Skip to content

Commit 2605e73

Browse files
betegonclaude
andauthored
fix(setup): auto-configure zsh fpath for shell completions (#509)
## Summary Zsh completions installed by `sentry cli setup` don't work out of the box because `~/.local/share/zsh/site-functions` isn't in zsh's default `fpath`. The setup previously only printed a hint asking users to manually edit `.zshrc`, while PATH was auto-configured — inconsistent and easy to miss. Now setup automatically appends `fpath=("dir" $fpath)` to `.zshrc`, mirroring how `addToPath()` already works. Also runs on upgrades so existing users get their fpath fixed on the next `sentry cli setup`. ## Changes - Add `addToFpath()` and `getFpathCommand()` to `shell.ts` (mirrors `addToPath`/`getPathCommand`) - Extract `handleZshFpath()` in `setup.ts` to keep complexity in check - Replace passive fpath hint with active `.zshrc` modification - Run fpath check even on completion file updates (one-time migration for existing installs) ## Test Plan - [x] `bun test test/lib/shell.test.ts` — 23 tests pass (5 new for `addToFpath`) - [x] `bun test test/commands/cli/setup.test.ts` — 29 tests pass (updated zsh test + new idempotency test) - [x] `bun run typecheck` — clean - [x] `bun run lint` — clean (only pre-existing warning in markdown.ts) - Manual: run `sentry cli setup` on zsh, verify `.zshrc` gets fpath line, restart shell, confirm tab completion works --------- Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
1 parent 4b4ed91 commit 2605e73

File tree

4 files changed

+240
-32
lines changed

4 files changed

+240
-32
lines changed

src/commands/cli/setup.ts

Lines changed: 40 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -30,9 +30,11 @@ import {
3030
import { CommandOutput } from "../../lib/formatters/output.js";
3131
import { logger } from "../../lib/logger.js";
3232
import {
33+
addToFpath,
3334
addToGitHubPath,
3435
addToPath,
3536
detectShell,
37+
getFpathCommand,
3638
getPathCommand,
3739
isBashAvailable,
3840
isInPath,
@@ -180,6 +182,37 @@ async function tryBashCompletionFallback(
180182
return await installCompletions("bash", homeDir, xdgDataHome);
181183
}
182184

185+
/**
186+
* Ensure the zsh completion directory is in fpath.
187+
*
188+
* Runs even on updates so existing installs get fpath configured (one-time migration).
189+
* Returns status messages for the user.
190+
*/
191+
async function handleZshFpath(
192+
shell: ShellInfo,
193+
completionDir: string,
194+
isNewInstall: boolean
195+
): Promise<string[]> {
196+
const lines: string[] = [];
197+
198+
if (shell.configFile) {
199+
const result = await addToFpath(shell.configFile, completionDir);
200+
if (result.modified) {
201+
lines.push(`Completions: ${result.message}`);
202+
lines.push(` Restart your shell or run: source ${shell.configFile}`);
203+
} else if (result.manualCommand) {
204+
lines.push(`Completions: ${result.message}`);
205+
lines.push(
206+
` Add manually to ${shell.configFile}: ${result.manualCommand}`
207+
);
208+
}
209+
} else if (isNewInstall) {
210+
lines.push(` Add to your .zshrc: ${getFpathCommand(completionDir)}`);
211+
}
212+
213+
return lines;
214+
}
215+
183216
/**
184217
* Handle shell completion installation.
185218
*
@@ -200,20 +233,19 @@ async function handleCompletions(
200233
const location = await installCompletions(shell.type, homeDir, xdgDataHome);
201234

202235
if (location) {
203-
// Silently updated — no need to tell the user on every upgrade
204-
if (!location.created) {
205-
return [];
206-
}
207-
208-
const lines = [`Completions: Installed to ${location.path}`];
236+
const lines: string[] = [];
209237

210-
// Zsh may need fpath hint
211238
if (shell.type === "zsh") {
212239
const completionDir = dirname(location.path);
213240
lines.push(
214-
` You may need to add to .zshrc: fpath=(${completionDir} $fpath)`
241+
...(await handleZshFpath(shell, completionDir, location.created))
215242
);
216243
}
244+
245+
if (location.created) {
246+
lines.unshift(`Completions: Installed to ${location.path}`);
247+
}
248+
217249
return lines;
218250
}
219251

src/lib/shell.ts

Lines changed: 58 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -171,79 +171,115 @@ export function isInPath(
171171
}
172172

173173
/**
174-
* Add a directory to PATH in a shell config file.
174+
* Append a shell config line to a config file with idempotency.
175175
*
176-
* @param configFile - Path to the config file
177-
* @param directory - Directory to add to PATH
178-
* @param shellType - The shell type (for correct syntax)
179-
* @returns Result of the modification attempt
176+
* Shared implementation for `addToPath` and `addToFpath`. Handles file
177+
* creation, duplicate detection, newline-aware appending, and error fallback.
178+
*
179+
* @param configFile - Path to the shell config file
180+
* @param directory - Directory being configured (used for duplicate check)
181+
* @param command - The full shell command to append (e.g. `export PATH="..."`)
182+
* @param label - Human-readable label for messages (e.g. "PATH", "fpath")
180183
*/
181-
export async function addToPath(
184+
async function addToShellConfig(
182185
configFile: string,
183186
directory: string,
184-
shellType: ShellType
187+
command: string,
188+
label: string
185189
): Promise<PathModificationResult> {
186-
const pathCommand = getPathCommand(shellType, directory);
187-
188-
// Read current content
189190
const file = Bun.file(configFile);
190191
const exists = await file.exists();
191192

192193
if (!exists) {
193-
// Create the file with the PATH command
194194
try {
195-
await Bun.write(configFile, `# sentry\n${pathCommand}\n`);
195+
await Bun.write(configFile, `# sentry\n${command}\n`);
196196
return {
197197
modified: true,
198198
configFile,
199-
message: `Created ${configFile} with PATH configuration`,
199+
message: `Created ${configFile} with ${label} configuration`,
200200
manualCommand: null,
201201
};
202202
} catch {
203203
return {
204204
modified: false,
205205
configFile: null,
206206
message: `Could not create ${configFile}`,
207-
manualCommand: pathCommand,
207+
manualCommand: command,
208208
};
209209
}
210210
}
211211

212212
const content = await file.text();
213213

214-
// Check if already configured
215-
if (content.includes(pathCommand) || content.includes(`"${directory}"`)) {
214+
if (content.includes(command) || content.includes(`"${directory}"`)) {
216215
return {
217216
modified: false,
218217
configFile,
219-
message: `PATH already configured in ${configFile}`,
218+
message: `${label} already configured in ${configFile}`,
220219
manualCommand: null,
221220
};
222221
}
223222

224-
// Append to file
225223
try {
226224
const newContent = content.endsWith("\n")
227-
? `${content}\n# sentry\n${pathCommand}\n`
228-
: `${content}\n\n# sentry\n${pathCommand}\n`;
225+
? `${content}\n# sentry\n${command}\n`
226+
: `${content}\n\n# sentry\n${command}\n`;
229227

230228
await Bun.write(configFile, newContent);
231229
return {
232230
modified: true,
233231
configFile,
234-
message: `Added sentry to PATH in ${configFile}`,
232+
message: `Added sentry ${label} in ${configFile}`,
235233
manualCommand: null,
236234
};
237235
} catch {
238236
return {
239237
modified: false,
240238
configFile: null,
241239
message: `Could not write to ${configFile}`,
242-
manualCommand: pathCommand,
240+
manualCommand: command,
243241
};
244242
}
245243
}
246244

245+
export function addToPath(
246+
configFile: string,
247+
directory: string,
248+
shellType: ShellType
249+
): Promise<PathModificationResult> {
250+
return addToShellConfig(
251+
configFile,
252+
directory,
253+
getPathCommand(shellType, directory),
254+
"PATH"
255+
);
256+
}
257+
258+
/**
259+
* Generate the fpath command for zsh completion directory.
260+
*/
261+
export function getFpathCommand(directory: string): string {
262+
return `fpath=("${directory}" $fpath)`;
263+
}
264+
265+
/**
266+
* Add a directory to zsh's fpath in a shell config file.
267+
*
268+
* @param configFile - Path to the zsh config file (e.g. ~/.zshrc)
269+
* @param directory - Directory to add to fpath
270+
*/
271+
export function addToFpath(
272+
configFile: string,
273+
directory: string
274+
): Promise<PathModificationResult> {
275+
return addToShellConfig(
276+
configFile,
277+
directory,
278+
getFpathCommand(directory),
279+
"fpath"
280+
);
281+
}
282+
247283
/**
248284
* Add to GitHub Actions PATH if running in CI.
249285
*/

test/commands/cli/setup.test.ts

Lines changed: 44 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -291,7 +291,46 @@ describe("sentry cli setup", () => {
291291
expect(getOutput()).toContain("Completions:");
292292
});
293293

294-
test("shows zsh fpath hint for zsh completions", async () => {
294+
test("adds fpath to .zshrc for zsh completions", async () => {
295+
const zshrc = join(testDir, ".zshrc");
296+
writeFileSync(zshrc, "# existing zshrc\n");
297+
298+
const { context, getOutput, restore } = createMockContext({
299+
homeDir: testDir,
300+
execPath: join(testDir, "bin", "sentry"),
301+
env: {
302+
PATH: `/usr/bin:${join(testDir, "bin")}:/bin`,
303+
SHELL: "/bin/zsh",
304+
},
305+
});
306+
restoreStderr = restore;
307+
308+
await run(
309+
app,
310+
["cli", "setup", "--no-modify-path", "--no-agent-skills"],
311+
context
312+
);
313+
314+
expect(getOutput()).toContain("fpath");
315+
expect(getOutput()).toContain("Completions:");
316+
317+
// Verify .zshrc was actually modified
318+
const content = await Bun.file(zshrc).text();
319+
expect(content).toContain("fpath=");
320+
expect(content).toContain("site-functions");
321+
});
322+
323+
test("skips fpath modification when already configured in .zshrc", async () => {
324+
const zshrc = join(testDir, ".zshrc");
325+
const completionDir = join(
326+
testDir,
327+
".local",
328+
"share",
329+
"zsh",
330+
"site-functions"
331+
);
332+
writeFileSync(zshrc, `# existing\nfpath=("${completionDir}" $fpath)\n`);
333+
295334
const { context, getOutput, restore } = createMockContext({
296335
homeDir: testDir,
297336
execPath: join(testDir, "bin", "sentry"),
@@ -308,7 +347,10 @@ describe("sentry cli setup", () => {
308347
context
309348
);
310349

311-
expect(getOutput()).toContain("fpath=");
350+
// Should still show "Installed to" but not "Added ... to fpath"
351+
const output = getOutput();
352+
expect(output).toContain("Completions: Installed to");
353+
expect(output).not.toContain("Added sentry fpath in");
312354
});
313355

314356
test("handles GitHub Actions PATH when GITHUB_ACTIONS is set", async () => {

test/lib/shell.test.ts

Lines changed: 98 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ import { afterEach, beforeEach, describe, expect, test } from "bun:test";
99
import { mkdirSync, rmSync, writeFileSync } from "node:fs";
1010
import { dirname, join } from "node:path";
1111
import {
12+
addToFpath,
1213
addToGitHubPath,
1314
addToPath,
1415
detectShell,
@@ -193,6 +194,103 @@ describe("shell utilities", () => {
193194
});
194195
});
195196

197+
describe("addToFpath", () => {
198+
let testDir: string;
199+
200+
beforeEach(() => {
201+
testDir = join(
202+
"/tmp",
203+
`shell-test-${Date.now()}-${Math.random().toString(36).slice(2)}`
204+
);
205+
mkdirSync(testDir, { recursive: true });
206+
});
207+
208+
afterEach(() => {
209+
rmSync(testDir, { recursive: true, force: true });
210+
});
211+
212+
test("creates config file if it doesn't exist", async () => {
213+
const configFile = join(testDir, ".zshrc");
214+
const result = await addToFpath(
215+
configFile,
216+
"/home/user/.local/share/zsh/site-functions"
217+
);
218+
219+
expect(result.modified).toBe(true);
220+
expect(result.configFile).toBe(configFile);
221+
222+
const content = await Bun.file(configFile).text();
223+
expect(content).toContain(
224+
'fpath=("/home/user/.local/share/zsh/site-functions" $fpath)'
225+
);
226+
});
227+
228+
test("appends to existing config file", async () => {
229+
const configFile = join(testDir, ".zshrc");
230+
writeFileSync(configFile, "# existing content\n");
231+
232+
const result = await addToFpath(
233+
configFile,
234+
"/home/user/.local/share/zsh/site-functions"
235+
);
236+
237+
expect(result.modified).toBe(true);
238+
239+
const content = await Bun.file(configFile).text();
240+
expect(content).toContain("# existing content");
241+
expect(content).toContain("# sentry");
242+
expect(content).toContain(
243+
'fpath=("/home/user/.local/share/zsh/site-functions" $fpath)'
244+
);
245+
});
246+
247+
test("skips if already configured", async () => {
248+
const configFile = join(testDir, ".zshrc");
249+
writeFileSync(
250+
configFile,
251+
'# sentry\nfpath=("/home/user/.local/share/zsh/site-functions" $fpath)\n'
252+
);
253+
254+
const result = await addToFpath(
255+
configFile,
256+
"/home/user/.local/share/zsh/site-functions"
257+
);
258+
259+
expect(result.modified).toBe(false);
260+
expect(result.message).toContain("already configured");
261+
});
262+
263+
test("appends newline separator when file doesn't end with newline", async () => {
264+
const configFile = join(testDir, ".zshrc");
265+
writeFileSync(configFile, "# existing content without newline");
266+
267+
const result = await addToFpath(
268+
configFile,
269+
"/home/user/.local/share/zsh/site-functions"
270+
);
271+
272+
expect(result.modified).toBe(true);
273+
274+
const content = await Bun.file(configFile).text();
275+
expect(content).toContain(
276+
"# existing content without newline\n\n# sentry\n"
277+
);
278+
});
279+
280+
test("returns manualCommand when config file cannot be created", async () => {
281+
const configFile = "/dev/null/impossible/path/.zshrc";
282+
const result = await addToFpath(
283+
configFile,
284+
"/home/user/.local/share/zsh/site-functions"
285+
);
286+
287+
expect(result.modified).toBe(false);
288+
expect(result.manualCommand).toBe(
289+
'fpath=("/home/user/.local/share/zsh/site-functions" $fpath)'
290+
);
291+
});
292+
});
293+
196294
describe("addToGitHubPath", () => {
197295
let testDir: string;
198296

0 commit comments

Comments
 (0)