From 4dfe90a0c12d6aa9e716c21a9f309d2fcf41ec28 Mon Sep 17 00:00:00 2001 From: Burak Yigit Kaya Date: Sat, 27 Jun 2026 14:42:19 +0000 Subject: [PATCH 1/2] fix(resolve): guide users to org/project instead of silencing the auto-detect error (CLI-3B) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit "Could not auto-detect organization and project" was being silenced, which hid the signal AND skipped the underlying UX fix. Instead of dropping it, help users succeed and keep the error visible to measure the impact: - Sole-account auto-select: when an authenticated account has exactly one accessible org with exactly one project, resolveOrgAndProject now returns that pair instead of null (step 7 of the cascade). This is strictly additive — callers that depend on a null return (e.g. event view's cross-org search) are unaffected, since it only turns a null into a uniquely-determined target. - Interactive picker: on a failed auto-detect in a TTY, guideOrgProjectFailure prompts the user to choose an org/project and saves it as the default. Gated on stdin AND stdout being TTYs so piped/--json runs never block. - Enriched error: authenticated users who can't be auto-resolved get a ContextError listing their accessible orgs (copy-pasteable), instead of the generic "run org list" guidance. - Un-silence ContextError: reverted the classifySilenced ContextError branch (and the dead recordSilencedError resource attribute) so its volume stays visible as the signal for these UX improvements. Wired into the auto-detect paths of resolveOrgProjectTarget, resolveTargetsFromParsedArg (issue list), and trace-target. Upload commands get the sole-account auto-select for free via resolveOrgAndProject. --- src/lib/error-reporting.ts | 38 ++- src/lib/resolve-target.ts | 292 +++++++++++++++++++++++- src/lib/telemetry.ts | 14 +- src/lib/trace-target.ts | 15 +- test/commands/trace/view.func.test.ts | 7 +- test/lib/error-reporting.test.ts | 20 +- test/lib/resolve-target-listing.test.ts | 109 +++++++++ test/lib/telemetry.test.ts | 10 +- 8 files changed, 443 insertions(+), 62 deletions(-) diff --git a/src/lib/error-reporting.ts b/src/lib/error-reporting.ts index ac145c6e5..edb9223dd 100644 --- a/src/lib/error-reporting.ts +++ b/src/lib/error-reporting.ts @@ -4,10 +4,15 @@ * Provides two things: * * 1. **Silencing rules** — `OutputError`, network failures (offline/DNS/proxy), - * `ContextError` (a required value the user omitted), `AuthError` (expected - * auth states the user must act on), 401–499 `ApiError`, and 400 `ApiError`s - * that report an unparseable user search query are not sent to Sentry as - * issues. A `cli.error.silenced` metric preserves volume + user/org context. + * `AuthError` (expected auth states the user must act on), 401–499 `ApiError`, + * and 400 `ApiError`s that report an unparseable user search query are not + * sent to Sentry as issues. A `cli.error.silenced` metric preserves volume + + * user/org context. + * + * `ContextError` (a required value the user omitted) is deliberately NOT + * silenced: its volume is the signal driving auto-detection/UX improvements + * (e.g. single-org auto-select, the interactive picker), so it must stay + * visible (CLI-3B). * * 2. **Grouping tags** — enriches every error event with `cli_error.*` tags * that Sentry's server-side fingerprint rules use for stable grouping. @@ -48,7 +53,6 @@ import { */ type SilenceReason = | "output_error" - | "context_missing" | "auth_expected" | "api_user_error" | "api_query_error" @@ -72,16 +76,14 @@ export function classifySilenced(error: unknown): SilenceReason | null { if (isNetworkError(error)) { return "network_error"; } - // A ContextError always means the user omitted a required value (no - // org/project could be resolved, a required ID was not provided, etc.). It is - // never a CLI bug — unlike ResolutionError, where a *provided* value that - // can't be matched may signal a product/access issue worth observing. There - // is nothing per-instance to investigate, so silence the whole class; the - // `cli.error.silenced` metric (keyed by `error_class` + `resource`) preserves - // the volume and which value was missing. (CLI-3B: ~2000 users.) - if (error instanceof ContextError) { - return "context_missing"; - } + // A ContextError means the user omitted a required value (no org/project + // could be resolved, a required ID was not provided, etc.). It is NOT + // silenced: the volume of these is the signal we use to drive auto-detection + // and UX improvements (single-org auto-select, the interactive picker, the + // enriched error). Silencing it hid that signal AND skipped the fix, so it + // stays captured (CLI-3B). The accompanying `resolveOrgProjectOrGuide` + // changes aim to drive this volume down by helping users succeed instead. + // // All AuthError reasons are expected auth states the user must act on, not // CLI bugs: `not_authenticated` (no token), `expired` (token aged out), and // `invalid` (a bad/insufficiently-scoped token the user supplied). `invalid` @@ -117,12 +119,6 @@ function recordSilencedError(error: unknown, reason: SilenceReason): void { if (error instanceof AuthError) { attributes.auth_reason = error.reason; } - // Preserve which required value was missing so the silenced-volume metric - // keeps sub-grouping context (e.g. "Organization and project" vs "Event ID"). - // ContextError resources are a small fixed set, so cardinality stays low. - if (error instanceof ContextError) { - attributes.resource = error.resource; - } try { Sentry.metrics.distribution("cli.error.silenced", 1, { attributes }); diff --git a/src/lib/resolve-target.ts b/src/lib/resolve-target.ts index e02196a78..cc1a9aa6d 100644 --- a/src/lib/resolve-target.ts +++ b/src/lib/resolve-target.ts @@ -15,8 +15,9 @@ */ import { basename } from "node:path"; +import { isatty } from "node:tty"; import pLimit from "p-limit"; -import type { SentryProject } from "../types/index.js"; +import type { SentryOrganization, SentryProject } from "../types/index.js"; import { findProjectByDsnKey, findProjectsByPattern, @@ -31,7 +32,13 @@ import { type ParsedOrgProject, parseOrgProjectArg, } from "./arg-parsing.js"; -import { getDefaultOrganization, getDefaultProject } from "./db/defaults.js"; +import { isAuthenticated } from "./db/auth.js"; +import { + getDefaultOrganization, + getDefaultProject, + setDefaultOrganization, + setDefaultProject, +} from "./db/defaults.js"; import { getCachedDsn, setCachedDsn } from "./db/dsn-cache.js"; import { getCachedProject, @@ -152,6 +159,13 @@ export type ResolveOptions = { cwd: string; /** Usage hint shown when only one of org/project is provided */ usageHint?: string; + /** + * Whether an interactive org/project picker may be shown when auto-detection + * fails. Only consulted by {@link resolveOrgProjectOrGuide}. When omitted, it + * is inferred from whether stdin **and** stdout are TTYs (so piped/`--json` + * invocations never block on a prompt). + */ + interactive?: boolean; }; /** @@ -1309,6 +1323,18 @@ export async function resolveOrgAndProject( const inferred = await inferFromDirectoryName(cwd); const [first] = inferred.targets; if (!first) { + // 7. Authenticated last resort: if the account has exactly one + // accessible org with exactly one project, that pair is the only + // possible target — use it instead of failing. This removes the + // "Could not auto-detect organization and project" dead-end for + // single-org/single-project accounts (CLI-3B). Callers that rely on + // a null return (e.g. event view's cross-org search) are unaffected: + // this only ever turns a null into a uniquely-determined target. + const sole = await resolveSoleAccountTarget(); + if (sole) { + span.setAttribute("resolve.method", "account_sole"); + return withTelemetryContext(sole); + } span.setAttribute("resolve.method", "none"); return null; } @@ -1327,6 +1353,250 @@ export async function resolveOrgAndProject( ); } +/** + * Build a {@link ResolvedTarget} from an org/project pair, filling display + * names and the numeric project ID from the fetched API objects. + */ +function toAccountTarget( + org: SentryOrganization, + project: SentryProject, + detectedFrom: string +): ResolvedTarget { + return { + org: org.slug, + project: project.slug, + projectId: toNumericId(project.id), + orgDisplay: org.name || org.slug, + projectDisplay: project.name || project.slug, + projectData: project, + detectedFrom, + }; +} + +/** + * Last-resort resolution for authenticated users: when the account has exactly + * one accessible organization containing exactly one project, that pair is the + * only possible target, so return it instead of failing auto-detection. + * + * Returns `null` when not authenticated, when a lookup fails, or when there is + * more than one (or zero) org/project — in those cases the choice is genuinely + * ambiguous and must be made explicitly or via {@link resolveOrgProjectOrGuide}. + * + * This never throws and never prompts, so it is safe to call from the shared + * {@link resolveOrgAndProject} cascade without affecting callers that depend on + * a `null` return. + */ +async function resolveSoleAccountTarget(): Promise { + if (!isAuthenticated()) { + return null; + } + try { + const orgs = await listOrganizations(); + const org = orgs.length === 1 ? orgs[0] : undefined; + if (!org) { + return null; + } + const projects = await listProjects(org.slug); + const project = projects.length === 1 ? projects[0] : undefined; + if (!project) { + return null; + } + return toAccountTarget(org, project, "your only accessible org/project"); + } catch (error) { + log.debug("Account-based auto-detect failed", error); + return null; + } +} + +/** + * Whether an interactive org/project picker may be shown. When `override` is + * provided (e.g. derived from `--json`) it wins; otherwise the picker is only + * offered when **both** stdin and stdout are TTYs, so piped or redirected + * invocations never block on a prompt or corrupt machine-readable output. + */ +function canPromptForTarget(override?: boolean): boolean { + if (override !== undefined) { + return override; + } + return isatty(0) && isatty(1); +} + +/** + * Prompt the user (consola) to pick a value from a list, returning the selected + * value or `null` if cancelled. A single-element list is auto-selected without + * prompting. + */ +async function promptSelect( + message: string, + options: { label: string; value: string }[] +): Promise { + const sole = options[0]; + if (options.length === 1 && sole) { + return sole.value; + } + const response = await log.prompt(message, { type: "select", options }); + // consola returns Symbol(clack:cancel) on Ctrl+C — a truthy non-string. + return typeof response === "string" ? response : null; +} + +/** + * Interactively resolve an org/project for authenticated users and persist the + * choice as the default. Returns `null` when not authenticated, when the + * account has no accessible orgs/projects, or when the user cancels. + */ +async function promptForOrgProject(): Promise { + if (!isAuthenticated()) { + return null; + } + + let orgs: SentryOrganization[]; + try { + orgs = await listOrganizations(); + } catch (error) { + log.debug("Failed to list organizations for interactive picker", error); + return null; + } + if (orgs.length === 0) { + return null; + } + + const orgSlug = await promptSelect( + "Select an organization:", + orgs.map((o) => ({ label: o.name || o.slug, value: o.slug })) + ); + if (!orgSlug) { + return null; + } + const org = orgs.find((o) => o.slug === orgSlug); + if (!org) { + return null; + } + + let projects: SentryProject[]; + try { + projects = await listProjects(orgSlug); + } catch (error) { + log.debug("Failed to list projects for interactive picker", error); + return null; + } + if (projects.length === 0) { + throw new ResolutionError( + `Organization '${orgSlug}'`, + "has no accessible projects", + `sentry project list ${orgSlug}/` + ); + } + + const projectSlug = await promptSelect( + "Select a project:", + projects.map((p) => ({ label: p.name || p.slug, value: p.slug })) + ); + if (!projectSlug) { + return null; + } + const project = projects.find((p) => p.slug === projectSlug); + if (!project) { + return null; + } + + // Persist as the default so the user is not prompted again. Best-effort: a + // read-only DB must not fail the command after a successful selection. + try { + setDefaultOrganization(orgSlug); + setDefaultProject(projectSlug); + log.info( + `Saved ${orgSlug}/${projectSlug} as your default. Change it with: sentry cli defaults org ` + ); + } catch (error) { + log.debug("Failed to persist selected org/project as default", error); + } + + return toAccountTarget(org, project, "interactive selection"); +} + +/** + * Build an actionable {@link ContextError} for authenticated users whose + * org/project could not be auto-detected. Lists the accessible organizations so + * the next command is copy-pasteable, instead of the generic "run org list" + * guidance shown to logged-out users. + */ +async function buildAccountContextError( + usageHint: string +): Promise { + try { + const orgs = await listOrganizations(); + if (orgs.length > 0) { + const shown = orgs.slice(0, 10).map((o) => ` ${o.slug}`); + const more = + orgs.length > shown.length + ? ` …and ${orgs.length - shown.length} more` + : ""; + const orgList = [...shown, more].filter(Boolean).join("\n"); + return new ContextError("Organization and project", usageHint, [ + `Specify one of your organizations:\n${orgList}`, + "List a project: sentry project list /", + "Save a default: sentry cli defaults org ", + ]); + } + } catch (error) { + log.debug("Failed to list organizations for context error", error); + } + // Fall back to the standard auto-detect guidance. + return new ContextError("Organization and project", usageHint); +} + +/** + * Guide the user to a target after auto-detection has already failed. + * + * Call this only when `resolveOrgAndProject` (or `resolveAllTargets`) returned + * no target. In order it: + * + * 1. Offers an interactive org/project picker (TTY only; saves the choice as a + * default), then + * 2. For authenticated users, throws a {@link ContextError} listing their + * accessible organizations, otherwise + * 3. Throws the standard auto-detect {@link ContextError}. + * + * Kept separate from {@link resolveOrgAndProject} so callers that mock the + * resolver in tests (and rely on its `null` return, e.g. event view's cross-org + * search) keep working — this is only invoked on the explicit failure path. + * + * @throws {ContextError} When the target cannot be resolved or chosen. + */ +export async function guideOrgProjectFailure( + options: Pick +): Promise { + const usageHint = options.usageHint ?? "sentry /"; + + if (canPromptForTarget(options.interactive)) { + const picked = await promptForOrgProject(); + if (picked) { + return withTelemetryContext(picked); + } + } + + if (isAuthenticated()) { + throw await buildAccountContextError(usageHint); + } + throw new ContextError("Organization and project", usageHint); +} + +/** + * Resolve an org/project target, guiding the user when auto-detection fails. + * + * Drop-in replacement for the `resolveOrgAndProject(...)` + "throw + * `ContextError` on null" pattern, combining {@link resolveOrgAndProject} with + * {@link guideOrgProjectFailure}. + * + * @throws {ContextError} When the target cannot be resolved or chosen. + */ +export async function resolveOrgProjectOrGuide( + options: ResolveOptions +): Promise { + const resolved = await resolveOrgAndProject(options); + return resolved ?? (await guideOrgProjectFailure(options)); +} + /** * Resolve organization only from multiple sources. * @@ -1726,14 +1996,10 @@ export async function resolveOrgProjectTarget( } case "auto-detect": { - // resolveOrgAndProject already sets telemetry context - const resolved = await resolveOrgAndProject({ - cwd, - usageHint, - }); - if (!resolved) { - throw new ContextError("Organization and project", usageHint); - } + // resolveOrgProjectOrGuide sets telemetry context and, when + // auto-detection fails, offers an interactive picker (TTY) or an + // actionable error listing the user's accessible orgs. + const resolved = await resolveOrgProjectOrGuide({ cwd, usageHint }); return { org: resolved.org, project: resolved.project }; } @@ -1821,6 +2087,12 @@ export async function resolveTargetsFromParsedArg( switch (parsed.type) { case "auto-detect": { const result = await resolveAllTargets({ cwd, usageHint }); + if (result.targets.length === 0) { + // Auto-detection found nothing. Offer an interactive picker (TTY) or an + // actionable error listing the user's accessible orgs, and let a + // single-org/single-project account resolve automatically (CLI-3B). + result.targets = [await resolveOrgProjectOrGuide({ cwd, usageHint })]; + } if (enrichProjectIds) { result.targets = await Promise.all( result.targets.map(async (t) => { diff --git a/src/lib/telemetry.ts b/src/lib/telemetry.ts index e29fb3f0f..694bdac1a 100644 --- a/src/lib/telemetry.ts +++ b/src/lib/telemetry.ts @@ -226,15 +226,15 @@ export async function withTelemetry( } ); } catch (e) { - // Route through reportCliError so silencing (OutputError, ContextError, - // expected-auth AuthError, 401–499 ApiError) and fingerprint normalization - // are applied consistently. Silenced errors emit a `cli.error.silenced` - // metric + optional structured log instead of creating a Sentry issue. + // Route through reportCliError so silencing (OutputError, expected-auth + // AuthError, 401–499 ApiError) and fingerprint normalization are applied + // consistently. Silenced errors emit a `cli.error.silenced` metric + + // optional structured log instead of creating a Sentry issue. (ContextError + // is intentionally NOT silenced — see classifySilenced.) reportCliError(e); // Only mark session crashed for errors that weren't silenced. - // Silenced errors (OutputError, ContextError, expected AuthError, user 4xx - // ApiError) are expected states — marking them crashed would skew - // release-health. + // Silenced errors (OutputError, expected AuthError, user 4xx ApiError) are + // expected states — marking them crashed would skew release-health. if (!classifySilenced(e)) { markSessionCrashed(); } diff --git a/src/lib/trace-target.ts b/src/lib/trace-target.ts index 5b2cbeee8..04f27444b 100644 --- a/src/lib/trace-target.ts +++ b/src/lib/trace-target.ts @@ -25,6 +25,7 @@ import { } from "./hex-id-recovery.js"; import { logger } from "./logger.js"; import { + guideOrgProjectFailure, resolveOrg, resolveOrgAndProject, resolveProjectBySlug, @@ -444,14 +445,12 @@ export async function resolveTraceOrgProject( ]); case "auto-detect": { - // resolveOrgAndProject already sets telemetry context - const resolved = await resolveOrgAndProject({ - cwd, - usageHint, - }); - if (!resolved) { - throw new ContextError("Organization and project", usageHint); - } + // resolveOrgAndProject already sets telemetry context. On a failed + // auto-detect, guideOrgProjectFailure offers an interactive picker (TTY) + // or an actionable error listing the user's accessible orgs. + const resolved = + (await resolveOrgAndProject({ cwd, usageHint })) ?? + (await guideOrgProjectFailure({ usageHint })); return { traceId: parsed.traceId, org: resolved.org, diff --git a/test/commands/trace/view.func.test.ts b/test/commands/trace/view.func.test.ts index 151a1a1eb..1d575893c 100644 --- a/test/commands/trace/view.func.test.ts +++ b/test/commands/trace/view.func.test.ts @@ -326,8 +326,13 @@ describe("viewCommand.func", () => { ).rejects.toThrow(ContextError); }); - test("throws ContextError when auto-detect returns null", async () => { + test("throws ContextError when auto-detect cannot resolve a target", async () => { + // Auto-detect returns null, then guideOrgProjectFailure (picker/enriched + // error) also cannot resolve a target. resolveOrgAndProjectSpy.mockResolvedValue(null); + vi.spyOn(resolveTarget, "guideOrgProjectFailure").mockRejectedValue( + new ContextError("Organization and project", "sentry trace view ") + ); const { context } = createMockContext(); const func = await viewCommand.loader(); diff --git a/test/lib/error-reporting.test.ts b/test/lib/error-reporting.test.ts index 75582ded8..693776db1 100644 --- a/test/lib/error-reporting.test.ts +++ b/test/lib/error-reporting.test.ts @@ -304,8 +304,10 @@ describe("classifySilenced", () => { test.each([ ["auto-detect failure", new ContextError("Organization and project", "x")], ["missing ID", new ContextError("Event ID", "sentry event view ", [])], - ])("silences ContextError (%s)", (_label, err) => { - expect(classifySilenced(err)).toBe("context_missing"); + ])("does NOT silence ContextError (%s)", (_label, err) => { + // ContextError volume is the signal driving auto-detection/UX fixes + // (single-org auto-select, interactive picker), so it must stay captured. + expect(classifySilenced(err)).toBeNull(); }); test.each([ @@ -465,20 +467,18 @@ describe("reportCliError integration", () => { return { tags, contexts }; } - test("silences ContextError and emits metric with resource", () => { + test("captures ContextError (no longer silenced) so its volume stays visible", () => { reportCliError( new ContextError("Organization and project", "sentry org view ") ); - expect(captureSpy).not.toHaveBeenCalled(); - expect(metricSpy).toHaveBeenCalledWith( + // CLI-3B: ContextError is the signal for auto-detection/UX improvements, so + // it is captured rather than dropped to a silenced-metric. + expect(captureSpy).toHaveBeenCalled(); + expect(metricSpy).not.toHaveBeenCalledWith( "cli.error.silenced", 1, expect.objectContaining({ - attributes: expect.objectContaining({ - error_class: "ContextError", - reason: "context_missing", - resource: "Organization and project", - }), + attributes: expect.objectContaining({ error_class: "ContextError" }), }) ); }); diff --git a/test/lib/resolve-target-listing.test.ts b/test/lib/resolve-target-listing.test.ts index 191b4fd2d..f0f961a78 100644 --- a/test/lib/resolve-target-listing.test.ts +++ b/test/lib/resolve-target-listing.test.ts @@ -36,6 +36,20 @@ vi.mock("../../src/lib/db/defaults.js", async (importOriginal) => { // biome-ignore lint/performance/noNamespaceImport: needed for spyOn mocking import * as defaults from "../../src/lib/db/defaults.js"; + +vi.mock("../../src/lib/db/auth.js", async (importOriginal) => { + const actual = + await importOriginal(); + return Object.fromEntries( + Object.entries(actual).map(([k, v]) => [ + k, + typeof v === "function" ? vi.fn(v) : v, + ]) + ); +}); + +// biome-ignore lint/performance/noNamespaceImport: needed for spyOn mocking +import * as auth from "../../src/lib/db/auth.js"; import { setOrgRegion, setOrgRegions } from "../../src/lib/db/regions.js"; import { ContextError, ResolutionError } from "../../src/lib/errors.js"; @@ -54,6 +68,7 @@ vi.mock("../../src/lib/resolve-target.js", async (importOriginal) => { import * as resolveTargetModule from "../../src/lib/resolve-target.js"; import { resolveOrgProjectFromArg, + resolveOrgProjectOrGuide, resolveOrgProjectTarget, resolveOrgsForListing, resolveTargetsFromParsedArg, @@ -166,6 +181,7 @@ describe("resolveOrgsForListing", () => { describe("resolveOrgProjectTarget", () => { let findProjectsBySlugSpy: ReturnType; let resolveOrgAndProjectSpy: ReturnType; + let listOrganizationsSpy: ReturnType; beforeEach(() => { findProjectsBySlugSpy = vi.spyOn(apiClient, "findProjectsBySlug"); @@ -173,6 +189,11 @@ describe("resolveOrgProjectTarget", () => { resolveTargetModule, "resolveOrgAndProject" ); + // No accessible orgs by default so the authenticated account fallback in + // resolveOrgProjectOrGuide can't resolve and makes no real HTTP calls. + listOrganizationsSpy = vi + .spyOn(apiClient, "listOrganizations") + .mockResolvedValue([]); // Pre-populate org region cache so resolveEffectiveOrg doesn't fetch setOrgRegion("my-org", DEFAULT_SENTRY_URL); }); @@ -180,6 +201,7 @@ describe("resolveOrgProjectTarget", () => { afterEach(() => { findProjectsBySlugSpy.mockRestore(); resolveOrgAndProjectSpy.mockRestore(); + listOrganizationsSpy.mockRestore(); }); test("returns org and project for explicit type", async () => { @@ -463,3 +485,90 @@ describe("resolveTargetsFromParsedArg", () => { }); }); }); + +// --------------------------------------------------------------------------- +// resolveOrgProjectOrGuide (authenticated account fallback) +// --------------------------------------------------------------------------- + +describe("resolveOrgProjectOrGuide", () => { + let isAuthenticatedSpy: ReturnType; + let listOrganizationsSpy: ReturnType; + let listProjectsSpy: ReturnType; + let getDefaultOrgSpy: ReturnType; + let getDefaultProjectSpy: ReturnType; + + beforeEach(() => { + isAuthenticatedSpy = vi.spyOn(auth, "isAuthenticated"); + listOrganizationsSpy = vi.spyOn(apiClient, "listOrganizations"); + listProjectsSpy = vi.spyOn(apiClient, "listProjects"); + // Force the cascade to miss config defaults so it reaches the account + // fallback (CWD has no DSN/config either). + getDefaultOrgSpy = vi + .spyOn(defaults, "getDefaultOrganization") + .mockReturnValue(null); + getDefaultProjectSpy = vi + .spyOn(defaults, "getDefaultProject") + .mockReturnValue(null); + }); + + afterEach(() => { + isAuthenticatedSpy.mockRestore(); + listOrganizationsSpy.mockRestore(); + listProjectsSpy.mockRestore(); + getDefaultOrgSpy.mockRestore(); + getDefaultProjectSpy.mockRestore(); + }); + + test("auto-selects the sole org/project for a single-org account", async () => { + isAuthenticatedSpy.mockReturnValue(true); + listOrganizationsSpy.mockResolvedValue([ + { slug: "solo", name: "Solo Inc" }, + ]); + listProjectsSpy.mockResolvedValue([ + { id: "42", slug: "only-proj", name: "Only Project" }, + ]); + + const result = await resolveOrgProjectOrGuide({ + cwd: CWD, + usageHint: "sentry issue list /", + interactive: false, + }); + + expect(result).toMatchObject({ org: "solo", project: "only-proj" }); + }); + + test("throws a ContextError listing accessible orgs for a multi-org account", async () => { + isAuthenticatedSpy.mockReturnValue(true); + listOrganizationsSpy.mockResolvedValue([ + { slug: "org-a", name: "Org A" }, + { slug: "org-b", name: "Org B" }, + ]); + + try { + await resolveOrgProjectOrGuide({ + cwd: CWD, + usageHint: "sentry issue list /", + interactive: false, + }); + expect.unreachable("should have thrown"); + } catch (err) { + expect(err).toBeInstanceOf(ContextError); + expect((err as Error).message).toContain("org-a"); + expect((err as Error).message).toContain("org-b"); + } + }); + + test("throws a generic ContextError when unauthenticated", async () => { + isAuthenticatedSpy.mockReturnValue(false); + + await expect( + resolveOrgProjectOrGuide({ + cwd: CWD, + usageHint: "sentry issue list /", + interactive: false, + }) + ).rejects.toBeInstanceOf(ContextError); + // Unauthenticated path must not attempt to list orgs. + expect(listOrganizationsSpy).not.toHaveBeenCalled(); + }); +}); diff --git a/test/lib/telemetry.test.ts b/test/lib/telemetry.test.ts index f493243f2..e93008d5c 100644 --- a/test/lib/telemetry.test.ts +++ b/test/lib/telemetry.test.ts @@ -289,7 +289,7 @@ describe("withTelemetry", () => { captureSpy.mockRestore(); }); - test("silences ContextError (missing user input, not a crash)", async () => { + test("captures ContextError so its volume stays visible (CLI-3B)", async () => { const captureSpy = vi.spyOn(Sentry, "captureException"); const metricSpy = vi.spyOn(Sentry.metrics, "distribution"); const { ContextError } = await import("../../src/lib/errors.js"); @@ -302,13 +302,13 @@ describe("withTelemetry", () => { throw error; }) ).rejects.toThrow(error); - // ContextError is an expected "missing input" error — not reported as a - // Sentry issue; volume is preserved via the cli.error.silenced metric. - expect(captureSpy).not.toHaveBeenCalled(); + // ContextError is no longer silenced — its volume drives auto-detection + // and UX improvements, so it must be reported to Sentry. + expect(captureSpy).toHaveBeenCalled(); const silencedCall = metricSpy.mock.calls.find( (c) => c[0] === "cli.error.silenced" ); - expect(silencedCall).toBeDefined(); + expect(silencedCall).toBeUndefined(); captureSpy.mockRestore(); metricSpy.mockRestore(); }); From 7abe866db6438f53ea3c0f99e0076b99b48f4c38 Mon Sep 17 00:00:00 2001 From: Burak Yigit Kaya Date: Sat, 27 Jun 2026 15:23:52 +0000 Subject: [PATCH 2/2] =?UTF-8?q?fix(resolve):=20address=20review=20?= =?UTF-8?q?=E2=80=94=20preserve=20skipped-DSN=20error,=20suppress=20prompt?= =?UTF-8?q?s=20in=20JSON=20mode?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Cursor Bugbot review on #1155: - HIGH: when auto-detect found DSNs but none resolved (self-hosted/no access), resolveTargetsFromParsedArg now preserves the empty result + skippedSelfHosted instead of calling the guide, so issue list surfaces the inaccessible-DSN ContextError rather than silently resolving a different org/project. - MEDIUM: added a process-wide interactive-prompts gate (interactive-prompts.ts) that the command wrapper disables in JSON mode; canPromptForTarget now consults it, so --json runs never block on or interleave with the org/project picker. - LOW (Seer): documented that guideOrgProjectFailure can also throw ResolutionError (selected org has no accessible projects). --- src/lib/command.ts | 5 +++++ src/lib/interactive-prompts.ts | 29 ++++++++++++++++++++++++++++ src/lib/resolve-target.ts | 25 ++++++++++++++++-------- test/lib/interactive-prompts.test.ts | 23 ++++++++++++++++++++++ 4 files changed, 74 insertions(+), 8 deletions(-) create mode 100644 src/lib/interactive-prompts.ts create mode 100644 test/lib/interactive-prompts.test.ts diff --git a/src/lib/command.ts b/src/lib/command.ts index f9554304c..f1a2e7580 100644 --- a/src/lib/command.ts +++ b/src/lib/command.ts @@ -57,6 +57,7 @@ import { } from "./formatters/output.js"; import { isPlainOutput } from "./formatters/plain-detect.js"; import { GLOBAL_FLAGS } from "./global-flags.js"; +import { setInteractivePromptsAllowed } from "./interactive-prompts.js"; import { LOG_LEVEL_NAMES, type LogLevelName, @@ -704,6 +705,10 @@ export function buildCommand< } } + // Suppress interactive prompts (e.g. the org/project picker) in JSON mode so + // a prompt never blocks a scripted run or interleaves with stdout JSON. + setInteractivePromptsAllowed(!cleanFlags.json); + const stdout = (this as unknown as { stdout: Writer }).stdout; // Reset per-invocation state diff --git a/src/lib/interactive-prompts.ts b/src/lib/interactive-prompts.ts new file mode 100644 index 000000000..e7ef9eec0 --- /dev/null +++ b/src/lib/interactive-prompts.ts @@ -0,0 +1,29 @@ +/** + * Process-wide gate for interactive prompts. + * + * Commands that emit machine-readable output (`--json`, or `SENTRY_OUTPUT_FORMAT=json`) + * must never block on an interactive prompt or interleave prompt UI with JSON on + * stdout. The command wrapper disables prompts for such runs via + * {@link setInteractivePromptsAllowed}, and prompt sites (e.g. the org/project + * picker in `resolve-target.ts`) consult {@link interactivePromptsAllowed} + * before showing a prompt. + * + * Defaults to `true` so code paths that run outside the command wrapper (tests, + * library callers) fall back to their own TTY checks rather than being silently + * blocked. + */ + +let allowed = true; + +/** + * Enable or disable interactive prompts for the current command run. Called by + * the command wrapper with `false` when JSON output is active. + */ +export function setInteractivePromptsAllowed(value: boolean): void { + allowed = value; +} + +/** Whether interactive prompts are currently permitted. */ +export function interactivePromptsAllowed(): boolean { + return allowed; +} diff --git a/src/lib/resolve-target.ts b/src/lib/resolve-target.ts index cc1a9aa6d..d6c4c84df 100644 --- a/src/lib/resolve-target.ts +++ b/src/lib/resolve-target.ts @@ -65,6 +65,7 @@ import { withAuthGuard, } from "./errors.js"; import { fuzzyMatch } from "./fuzzy.js"; +import { interactivePromptsAllowed } from "./interactive-prompts.js"; import { logger } from "./logger.js"; import { resolveEffectiveOrg } from "./region.js"; import { CONFIG_FILENAME, loadSentryCliRc } from "./sentryclirc.js"; @@ -1410,15 +1411,16 @@ async function resolveSoleAccountTarget(): Promise { /** * Whether an interactive org/project picker may be shown. When `override` is - * provided (e.g. derived from `--json`) it wins; otherwise the picker is only - * offered when **both** stdin and stdout are TTYs, so piped or redirected - * invocations never block on a prompt or corrupt machine-readable output. + * provided (e.g. derived from `--json`) it wins. Otherwise the picker is only + * offered when JSON output is not active (so it never blocks a scripted run or + * corrupts stdout JSON) **and** both stdin and stdout are TTYs (so piped or + * redirected invocations never block on a prompt). */ function canPromptForTarget(override?: boolean): boolean { if (override !== undefined) { return override; } - return isatty(0) && isatty(1); + return interactivePromptsAllowed() && isatty(0) && isatty(1); } /** @@ -1562,6 +1564,8 @@ async function buildAccountContextError( * search) keep working — this is only invoked on the explicit failure path. * * @throws {ContextError} When the target cannot be resolved or chosen. + * @throws {ResolutionError} When the interactively-selected organization has no + * accessible projects. */ export async function guideOrgProjectFailure( options: Pick @@ -2087,10 +2091,15 @@ export async function resolveTargetsFromParsedArg( switch (parsed.type) { case "auto-detect": { const result = await resolveAllTargets({ cwd, usageHint }); - if (result.targets.length === 0) { - // Auto-detection found nothing. Offer an interactive picker (TTY) or an - // actionable error listing the user's accessible orgs, and let a - // single-org/single-project account resolve automatically (CLI-3B). + // Only guide when there is genuinely no context. If DSNs WERE found but + // could not be resolved (self-hosted / no access), `skippedSelfHosted` is + // set — preserve the empty result so the caller surfaces the + // inaccessible-DSN error instead of silently resolving a different + // org/project (which would mask the real problem). + if (result.targets.length === 0 && !result.skippedSelfHosted) { + // Offer an interactive picker (TTY) or an actionable error listing the + // user's accessible orgs, and let a single-org/single-project account + // resolve automatically (CLI-3B). result.targets = [await resolveOrgProjectOrGuide({ cwd, usageHint })]; } if (enrichProjectIds) { diff --git a/test/lib/interactive-prompts.test.ts b/test/lib/interactive-prompts.test.ts new file mode 100644 index 000000000..cae56abf6 --- /dev/null +++ b/test/lib/interactive-prompts.test.ts @@ -0,0 +1,23 @@ +import { afterEach, describe, expect, test } from "vitest"; +import { + interactivePromptsAllowed, + setInteractivePromptsAllowed, +} from "../../src/lib/interactive-prompts.js"; + +describe("interactive-prompts gate", () => { + afterEach(() => { + // Restore the default so other tests/files aren't affected. + setInteractivePromptsAllowed(true); + }); + + test("defaults to allowed so non-wrapper callers fall back to their own checks", () => { + expect(interactivePromptsAllowed()).toBe(true); + }); + + test("reflects the value set by the command wrapper (JSON mode disables it)", () => { + setInteractivePromptsAllowed(false); + expect(interactivePromptsAllowed()).toBe(false); + setInteractivePromptsAllowed(true); + expect(interactivePromptsAllowed()).toBe(true); + }); +});