From 2659a11825187710f7b3c8d5e0edd1f48fd899d0 Mon Sep 17 00:00:00 2001 From: Burak Yigit Kaya Date: Tue, 28 Apr 2026 21:40:33 +0000 Subject: [PATCH 1/7] fix: replace dead-end @latest selector error hint with actionable alternative (CLI-1ET) --- src/commands/issue/utils.ts | 3 ++- test/commands/issue/utils.test.ts | 2 ++ 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/src/commands/issue/utils.ts b/src/commands/issue/utils.ts index 50d3ee1a0..e707db5b8 100644 --- a/src/commands/issue/utils.ts +++ b/src/commands/issue/utils.ts @@ -455,10 +455,11 @@ async function resolveSelector( throw new ResolutionError( `Selector '${selector}'`, "no unresolved issues found", - commandHint, + `sentry issue list ${orgSlug}/ -q ""`, [ `No unresolved issues found in org '${orgSlug}'.`, `The ${label} issue selector only matches unresolved issues.`, + `Use -q "" to list all issues regardless of status.`, ] ); } diff --git a/test/commands/issue/utils.test.ts b/test/commands/issue/utils.test.ts index 3c8ab1f7b..41f383e45 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 ""'); }); test("throws ContextError when org cannot be resolved for bare @selector", async () => { From a9763693d5ac964e5b48f3fb9fda7d7000fefbc5 Mon Sep 17 00:00:00 2001 From: Burak Yigit Kaya Date: Tue, 28 Apr 2026 21:50:22 +0000 Subject: [PATCH 2/7] fix: use is:resolved query hint instead of no-op empty string --- src/commands/issue/utils.ts | 4 ++-- test/commands/issue/utils.test.ts | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/commands/issue/utils.ts b/src/commands/issue/utils.ts index e707db5b8..023cd55f7 100644 --- a/src/commands/issue/utils.ts +++ b/src/commands/issue/utils.ts @@ -455,11 +455,11 @@ async function resolveSelector( throw new ResolutionError( `Selector '${selector}'`, "no unresolved issues found", - `sentry issue list ${orgSlug}/ -q ""`, + `sentry issue list ${orgSlug}/ -q "is:resolved"`, [ `No unresolved issues found in org '${orgSlug}'.`, `The ${label} issue selector only matches unresolved issues.`, - `Use -q "" to list all issues regardless of status.`, + `Use -q "is:resolved" to see resolved issues instead.`, ] ); } diff --git a/test/commands/issue/utils.test.ts b/test/commands/issue/utils.test.ts index 41f383e45..5bd2e0411 100644 --- a/test/commands/issue/utils.test.ts +++ b/test/commands/issue/utils.test.ts @@ -1992,7 +1992,7 @@ describe("resolveOrgAndIssueId: magic @ selectors", () => { 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 ""'); + expect(String(err)).toContain('-q "is:resolved"'); }); test("throws ContextError when org cannot be resolved for bare @selector", async () => { From 2b27b912174fd9b4db86a76199a0c4e86bf01862 Mon Sep 17 00:00:00 2001 From: Burak Yigit Kaya Date: Tue, 28 Apr 2026 22:00:59 +0000 Subject: [PATCH 3/7] chore: trim redundant suggestion lines from selector error --- src/commands/issue/utils.ts | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/commands/issue/utils.ts b/src/commands/issue/utils.ts index 023cd55f7..af1545327 100644 --- a/src/commands/issue/utils.ts +++ b/src/commands/issue/utils.ts @@ -457,9 +457,7 @@ async function resolveSelector( "no unresolved issues found", `sentry issue list ${orgSlug}/ -q "is:resolved"`, [ - `No unresolved issues found in org '${orgSlug}'.`, `The ${label} issue selector only matches unresolved issues.`, - `Use -q "is:resolved" to see resolved issues instead.`, ] ); } From 2ff78309e944656ab07877701211ff1e74579e1e Mon Sep 17 00:00:00 2001 From: Burak Yigit Kaya Date: Tue, 28 Apr 2026 22:03:34 +0000 Subject: [PATCH 4/7] style: collapse single-element array to one line (biome format) --- src/commands/issue/utils.ts | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/src/commands/issue/utils.ts b/src/commands/issue/utils.ts index af1545327..1331eae03 100644 --- a/src/commands/issue/utils.ts +++ b/src/commands/issue/utils.ts @@ -456,9 +456,7 @@ async function resolveSelector( `Selector '${selector}'`, "no unresolved issues found", `sentry issue list ${orgSlug}/ -q "is:resolved"`, - [ - `The ${label} issue selector only matches unresolved issues.`, - ] + [`The ${label} issue selector only matches unresolved issues.`] ); } From 85a07bce557ac273ba3c5d5d22861cd89b4c2b34 Mon Sep 17 00:00:00 2001 From: Burak Yigit Kaya Date: Tue, 28 Apr 2026 22:19:10 +0000 Subject: [PATCH 5/7] fix(completions): handle permission errors gracefully in installCompletions (CLI-1A5) installCompletions now catches EPERM, EACCES, and EROFS filesystem errors and returns null instead of throwing. This prevents noisy Sentry reports when the completion target directory is not writable (common on macOS with SIP-protected paths or during Homebrew post-install). --- src/lib/completions.ts | 57 +++++++++++++++++++++++++++++------- test/lib/completions.test.ts | 42 +++++++++++++++++++++++++- 2 files changed, 87 insertions(+), 12 deletions(-) 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/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); + } + }); }); }); From 39bc7eca5241fe58b5a187ada278a7cbb530ce32 Mon Sep 17 00:00:00 2001 From: Burak Yigit Kaya Date: Tue, 28 Apr 2026 22:22:43 +0000 Subject: [PATCH 6/7] fix: update setup test to expect graceful permission handling The setup test expected bestEffort to catch the EPERM error, but now installCompletions handles it internally and returns null. Update the test to verify setup completes without error messages. --- test/commands/cli/setup.test.ts | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/test/commands/cli/setup.test.ts b/test/commands/cli/setup.test.ts index a81459687..91aab5090 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,11 @@ 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"); + // Setup still completes successfully + expect(combined).not.toContain("error"); chmod(zshDir, 0o755); }); From 65303b54408c759502872b5cad3eecf3a8c1792d Mon Sep 17 00:00:00 2001 From: Burak Yigit Kaya Date: Tue, 28 Apr 2026 22:33:13 +0000 Subject: [PATCH 7/7] fix: prevent misleading fallback message for supported shells with permission errors When installCompletions returns null for a supported shell (bash/zsh/fish) due to permission errors, don't fall through to tryBashCompletionFallback which would misleadingly say 'Your shell (zsh) is not directly supported'. Skip the fallback for all shells with native completion support. Addresses Cursor Bugbot review comment. --- src/commands/cli/setup.ts | 14 ++++++++++++-- test/commands/cli/setup.test.ts | 2 ++ 2 files changed, 14 insertions(+), 2 deletions(-) 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/test/commands/cli/setup.test.ts b/test/commands/cli/setup.test.ts index 91aab5090..419ee2632 100644 --- a/test/commands/cli/setup.test.ts +++ b/test/commands/cli/setup.test.ts @@ -895,6 +895,8 @@ describe("sentry cli setup", () => { // 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");