diff --git a/src/commands/cli/setup.ts b/src/commands/cli/setup.ts index 0d5a12892..1f42b3c19 100644 --- a/src/commands/cli/setup.ts +++ b/src/commands/cli/setup.ts @@ -250,8 +250,18 @@ async function handleCompletions( return lines; } - // sh/ash are minimal POSIX shells — completions aren't expected - if (shell.type === "sh" || shell.type === "ash") { + // Shells with native completion support (bash/zsh/fish) return null only + // when the target directory is not writable (permission error handled + // inside installCompletions). Don't fall through to the bash fallback — + // that would misleadingly say the shell "is not directly supported". + // sh/ash are minimal POSIX shells — completions aren't expected either. + if ( + shell.type === "bash" || + shell.type === "zsh" || + shell.type === "fish" || + shell.type === "sh" || + shell.type === "ash" + ) { return []; } diff --git a/src/commands/issue/utils.ts b/src/commands/issue/utils.ts index 50d3ee1a0..1331eae03 100644 --- a/src/commands/issue/utils.ts +++ b/src/commands/issue/utils.ts @@ -455,11 +455,8 @@ async function resolveSelector( throw new ResolutionError( `Selector '${selector}'`, "no unresolved issues found", - commandHint, - [ - `No unresolved issues found in org '${orgSlug}'.`, - `The ${label} issue selector only matches unresolved issues.`, - ] + `sentry issue list ${orgSlug}/ -q "is:resolved"`, + [`The ${label} issue selector only matches unresolved issues.`] ); } diff --git a/src/lib/completions.ts b/src/lib/completions.ts index b1e97e39f..860cbcdb7 100644 --- a/src/lib/completions.ts +++ b/src/lib/completions.ts @@ -615,10 +615,16 @@ export function getCompletionPath( /** * Install completion script for a shell type. * + * Returns `null` when the shell is unsupported or when the target directory + * cannot be created due to filesystem permission restrictions (EPERM/EACCES). + * This is common on macOS where `~/.local/share/zsh` may be owned by root + * or protected by SIP, and during Homebrew post-install when the home + * directory has restrictive permissions. + * * @param shellType - The shell type * @param homeDir - User's home directory * @param xdgDataHome - XDG_DATA_HOME or undefined for default - * @returns Location info if installed, null if shell not supported + * @returns Location info if installed, null if shell not supported or path not writable */ export async function installCompletions( shellType: ShellType, @@ -635,17 +641,46 @@ export async function installCompletions( return null; } - // Create directory if needed - const dir = dirname(path); - if (!existsSync(dir)) { - mkdirSync(dir, { recursive: true, mode: 0o755 }); + try { + // Create directory if needed + const dir = dirname(path); + if (!existsSync(dir)) { + mkdirSync(dir, { recursive: true, mode: 0o755 }); + } + + const alreadyExists = existsSync(path); + await Bun.write(path, script); + + return { + path, + created: !alreadyExists, + }; + } catch (error: unknown) { + // Permission errors are expected when the target directory is owned by + // root, protected by macOS SIP, or when running in a restricted context + // (e.g. Homebrew post-install). Silently skip — the caller (bestEffort + // in setup.ts) would catch anyway, but handling here avoids noisy Sentry + // reports for a non-critical operation. + if (isPermissionError(error)) { + return null; + } + throw error; } +} - const alreadyExists = existsSync(path); - await Bun.write(path, script); +/** Filesystem error codes that indicate a permission problem */ +const PERMISSION_ERROR_CODES = new Set(["EPERM", "EACCES", "EROFS"]); - return { - path, - created: !alreadyExists, - }; +/** + * Check whether an error is a filesystem permission error. + * + * Matches EPERM (operation not permitted), EACCES (permission denied), + * and EROFS (read-only filesystem). + */ +function isPermissionError(error: unknown): boolean { + if (!(error instanceof Error && "code" in error)) { + return false; + } + const code = (error as NodeJS.ErrnoException).code; + return typeof code === "string" && PERMISSION_ERROR_CODES.has(code); } diff --git a/test/commands/cli/setup.test.ts b/test/commands/cli/setup.test.ts index a81459687..419ee2632 100644 --- a/test/commands/cli/setup.test.ts +++ b/test/commands/cli/setup.test.ts @@ -862,9 +862,10 @@ describe("sentry cli setup", () => { expect(getOutput()).toContain("Agent skills:"); }); - test("bestEffort catches errors from steps and setup still completes", async () => { - // Make the completions dir unwritable so installCompletions() fails. - // bestEffort() must catch the error and continue — setup still completes. + test("setup completes gracefully when completion directory is not writable", async () => { + // Make the completions dir unwritable so Bun.write() can't write the + // completion script. installCompletions() catches the permission error + // and returns null — setup completes without error or warning. const { chmodSync: chmod } = await import("node:fs"); const homeDir = join(testDir, "home"); const xdgData = join(homeDir, ".local", "share"); @@ -891,9 +892,13 @@ describe("sentry cli setup", () => { ); const combined = getOutput(); - // Setup must complete even though the completions step threw — - // the warning appears in the formatted output - expect(combined).toContain("Shell completions failed"); + // installCompletions handles permission errors gracefully and returns + // null, so bestEffort never sees an error — no failure message appears + expect(combined).not.toContain("Shell completions failed"); + // No misleading fallback message for a supported shell (zsh) + expect(combined).not.toContain("not directly supported"); + // Setup still completes successfully + expect(combined).not.toContain("error"); chmod(zshDir, 0o755); }); diff --git a/test/commands/issue/utils.test.ts b/test/commands/issue/utils.test.ts index 3c8ab1f7b..5bd2e0411 100644 --- a/test/commands/issue/utils.test.ts +++ b/test/commands/issue/utils.test.ts @@ -1991,6 +1991,8 @@ describe("resolveOrgAndIssueId: magic @ selectors", () => { expect(err).toBeInstanceOf(ResolutionError); expect(String(err)).toContain("no unresolved issues found"); expect(String(err)).toContain("most recent"); + expect(String(err)).toContain("sentry issue list"); + expect(String(err)).toContain('-q "is:resolved"'); }); test("throws ContextError when org cannot be resolved for bare @selector", async () => { diff --git a/test/lib/completions.test.ts b/test/lib/completions.test.ts index 5fba91ed4..57c1a970d 100644 --- a/test/lib/completions.test.ts +++ b/test/lib/completions.test.ts @@ -7,7 +7,7 @@ */ import { afterEach, beforeEach, describe, expect, test } from "bun:test"; -import { existsSync, mkdirSync, rmSync } from "node:fs"; +import { chmodSync, existsSync, mkdirSync, rmSync } from "node:fs"; import { join } from "node:path"; import { extractCommandTree, @@ -167,5 +167,45 @@ describe("completions", () => { expect(second!.created).toBe(false); expect(second!.path).toBe(first!.path); }); + + test("returns null when directory creation fails with EACCES", async () => { + // Create a read-only parent directory so mkdir inside it fails + const restrictedDir = join(testDir, "restricted"); + mkdirSync(restrictedDir, { recursive: true, mode: 0o755 }); + // Make it non-writable so child directory creation fails + chmodSync(restrictedDir, 0o444); + + try { + // XDG_DATA_HOME points into the restricted directory + const result = await installCompletions( + "bash", + "/nonexistent", + join(restrictedDir, "subdir") + ); + expect(result).toBeNull(); + } finally { + // Restore write permission for cleanup + chmodSync(restrictedDir, 0o755); + } + }); + + test("returns null when parent directory is read-only (EPERM scenario)", async () => { + // Simulate the macOS EPERM scenario: a parent directory exists but + // we can't create subdirectories inside it + const lockedParent = join(testDir, "locked"); + mkdirSync(lockedParent, { recursive: true }); + chmodSync(lockedParent, 0o555); + + try { + const result = await installCompletions( + "zsh", + "/nonexistent", + join(lockedParent, "deep", "nested") + ); + expect(result).toBeNull(); + } finally { + chmodSync(lockedParent, 0o755); + } + }); }); });