Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
46 changes: 38 additions & 8 deletions src/commands/cli/setup.ts
Original file line number Diff line number Diff line change
Expand Up @@ -30,9 +30,11 @@ import {
import { CommandOutput } from "../../lib/formatters/output.js";
import { logger } from "../../lib/logger.js";
import {
addToFpath,
addToGitHubPath,
addToPath,
detectShell,
getFpathCommand,
getPathCommand,
isBashAvailable,
isInPath,
Expand Down Expand Up @@ -180,6 +182,35 @@ async function tryBashCompletionFallback(
return await installCompletions("bash", homeDir, xdgDataHome);
}

/**
* Ensure the zsh completion directory is in fpath.
*
* Runs even on updates so existing installs get fpath configured (one-time migration).
* Returns status messages for the user.
*/
async function handleZshFpath(
shell: ShellInfo,
completionDir: string,
isNewInstall: boolean
): Promise<string[]> {
const lines: string[] = [];

if (shell.configFile) {
const result = await addToFpath(shell.configFile, completionDir);
if (result.modified) {
lines.push(`Completions: ${result.message}`);
lines.push(` Restart your shell or run: source ${shell.configFile}`);
}
if (result.manualCommand) {
lines.push(` Add manually to .zshrc: ${result.manualCommand}`);
}
} else if (isNewInstall) {
lines.push(` Add to your .zshrc: ${getFpathCommand(completionDir)}`);
}

return lines;
}

/**
* Handle shell completion installation.
*
Expand All @@ -200,20 +231,19 @@ async function handleCompletions(
const location = await installCompletions(shell.type, homeDir, xdgDataHome);

if (location) {
// Silently updated — no need to tell the user on every upgrade
if (!location.created) {
return [];
}

const lines = [`Completions: Installed to ${location.path}`];
const lines: string[] = [];

// Zsh may need fpath hint
if (shell.type === "zsh") {
const completionDir = dirname(location.path);
lines.push(
` You may need to add to .zshrc: fpath=(${completionDir} $fpath)`
...(await handleZshFpath(shell, completionDir, location.created))
);
}

Comment on lines 242 to +244
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bug: On upgrade, handleZshFpath silently fails to provide setup instructions if a zsh config file is not found, because it incorrectly checks the isNewInstall flag.
Severity: MEDIUM

Suggested Fix

Modify handleZshFpath to provide manual setup instructions whenever shell.configFile is null, regardless of the isNewInstall flag's value. This will align its behavior with handlePathModification and ensure users always receive guidance.

Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent.
Verify if this is a real issue. If it is, propose a fix; if not, explain why it's not
valid.

Location: src/commands/cli/setup.ts#L242-L244

Potential issue: The `handleZshFpath` function fails to provide setup instructions
during an upgrade if the user's zsh configuration file was not found on the initial
install. The logic checks for a config file, and if not found, it only provides guidance
if `isNewInstall` is true. On an upgrade, `isNewInstall` is false, causing the function
to silently do nothing. This leaves the user with non-functional shell completions and
no information on how to fix it. This behavior is inconsistent with the
`handlePathModification` function, which always provides guidance when a configuration
file is missing, regardless of whether it's a new install or an upgrade.

if (location.created) {
lines.unshift(`Completions: Installed to ${location.path}`);
}

return lines;
}

Expand Down
78 changes: 78 additions & 0 deletions src/lib/shell.ts
Original file line number Diff line number Diff line change
Expand Up @@ -244,6 +244,84 @@ export async function addToPath(
}
}

/**
* Generate the fpath command for zsh completion directory.
*/
export function getFpathCommand(directory: string): string {
return `fpath=("${directory}" $fpath)`;
}

/**
* Add a directory to zsh's fpath in a shell config file.
*
* Mirrors `addToPath` — reads the config file, checks idempotency,
* and appends `fpath=("dir" $fpath)` if not already present.
*
* @param configFile - Path to the zsh config file (e.g. ~/.zshrc)
* @param directory - Directory to add to fpath
* @returns Result of the modification attempt
*/
export async function addToFpath(
configFile: string,
directory: string
): Promise<PathModificationResult> {
const fpathCommand = getFpathCommand(directory);

const file = Bun.file(configFile);
const exists = await file.exists();

if (!exists) {
try {
await Bun.write(configFile, `# sentry\n${fpathCommand}\n`);
return {
modified: true,
configFile,
message: `Created ${configFile} with fpath configuration`,
manualCommand: null,
};
} catch {
return {
modified: false,
configFile: null,
message: `Could not create ${configFile}`,
manualCommand: fpathCommand,
};
}
}

const content = await file.text();

if (content.includes(fpathCommand) || content.includes(`"${directory}"`)) {
return {
modified: false,
configFile,
message: `fpath already configured in ${configFile}`,
manualCommand: null,
};
}

try {
const newContent = content.endsWith("\n")
? `${content}\n# sentry\n${fpathCommand}\n`
: `${content}\n\n# sentry\n${fpathCommand}\n`;

await Bun.write(configFile, newContent);
return {
modified: true,
configFile,
message: `Added sentry completions to fpath in ${configFile}`,
manualCommand: null,
};
} catch {
return {
modified: false,
configFile: null,
message: `Could not write to ${configFile}`,
manualCommand: fpathCommand,
};
}
}

/**
* Add to GitHub Actions PATH if running in CI.
*/
Expand Down
46 changes: 44 additions & 2 deletions test/commands/cli/setup.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -291,7 +291,46 @@ describe("sentry cli setup", () => {
expect(getOutput()).toContain("Completions:");
});

test("shows zsh fpath hint for zsh completions", async () => {
test("adds fpath to .zshrc for zsh completions", async () => {
const zshrc = join(testDir, ".zshrc");
writeFileSync(zshrc, "# existing zshrc\n");

const { context, getOutput, restore } = createMockContext({
homeDir: testDir,
execPath: join(testDir, "bin", "sentry"),
env: {
PATH: `/usr/bin:${join(testDir, "bin")}:/bin`,
SHELL: "/bin/zsh",
},
});
restoreStderr = restore;

await run(
app,
["cli", "setup", "--no-modify-path", "--no-agent-skills"],
context
);

expect(getOutput()).toContain("fpath");
expect(getOutput()).toContain("Completions:");

// Verify .zshrc was actually modified
const content = await Bun.file(zshrc).text();
expect(content).toContain("fpath=");
expect(content).toContain("site-functions");
});

test("skips fpath modification when already configured in .zshrc", async () => {
const zshrc = join(testDir, ".zshrc");
const completionDir = join(
testDir,
".local",
"share",
"zsh",
"site-functions"
);
writeFileSync(zshrc, `# existing\nfpath=("${completionDir}" $fpath)\n`);

const { context, getOutput, restore } = createMockContext({
homeDir: testDir,
execPath: join(testDir, "bin", "sentry"),
Expand All @@ -308,7 +347,10 @@ describe("sentry cli setup", () => {
context
);

expect(getOutput()).toContain("fpath=");
// Should still show "Installed to" but not "Added ... to fpath"
const output = getOutput();
expect(output).toContain("Completions: Installed to");
expect(output).not.toContain("Added sentry completions to fpath");
});

test("handles GitHub Actions PATH when GITHUB_ACTIONS is set", async () => {
Expand Down
98 changes: 98 additions & 0 deletions test/lib/shell.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import { afterEach, beforeEach, describe, expect, test } from "bun:test";
import { mkdirSync, rmSync, writeFileSync } from "node:fs";
import { dirname, join } from "node:path";
import {
addToFpath,
addToGitHubPath,
addToPath,
detectShell,
Expand Down Expand Up @@ -193,6 +194,103 @@ describe("shell utilities", () => {
});
});

describe("addToFpath", () => {
let testDir: string;

beforeEach(() => {
testDir = join(
"/tmp",
`shell-test-${Date.now()}-${Math.random().toString(36).slice(2)}`
);
mkdirSync(testDir, { recursive: true });
});

afterEach(() => {
rmSync(testDir, { recursive: true, force: true });
});

test("creates config file if it doesn't exist", async () => {
const configFile = join(testDir, ".zshrc");
const result = await addToFpath(
configFile,
"/home/user/.local/share/zsh/site-functions"
);

expect(result.modified).toBe(true);
expect(result.configFile).toBe(configFile);

const content = await Bun.file(configFile).text();
expect(content).toContain(
'fpath=("/home/user/.local/share/zsh/site-functions" $fpath)'
);
});

test("appends to existing config file", async () => {
const configFile = join(testDir, ".zshrc");
writeFileSync(configFile, "# existing content\n");

const result = await addToFpath(
configFile,
"/home/user/.local/share/zsh/site-functions"
);

expect(result.modified).toBe(true);

const content = await Bun.file(configFile).text();
expect(content).toContain("# existing content");
expect(content).toContain("# sentry");
expect(content).toContain(
'fpath=("/home/user/.local/share/zsh/site-functions" $fpath)'
);
});

test("skips if already configured", async () => {
const configFile = join(testDir, ".zshrc");
writeFileSync(
configFile,
'# sentry\nfpath=("/home/user/.local/share/zsh/site-functions" $fpath)\n'
);

const result = await addToFpath(
configFile,
"/home/user/.local/share/zsh/site-functions"
);

expect(result.modified).toBe(false);
expect(result.message).toContain("already configured");
});

test("appends newline separator when file doesn't end with newline", async () => {
const configFile = join(testDir, ".zshrc");
writeFileSync(configFile, "# existing content without newline");

const result = await addToFpath(
configFile,
"/home/user/.local/share/zsh/site-functions"
);

expect(result.modified).toBe(true);

const content = await Bun.file(configFile).text();
expect(content).toContain(
"# existing content without newline\n\n# sentry\n"
);
});

test("returns manualCommand when config file cannot be created", async () => {
const configFile = "/dev/null/impossible/path/.zshrc";
const result = await addToFpath(
configFile,
"/home/user/.local/share/zsh/site-functions"
);

expect(result.modified).toBe(false);
expect(result.manualCommand).toBe(
'fpath=("/home/user/.local/share/zsh/site-functions" $fpath)'
);
});
});

describe("addToGitHubPath", () => {
let testDir: string;

Expand Down
Loading