diff --git a/AGENTS.md b/AGENTS.md index 6b9e22784..cd37e3691 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -986,86 +986,91 @@ mock.module("./some-module", () => ({ | Add documentation | `docs/src/content/docs/` | | Hand-written command doc content | `docs/src/fragments/commands/` | - + ## Long-term Knowledge ### Architecture - -* **Auth token env var override pattern: SENTRY\_AUTH\_TOKEN > SENTRY\_TOKEN > SQLite**: Auth in \`src/lib/db/auth.ts\` follows layered precedence: \`SENTRY\_AUTH\_TOKEN\` > \`SENTRY\_TOKEN\` > SQLite OAuth token. \`getEnvToken()\` trims env vars (empty/whitespace = unset). \`AuthSource\` tracks provenance. \`ENV\_SOURCE\_PREFIX = "env:"\` — use \`.length\` not hardcoded 4. Env tokens bypass refresh/expiry. \`isEnvTokenActive()\` guards auth commands. Logout must NOT clear stored auth when env token active. These functions stay in \`db/auth.ts\` despite not touching DB because they're tightly coupled with token retrieval. + +* **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\`. - -* **Consola chosen as CLI logger with Sentry createConsolaReporter integration**: Consola is the CLI logger with Sentry \`createConsolaReporter\` integration. Two reporters: FancyReporter (stderr) + Sentry structured logs. Level via \`SENTRY\_LOG\_LEVEL\`. \`buildCommand\` injects hidden \`--log-level\`/\`--verbose\` flags. \`withTag()\` creates independent instances; \`setLogLevel()\` propagates via registry. All user-facing output must use consola, not raw stderr. \`HandlerContext\` intentionally omits stderr. + +* **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. - -* **DSN cache invalidation uses two-level mtime tracking (sourceMtimes + dirMtimes)**: \*\*DSN cache invalidation uses two-level mtime tracking\*\*: \`sourceMtimes\` (DSN-bearing files, catches in-place edits) + \`dirMtimes\` (every walked dir, catches new files) + root mtime fast-path + 24h TTL. Dropping either map is a correctness regression. Walker emits mtimes via \`onDirectoryVisit\` hook + \`recordMtimes\` option; DSN scanner uses \`grepFiles({pattern: DSN\_PATTERN, recordMtimes: true, onDirectoryVisit})\` as full-scan pipeline (~20% faster than old walkFiles path). \`scanCodeForFirstDsn\` stays on direct walker loop — worker-pool init cost (~20ms) dominates for single-DSN. \*\*sourceMtimes invariant\*\*: \`processMatch\` must record mtime for EVERY file containing a host-validated DSN, not just deduplicated new ones — track via \`fileHadValidDsn\` flag independent of \`seen.has(raw)\`. \*\*Error-path invariant\*\*: \`scanDirectory\` catch MUST return empty \`dirMtimes: {}\`, NOT partial map from callback (would silently bless unvisited dirs). \`ConfigError\` re-throws instead. + +* **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). - -* **Grep worker pool: binary-transferable matches + streaming dispatch in src/lib/scan/**: \*\*Grep worker pool\*\* (\`src/lib/scan/worker-pool.ts\` + \`grep-worker.js\`): lazy singleton, size \`min(8, max(2, availableParallelism()))\`. Matches encoded as \`Uint32Array\` quads \`\[pathIdx, lineNum, lineOffset, lineLength]\` + \`linePool\` string, transferred via \`postMessage(msg, \[ints.buffer])\` — ~40% faster than structuredClone. Worker imported via \`with { type: "text" }\` → \`Blob\` + \`URL.createObjectURL\`; \`new Worker(new URL(...))\` HANGS in \`bun build --compile\` binaries. \*\*FIFO \`pending\` queue per worker\*\* — per-dispatch \`addEventListener\` causes wrong-request resolution. \*\*\`ref()\`/\`unref()\` are idempotent booleans, NOT refcounted\*\* — only unref when \`inflight\` drops to 0; workers spawn unref'd so idle pool doesn't block CLI exit. Readiness-failure handler must close over its own slot, not \`pop()\`. Disable via \`SENTRY\_SCAN\_DISABLE\_WORKERS=1\`. Surface pipeline failures: track \`dispatchedBatches\`/\`failedBatches\`, \`await Promise.allSettled(dispatchPromises)\` in \`finally\`; throw if all batches failed so DSN cache doesn't persist false-negatives. + +* **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. - -* **Input validation layer: src/lib/input-validation.ts guards CLI arg parsing**: CLI arg input validation (\`src/lib/input-validation.ts\`): Four validators — \`rejectControlChars\` (ASCII < 0x20), \`rejectPreEncoded\` (%XX), \`validateResourceId\` (rejects ?, #, %, whitespace), \`validateEndpoint\` (rejects \`..\` traversal). Applied in \`parseSlashOrgProject\`, \`parseOrgProjectArg\`, \`parseIssueArg\`, \`normalizeEndpoint\`. NOT applied in \`parseSlashSeparatedArg\` for plain IDs (may contain structural separators). Env vars and DB cache values are trusted. + +* **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. - -* **Magic @ selectors resolve issues dynamically via sort-based list API queries**: Magic @ selectors resolve issues dynamically: \`@latest\`, \`@most\_frequent\` in \`parseIssueArg\` detected before \`validateResourceId\` (@ not in forbidden charset). \`SELECTOR\_MAP\` provides case-insensitive matching. \`resolveSelector\` maps to \`IssueSort\` values, calls \`listIssuesPaginated\` with \`perPage: 1\`, \`query: 'is:unresolved'\`. Supports org-prefixed: \`sentry/@latest\`. Unrecognized \`@\`-prefixed strings fall through. \`ParsedIssueArg\` union includes \`{ type: 'selector' }\`. + +* **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()\`. - -* **Sentry SDK uses @sentry/node-core/light instead of @sentry/bun to avoid OTel overhead**: Sentry SDK uses \`@sentry/node-core/light\` instead of \`@sentry/bun\` to avoid OpenTelemetry overhead (~150ms, 24MB). \`@sentry/core\` barrel patched via \`bun patch\` to remove ~32 unused exports. Gotcha: \`LightNodeClient\` hardcodes \`runtime: { name: 'node' }\` AFTER spreading options — fix by patching \`client.getOptions().runtime\` post-init (mutable ref). Transport uses Node \`http\` instead of native \`fetch\`. Upstream: getsentry/sentry-javascript#19885, #19886. + +* **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. - -* **src/lib/scan/ module: policy-free file walker with IgnoreStack**: \`src/lib/scan/\` — policy-free walker + grep/glob engine. Files: \`walker.ts\` (DFS async gen; \`minDepth\`/\`maxDepth\` cap DESCENT not yield; \`timeBudgetMs\`, \`AbortSignal\`, \`onDirectoryVisit\`, \`recordMtimes\`), \`ignore.ts\` (IgnoreStack two-tier \`#rootIg\`+\`#nestedByRelDir\`), \`binary.ts\` (8KB NUL sniff + ext fast-path), \`regex.ts\` (\`(?i)\`/\`(?im)\`/\`(?U)\` → JS flags), \`grep.ts\`/\`glob.ts\` (picomatch \`{dot:true}\`). DSN policy lives in \`src/lib/dsn/scan-options.ts\`. \*\*Unification ceiling\*\*: downward tree-walker only — NOT for upward walks (\`walk-up.ts\`), single-file reads, cache sweeps, or writes. Gotchas: \`classifyFile\` short-circuits when \`cfg.extensions\` set; \`buildWalkOptions\` MUST forward \`descentHook\`+\`alwaysSkipDirs\`+\`followSymlinks\`+\`recordMtimes\`+\`clock\`; NEVER \`pLimit.clearQueue()\` (hangs) — use \`onResult\` returning \`{done:true}\`; \`path.join\`/\`relative\`/\`extname\` ~10× slower than manual string ops in hot loop; scan module trusts \`opts.path\`, caller enforces sandbox. + +* **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. - -* **Telemetry opt-out is env-var-only — no persistent preference or DO\_NOT\_TRACK**: Telemetry opt-out priority: (1) \`SENTRY\_CLI\_NO\_TELEMETRY=1\`, (2) \`DO\_NOT\_TRACK=1\`, (3) \`metadata.defaults.telemetry\`, (4) default on. DB read try/catch wrapped (runs before DB init). Schema v13 merged \`defaults\` table into \`metadata\` KV with keys \`defaults.{org,project,telemetry,url}\`; getters/setters in \`src/lib/db/defaults.ts\`. \`sentry cli defaults\` uses variadic \`\[key, value?]\`: no args → show all; 1 arg → show key; 2 args → set; \`--clear\` without args → clear all (guarded); \`--clear key\` → clear specific. \`computeTelemetryEffective()\` returns resolved source for display. + +* **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/event IDs are NOT v7. \`decodeUuidV7Timestamp()\` and \`ageInDaysFromUuidV7()\` in \`src/lib/hex-id.ts\` return null for non-v7, safe to call unconditionally. Enables deterministic 'past retention' messages; wired in \`recoverHexId\` 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'\`. Shared hex primitives (\`HEX\_ID\_RE\`, \`SPAN\_ID\_RE\`, \`UUID\_DASH\_RE\`, \`LEADING\_HEX\_RE\`, \`MIDDLE\_ELLIPSIS\_RE\`, \`HexEntityType\`) live in \`hex-id.ts\`. - -* **Zod schema on OutputConfig enables self-documenting JSON fields in help and SKILL.md**: Zod schema on OutputConfig enables self-documenting JSON fields: List commands register \`schema?: ZodType\` on \`OutputConfig\\`. \`extractSchemaFields()\` produces \`SchemaFieldInfo\[]\` from Zod shapes. \`buildFieldsFlag()\` enriches \`--fields\` brief; \`enrichDocsWithSchema()\` appends fields to \`fullDescription\`. Schema exposed as \`\_\_jsonSchema\` on built commands — \`introspect.ts\` reads it into \`CommandInfo.jsonFields\`, \`help.ts\` and \`generate-skill.ts\` render it. For \`buildOrgListCommand\`/\`dispatchOrgScopedList\`, pass \`schema\` via \`OrgListConfig\`. + +* **Stricli route errors are uninterceptable — only post-run detection works**: Stricli error gaps: (1) Route failures uninterceptable — Stricli writes stderr and 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\` using \`fuzzyMatch()\` (up to 3 suggestions); JSON includes \`suggestions\`. (4) Plural alias detection in \`app.ts\`. + + +* **Three Sentry APIs for span custom attributes with different capabilities**: Three Sentry span APIs with different attribute capabilities: (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. ### Decision - -* **All view subcommands should use \ \ positional pattern**: All \`\* view\` subcommands use \`\ \\` positional pattern (Intent-First Correction UX): target is optional \`org/project\`. Use opportunistic arg swapping with \`log.warn()\` when args are wrong order — when intent is unambiguous, do what they meant. Normalize at command level, keep parsers pure. Model after \`gh\` CLI. Exception: \`auth\` uses \`defaultCommand: "status"\` (no viewable entity). Routes without defaults: \`cli\`, \`sourcemap\`, \`repo\`, \`team\`, \`trial\`, \`release\`, \`dashboard/widget\`. + +* **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. + + +* **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-derived terminal color palette tuned for dual-background contrast**: Terminal color palette tuned for dual-background contrast: 10-color chart palette derived from Sentry's categorical hues (\`static/app/utils/theme/scraps/tokens/color.tsx\`), adjusted to mid-luminance for ≥3:1 contrast on both dark and light backgrounds. Adjustments: orange #FF9838→#C06F20, green #67C800→#3D8F09, yellow #FFD00E→#9E8B18, purple #5D3EB2→#8B6AC8, indigo #50219C→#7B50D0; blurple/pink/magenta unchanged; teal #228A83 added. Hex preferred over ANSI 16-color for guaranteed contrast. + +* **Trace-related commands must handle project consistently across CLI**: Trace/log commands 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()\`. Endpoint is PRIVATE (no \`@sentry/api\` types); hand-written \`TraceLogSchema\` in \`src/types/sentry.ts\` required. ### Gotcha - -* **AuthError constructor takes reason first, message second**: \`AuthError(reason: AuthErrorReason, message?: string)\` where \`AuthErrorReason\` is \`"not\_authenticated" | "expired" | "invalid"\`. Easy to accidentally swap args as \`new AuthError("Token expired", "expired")\` — the string \`"Token expired"\` gets assigned as \`reason\` (invalid enum value). Tests aren't type-checked (tsconfig excludes them), so TypeScript won't catch this. Correct: \`new AuthError("expired", "Token expired")\`. Default messages exist for each reason, so the second arg is often unnecessary. + +* **Biome lint differs between local lint:fix and CI lint**: Biome \`lint:fix\` (local) differs from CI \`lint\` — auto-fix can hide issues CI still catches: (1) \`noPrecisionLoss\` on integer literals >2^53, (2) \`noIncrementDecrement\` on \`count++\`, (3) import ordering when a named import follows non-import runtime code. Formatter rewrites multi-line imports to single-line when they fit. Always run \`bun run lint\` before pushing. Use \`for...of\` destructuring or \`i += 1\` instead of \`++\`; use \`Number(string)\` or split literals instead of \`1\_735\_689\_600\_000\_000\_001\`. - -* **DEFAULT\_SKIP\_DIRS prunes dist/build/.next — narrow for build-output scans**: \*\*scan module walkFiles migration gotchas\*\*: (1) \`DEFAULT\_SKIP\_DIRS\` in \`src/lib/scan/ignore.ts\` prunes broadly: \`node\_modules\`, VCS, \`dist\`, \`build\`, \`target\`, \`.next\`, \`.nuxt\`, \`.output\`, \`vendor\`, \`.gradle\`, \`.bundle\`, \`coverage\`, \`.cache\`, \`.turbo\`. Walker starting AT \`dist/\` doesn't prune cwd itself. For \`sourcemap/inject.ts\`, pass narrowed \`alwaysSkipDirs: \["node\_modules"]\` + \`respectGitignore: false\`. (2) \`walkFiles\` defaults \`maxFileSize: 256KB\` — silently skips large JS bundles (webpack/rollup/Next.js chunks routinely exceed this). Pass \`maxFileSize: Number.POSITIVE\_INFINITY\` when replacing unlimited \`readdir\` loops. (3) \`walkFiles\` enforces \`path.isAbsolute(opts.cwd)\` and throws on relative paths. CLI users pass \`./dist\`; callers must \`resolvePath(dir)\` before handing to walker. (4) When \`extensions\` is set, walker skips binary NUL-sniff. (5) Migration checklist: verify \`respectGitignore\`, \`hidden\`, \`alwaysSkipDirs\`, \`extensions\`, \`followSymlinks\`, \`maxFileSize\`, absolute-cwd. DFS order differs from files-first-then-dirs. + +* **Bun mock.module for node:tty requires default export and class stubs**: Bun testing gotchas: (1) \`mock.module()\` for CJS built-ins (e.g. \`node:tty\`) needs \`default\` re-export plus named exports, declared 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())\` for read-only. (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(){} })\`. - -* **mapFilesConcurrent skips null but not empty arrays — callers must return null for no-op**: Scan/formatter off-by-ones: (1) \`mapFilesConcurrent\` in \`src/lib/scan/concurrent.ts\` filters \`null\` but NOT empty arrays — fires \`onResult\` per empty result. Callers (e.g. \`processEntry\` in \`dsn/code-scanner.ts\`) must return \`null\` (not \`\[]\`) for no-op files (~99% of 10k-file walks). Stream variant filters both. (2) \`collectGlob\`/\`collectGrep\` must NOT forward \`maxResults\` to underlying iterator — collector drains uncapped, sets \`truncated=true\` on overshoot; forwarding stops iterator at exactly N without \`truncated\`. (3) \`filterFields\` in \`formatters/json.ts\` uses dot-notation — property tests must use \`\[a-zA-Z0-9\_]\` charset to avoid ambiguous keys with dots. +### Pattern - -* **process.stdin.isTTY unreliable in Bun — use isatty(0) and backfill for clack**: \`process.stdin.isTTY\` unreliable in Bun — use \`isatty(0)\` from \`node:tty\`. Bun's single-file binary can leave \`process.stdin.isTTY === undefined\` on TTY fds inherited via redirects like \`exec … \ +* **403 scope extraction via api-scope.ts helpers**: \`src/lib/api-scope.ts\` \`extractRequiredScopes(detail)\` 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()\`. - -* **Spinner stdout/stderr collision: log messages inside withProgress appear on spinner line**: Spinner stdout/stderr collision: \`withProgress\` in \`src/lib/polling.ts\` writes to stdout with \`\r\x1b\[K\` (no newline); consola writes to stderr. Any \`log.info()\` called \*\*inside\*\* the callback appears on the spinner line because stderr doesn't know stdout's carriage-return position. Fix: propagate data out via return value, call \`log.info()\` \*\*after\*\* \`withProgress\` completes (finally block clears the line). Affected \`downloadBinaryToTemp\` in \`upgrade.ts\`. + +* **buildApiUrl helper for safe Sentry API URL construction**: \`buildApiUrl(regionUrl, ...segments)\` in \`src/lib/api/infrastructure.ts\` composes Sentry API URLs. Owns \`/api/0/\` prefix, trailing slash, per-segment \`encodeURIComponent\`. Safety: slugs containing \`/\` get encoded correctly. Zero segments → \`base/api/0/\`. Replaces error-prone \`${base}/api/0/organizations/${encodeURIComponent(org)}/...\` patterns. Use for all URL-composition sites in domain API modules. Since #788 (cache identity scoping), all cache invalidation prefix construction uses it. \`stripTrailingSlash\` is no longer exported. - -* **Stricli rejects unknown flags — pre-parsed global flags must be consumed from argv**: Stricli flag parsing traps: (1) Unknown \`--flag\`s rejected — global flags parsed in \`bin.ts\` MUST be spliced from argv (check both \`--flag value\` and \`--flag=value\`). (2) \`FLAG\_NAME\_PATTERN\` requires 2+ chars after \`--\`; single-char flags like \`--x\` silently become positionals — use aliases (\`-x\` → longer name). Bit \`dashboard widget --x\`/\`--y\`. (3) \`FlagDef.hidden\` is propagated by \`extractFlags\` so \`generateCommandDoc\` filters hidden flags alongside \`help\`/\`helpAll\`; hidden \`--log-level\`/\`--verbose\` appear only in global options docs. + +* **fetchWithTimeout uses bare fetch reference for test mockability**: \`fetchWithTimeout\` in \`src/lib/sentry-client.ts\` calls \`fetch(input, ...)\` as a bare global reference — this is load-bearing for tests that swap \`globalThis.fetch\`. Do NOT refactor to capture \`fetch\` at module load (via destructuring or aliasing) — all tests using \`mockFetch()\` would silently fall through to real network. \`resetAuthenticatedFetch()\` in test \`beforeEach\` clears the authenticated-fetch singleton (for auth state), NOT the fetch mock itself. If refactoring, add explicit \`// must remain bare fetch() for test mockability\` comment. - -* **test:unit glob only picks up test/lib, test/commands, test/types, test/isolated**: \`bun test\` script globs in \`package.json\` are narrow: \`test:unit\` = \`test/lib test/commands test/types\`, \`test:isolated\` = \`test/isolated\`, \`test:e2e\` = \`test/e2e\`. Tests placed under \`test/fixtures/\`, \`test/scripts/\`, or \`test/script/\` are NOT picked up by any standard CI script despite being valid Bun test files. Place new tests under \`test/lib/\/\` to inherit coverage. Note: \`test/script/\` (singular) exists with script tests but is also outside the globs. + +* **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. - -* **Whole-buffer matchAll slower than split+test when aggregated over many files**: grep perf/correctness traps in \`src/lib/scan/grep.ts\`: (1) Whole-buffer \`regex.exec\` 12× faster per-file but ~1.6× SLOWER than \`split('\n')+test\` aggregated over 10k files — early-exit at \`maxResults\` via \`mapFilesConcurrent.onResult\` wins. (2) Literal prefilter is FILE-LEVEL gate (\`indexOf\` → skip), then whole-buffer exec. Per-line-verify breaks cross-newline patterns and Unicode length-changing \`toLowerCase\` (Turkish \`İ\`→\`i̇\`). Extractor handles multi-char escapes via \`escapeSequenceLength\`; \`hasTopLevelAlternation\`+\`skipGroup\` must call \`skipCharacterClass\` (PCRE \`\[]abc]\` ≠ JS empty class). (3) \`GrepOptions.multiline\` defaults \`true\` (rg/git-grep); \`compilePattern\` default stays \`false\`. (4) Wake-latch race: \`let notify=null; await new Promise(r=>notify=r)\` loses signals if producer wakes before assignment — use latched \`pendingWake\` flag. + +* **Identity-scoped response cache via fingerprint mixin**: Identity-scoped response cache: \`buildCacheKey(method, url)\` mixes in memoized \`getIdentityFingerprint()\` (MD5 of \`kind|secret\` truncated to 16 hex; CodeQL dismissed — namespacing, not auth). \`CacheEntry\` persists identity so \`invalidateCachedResponsesMatching(prefix)\` skips other identities. Invalidation centralized at \`authenticatedFetch\` in \`sentry-client.ts\` — after 2xx non-GET, runs \`computeInvalidationPrefixes(fullUrl, getApiBaseUrl())\` walking hierarchy up to \`organizations/{org}/\` plus cross-endpoint rules via \`extra\`/\`extraAbsolute\` (control-silo vs region-silo). \*\*Contract: never throws\*\* — wrapped in try/catch. \`SKIP\_INVALIDATION\_PATTERNS\` short-circuits chunk-upload/assemble. \`clearAuth()\` dynamically imports \`clearResponseCache\` to break cycle. Always use prefix-match with trailing slash; exact-match removed. URL-only hook can't decode bulk mutations with IDs in query params (e.g. \`mergeIssues\`) — invalidate per-ID at caller. -### Pattern + +* **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\`. - -* **Bench harness with concurrency sweep + deterministic fixtures**: Bench harness (not in CI): \`script/bench.ts\` + \`script/bench-sweep.ts\`. Scripts: \`bun run bench\`, \`bench:save\` (\`.bench/baseline.json\`), \`bench:compare\` (fails >20% regression), \`bench:sweep\` (concurrency tuning). Flags: \`--size\`, \`--op\`, \`--repo \\` (refuses \`--save-baseline\`), \`--runs\`, \`--warmup\`, \`--json\`. Fixtures in \`test/fixtures/bench/\` use xorshift32-seeded generator for byte-identical trees in \`os.tmpdir()/sentry-cli-bench/\`. Op labels match Sentry span names. \`withBenchDb(fn)\` scopes \`SENTRY\_CONFIG\_DIR\`; \`clearDsnDetectionCache()\` between cold iterations. \*\*CONCURRENCY\_LIMIT\*\*: sweep showed knee=availableParallelism for walker-fed workloads; pure I/O wants 16-32. Kept at \`min(16, max(2, availableParallelism()))\`. + +* **Memoize identity fingerprint with test-reset hook + setAuthToken invalidation**: Memoize + test-reset pattern in src/lib/db/auth.ts: \`getIdentityFingerprint()\`, \`getAuthToken()\` (as \`cachedAuthToken\`), and the full auth row used by \`refreshToken()\` (as \`cachedAuthRow\`) are all memoized at module scope. Use wrapper-object sentinels \`{ value }\` to distinguish 'not cached' from 'cached as undefined' (logged out). Invalidate via \`reset\*Cache()\` exports at the only mutation points: \`setAuthToken()\` and \`clearAuth()\`. Safe under OAuth rotation (refresh\_token preserved) and 401 refresh (routes through setAuthToken). Tests mutating \`process.env.SENTRY\_AUTH\_TOKEN\` bypass the mutation hooks — must call reset functions manually in beforeEach and inside property-test bodies. \`useTestConfigDir\` calls all three resets in beforeEach/afterEach to prevent cross-file pollution in Bun's sequential test runner. Same memo+reset pattern mirrors \`resetUpdateNotificationState\`, \`resetCacheState\`, \`resetAuthHintState\`. Fixed N+1 SQL hits per API request (CLI-13V). - -* **Event view cross-org fallback chain: project → org → all orgs**: Event view cross-org fallback chain: \`fetchEventWithContext\` in \`src/commands/event/view.ts\` tries (1) \`getEvent(org, project, eventId)\`, (2) \`resolveEventInOrg(org, eventId)\`, (3) \`findEventAcrossOrgs(eventId, { excludeOrgs })\`. Extracted into \`tryEventFallbacks()\` for Biome complexity limit. \`excludeOrgs\` only set when same-org search returned null (not on transient error). Both catch blocks re-throw \`AuthError\` while swallowing transient errors. Warning distinguishes same-org/different-project from different-org via \`crossOrg.org === org\`. + +* **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 (including \`ValidationError\`) and log warnings. Uses: \`parseCursorFlag\`, \`sanitizeQuery\`, \`parsePeriod\` (returns \`TimeRange\`), \`parseSort\`/\`parseSortFlag\`, \`numberParser\`/\`parseLimit\`. Optional period flags: \`flags.period\` is \`TimeRange | undefined\` — commands default to \`TIME\_RANGE\_\*\` constants. \`formatTimeRangeFlag()\` converts back; \`appendPeriodHint()\` in \`time-range.ts\` encapsulates hint-building across 4+ commands. - -* **PR review workflow: Cursor Bugbot + Seer + human cycle**: PR review workflow (Cursor Bugbot + Seer + human): \`gh pr checks \ --json state,link\` + \`gh run view --log-failed\` for CI. Unresolved threads via \`gh api graphql\` with \`reviewThreads\` query filtering \`isResolved:false\`+\`isMinimized:false\`. After fixes: reply via \`gh api repos/.../pulls/comments/\/replies\` then resolve via \`resolveReviewThread\` mutation. Bots auto-resolve on detected fix. Statuses: \`UNSTABLE\`=non-blocking bots running, \`BLOCKED\`=required CI pending, \`CLEAN\`+\`MERGEABLE\`=ready. Repo is squash-merge only. Expect 4-6 rounds on subtle regex/Unicode PRs. Bugbot findings usually real but occasionally assume PCRE/Python semantics (e.g. \`\[]abc]\` is class-with-\`]\` in PCRE but empty class in JS) — verify with reproduction. Dispatch self-review subagent between rounds. +### Preference - -* **Worker bodies as real .js files via text-import, not inline strings**: \*\*Worker bodies as real .js files via text-import\*\*: Prefer real \`.js\` files imported as text over inline template-literal worker sources — inline strings invisible to Biome/TypeScript. Pattern: write worker as plain JS (CJS-style \`require("node:fs")\` to avoid TLA parsing), import via \`with { type: "text" }\`, feed to \`Blob\` + \`URL.createObjectURL\`. Must be self-contained (no user imports). \*\*esbuild doesn't support \`with { type: "text" }\`\*\* — fix: \`script/text-import-plugin.ts\` intercepts via \`onResolve\`/\`onLoad\`, reads file, emits \`export default "\"\`. Shared between \`script/build.ts\` and \`script/bundle.ts\`. Bun runtime handles natively. TypeScript doesn't model the attribute — cast through \`as unknown as string\`. + +* **Code review style: BYK values brevity; trim JSDoc essays aggressively**: BYK code-review style — brevity first: terse 1-3 line JSDoc; remove comments that restate code; don't wrap try/catch around no-throw helpers (but DO wrap post-success housekeeping like cache invalidation — defense-in-depth); MD5 over HMAC for non-auth hashing; no lazy imports without documented reason. Prefer \`\[...new Set(items)]\` over hand-rolled dedupe; \`toSpliced\` over spread+new-array; spread/slice over \`.unshift()\` on returned API objects. Direct questions drive simplification ('inputs never change, why not memoize?' → memoize+reset). Dismiss CodeQL false positives via \`gh api\` with rationale. 'Centralized mechanism' → file follow-up issue, not scope creep. Implement trivial reviewer suggestions in-PR rather than deferring. Run subagent self-review on merge-ready PRs — typical yield 1-3 items (stale PR descriptions, CI-only lint, doc drift). Take bot findings (Cursor Bugbot, Seer) seriously even after self-review approval — expect 4-6 rounds on subtle Unicode/regex/error-handling PRs. diff --git a/src/lib/db/auth.ts b/src/lib/db/auth.ts index 44c0be11d..0e8e9260e 100644 --- a/src/lib/db/auth.ts +++ b/src/lib/db/auth.ts @@ -153,6 +153,9 @@ export function getAuthConfig(): AuthConfig | undefined { return; } +/** Memoized token. Wrapper distinguishes "not cached" from "cached as undefined". */ +let cachedAuthToken: { value: string | undefined } | undefined; + /** * Get the active auth token. * @@ -160,6 +163,15 @@ export function getAuthConfig(): AuthConfig | undefined { * With `SENTRY_FORCE_ENV_TOKEN=1`: checks env vars first (old behavior). */ export function getAuthToken(): string | undefined { + if (cachedAuthToken !== undefined) { + return cachedAuthToken.value; + } + const value = computeAuthToken(); + cachedAuthToken = { value }; + return value; +} + +function computeAuthToken(): string | undefined { const forceEnv = getEnv().SENTRY_FORCE_ENV_TOKEN?.trim(); if (forceEnv) { const envToken = getEnvToken(); @@ -196,6 +208,31 @@ export function getAuthToken(): string | undefined { return; } +/** Reset the memoized auth token. Tests only — call between auth-state mutations. */ +export function resetAuthTokenCache(): void { + cachedAuthToken = undefined; +} + +/** Memoized full auth row for {@link refreshToken}. Same wrapper contract as {@link cachedAuthToken}. */ +let cachedAuthRow: { value: AuthRow | undefined } | undefined; + +function getCachedAuthRow(): AuthRow | undefined { + if (cachedAuthRow !== undefined) { + return cachedAuthRow.value; + } + const db = getDatabase(); + const row = db.query("SELECT * FROM auth WHERE id = 1").get() as + | AuthRow + | undefined; + cachedAuthRow = { value: row }; + return row; +} + +/** Reset the memoized auth row. Tests only — call between auth-state mutations. */ +export function resetAuthRowCache(): void { + cachedAuthRow = undefined; +} + export function setAuthToken( token: string, expiresIn?: number, @@ -221,9 +258,11 @@ export function setAuthToken( ["id"] ); }); - // Auth row changed — drop the memoized fingerprint so the next - // `getIdentityFingerprint()` call reflects the new row. + // Auth row changed — drop memoized fingerprint, token, and row so the next + // read reflects the new row. resetIdentityFingerprintCache(); + resetAuthTokenCache(); + resetAuthRowCache(); } export async function clearAuth(): Promise { @@ -239,6 +278,8 @@ export async function clearAuth(): Promise { clearAllIssueOrgCache(); }); resetIdentityFingerprintCache(); + resetAuthTokenCache(); + resetAuthRowCache(); // Dynamic import avoids the auth→response-cache→auth cycle. try { @@ -424,10 +465,7 @@ export async function refreshToken( const { force = false } = options; const { AuthError } = await import("../errors.js"); - const db = getDatabase(); - const row = db.query("SELECT * FROM auth WHERE id = 1").get() as - | AuthRow - | undefined; + const row = getCachedAuthRow(); if (!row?.token) { // No stored token — try env token as fallback diff --git a/test/helpers.ts b/test/helpers.ts index 3d7b4429b..2d82a058e 100644 --- a/test/helpers.ts +++ b/test/helpers.ts @@ -8,6 +8,11 @@ import { afterEach, beforeEach } from "bun:test"; import { mkdirSync } from "node:fs"; import { mkdtemp, rm } from "node:fs/promises"; import { join } from "node:path"; +import { + resetAuthRowCache, + resetAuthTokenCache, + resetIdentityFingerprintCache, +} from "../src/lib/db/auth.js"; import { CONFIG_DIR_ENV_VAR, closeDatabase } from "../src/lib/db/index.js"; // biome-ignore lint/performance/noBarrelFile: re-exporting a single constant, not a barrel @@ -105,12 +110,19 @@ export function useTestConfigDir( beforeEach(async () => { savedConfigDir = process.env[CONFIG_DIR_ENV_VAR]; closeDatabase(); + // Fresh DB — drop module-scoped auth caches from the previous test. + resetAuthTokenCache(); + resetAuthRowCache(); + resetIdentityFingerprintCache(); dir = await createTestConfigDir(prefix, options); process.env[CONFIG_DIR_ENV_VAR] = dir; }); afterEach(async () => { closeDatabase(); + resetAuthTokenCache(); + resetAuthRowCache(); + resetIdentityFingerprintCache(); // Always restore the previous value — never delete. // Deleting process.env.SENTRY_CONFIG_DIR causes failures in test files // that load after this afterEach runs, because their module-level code diff --git a/test/lib/db/auth.property.test.ts b/test/lib/db/auth.property.test.ts index ce79c381e..e91b06d4d 100644 --- a/test/lib/db/auth.property.test.ts +++ b/test/lib/db/auth.property.test.ts @@ -22,6 +22,8 @@ import { getAuthToken, isEnvTokenActive, refreshToken, + resetAuthRowCache, + resetAuthTokenCache, setAuthToken, } from "../../../src/lib/db/auth.js"; import { useTestConfigDir } from "../../helpers.js"; @@ -43,6 +45,8 @@ beforeEach(() => { savedSentryToken = process.env.SENTRY_TOKEN; delete process.env.SENTRY_AUTH_TOKEN; delete process.env.SENTRY_TOKEN; + resetAuthTokenCache(); + resetAuthRowCache(); }); afterEach(() => { @@ -58,10 +62,17 @@ afterEach(() => { } }); +/** Invalidate between property iterations — env-var mutations bypass setAuthToken. */ +function resetAuthCaches() { + resetAuthTokenCache(); + resetAuthRowCache(); +} + describe("property: env var priority", () => { test("SENTRY_AUTH_TOKEN always wins over SENTRY_TOKEN", () => { fcAssert( property(tokenArb, tokenArb, (authToken, sentryToken) => { + resetAuthCaches(); process.env.SENTRY_AUTH_TOKEN = authToken; process.env.SENTRY_TOKEN = sentryToken; @@ -77,6 +88,7 @@ describe("property: env var priority", () => { test("stored OAuth wins over env var (default behavior)", () => { fcAssert( property(tokenArb, tokenArb, (envToken, storedToken) => { + resetAuthCaches(); setAuthToken(storedToken); process.env.SENTRY_AUTH_TOKEN = envToken; @@ -91,16 +103,19 @@ describe("property: env var priority", () => { test("SENTRY_FORCE_ENV_TOKEN overrides stored OAuth", () => { fcAssert( property(tokenArb, tokenArb, (envToken, storedToken) => { + resetAuthCaches(); setAuthToken(storedToken); process.env.SENTRY_AUTH_TOKEN = envToken; try { process.env.SENTRY_FORCE_ENV_TOKEN = "1"; + resetAuthCaches(); expect(getAuthToken()).toBe(envToken.trim()); expect(getAuthConfig()?.source).toBe( "env:SENTRY_AUTH_TOKEN" satisfies AuthSource ); } finally { delete process.env.SENTRY_FORCE_ENV_TOKEN; + resetAuthCaches(); } }), { numRuns: DEFAULT_NUM_RUNS } @@ -110,6 +125,7 @@ describe("property: env var priority", () => { test("stored token used when no env vars set", () => { fcAssert( property(tokenArb, (storedToken) => { + resetAuthCaches(); setAuthToken(storedToken); expect(getAuthToken()).toBe(storedToken); @@ -124,6 +140,7 @@ describe("property: env tokens never trigger refresh", () => { test("refreshToken returns env token without refreshing", async () => { await fcAssert( asyncProperty(tokenArb, async (envToken) => { + resetAuthCaches(); process.env.SENTRY_AUTH_TOKEN = envToken; const result = await refreshToken(); @@ -139,6 +156,7 @@ describe("property: env tokens never trigger refresh", () => { test("refreshToken with force=true still returns env token without refreshing", async () => { await fcAssert( asyncProperty(tokenArb, async (envToken) => { + resetAuthCaches(); process.env.SENTRY_AUTH_TOKEN = envToken; const result = await refreshToken({ force: true }); @@ -154,6 +172,7 @@ describe("property: isEnvTokenActive consistency", () => { test("when no env token, getAuthConfig never returns env source", () => { fcAssert( property(option(tokenArb), (storedTokenOpt) => { + resetAuthCaches(); // Clean slate — no env tokens delete process.env.SENTRY_AUTH_TOKEN; delete process.env.SENTRY_TOKEN; @@ -177,6 +196,7 @@ describe("property: isEnvTokenActive consistency", () => { test("stored OAuth takes priority: getAuthConfig returns oauth even when env token is set", () => { fcAssert( property(tokenArb, tokenArb, (envToken, storedToken) => { + resetAuthCaches(); process.env.SENTRY_AUTH_TOKEN = envToken; setAuthToken(storedToken); @@ -194,6 +214,7 @@ describe("property: source round-trip", () => { test("source correctly identifies SENTRY_AUTH_TOKEN", () => { fcAssert( property(tokenArb, (token) => { + resetAuthCaches(); process.env.SENTRY_AUTH_TOKEN = token; const config = getAuthConfig(); expect(config?.source).toBe("env:SENTRY_AUTH_TOKEN"); @@ -207,6 +228,7 @@ describe("property: source round-trip", () => { test("source correctly identifies SENTRY_TOKEN", () => { fcAssert( property(tokenArb, (token) => { + resetAuthCaches(); process.env.SENTRY_TOKEN = token; const config = getAuthConfig(); expect(config?.source).toBe("env:SENTRY_TOKEN"); diff --git a/test/lib/db/auth.test.ts b/test/lib/db/auth.test.ts index 132892926..cd0524cbf 100644 --- a/test/lib/db/auth.test.ts +++ b/test/lib/db/auth.test.ts @@ -10,6 +10,7 @@ import { afterEach, beforeEach, describe, expect, test } from "bun:test"; import { ANON_IDENTITY, + clearAuth, getActiveEnvVarName, getAuthConfig, getAuthToken, @@ -18,9 +19,12 @@ import { isAuthenticated, isEnvTokenActive, refreshToken, + resetAuthRowCache, + resetAuthTokenCache, resetIdentityFingerprintCache, setAuthToken, } from "../../../src/lib/db/auth.js"; +import { getDatabase } from "../../../src/lib/db/index.js"; import { useTestConfigDir } from "../../helpers.js"; useTestConfigDir("auth-env-"); @@ -34,6 +38,8 @@ beforeEach(() => { delete process.env.SENTRY_AUTH_TOKEN; delete process.env.SENTRY_TOKEN; resetIdentityFingerprintCache(); + resetAuthTokenCache(); + resetAuthRowCache(); }); afterEach(() => { @@ -204,7 +210,6 @@ describe("OAuth-preferred auth (#646)", () => { describe("clearAuth: integration with per-account caches", () => { test("clearAuth drops issue_org_cache entries (prevents cross-account leakage)", async () => { - const { clearAuth } = await import("../../../src/lib/db/auth.js"); const { setCachedIssueOrg, getCachedIssueOrg } = await import( "../../../src/lib/db/issue-org-cache.js" ); @@ -323,3 +328,89 @@ describe("getIdentityFingerprint", () => { expect(getIdentityFingerprint()).toBe(fp); }); }); + +describe("getAuthToken memoization", () => { + test("returns cached value on repeated calls without re-reading the DB", () => { + setAuthToken("stored_token"); + const first = getAuthToken(); + expect(first).toBe("stored_token"); + + // Mutate the DB row directly behind getAuthToken's back — a cached + // read must not reflect this change until the cache is invalidated. + getDatabase().query("UPDATE auth SET token = 'mutated' WHERE id = 1").run(); + + // Still returns the cached value + expect(getAuthToken()).toBe("stored_token"); + + // After reset, the new value is read + resetAuthTokenCache(); + expect(getAuthToken()).toBe("mutated"); + }); + + test("caches the logged-out state (undefined) without re-reading", () => { + expect(getAuthToken()).toBeUndefined(); + + // Write a token directly to DB, bypassing setAuthToken. The cached + // undefined must persist until invalidated. + getDatabase() + .query("INSERT INTO auth (id, token, updated_at) VALUES (1, 'sneaky', ?)") + .run(Date.now()); + + expect(getAuthToken()).toBeUndefined(); + + resetAuthTokenCache(); + expect(getAuthToken()).toBe("sneaky"); + }); + + test("setAuthToken invalidates the cache", () => { + setAuthToken("token_a"); + expect(getAuthToken()).toBe("token_a"); + + setAuthToken("token_b"); + // No manual reset — setAuthToken must have invalidated the cache + expect(getAuthToken()).toBe("token_b"); + }); + + test("clearAuth invalidates the cache", async () => { + setAuthToken("token_to_clear"); + expect(getAuthToken()).toBe("token_to_clear"); + + await clearAuth(); + expect(getAuthToken()).toBeUndefined(); + }); + + test("env-var change requires manual cache reset (documented contract)", () => { + expect(getAuthToken()).toBeUndefined(); + + // Env mutation without reset: cache stays stale (by design). + process.env.SENTRY_AUTH_TOKEN = "env_token"; + expect(getAuthToken()).toBeUndefined(); + + resetAuthTokenCache(); + expect(getAuthToken()).toBe("env_token"); + }); +}); + +describe("refreshToken row-read memoization", () => { + test("setAuthToken between refreshToken calls is reflected", async () => { + // refreshToken reads the full row; invalidation must propagate so the + // second call sees the freshly stored token. + setAuthToken("first_token", 3600, "refresh_1"); + const r1 = await refreshToken(); + expect(r1.token).toBe("first_token"); + + setAuthToken("second_token", 3600, "refresh_2"); + const r2 = await refreshToken(); + expect(r2.token).toBe("second_token"); + }); + + test("clearAuth invalidates the row cache", async () => { + setAuthToken("will_be_cleared", 3600, "refresh_x"); + const r1 = await refreshToken(); + expect(r1.token).toBe("will_be_cleared"); + + await clearAuth(); + // With nothing stored and no env var, refreshToken throws not_authenticated + await expect(refreshToken()).rejects.toThrow(); + }); +}); diff --git a/test/lib/db/model-based.test.ts b/test/lib/db/model-based.test.ts index 8e106818b..0f3d84caa 100644 --- a/test/lib/db/model-based.test.ts +++ b/test/lib/db/model-based.test.ts @@ -36,6 +36,8 @@ import { getAuthToken, isAuthenticated, isEnvTokenActive, + resetAuthRowCache, + resetAuthTokenCache, setAuthToken, } from "../../../src/lib/db/auth.js"; import { @@ -299,6 +301,9 @@ class SetEnvAuthTokenCommand implements AsyncCommand { async run(model: DbModel, _real: RealDb): Promise { process.env.SENTRY_AUTH_TOKEN = this.token; + // Env mutation bypasses setAuthToken's invalidation. + resetAuthTokenCache(); + resetAuthRowCache(); // Model stores trimmed value — matches real getEnvToken() which trims const trimmed = this.token.trim(); model.envAuthToken = trimmed || null; @@ -312,6 +317,8 @@ class ClearEnvAuthTokenCommand implements AsyncCommand { async run(model: DbModel, _real: RealDb): Promise { delete process.env.SENTRY_AUTH_TOKEN; + resetAuthTokenCache(); + resetAuthRowCache(); model.envAuthToken = null; } @@ -329,6 +336,8 @@ class SetEnvSentryTokenCommand implements AsyncCommand { async run(model: DbModel, _real: RealDb): Promise { process.env.SENTRY_TOKEN = this.token; + resetAuthTokenCache(); + resetAuthRowCache(); // Model stores trimmed value — matches real getEnvToken() which trims const trimmed = this.token.trim(); model.envSentryToken = trimmed || null; @@ -342,6 +351,8 @@ class ClearEnvSentryTokenCommand implements AsyncCommand { async run(model: DbModel, _real: RealDb): Promise { delete process.env.SENTRY_TOKEN; + resetAuthTokenCache(); + resetAuthRowCache(); model.envSentryToken = null; } @@ -775,6 +786,8 @@ describe("model-based: database layer", () => { const savedSentryToken = process.env.SENTRY_TOKEN; delete process.env.SENTRY_AUTH_TOKEN; delete process.env.SENTRY_TOKEN; + resetAuthTokenCache(); + resetAuthRowCache(); try { const setup = () => ({ model: createEmptyModel(), @@ -909,6 +922,8 @@ describe("model-based: database layer", () => { const cleanup = createIsolatedDbContext(); const savedAuthToken = process.env.SENTRY_AUTH_TOKEN; delete process.env.SENTRY_AUTH_TOKEN; + resetAuthTokenCache(); + resetAuthRowCache(); try { // Set token that expires immediately (negative expiresIn) setAuthToken(token, -1);