Skip to content

Commit 48f5e63

Browse files
authored
fix(resolve): guide users to org/project instead of silencing the auto-detect error (#1155)
## Why Follow-up to #1149 based on review feedback: **"Could not auto-detect organization and project" was being silenced, which hid the signal AND skipped the underlying UX fix.** We keep these errors around to learn where users get stuck — so the right move is to help users succeed *and* keep the error visible to measure impact. CLI-3B: ~2000 users. ## What changed **1. Sole-account auto-select (helps every command, zero risk)** `resolveOrgAndProject` gains a final cascade step: when an authenticated account has exactly **one** accessible org with exactly **one** project, that pair is the only possible answer, so it's returned instead of `null`. This is strictly additive — callers that depend on a `null` return (notably `event view`'s cross-org search) are unaffected, since it only ever turns a `null` into a uniquely-determined target. **2. Interactive picker (TTY)** On a failed auto-detect, `guideOrgProjectFailure` prompts the user to choose an org then a project, and saves the choice as the default (so they're not asked again). Gated on **both** stdin and stdout being TTYs, so piped/`--json` invocations never block on a prompt. **3. Enriched error** Authenticated users who still can't be resolved get a `ContextError` listing their accessible orgs (copy-pasteable), instead of the generic "run `sentry org list`" guidance shown to logged-out users. **4. Un-silence `ContextError`** Reverted the `classifySilenced` `ContextError` branch (and the now-dead `recordSilencedError` resource attribute) so its volume stays visible — that volume is the signal for whether the UX changes above actually work. ## Wiring `guideOrgProjectFailure` is invoked on the auto-detect failure path of `resolveOrgProjectTarget`, `resolveTargetsFromParsedArg` (backs `issue list`), and `trace-target`. The fallback is kept **separate** from `resolveOrgAndProject` (rather than wrapping it) so command tests that mock the resolver — and rely on its `null` return — keep working; the guide only runs on the explicit failure path. Upload commands (`sourcemap`/`debug-files`/`proguard`/`code-mappings`/`dart-symbol-map`) get the sole-account auto-select for free via `resolveOrgAndProject`; their interactive-picker/enriched-error wiring can follow up (they're usually non-interactive CI runs with DSN/config context). ## Tests - `resolveOrgProjectOrGuide`: sole-org/project auto-select, multi-org enriched `ContextError`, unauthenticated generic `ContextError` (no org-list call). - Updated `classifySilenced` / `recordSilencedError` / telemetry tests to assert `ContextError` is now **captured**, not silenced. - Updated trace/span resolver tests for the new fallback path. - Full `test/lib` + `test/commands` suites green (7964 passing); lint + typecheck clean.
1 parent 0725a14 commit 48f5e63

12 files changed

Lines changed: 640 additions & 65 deletions

.lore.md

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -474,9 +474,15 @@
474474
<!-- lore:019e5ae3-dee8-7d8c-97cb-69a50bf223ed -->
475475
* **Always create a dedicated branch when updating fossilize versions**: When a new version of fossilize is released, always create a branch named \`chore/fossilize-{version}\` tracking origin/main, update the dependency, remove any functionality now handled natively by fossilize (e.g., \`stripCachedNodeBinaries()\` removed in 0.7.0), verify the build succeeds, then commit with \`chore: update fossilize to X.Y.Z\`. Follow this exact pattern: branch → update dep → remove superseded code → build verify → commit → PR.
476476

477+
<!-- lore:019f60d4-72d1-703c-9c8f-ee44d9e61217 -->
478+
* **Always Directs to Resolve Merge Conflicts and Ensure Compatibility**: The user consistently directs the assistant to resolve merge conflicts, ensure compatibility, and verify code changes before merging. This includes rebasing branches, resolving conflicts, and verifying code changes. The user also emphasizes the importance of backwards and forwards compatibility during transitions and releases.
479+
477480
<!-- lore:019e51da-a4fd-75f7-85e9-6627f83e74f4 -->
478481
* **Always explore e2e test infrastructure thoroughly before debugging or modifying tests**: When approaching e2e test work, always explore the full infrastructure before making changes: \`test/e2e/\` (14 files: api, auth, bundle, completion, delta-upgrade, event, issue, library, log, multiregion, project, skill-eval, telemetry-exit, trace), \`test/fixture.ts\` (getCliCommand, runCli, createE2EContext), \`test/helpers.ts\` (useTestConfigDir, useEnvSandbox, resetHostScopingState, mintSntrysToken, extractFetchUrl), \`test/mocks/\` (server.ts, routes.js, multiregion.ts), \`src/bin.ts\`, \`src/cli.ts\`. Key: \`getCliCommand()\` returns \`\[SENTRY\_CLI\_BINARY]\` if set, else \`\[process.execPath, 'run', 'src/bin.ts']\`. \`createE2EContext.run()\` sets \`SENTRY\_AUTH\_TOKEN: ''\`, \`SENTRY\_TOKEN: ''\`, \`SENTRY\_CLI\_NO\_TELEMETRY: '1'\`. \`test:e2e\` runs without \`--isolate --parallel\`. Map full infrastructure before proposing fixes.
479482

483+
<!-- lore:019f60d5-d995-782e-a4e1-5fe53abb792c -->
484+
* **Always fetchable from the base repo with github**: User stated always fetchable from the base repo with github.
485+
480486
<!-- lore:019f3852-932f-7ff6-9192-c9efc7457b18 -->
481487
* **Always fix root causes and add preventive safeguards together**: When the user identifies a recurring or accumulated problem (e.g., 61 stale PR previews bloating gh-pages), they direct: (1) fix the immediate root cause, (2) port or add automation to prevent recurrence (e.g., on-PR-close cleanup), and (3) add a scheduled safety-net sweep as a belt-and-suspenders measure. Similarly with code bugs: they want the bug fixed AND the underlying gap addressed. Never just patch the symptom — always pair the fix with structural prevention. This applies to CI/CD workflows, dependency issues, and code correctness problems alike.
482488

@@ -519,6 +525,9 @@
519525
<!-- lore:019f60a3-616e-77a1-bbd2-7376005e41df -->
520526
* **Always requests a thorough review before merging PRs**: The user consistently requests detailed reviews of pull requests (PRs) before merging them. This includes scrutinizing changes, verifying git status is clean, and checking test coverage. The user also focuses on specific areas such as error handling, edge cases, and code correctness. Additionally, the user often performs multiple review passes, requesting objective and critical reviews to ensure high-quality code changes.
521527

528+
<!-- lore:019f60d4-cf8a-7213-a62e-41e6b23b80e0 -->
529+
* **Always requests exhaustive and detailed research**: The user consistently requests thorough and precise investigations into specific aspects of the codebase, infrastructure, or related tools. They emphasize the need for exhaustive research, often specifying that no code should be written and that findings should be detailed, including file paths, line numbers, and config values. This pattern is evident across multiple sessions, where the user asks for detailed inventories, code reviews, and investigations into various components such as release infrastructure, build processes, and versioning mechanisms.
530+
522531
<!-- lore:019e56f4-d4d9-7d15-b729-2f11a05dd23d -->
523532
* **Always research technical approaches thoroughly before implementation**: When facing a significant technical decision or migration, the user consistently requests deep research into multiple approaches before writing any code. This includes: fetching specific upstream documentation/source files (e.g., BUILDING.md, configure.py), identifying concrete flags/options, estimating build times, and evaluating cross-compilation feasibility. The user wants tradeoffs between paths laid out explicitly. Only after research is complete does implementation begin. When presenting research, include specific flags, URLs, estimated costs (time/size), and platform constraints.
524533

@@ -597,6 +606,12 @@
597606
<!-- lore:019f609c-b01f-79db-ad40-f0e299775799 -->
598607
* **Never exceed the terminal width (it would wrap into a broken layout otherwise)**: Never exceed the terminal width (it would wrap into a broken layout otherwise). This is a standing directive to ensure readable output.
599608

609+
<!-- lore:019f60d6-3f67-78ee-9e4a-ba550489b4d2 -->
610+
* **Never target the same path**: User stated never target the same path.
611+
612+
<!-- lore:019f60d6-b41b-7bc0-9b92-3c9e391be71b -->
613+
* **Never throws - errors are caught and reported to Sentry**: User stated Never throws - errors are caught and reported to Sentry.
614+
600615
<!-- lore:019efaea-f5bc-7140-abd0-996eb16f4d5d -->
601616
* **Never treat incomplete operations as successful — always surface silent failures**: When reviewing code, the user explicitly asserts that incomplete operations must never report success. Examples from \`debug-files upload\`: \`not\_found\` after deadline is a failure (exit 1), not success (exit 0). Symlink cycles must not hang silently. Missing requested IDs must be surfaced. Any failure path that silently exits 0 or produces no diagnostic is a blocker.
602617

src/lib/command.ts

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -57,6 +57,7 @@ import {
5757
} from "./formatters/output.js";
5858
import { isPlainOutput } from "./formatters/plain-detect.js";
5959
import { GLOBAL_FLAGS } from "./global-flags.js";
60+
import { setInteractivePromptsAllowed } from "./interactive-prompts.js";
6061
import {
6162
LOG_LEVEL_NAMES,
6263
type LogLevelName,
@@ -704,6 +705,10 @@ export function buildCommand<
704705
}
705706
}
706707

708+
// Suppress interactive prompts (e.g. the org/project picker) in JSON mode so
709+
// a prompt never blocks a scripted run or interleaves with stdout JSON.
710+
setInteractivePromptsAllowed(!cleanFlags.json);
711+
707712
const stdout = (this as unknown as { stdout: Writer }).stdout;
708713

709714
// Reset per-invocation state

src/lib/error-reporting.ts

Lines changed: 17 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -4,10 +4,15 @@
44
* Provides two things:
55
*
66
* 1. **Silencing rules** — `OutputError`, network failures (offline/DNS/proxy),
7-
* `ContextError` (a required value the user omitted), `AuthError` (expected
8-
* auth states the user must act on), 401–499 `ApiError`, and 400 `ApiError`s
9-
* that report an unparseable user search query are not sent to Sentry as
10-
* issues. A `cli.error.silenced` metric preserves volume + user/org context.
7+
* `AuthError` (expected auth states the user must act on), 401–499 `ApiError`,
8+
* and 400 `ApiError`s that report an unparseable user search query are not
9+
* sent to Sentry as issues. A `cli.error.silenced` metric preserves volume +
10+
* user/org context.
11+
*
12+
* `ContextError` (a required value the user omitted) is deliberately NOT
13+
* silenced: its volume is the signal driving auto-detection/UX improvements
14+
* (e.g. single-org auto-select, the interactive picker), so it must stay
15+
* visible (CLI-3B).
1116
*
1217
* 2. **Grouping tags** — enriches every error event with `cli_error.*` tags
1318
* that Sentry's server-side fingerprint rules use for stable grouping.
@@ -47,7 +52,6 @@ import {
4752
*/
4853
type SilenceReason =
4954
| "output_error"
50-
| "context_missing"
5155
| "auth_expected"
5256
| "api_user_error"
5357
| "network_error";
@@ -70,16 +74,14 @@ export function classifySilenced(error: unknown): SilenceReason | null {
7074
if (isNetworkError(error)) {
7175
return "network_error";
7276
}
73-
// A ContextError always means the user omitted a required value (no
74-
// org/project could be resolved, a required ID was not provided, etc.). It is
75-
// never a CLI bug — unlike ResolutionError, where a *provided* value that
76-
// can't be matched may signal a product/access issue worth observing. There
77-
// is nothing per-instance to investigate, so silence the whole class; the
78-
// `cli.error.silenced` metric (keyed by `error_class` + `resource`) preserves
79-
// the volume and which value was missing. (CLI-3B: ~2000 users.)
80-
if (error instanceof ContextError) {
81-
return "context_missing";
82-
}
77+
// A ContextError means the user omitted a required value (no org/project
78+
// could be resolved, a required ID was not provided, etc.). It is NOT
79+
// silenced: the volume of these is the signal we use to drive auto-detection
80+
// and UX improvements (single-org auto-select, the interactive picker, the
81+
// enriched error). Silencing it hid that signal AND skipped the fix, so it
82+
// stays captured (CLI-3B). The accompanying `resolveOrgProjectOrGuide`
83+
// changes aim to drive this volume down by helping users succeed instead.
84+
//
8385
// All AuthError reasons are expected auth states the user must act on, not
8486
// CLI bugs: `not_authenticated` (no token), `expired` (token aged out), and
8587
// `invalid` (a bad/insufficiently-scoped token the user supplied). `invalid`
@@ -112,12 +114,6 @@ function recordSilencedError(error: unknown, reason: SilenceReason): void {
112114
if (error instanceof AuthError) {
113115
attributes.auth_reason = error.reason;
114116
}
115-
// Preserve which required value was missing so the silenced-volume metric
116-
// keeps sub-grouping context (e.g. "Organization and project" vs "Event ID").
117-
// ContextError resources are a small fixed set, so cardinality stays low.
118-
if (error instanceof ContextError) {
119-
attributes.resource = error.resource;
120-
}
121117

122118
try {
123119
Sentry.metrics.distribution("cli.error.silenced", 1, { attributes });

src/lib/interactive-prompts.ts

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,29 @@
1+
/**
2+
* Process-wide gate for interactive prompts.
3+
*
4+
* Commands that emit machine-readable output (`--json`, or `SENTRY_OUTPUT_FORMAT=json`)
5+
* must never block on an interactive prompt or interleave prompt UI with JSON on
6+
* stdout. The command wrapper disables prompts for such runs via
7+
* {@link setInteractivePromptsAllowed}, and prompt sites (e.g. the org/project
8+
* picker in `resolve-target.ts`) consult {@link interactivePromptsAllowed}
9+
* before showing a prompt.
10+
*
11+
* Defaults to `true` so code paths that run outside the command wrapper (tests,
12+
* library callers) fall back to their own TTY checks rather than being silently
13+
* blocked.
14+
*/
15+
16+
let allowed = true;
17+
18+
/**
19+
* Enable or disable interactive prompts for the current command run. Called by
20+
* the command wrapper with `false` when JSON output is active.
21+
*/
22+
export function setInteractivePromptsAllowed(value: boolean): void {
23+
allowed = value;
24+
}
25+
26+
/** Whether interactive prompts are currently permitted. */
27+
export function interactivePromptsAllowed(): boolean {
28+
return allowed;
29+
}

0 commit comments

Comments
 (0)