From 846bc41b1ed6cfdcd46030886760c858e7ff44df Mon Sep 17 00:00:00 2001 From: Burak Yigit Kaya Date: Mon, 20 Apr 2026 17:13:43 +0000 Subject: [PATCH 1/2] feat(auth): hint when env token is shadowed by stored OAuth (#785) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The most painful UX item from getsentry/cli#785: a user sets SENTRY_AUTH_TOKEN (e.g. from a Stripe Projects or Vercel integration) but also has a stored OAuth login from `sentry auth login`. The CLI silently prefers the stored login — the user's 30 minutes of debugging invariably ends at discovering `SENTRY_FORCE_ENV_TOKEN=1`. Surface the collision on stderr the first time an authenticated command hits the API: [info] [auth] Detected SENTRY_AUTH_TOKEN env var but using stored login for alice. Set SENTRY_FORCE_ENV_TOKEN=1 to prefer the env var. Gating: - Fires only when an env token is set AND a stored OAuth login exists AND SENTRY_FORCE_ENV_TOKEN is not set. - Fires at most once per process (module-local latch). - Fires inside `authenticatedFetch` so local-only commands like `sentry help` or `sentry cli upgrade` stay quiet. User label resolution prefers `username` → `email` → `name` → "stored OAuth user" fallback, matching what `sentry auth whoami` shows when the cache is cold. Addresses getsentry/cli#785 item #4. --- src/lib/auth-hint.ts | 111 +++++++++++++++++++ src/lib/sentry-client.ts | 8 ++ test/lib/auth-hint.test.ts | 213 +++++++++++++++++++++++++++++++++++++ 3 files changed, 332 insertions(+) create mode 100644 src/lib/auth-hint.ts create mode 100644 test/lib/auth-hint.test.ts diff --git a/src/lib/auth-hint.ts b/src/lib/auth-hint.ts new file mode 100644 index 000000000..9ec20dbf1 --- /dev/null +++ b/src/lib/auth-hint.ts @@ -0,0 +1,111 @@ +/** + * One-shot env-token-ignored hint. + * + * When a user sets SENTRY_AUTH_TOKEN (or SENTRY_TOKEN) but also has a + * stored OAuth login from `sentry auth login`, the CLI silently prefers + * the stored login. Users then wonder why their valid provisioned + * token isn't being used — this was the most painful item in the CLI + * UX feedback issue (getsentry/cli#785 #4). The hint surfaces the + * collision on stderr the first time the CLI reaches for auth in a + * given process. + * + * Design notes: + * - Fires at most once per process (module-local latch). CLI invocations + * are short-lived, so "once per invocation" is the intended scope — + * persisting across invocations would require a DB key and add noise + * without a clear benefit. + * - Gated behind `!SENTRY_FORCE_ENV_TOKEN`: when the user has already + * opted in to the env-var path, the hint is moot. + * - Gated behind `hasStoredAuthCredentials()`: without a stored OAuth + * login there's no collision to surface. + * - Uses `log.info` to stay clearly advisory (not a warning — nothing + * is wrong, just a helpful breadcrumb). + */ + +import { + getActiveEnvVarName, + getRawEnvToken, + hasStoredAuthCredentials, +} from "./db/auth.js"; +import { getUserInfo } from "./db/user.js"; +import { getEnv } from "./env.js"; +import { logger } from "./logger.js"; + +const log = logger.withTag("auth"); + +/** Per-process latch — flipped the first time we emit the hint. */ +let hintEmitted = false; + +/** + * Emit the env-token-ignored hint if the current process has the + * collision and hasn't yet notified the user. + * + * Safe to call on every auth'd request — the per-process latch and + * quick environment / DB checks mean the cost is negligible after the + * first call. + */ +export function maybeWarnEnvTokenIgnored(): void { + if (hintEmitted) { + return; + } + + // Fast path: no env token to ignore. + const envToken = getRawEnvToken(); + if (!envToken) { + return; + } + + // Fast path: user opted into the env token explicitly. + if (getEnv().SENTRY_FORCE_ENV_TOKEN?.trim()) { + return; + } + + // No stored OAuth → no collision. + let hasStored = false; + try { + hasStored = hasStoredAuthCredentials(); + } catch { + // DB access failed — silently skip; this is a best-effort hint. + return; + } + if (!hasStored) { + return; + } + + hintEmitted = true; + + const envVar = getActiveEnvVarName(); + const userLabel = resolveStoredUserLabel(); + + log.info( + `Detected ${envVar} env var but using stored login for ${userLabel}.\n` + + " Set SENTRY_FORCE_ENV_TOKEN=1 to prefer the env var." + ); +} + +/** + * Resolve a user-friendly label for the stored OAuth user. + * + * Prefers `username`, then `email`, then `name` — matching what + * `sentry auth whoami` surfaces. Falls back to a neutral "stored + * OAuth user" when the cached `user_info` row is missing (fresh DB, + * never-ran-whoami, or read error). + */ +function resolveStoredUserLabel(): string { + try { + const user = getUserInfo(); + return user?.username ?? user?.email ?? user?.name ?? "stored OAuth user"; + } catch { + return "stored OAuth user"; + } +} + +/** + * Reset the one-shot latch. Tests call this between scenarios so each + * case starts with a fresh "has this process already notified" state. + * + * @internal + */ +export function resetAuthHintState(): void { + hintEmitted = false; +} diff --git a/src/lib/sentry-client.ts b/src/lib/sentry-client.ts index 8b011ca45..671275c67 100644 --- a/src/lib/sentry-client.ts +++ b/src/lib/sentry-client.ts @@ -9,6 +9,7 @@ */ import { getTraceData } from "@sentry/node-core/light"; +import { maybeWarnEnvTokenIgnored } from "./auth-hint.js"; import { DEFAULT_SENTRY_URL, getConfiguredSentryUrl, @@ -361,6 +362,13 @@ function createAuthenticatedFetch(): ( input: Request | string | URL, init?: RequestInit ): Promise { + // Once-per-process hint when env-var auth token is shadowed by a + // stored OAuth login. Runs here (rather than at command entry) so + // the hint only fires for commands that actually exercise auth — + // `sentry help` and similar local-only commands stay quiet. + // Internally rate-limited and cheap on repeat calls. + maybeWarnEnvTokenIgnored(); + const method = init?.method ?? (input instanceof Request ? input.method : "GET"); const urlPath = extractUrlPath(input); diff --git a/test/lib/auth-hint.test.ts b/test/lib/auth-hint.test.ts new file mode 100644 index 000000000..742c91c54 --- /dev/null +++ b/test/lib/auth-hint.test.ts @@ -0,0 +1,213 @@ +/** + * Tests for the one-shot env-token-ignored hint + * (`maybeWarnEnvTokenIgnored`). + * + * Covers: + * - Hint fires when env token + stored OAuth coexist. + * - Hint does NOT fire without an env token, without a stored login, + * or when SENTRY_FORCE_ENV_TOKEN is set. + * - Repeat calls in the same process are silent. + * - User label preference order (username > email > name > fallback). + */ + +import { afterEach, beforeEach, describe, expect, spyOn, test } from "bun:test"; +import { + maybeWarnEnvTokenIgnored, + resetAuthHintState, +} from "../../src/lib/auth-hint.js"; +import { setAuthToken } from "../../src/lib/db/auth.js"; +import { setUserInfo } from "../../src/lib/db/user.js"; +import { useTestConfigDir } from "../helpers.js"; + +useTestConfigDir("auth-hint-"); + +let savedAuthToken: string | undefined; +let savedSentryToken: string | undefined; +let savedForceEnv: string | undefined; + +beforeEach(() => { + savedAuthToken = process.env.SENTRY_AUTH_TOKEN; + savedSentryToken = process.env.SENTRY_TOKEN; + savedForceEnv = process.env.SENTRY_FORCE_ENV_TOKEN; + delete process.env.SENTRY_AUTH_TOKEN; + delete process.env.SENTRY_TOKEN; + delete process.env.SENTRY_FORCE_ENV_TOKEN; + resetAuthHintState(); +}); + +afterEach(() => { + if (savedAuthToken !== undefined) { + process.env.SENTRY_AUTH_TOKEN = savedAuthToken; + } else { + delete process.env.SENTRY_AUTH_TOKEN; + } + if (savedSentryToken !== undefined) { + process.env.SENTRY_TOKEN = savedSentryToken; + } else { + delete process.env.SENTRY_TOKEN; + } + if (savedForceEnv !== undefined) { + process.env.SENTRY_FORCE_ENV_TOKEN = savedForceEnv; + } else { + delete process.env.SENTRY_FORCE_ENV_TOKEN; + } +}); + +/** + * Capture stderr output from consola's default reporter. + * + * We can't reliably spy on `logger.withTag("auth").info` from a test + * because each `withTag()` call returns a fresh consola instance — the + * spy target and the instance used inside `auth-hint.ts` are + * different objects. Hooking `process.stderr.write` captures the final + * rendered output regardless of which instance emitted it. + */ +function captureStderr() { + const stderrSpy = spyOn(process.stderr, "write").mockImplementation( + () => true + ); + return { + /** Number of calls whose first argument contained the env-hint body. */ + hintCalls: () => + stderrSpy.mock.calls.filter((call) => + String(call[0] ?? "").includes("SENTRY_FORCE_ENV_TOKEN=1") + ).length, + /** Flattened first-arg text across all calls (for substring assertions). */ + text: () => + stderrSpy.mock.calls.map((call) => String(call[0] ?? "")).join(""), + restore: () => stderrSpy.mockRestore(), + }; +} + +describe("maybeWarnEnvTokenIgnored", () => { + test("does nothing when no env token is set", () => { + setAuthToken("stored_oauth", 3600, "refresh_a"); + const cap = captureStderr(); + try { + maybeWarnEnvTokenIgnored(); + expect(cap.hintCalls()).toBe(0); + } finally { + cap.restore(); + } + }); + + test("does nothing when no stored OAuth login exists", () => { + process.env.SENTRY_AUTH_TOKEN = "env_token"; + const cap = captureStderr(); + try { + maybeWarnEnvTokenIgnored(); + expect(cap.hintCalls()).toBe(0); + } finally { + cap.restore(); + } + }); + + test("does nothing when SENTRY_FORCE_ENV_TOKEN is set", () => { + setAuthToken("stored_oauth", 3600, "refresh_a"); + process.env.SENTRY_AUTH_TOKEN = "env_token"; + process.env.SENTRY_FORCE_ENV_TOKEN = "1"; + const cap = captureStderr(); + try { + maybeWarnEnvTokenIgnored(); + expect(cap.hintCalls()).toBe(0); + } finally { + cap.restore(); + } + }); + + test("fires once when env token collides with stored OAuth", () => { + setAuthToken("stored_oauth", 3600, "refresh_a"); + setUserInfo({ + userId: "u-1", + username: "alice", + email: "alice@example.com", + }); + process.env.SENTRY_AUTH_TOKEN = "env_token"; + + const cap = captureStderr(); + try { + maybeWarnEnvTokenIgnored(); + expect(cap.hintCalls()).toBe(1); + const text = cap.text(); + expect(text).toContain("SENTRY_AUTH_TOKEN env var"); + expect(text).toContain("stored login for alice"); + expect(text).toContain("SENTRY_FORCE_ENV_TOKEN=1"); + } finally { + cap.restore(); + } + }); + + test("is silent on repeat calls within the same process", () => { + setAuthToken("stored_oauth", 3600, "refresh_a"); + process.env.SENTRY_AUTH_TOKEN = "env_token"; + + const cap = captureStderr(); + try { + maybeWarnEnvTokenIgnored(); + maybeWarnEnvTokenIgnored(); + maybeWarnEnvTokenIgnored(); + expect(cap.hintCalls()).toBe(1); + } finally { + cap.restore(); + } + }); + + test("uses SENTRY_TOKEN when only that legacy var is set", () => { + setAuthToken("stored_oauth", 3600, "refresh_a"); + process.env.SENTRY_TOKEN = "legacy_token"; + + const cap = captureStderr(); + try { + maybeWarnEnvTokenIgnored(); + expect(cap.hintCalls()).toBe(1); + expect(cap.text()).toContain("SENTRY_TOKEN env var"); + } finally { + cap.restore(); + } + }); + + test("falls back to email when username is unavailable", () => { + setAuthToken("stored_oauth", 3600, "refresh_a"); + setUserInfo({ + userId: "u-1", + email: "alice@example.com", + }); + process.env.SENTRY_AUTH_TOKEN = "env_token"; + + const cap = captureStderr(); + try { + maybeWarnEnvTokenIgnored(); + expect(cap.text()).toContain("stored login for alice@example.com"); + } finally { + cap.restore(); + } + }); + + test("falls back to name when username and email are unavailable", () => { + setAuthToken("stored_oauth", 3600, "refresh_a"); + setUserInfo({ userId: "u-1", name: "Alice Wonderland" }); + process.env.SENTRY_AUTH_TOKEN = "env_token"; + + const cap = captureStderr(); + try { + maybeWarnEnvTokenIgnored(); + expect(cap.text()).toContain("stored login for Alice Wonderland"); + } finally { + cap.restore(); + } + }); + + test("uses a neutral label when no user info is cached", () => { + setAuthToken("stored_oauth", 3600, "refresh_a"); + // No setUserInfo — user_info table is empty. + process.env.SENTRY_AUTH_TOKEN = "env_token"; + + const cap = captureStderr(); + try { + maybeWarnEnvTokenIgnored(); + expect(cap.text()).toContain("stored login for stored OAuth user"); + } finally { + cap.restore(); + } + }); +}); From 83076f5dbff9974dcb422864757ff4ca7d89006b Mon Sep 17 00:00:00 2001 From: Burak Yigit Kaya Date: Tue, 21 Apr 2026 08:59:22 +0000 Subject: [PATCH 2/2] fix(auth-hint): report DB read failures to Sentry MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Address review comment on #790: the best-effort guards in maybeWarnEnvTokenIgnored() and resolveStoredUserLabel() silently swallowed DB read failures, making persistent cache-read problems invisible. Report them via Sentry.captureException while keeping the CLI-UX behavior unchanged (still falls back to silent-no-hint and neutral-user-label respectively). Also drops the redundant early `return` in the hasStoredAuthCredentials catch block — `hasStored` stays `false` after the throw, so the existing `if (!hasStored) return` guard handles it. --- AGENTS.md | 117 +++++++++++++++++++++---------------------- src/lib/auth-hint.ts | 19 +++++-- 2 files changed, 71 insertions(+), 65 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index 1c6faf186..a1724a17d 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -984,93 +984,90 @@ mock.module("./some-module", () => ({ ### Architecture - -* **api-client.ts split into domain modules under src/lib/api/**: Monolithic \`src/lib/api-client.ts\` was split into domain modules under \`src/lib/api/\`: infrastructure.ts, organizations.ts, projects.ts, teams.ts, repositories.ts, issues.ts, events.ts, traces.ts, logs.ts, seer.ts, trials.ts, users.ts. Original file is now a ~100-line barrel re-export (already in \`biome.jsonc\` \`noBarrelFile\` override). Add new API functions to the appropriate domain module, not the barrel file. + +* **Dashboard widget interval computed from terminal width and layout before API calls**: Dashboard widget interval from terminal width: \`computeOptimalInterval()\` in \`src/lib/api/dashboards.ts\` calculates chart interval before API calls. Formula: \`colWidth = floor(layout.w / 6 \* termWidth)\`, \`chartWidth = colWidth - 4 - gutterW\`, \`idealSeconds = periodSeconds / chartWidth\`. Snaps to nearest Sentry bucket (1m–1d). \`periodToSeconds()\` parses \`"24h"\`, \`"7d"\` etc. \`queryWidgetTimeseries\` uses \`params.interval ?? widget.interval\`. - -* **cli.sentry.dev is served from gh-pages branch via GitHub Pages**: \`cli.sentry.dev\` is served from gh-pages branch via GitHub Pages. Craft's gh-pages target runs \`git rm -r -f .\` before extracting docs — persist extra files via \`postReleaseCommand\` in \`.craft.yml\`. Install script supports \`--channel nightly\`, downloading from the \`nightly\` release tag directly. version.json is only used by upgrade/version-check flow. + +* **DSN org prefix normalization in arg-parsing.ts**: DSN/numeric org prefix normalization — four code paths must all convert to slugs before API calls (many endpoints reject numeric org IDs with 404/403): (1) \`extractOrgIdFromHost\` strips \`o\` prefix during DSN parsing. (2) \`stripDsnOrgPrefix()\` handles user-typed \`o1081365/\` in \`parseOrgProjectArg()\`. (3) \`normalizeNumericOrg()\` in \`resolve-target.ts\` resolves bare numeric IDs via DB cache or uncached API call. (4) Dashboard's \`resolveOrgFromTarget()\` pipes through \`resolveEffectiveOrg()\`, also used by \`tryResolveRecoveryOrg()\` in hex-id-recovery. - -* **Debug ID sourcemap matching works but each build generates independent debug IDs**: Binary build pipeline: esbuild bundles TS→minified JS+\`.map\` (not \`Bun.build()\` — collision bug oven-sh/bun#14585 + empty sourcemap \`names\`); config: \`platform:"node"\`, \`format:"esm"\`, \`external:\["bun:\*"]\`. Then \`injectDebugId()\` + \`uploadSourcemaps()\` (prefix \`~/$bunfs/root/\`), then \`Bun.build({compile:true, minify:false})\`. Debug IDs via \`globalThis.\_sentryDebugIds\`. \`binpunch\` strips unused ICU data. CRITICAL: \`SENTRY\_AUTH\_TOKEN\` is a GitHub \*\*environment\*\* secret (in \`production\`), not repo secret. Jobs needing it must declare \`environment: ${{ (github.ref == 'refs/heads/main' || startsWith(github.ref, 'refs/heads/release/')) && 'production' || '' }}\` — otherwise secret resolves empty and uploads silently skip. Build also requires \`SENTRY\_CLIENT\_ID\` env var (exits 1 without it); use \`bun run --env-file=.env.local build\` locally. + +* **env-registry.ts drives --help env var section + docs**: \`src/lib/env-registry.ts\` (\`ENV\_VAR\_REGISTRY\`) is the single source for all env vars the CLI honors. Entries have \`{name, description, example?, defaultValue?, installOnly?, topLevel?, briefDescription?}\`. \`topLevel: true\` + \`briefDescription\` surfaces in \`sentry --help\` Environment Variables section (via \`formatEnvVarsSection()\` in \`help.ts\`) and in \`sentry help --json\` as \`envVars\` array on the full-tree envelope. Docs generator consumes the full registry for \`configuration.md\`. When adding a new env var, add it here with \`installOnly: true\` if install-script-only. Reserve \`topLevel: true\` for core-path vars only (auth, targeting, URL, key display/logging). - -* **Issue resolve --in grammar: release + @next + @commit sentinels**: \`sentry issue resolve --in\` grammar: (a) omitted → immediate resolve, (b) \`\\` → \`inRelease\` (monorepo strings like \`spotlight@1.2.3\` pass through as-is), (c) \`@next\` → \`inNextRelease\`, (d) \`@commit\` → auto-detect git HEAD + match against Sentry repos, (e) \`@commit:\@\\` → explicit. \`@commit:\` is the unambiguous anchor. \`parseResolveSpec\` returns \`ParsedResolveSpec\` (\`kind: 'static' | 'commit-auto' | 'commit-explicit'\`); \`resolveCommitSpec\` in \`src/commands/issue/resolve-commit-spec.ts\` uses \`getHeadCommit\`/\`getRepositoryName\` from \`src/lib/git.ts\` and matches against Sentry repo \`externalSlug\` or \`name\` via \`listRepositoriesCached\`. Sentry's \`statusDetails.inCommit\` requires \`{commit, repository}\` object — not bare SHA. All failure paths throw \`ValidationError\` with hints; never silent fallback. + +* **GHCR versioned nightly tags for delta upgrade support**: GHCR nightly with delta upgrades: Three tag types: \`:nightly\` (rolling), \`:nightly-\\` (immutable), \`:patch-\\` (delta). Delta patches use TRDIFF10 (zstd-compressed), ~50KB vs ~29MB full. Client via \`Bun.zstdDecompressSync()\`. N-1 only, full fallback, SHA-256 verify, 60% size threshold. npm/Node excluded. Test: \`mockGhcrNightlyVersion()\`. - -* **Nightly delta upgrade buildNightlyPatchGraph fetches ALL patch tags — O(N) HTTP calls**: Nightly delta upgrade via GHCR versioned tags (GitHub releases are immutable): nightlies publish to GHCR as \`nightly-0.14.0-dev.1772661724\`. \`src/lib/delta-upgrade.ts\` supports stable (GitHub Releases) and nightly (GHCR). \`filterAndSortChainTags\` filters \`patch-\*\` via \`Bun.semver.order()\`. GHCR uses \`fetchWithRetry\` (10s+1 retry) with \`AbortSignal.any()\`; \`isExternalAbort\` skips retries for external aborts (critical for background prefetch). Patches cached to \`~/.sentry/patch-cache/\` (7-day TTL). CI tag filter: \`grep '^nightly-\[0-9]'\`. \`fetchManifest()\` throws \`UpgradeError("network\_error")\` on network failure/non-200. + +* **Issue list auto-pagination beyond API's 100-item cap**: Sentry API silently caps \`limit\` at 100 per request. \`listIssuesAllPages()\` auto-paginates using Link headers, bounded by MAX\_PAGINATION\_PAGES (50). \`API\_MAX\_PER\_PAGE\` constant is shared across all paginated consumers. \`--limit\` means total results everywhere (max 1000, default 25). Org-all mode uses \`fetchOrgAllIssues()\`; explicit \`--cursor\` does single-page fetch to preserve cursor chain. - -* **npm bundle requires Node.js >= 22 due to node:sqlite polyfill**: The npm package (dist/bin.cjs) requires Node.js >= 22 because the bun:sqlite polyfill uses \`node:sqlite\`. A runtime version guard in the esbuild banner catches this early. When writing esbuild banner strings in TS template literals, double-escape: \`\\\\\\\n\` in TS → \`\\\n\` in output → newline at runtime. Single \`\\\n\` produces a literal newline inside a JS string, causing SyntaxError. + +* **resolveProjectBySlug carries full projectData to avoid redundant getProject calls**: resolveProjectBySlug carries full projectData to skip redundant API calls: Returns \`{ org, project, projectData: SentryProject }\` from \`findProjectsBySlug()\`. \`ResolvedOrgProject\`/\`ResolvedTarget\` have optional \`projectData?\` (populated only in project-search path). Downstream commands use \`resolved.projectData ?? await getProject(org, project)\` to save ~500-800ms. - -* **Numeric issue ID resolution returns org:undefined despite API success**: Numeric issue ID resolution in \`resolveNumericIssue()\`: (1) try DSN/env/config for org, (2) if found use \`getIssueInOrg(org, id)\` with region routing, (3) else fall back to unscoped \`getIssue(id)\`, (4) extract org from \`issue.permalink\` via \`parseSentryUrl\` as final fallback. \`parseSentryUrl\` handles path-based (\`/organizations/{org}/...\`) and subdomain-style URLs. \`matchSubdomainOrg()\` filters region subdomains by requiring slug length > 2. Self-hosted uses path-based only. + +* **Self-hosted OAuth device flow requires Sentry 26.1.0+ and SENTRY\_CLIENT\_ID**: Self-hosted OAuth requires Sentry 26.1.0+ with \`SENTRY\_URL\` and \`SENTRY\_CLIENT\_ID\` env vars. Users must create a public OAuth app in Settings → Developer Settings. Client ID NOT optional for self-hosted. Fallback: \`sentry auth login --token\`. \`getSentryUrl()\`/\`getClientId()\` read lazily so URL parsing from arguments can set \`SENTRY\_URL\` after import. - -* **repo\_cache SQLite table for offline Sentry repo lookups**: Schema v14 adds \`repo\_cache\` table in \`src/lib/db/schema.ts\` + helpers in \`src/lib/db/repo-cache.ts\` (7-day TTL). \`listAllRepositories(org)\` in \`src/lib/api/repositories.ts\` paginates through \`listRepositoriesPaginated\` using \`API\_MAX\_PER\_PAGE\` and \`MAX\_PAGINATION\_PAGES\` — never use the unpaginated \`listRepositories\` for cache-backed lookups (silently caps at ~25). \`listRepositoriesCached(org)\` wraps it with cache-first lookup and a try/catch around \`setCachedRepos\` so read-only databases (macOS \`sudo brew install\`) don't crash commands whose API fetch already succeeded. Used by \`@commit\` resolver to match git origin \`owner/repo\` against Sentry repo \`externalSlug\` or \`name\`. + +* **Sentry CLI markdown-first formatting pipeline replaces ad-hoc ANSI**: Formatters build CommonMark strings; \`renderMarkdown()\` renders to ANSI for TTY or raw markdown for non-TTY. Key helpers: \`colorTag()\`, \`mdKvTable()\`, \`mdRow()\`, \`mdTableHeader()\` (\`:\` suffix = right-aligned), \`renderTextTable()\`. \`isPlainOutput()\` checks \`SENTRY\_PLAIN\_OUTPUT\` > \`NO\_COLOR\` > \`!isTTY\`. Batch path: \`formatXxxTable()\`. Streaming path: \`StreamingTable\` (TTY) or raw markdown rows (plain). Both share \`buildXxxRowCells()\`. - -* **Seer trial prompt uses middleware layering in bin.ts error handling chain**: Seer trial prompt via error middleware layering: \`bin.ts\` chain is \`main() → executeWithAutoAuth() → executeWithSeerTrialPrompt() → runCommand()\`. Seer trial prompts (\`no\_budget\`/\`not\_enabled\`) caught by inner wrapper; auth errors bubble to outer. Trial API: \`GET /api/0/customers/{org}/\` → \`productTrials\[]\` (prefer \`seerUsers\`, fallback \`seerAutofix\`). Start: \`PUT /api/0/customers/{org}/product-trial/\`. SaaS-only; self-hosted 404s gracefully. \`ai\_disabled\` excluded. \`startSeerTrial\` accepts \`category\` from trial object — don't hardcode. + +* **Sentry dashboard API rejects discover/transaction-like widget types — use spans**: Sentry API dataset gotchas: (1) Events/Explore API accepts \`spans\`, \`transactions\`, \`logs\`, \`errors\`, \`discover\`; \`spansIndexed\` is INVALID (500). Valid list in \`EVENTS\_API\_DATASETS\`. (2) Dashboard \`widgetType\`: \`discover\` and \`transaction-like\` rejected as deprecated — use \`spans\`. \`WIDGET\_TYPES\` (active) vs \`ALL\_WIDGET\_TYPES\` (includes deprecated for parsing). Tests use \`error-events\` not \`discover\`. (3) \`sort\` param only on \`spans\` dataset. (4) \`tracemetrics\` uses comma-separated aggregates; only line/area/bar/table/big\_number displays. - -* **SQLite DB functions are synchronous — async signatures are historical artifacts**: All \`src/lib/db/\` functions do synchronous SQLite operations. Many have \`async\` signatures — historical artifact from PR #89 (JSON file→SQLite migration). Safe to convert to sync. Legitimately async exceptions: \`clearAuth()\` (cache dir cleanup), \`getCachedDetection()\`/\`getCachedProjectRoot()\`/\`setCachedProjectRoot()\` (stat for mtime), \`refreshToken()\`/\`performTokenRefresh()\` (HTTP calls). + +* **Sentry issue stats field: time-series controlled by groupStatsPeriod**: Issue stats and list layout: \`stats\` depends on \`groupStatsPeriod\` (\`""\`, \`"14d"\`, \`"24h"\`, \`"auto"\`). Critical: \`count\` is period-scoped — use \`lifetime.count\` for true total. \`--compact\` is tri-state (\`optional: true\`): explicit overrides, \`undefined\` triggers \`shouldAutoCompact(rowCount)\` — compact if \`3N + 3 > termHeight\`. TREND column hidden < 100 cols. Stricli boolean flags with \`optional: true\` produce \`boolean | undefined\` enabling this auto-detect pattern. -### Decision - - -* **Raw markdown output for non-interactive terminals, rendered for TTY**: Markdown-first output pipeline: custom renderer in \`src/lib/formatters/markdown.ts\` walks \`marked\` tokens to produce ANSI-styled output. Commands build CommonMark using helpers (\`mdKvTable()\`, \`mdRow()\`, \`colorTag()\`, \`escapeMarkdownCell()\`, \`safeCodeSpan()\`) and pass through \`renderMarkdown()\`. \`isPlainOutput()\` precedence: \`SENTRY\_PLAIN\_OUTPUT\` > \`NO\_COLOR\` > \`FORCE\_COLOR\` > \`!isTTY\`. \`--json\` always outputs JSON. Colors defined in \`COLORS\` object in \`colors.ts\`. Tests run non-TTY so assertions match raw CommonMark; use \`stripAnsi()\` helper for rendered-mode assertions. + +* **Sentry log IDs are UUIDv7 — enables deterministic retention checks**: Sentry log IDs are UUIDv7 (first 12 hex = ms timestamp, version char \`7\` at pos 13). Traces and event IDs are NOT v7. \`decodeUuidV7Timestamp()\` and \`ageInDaysFromUuidV7()\` in \`src/lib/hex-id.ts\` return null for non-v7 IDs, safe to call unconditionally. Enables deterministic 'past retention' messages instead of 'may have been deleted'. Wired in \`recoverHexId\` (short-circuit) and \`log/view.ts#throwNotFoundError\`. \`RETENTION\_DAYS.log = 90\` in \`src/lib/retention.ts\`; traces/events are \`null\` (plan-dependent). \`LOG\_RETENTION\_PERIOD\` is DERIVED as \`\` \`${RETENTION\_DAYS.log}d\` \`\` — never hardcode \`'90d'\` or \`'90 days'\`. Shared hex primitives (\`HEX\_ID\_RE\`, \`SPAN\_ID\_RE\`, \`UUID\_DASH\_RE\`, \`LEADING\_HEX\_RE\`, \`MIDDLE\_ELLIPSIS\_RE\`, \`HexEntityType\`) live in \`hex-id.ts\` — \`HexEntityType\` stays here (not recovery.ts) to avoid inverted dependency. -### Gotcha + +* **SKILL.md is fully generated — edit source files, not output**: SKILL.md is fully generated — edit sources not output: Skill files under \`plugins/sentry-cli/skills/sentry-cli/\` generated by \`bun run generate:skill\`. CI auto-commits. Edit sources: (1) \`docs/src/content/docs/agent-guidance.md\` for SKILL.md content, (2) \`src/commands/\*/\` flag \`brief\` strings for reference descriptions, (3) \`docs/src/content/docs/commands/\*.md\` for examples. \`bun run check:skill\` fails if stale. - -* **@sentry/api SDK passes Request object to custom fetch — headers lost on Node.js**: @sentry/api SDK calls \`\_fetch(request)\` with no init object. In \`authenticatedFetch\`, \`init\` is undefined so \`prepareHeaders\` creates empty headers — on Node.js this strips Content-Type (HTTP 415). Fix: fall back to \`input.headers\` when \`init\` is undefined. Use \`unwrapPaginatedResult\` (not \`unwrapResult\`) to access the Response's Link header for pagination. \`per\_page\` is not in SDK types; cast query to pass it at runtime. + +* **Stricli route errors are uninterceptable — only post-run detection works**: Stricli error gaps: (1) Route failures uninterceptable — Stricli writes stderr, returns \`ExitCode.UnknownCommand\` (-5 / 251 in Bun); only post-\`run()\` \`process.exitCode\` check works. (2) \`OutputError\` calls \`process.exit()\` immediately, bypassing telemetry. (3) \`defaultCommand: 'help'\` bypasses built-in fuzzy matching — fixed by \`resolveCommandPath()\` in \`introspect.ts\` with \`fuzzyMatch()\` (up to 3 suggestions); JSON includes \`suggestions\` array. (4) Plural alias detection in \`app.ts\`. - -* **Completion drift test requires new issue-positional commands be registered**: \`test/lib/completions.property.test.ts\` scans the route tree for commands whose positional placeholder contains "org" or equals "issue", then asserts every such command appears in \`ORG\_PROJECT\_COMMANDS\` or \`ORG\_ONLY\_COMMANDS\` (in \`src/lib/complete.ts\`). When adding a new issue subcommand (resolve, unresolve, merge, etc.) using \`issueIdPositional\`, register it in \`ORG\_PROJECT\_COMMANDS\` or the test fails with \`expect(combined.has(cmd)).toBe(true)\` → false. + +* **Three Sentry APIs for span custom attributes with different capabilities**: Three Sentry span APIs: (1) \`/trace/{traceId}/\` — hierarchical tree; \`additional\_attributes\` enumerates requested attrs; returns \`measurements\` (zero-filled on non-browser, stripped by \`filterSpanMeasurements()\`). (2) \`/projects/{org}/{project}/trace-items/{itemId}/\` — single span full detail; ALL attributes as \`{name,type,value}\[]\` automatically. (3) \`/events/?dataset=spans\&field=X\` — list/search; explicit \`field\` params. \`--fields\` flag filters JSON output AND requests extra API fields via \`extractExtraApiFields()\`. \`FIELD\_GROUP\_ALIASES\` supports shorthand expansion. - -* **Cross-org merge check must reject undefined orgs, not filter them**: In \`src/commands/issue/merge.ts\` \`resolveAllIssues\`, bare numeric issue IDs without DSN/config context resolve with \`org: undefined\`. Filtering \`undefined\` out of the org set before the cross-org check lets mixed-org merges slip through — call goes to known org's endpoint and fails with confusing API error instead of friendly "Cannot merge issues across organizations". Require every resolved issue to have defined org (or reject undefined) before proceeding. Applies anywhere resolved entities must share an org. + +* **Update notification rate-limited via last\_notified metadata + TTY gate**: \`src/lib/version-check.ts\` \`getUpdateNotification()\` now enforces a 24h rate limit via \`KEY\_LAST\_NOTIFIED\` metadata + an in-process latch (\`notifiedThisRun\`) plus a \`process.stderr.isTTY\` gate. Non-TTY invocations never see the banner (scripts, CI, \`| cat\`). \`setLastNotifiedNow()\` persists on emit; the latch prevents double-emit within a single process. Tests must mock \`process.stderr.isTTY = true\` AND reset the in-process latch between cases via the exported \`\_resetNotificationStateForTests()\` helper. Mirrors the pattern of other one-shot hints in the CLI. - -* **Install script: BSD sed and awk JSON parsing breaks OCI digest extraction**: The install script parses OCI manifests with awk (no jq). Key trap: BSD sed \`\n\` is literal, not newline. Fix: single awk pass tracking last-seen \`"digest"\`, printing when \`"org.opencontainers.image.title"\` matches target. The config digest (\`sha256:44136fa...\`) is a 2-byte \`{}\` blob — downloading it instead of the real binary causes \`gunzip: unexpected end of file\`. +### Decision - -* **Multi-region fan-out: distinguish all-403 from empty orgs with hasSuccessfulRegion flag**: In \`listOrganizationsUncached\` (\`src/lib/api/organizations.ts\`), \`Promise.allSettled\` collects multi-region results. Don't use \`flatResults.length === 0\` to detect all-regions-failed — a region returning 200 OK with zero orgs pushes nothing into \`flatResults\`. Track a \`hasSuccessfulRegion\` boolean on any \`"fulfilled"\` settlement. Only re-throw 403 \`ApiError\` when \`!hasSuccessfulRegion && lastScopeError\`. + +* **400 Bad Request from Sentry API indicates a CLI bug, not a user error**: Telemetry 400 convention: 400 = CLI bug (capture to Sentry), 401-499 = user error (skip). \`isUserApiError()\` uses \`> 400\` (exclusive). \`isExpectedUserError()\` guard in \`app.ts\` skips ContextError, ResolutionError, ValidationError, SeerError, 401-499 ApiErrors. Captures 400, 5xx, unknown. Skipped errors → breadcrumbs. For \`ApiError\`, call \`Sentry.setContext('api\_error', {...})\` before \`captureException\` — SDK doesn't auto-capture custom properties. - -* **Multiple mockFetch calls replace each other — use unified mocks for multi-endpoint tests**: Bun test mocking gotchas: (1) \`mockFetch()\` replaces \`globalThis.fetch\` — calling twice replaces the first. Use one unified mock dispatching by URL. (2) \`mock.module()\` pollutes module registry for ALL subsequent files — tests using it must live in \`test/isolated/\` and run via \`test:isolated\`. (3) For \`Bun.spawn\`, use direct property assignment in \`beforeEach\`/\`afterEach\`. + +* **CLI UX philosophy: auto-recover when intent is clear, warn gently**: UX principle: don't fail when intent is clear — do the intent and nudge via \`log.warn()\` to stderr. Keep errors in Sentry telemetry for visibility (e.g., SeerError for upsell tracking). Two recovery tiers: (1) auto-correct when semantics identical (AND→space), (2) auto-recover with warning when semantics differ (OR→space, warn about union→intersection). Only throw when intent can't be fulfilled. Model after \`gh\` CLI. AI agents are primary consumers constructing natural OR/AND queries. - -* **Sentry bulk merge --into is a preference, not a guarantee**: Sentry bulk merge API quirks: \`PUT /api/0/organizations/{org}/issues/?id=X\&id=Y\` with \`{merge:1}\` picks canonical parent by event count, not input order — \`--into\` is a preference only. Commands must check API response's \`parent\` against requested preference and warn when Sentry picked differently. Strip \`org/\` prefix from user input before comparing to \`issue.shortId\`. Empty-result merges return 204, not 200. Server-side fingerprint rules only apply to NEW events — existing fragmented issues must be merged via this endpoint (async, prefer org-scoped); verify by re-fetching child's \`id\`. + +* **Trace-related commands must handle project consistently across CLI**: Trace/log commands and project scoping: \`getDetailedTrace\` accepts optional numeric \`projectId\` (not hardcoded \`-1\`). Resolve slug→ID via \`getProject()\`. \`formatSimpleSpanTree\` shows orphan annotation only when \`projectFiltered\` is set. \`buildProjectQuery()\` in \`arg-parsing.ts\` prepends \`project:\\` to queries (used by \`trace/logs.ts\`, \`log/list.ts\`). Multi-project: \`--query 'project:\[cli,backend]'\`. Trace-logs endpoint (\`/organizations/{org}/trace-logs/\`) is org-scoped — uses \`resolveOrg()\` not \`resolveOrgAndProject()\`. Endpoint is PRIVATE (no \`@sentry/api\` types); hand-written \`TraceLogSchema\` in \`src/types/sentry.ts\` required. - -* **useTestConfigDir without isolateProjectRoot causes DSN scanning of repo tree**: \`useTestConfigDir()\` creates temp dirs under \`.test-tmp/\` in the repo tree. Without \`{ isolateProjectRoot: true }\`, \`findProjectRoot\` walks up and finds the repo's \`.git\`, causing DSN detection to scan real source code and trigger network calls against test mocks (timeouts). Always pass \`isolateProjectRoot: true\` when tests exercise \`resolveOrg\`, \`detectDsn\`, or \`findProjectRoot\`. +### Gotcha -### Pattern + +* **Biome lint differs between local lint:fix and CI lint**: \`bun run lint:fix\` may auto-apply fixes locally that hide issues CI's \`bun run lint\` will still catch. Specifically: (1) \`noPrecisionLoss\` on large integer literals (>2^53), (2) \`noIncrementDecrement\` on \`count++\`, (3) import ordering when a named import is placed after non-import runtime code. Biome's formatter also rewrites multi-line imports to single-line when they fit — the auto-fix may leave stale state. Always run \`bun run lint\` (not just \`lint:fix\`) before pushing. Use \`for...of\` with array destructuring or explicit \`i += 1\` instead of \`++\`. Use \`Number(string)\` or split literals instead of \`1\_735\_689\_600\_000\_000\_001\`. - -* **apiRequestToRegion throws ApiError on 204/205 — don't return null as T**: apiRequestToRegion throws ApiError on 204/205 — don't return null as T: Empty responses (204/205) throw \`ApiError(204)\` rather than returning \`null as T\`, keeping the \`T\` return type sound. Callers expecting 204 (e.g. Sentry's bulk mutate for "no matching issues") must catch \`ApiError\` with \`status === 204\` and translate to a domain-specific error. Don't use \`null as T\` to signal empty bodies — TypeScript hides the null and causes runtime crashes on property access. + +* **Bun mock.module for node:tty requires default export and class stubs**: Bun testing gotchas: (1) \`mock.module()\` for CJS built-ins needs \`default\` re-export plus named exports, top-level BEFORE \`await import()\`; lives in \`test/isolated/\`. (2) Destructuring imports capture binding at load; verify via call-count > 0. (3) \`Bun.mmap()\` always opens PROT\_WRITE — use \`new Uint8Array(await Bun.file(path).arrayBuffer())\`. (4) Wrap \`Bun.which()\` with optional \`pathEnv\` for deterministic testing. (5) Mocking \`@sentry/node-core/light\`: \`startSpan\` must pass mock span to callback: \`startSpan: (\_, fn) => fn({ setStatus(){}, setAttribute(){}, end(){} })\`. - -* **findProjectsByPattern as fuzzy fallback for exact slug misses**: When \`findProjectsBySlug\` returns empty (no exact match), use \`findProjectsByPattern\` as a fallback to suggest similar projects. \`findProjectsByPattern\` does bidirectional word-boundary matching (\`matchesWordBoundary\`) against all projects in all orgs — the same logic used for directory name inference. In the \`project-search\` handler, call it after the exact miss, format matches as \`\/\\` suggestions in the \`ResolutionError\`. This avoids a dead-end error for typos like 'patagonai' when 'patagon-ai' exists. Note: \`findProjectsByPattern\` makes additional API calls (lists all projects per org), so only call it on the failure path. +### Pattern - -* **Issue command hint: pass subcommand name only, not full path**: \`resolveIssue({command, commandBase})\` builds error hints via \`buildCommandHint(command, issueId, base="sentry issue")\` which concatenates \`${base} ${command} ...\`. Pass the subcommand name only (e.g. \`"resolve"\`, \`"view"\`) and let \`commandBase\` default to \`"sentry issue"\`. Passing \`command: "issue resolve"\` with \`commandBase: "issue"\` produces doubled prefixes like \`"issue issue resolve \"\`. Pattern followed by \`view\`/\`events\`/\`explain\`/\`plan\`/\`resolve\`/\`unresolve\`/\`merge\` in \`src/commands/issue/\`. + +* **403 scope extraction via api-scope.ts helpers**: \`src/lib/api-scope.ts\` exports \`extractRequiredScopes(detail)\` which scans Sentry 403 response detail (string or structured) for scope-like tokens (e.g. \`event:read\`, \`project:admin\`). Matches free-text and structured \`required\`/\`required\_scopes\` fields. Use in 403-enrichment paths instead of hardcoded generic scope lists; fall back to generic hint only when extraction returns empty. Wired into \`issue list\` \`build403Detail()\`, \`organizations.ts\` \`enrich403Error()\`, and anywhere a 403 is re-thrown with a required-scopes suggestion. - -* **Preserve ApiError type so classifySilenced can silence 4xx errors**: \`classifySilenced\` in \`src/lib/error-reporting.ts\` only silences \`ApiError\` with status 401-499 — wrapping API errors in generic \`CliError\` loses \`status\` and causes 403s etc. to be captured as Sentry issues. Re-throw as \`ApiError\` preserving \`status\`/\`detail\`/\`endpoint\`. Keep message terse: \`ApiError.format()\` appends \`detail\`/\`endpoint\` on own lines, so including detail in message causes duplicated output. Pattern: \`throw new ApiError(\\\`Failed to X (HTTP ${error.status}).\\\`, error.status, error.detail, error.endpoint)\`. Also: ValidationError without a \`field\` arg previously produced empty \`cli\_error.kind\`, fingerprint-collapsing all unfielded ValidationErrors into one group. Fix (PR #776): \`setGroupingTags\` falls back to normalized message prefix via \`extractMessagePrefix\`. When adding ValidationError sites, pass \`field\` or ensure distinctive message prefix. + +* **I/O concurrency limits belong at the call site, not in generic combinators**: I/O concurrency limits belong at the call site, not in generic combinators. Pattern: module-scoped \`pLimit()\` with named constant (e.g., \`STAT\_CONCURRENCY = 32\` in \`project-root.ts\`, \`CACHE\_IO\_CONCURRENCY\` in \`response-cache.ts\`, \`pLimit(50)\` in \`code-scanner.ts\`). Keeps combinators pure, makes budget explicit at I/O boundary. stat() lighter than full reads — ~32 for stats vs ~50 for reads, well below macOS's 256 FD ceiling. - -* **Resolve --into flag through resolveIssue for alias parity with positionals**: Merge/batch commands with a \`--into \\` (or similar canonical-parent) flag should pass it through \`resolveIssue()\` the same way positional args are resolved, then compare by numeric \`id\` — not by string-matching against \`issue.shortId\`. This gives alias support (\`f-g\`, \`fr-a3\`) and org-qualified forms (\`my-org/CLI-G5\`) for free, and avoids asymmetry where positionals accept aliases but flags don't. Makes \`orderForMerge\` async. Pattern in \`src/commands/issue/merge.ts\`. + +* **Identity-scoped response cache via fingerprint mixin**: Response cache key is \`sha256(identity|method|url)\` where identity comes from \`getIdentityFingerprint()\` in \`src/lib/db/auth.ts\`: \`oauth:\\` for stored OAuth (stable across hourly access-token refresh), \`env:\\` for env tokens, or \`ANON\_IDENTITY\` when no auth. \`getCachedResponse\`/\`storeCachedResponse\`/\`invalidateCachedResponse\` resolve identity internally; \`buildCacheKey(method, url, identity)\` takes it explicitly for tests. This auto-isolates cross-account state (whoami, org lists) without explicit invalidation. Added \`invalidateCachedResponsesMatching(urlPrefix)\` for paginated-list wipes after mutations. To avoid a \`db/auth.ts\` ↔ \`response-cache.ts\` cycle, \`clearAuth()\` dynamically imports \`clearResponseCache\`. - -* **Sentry SDK tree-shaking patches must be regenerated via bun patch workflow**: Sentry SDK tree-shaking via bun patch: \`patchedDependencies\` in \`package.json\` tree-shakes unused exports from \`@sentry/core\` and \`@sentry/node-core\` (AI integrations, feature flags, profiler). Bumping SDK versions: (1) remove old patches/entries, (2) \`rm -rf ~/.bun/install/cache/@sentry\` (edits persist in cache), (3) \`bun install\` fresh, (4) \`bun patch @sentry/core\`, edit, \`bun patch --commit\`; repeat for node-core. Preserved exports: \`\_INTERNAL\_safeUnref\`, \`\_INTERNAL\_safeDateNow\` (core), \`nodeRuntimeMetricsIntegration\` (node-core). Manual \`git diff\` patches fail — always use \`bun patch --commit\`. + +* **Isolated adapter coverage via fetch mocking in test/lib/**: To get CodeCov coverage on API-calling functions (e.g., hex-id-recovery adapters, api-client functions), write tests in \`test/lib/\*.coverage.test.ts\` or \`test/lib/\*.adapters.test.ts\` that mock \`globalThis.fetch\` via \`mockFetch()\` from \`test/helpers.js\`, call \`setAuthToken()\` + \`setOrgRegion()\` in \`beforeEach\`, and invoke the REAL function. Tests in \`test/e2e/\` or tests that stub the exports via \`spyOn\`/\`mock.module\` give ZERO coverage to the mocked function body. Use \`useTestConfigDir()\` for DB isolation. Pattern example: \`test/lib/api-client.coverage.test.ts\` and \`test/lib/hex-id-recovery.adapters.test.ts\`. Mock responses must include ALL Zod-required fields — minimal stubs fail schema validation with a noisy \`ApiError\`. - -* **Shared pagination infrastructure: buildPaginationContextKey and parseCursorFlag**: Bidirectional pagination via cursor stack in \`src/lib/db/pagination.ts\`. \`resolveCursor(flag, key, contextKey)\` maps keywords (next/prev/first/last) to \`{cursor, direction}\`. \`advancePaginationState\` manages stack — back-then-forward truncates stale entries. \`hasPreviousPage\` checks \`page\_index > 0\`. \`paginationHint()\` builds nav strings. All list commands use this. Critical: \`resolveCursor()\` must be called inside \`org-all\` override closures, not before \`dispatchOrgScopedList\`. + +* **Pagination contextKey must include all query-varying parameters with escaping**: Pagination \`contextKey\` must encode every query-varying parameter (sort, query, period) with \`escapeContextKeyValue()\` (replaces \`|\` with \`%7C\`). Provide fallback before escaping since \`flags.period\` may be \`undefined\` in tests despite defaults: \`flags.period ? escapeContextKeyValue(flags.period) : "90d"\`. \`DEFAULT\_PERIOD = "90d"\` is a module constant. Compare via \`formatTimeRangeFlag(flags.period) !== DEFAULT\_PERIOD\` or \`appendPeriodHint()\` from \`time-range.ts\`. - -* **Telemetry instrumentation pattern: withTracingSpan + captureException for handled errors**: For graceful-fallback operations, use \`withTracingSpan\` from \`src/lib/telemetry.ts\` for child spans and \`captureException\` from \`@sentry/bun\` (named import — Biome forbids namespace imports) with \`level: 'warning'\` for non-fatal errors. \`withTracingSpan\` uses \`onlyIfParent: true\` — no-op without active transaction. User-visible fallbacks use \`log.warn()\` not \`log.debug()\`. Several commands bypass telemetry by importing \`buildCommand\` from \`@stricli/core\` directly instead of \`../../lib/command.js\` (trace/list, trace/view, log/view, api.ts, help.ts). + +* **Redact sensitive flags in raw argv before sending to telemetry**: Telemetry argv redaction and context: \`withTelemetry\` calls \`initTelemetryContext()\` — sets user ID, email, runtime, is\_self\_hosted. Org from \`getDefaultOrganization()\` (SQLite). \`SENSITIVE\_FLAGS\` in \`telemetry.ts\` (currently \`token\`) — scans for \`--token\`/\`-token\`, replaces value with \`\[REDACTED]\`. Handles \`--flag value\` and \`--flag=value\`. Command tag format: \`sentry.issue.list\` (dot-joined with \`sentry.\` prefix). - -* **Testing Stricli command func() bodies via spyOn mocking**: To unit-test a Stricli command's \`func()\` body: (1) \`const func = await cmd.loader()\`, (2) \`func.call(mockContext, flags, ...args)\` with mock \`stdout\`, \`stderr\`, \`cwd\`, \`setContext\`. (3) \`spyOn\` namespace imports to mock dependencies (e.g., \`spyOn(apiClient, 'getLogs')\`). The \`loader()\` return type union causes \`.call()\` LSP errors — these are false positives that pass \`tsc --noEmit\`. When API functions are renamed (e.g., \`getLog\` → \`getLogs\`), update both spy target name AND mock return shape (single → array). Slug normalization (\`normalizeSlug\`) replaces underscores with dashes but does NOT lowercase — test assertions must match original casing (e.g., \`'CAM-82X'\` not \`'cam-82x'\`). + +* **Stricli parse functions can perform validation and sanitization at flag-parse time**: Stricli's \`parse\` fn on \`kind: "parsed"\` flags runs during argument parsing before \`func()\`. Can throw errors (including \`ValidationError\`) and log warnings. Current uses: \`parseCursorFlag\`, \`sanitizeQuery\`, \`parsePeriod\` (returns \`TimeRange\`), \`parseSort\`/\`parseSortFlag\`, \`numberParser\`/\`parseLimit\`. Optional period flags: \`flags.period\` is \`TimeRange | undefined\` — commands use \`TIME\_RANGE\_\*\` constants as defaults. \`formatTimeRangeFlag()\` converts back to display strings. \`appendPeriodHint(parts, period, flagName)\` in \`time-range.ts\` encapsulates \`if (formatted !== DEFAULT\_PERIOD) parts.push(...)\` across 4+ hint builders. diff --git a/src/lib/auth-hint.ts b/src/lib/auth-hint.ts index 9ec20dbf1..6ceacd433 100644 --- a/src/lib/auth-hint.ts +++ b/src/lib/auth-hint.ts @@ -22,6 +22,8 @@ * is wrong, just a helpful breadcrumb). */ +// biome-ignore lint/performance/noNamespaceImport: Sentry SDK recommends namespace import +import * as Sentry from "@sentry/node-core/light"; import { getActiveEnvVarName, getRawEnvToken, @@ -60,13 +62,16 @@ export function maybeWarnEnvTokenIgnored(): void { return; } - // No stored OAuth → no collision. + // No stored OAuth → no collision. DB access failures are reported + // to Sentry but must not crash the CLI — the hint is best-effort. + // When `hasStoredAuthCredentials()` throws, `hasStored` stays `false` + // and the subsequent guard suppresses the hint, which is the intended + // conservative behavior. let hasStored = false; try { hasStored = hasStoredAuthCredentials(); - } catch { - // DB access failed — silently skip; this is a best-effort hint. - return; + } catch (error) { + Sentry.captureException(error); } if (!hasStored) { return; @@ -95,7 +100,11 @@ function resolveStoredUserLabel(): string { try { const user = getUserInfo(); return user?.username ?? user?.email ?? user?.name ?? "stored OAuth user"; - } catch { + } catch (error) { + // DB read failure for the user-info cache is non-fatal — fall back + // to the neutral label — but still surface the error to Sentry so + // we can diagnose persistent cache-read failures. + Sentry.captureException(error); return "stored OAuth user"; } }