diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 490970d3c..07bfcb531 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -633,6 +633,9 @@ jobs: with: bun-version: "1.3.13" - uses: pnpm/action-setup@v4 + - uses: actions/setup-node@v6 + with: + node-version: "22" - uses: actions/cache@v5 id: cache with: diff --git a/.lore.md b/.lore.md index 93d44ae15..43c4ba4a7 100644 --- a/.lore.md +++ b/.lore.md @@ -4,9 +4,15 @@ ### Architecture + +* **@sentry/api SDK integration: type wrapping pattern and pagination helpers**: (architecture) @sentry/api SDK integration: wrap SDK types at \`src/lib/api/\*.ts\` with \`as unknown as SentryX\` casts; never leak to commands. \`src/types/sentry.ts\` uses \`Partial\ & RequiredCore\`. Avoid circular deps by importing SDK functions directly in \`src/lib/region.ts\`. \`unwrapResult\`/\`unwrapPaginatedResult\` must stay CLI-owned (SDK versions throw plain \`Error\`). Body-shape casts use \`Parameters\\[0]\["body"]\`. \`apiRequestToRegion/rawApiRequest\` expose only \`{ method, body, params, schema }\` (typed API); \`rawApiRequest\` adds \`headers?\`. Neither exposes \`timeout\`/\`signal\`. \`apiRequestToRegion\` auto-sets JSON Content-Type; \`rawApiRequest\` preserves strings. 204/205 throw \`ApiError\` not \`TypeError\`. + * **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. + +* **Completion fast-path skips Sentry SDK via SENTRY\_CLI\_NO\_TELEMETRY and SQLite telemetry queue**: (architecture) Completion fast-path + Sentry SDK setup: Shell completions set \`SENTRY\_CLI\_NO\_TELEMETRY=1\` in \`bin.ts\` before imports, skipping \`createTracedDatabase\` and \`@sentry/node-core/light\` load (~85ms). Timing queued to \`completion\_telemetry\_queue\` SQLite table; normal runs drain via \`DELETE ... RETURNING\`. Achieves ~60ms dev / ~140ms CI within 200ms budget. SDK uses \`@sentry/node-core/light\` (not \`@sentry/bun\`) to avoid OpenTelemetry overhead (~150ms, 24MB). \`@sentry/core\` barrel patched via \`bun patch\`. \`LightNodeClient\` hardcodes \`runtime:{name:'node'}\` AFTER spreading options — fix by patching \`client.getOptions().runtime\` post-init. Transport uses Node \`http\`. Always import from \`@sentry/node-core/light\`; root barrel pulls uninstalled @opentelemetry/instrumentation. When bumping SDK: remove patches, install, patch, edit, commit. + * **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. @@ -28,24 +34,27 @@ * **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' }\`. + +* **Project cache is org-scoped with three key formats and three population paths**: Project cache (\`project\_cache\` table) uses three key shapes: \`{orgId}:{projectId}\` (DSN), \`dsn:{publicKey}\` (DSN-only), \`list:{orgSlug}/{projectSlug}\` (batch). \`getCachedProjectBySlug\` queries all three for hot-path lookups. Population: DSN resolution, \`listProjects()\` batch, \`fetchProjectId\` seed. Resolution errors use live API via \`findSimilarProjectsAcrossOrgs\` — no cross-org cache search. + * **safe-read.ts wraps isRegularFile + Bun.file().text() for FIFO-safe user-path reads**: \`src/lib/safe-read.ts\` \`safeReadFile(path, operation): Promise\\` combines \`isRegularFile()\` + \`Bun.file().text()\` + broad error swallow (FIFO/ENOENT/EACCES/EPERM/EISDIR/ENOTDIR). Sole caller: \`apply-patchset.ts\`. \*\*Do NOT use for committed config loads\*\* — swallows EPERM/EISDIR, making \`chmod 000 .sentryclirc\` manifest as confusing 'no auth token'. For loud permission surfacing (\`tryReadSentryCliRc\`), call \`fs.promises.stat\` directly, gate on \`isFile()\`, catch only ENOENT/EACCES. \`read-files.ts\`/\`workflow-inputs.ts\` use direct stat to reuse one stat for size-gating. Test with real \`mkfifo\` + short timeout as hang detector. * **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 API: events require org+project, issues have legacy global endpoint**: Sentry API quirks: (1) Events need org+project (\`/projects/{org}/{project}/events/{id}/\`); issues use legacy global \`/api/0/issues/{id}/\`; traces need org only. (2) \`/users/me/\` returns 403 for OAuth — use \`/auth/\` instead via \`getControlSiloUrl()\`. (3) Chunk upload endpoint returns camelCase (\`chunkSize\`, etc.) — exception to snake\_case convention. + -* **Sentry CLI authenticated fetch architecture with response caching**: (architecture) Authenticated fetch + response cache: \`createAuthenticatedFetch\`: auth headers, 30s timeout, max 2 retries, 401 refresh, span tracing. \`buildAttemptFactory\` clones \`Request\`; do NOT materialize FormData (strips boundary). Per-endpoint timeout overrides (e.g. \`/autofix/\` 120s). Response cache RFC 7234 at \`~/.sentry/cache/responses/\`, GET 2xx only. TTL tiers: stable=5min, volatile=60s, immutable=24h. \`@sentry/api\` SDK passes Request with no init — undefined init → empty headers stripping Content-Type (HTTP 415); fall back to \`input.headers\` when init undefined. Guard \`Array.isArray(data)\` before \`.map()\` (SDK returns \`{}\` for 204/empty). GET response cache checked BEFORE fetch — tests asserting call counts see 0 calls if prior test cached same URL. +* **Sentry CLI authenticated fetch architecture with response caching**: (architecture) Authenticated fetch + response cache: \`createAuthenticatedFetch\`: auth headers, 30s timeout, max 2 retries, 401 refresh, span tracing. \`buildAttemptFactory\` clones \`Request\`; do NOT materialize FormData (strips boundary). Per-endpoint timeout overrides (e.g. \`/autofix/\` 120s). Response cache RFC 7234 at \`~/.sentry/cache/responses/\`, GET 2xx only. TTL tiers: stable=5min, volatile=60s, immutable=24h. \`@sentry/api\` SDK passes Request with no init — undefined init → empty headers stripping Content-Type (HTTP 415); fall back to \`input.headers\` when init undefined. Guard \`Array.isArray(data)\` before \`.map()\` (SDK returns \`{}\` for 204/empty; \`ReadableStream\` when Content-Type missing). GET response cache checked BEFORE fetch — tests asserting call counts see 0 calls if prior test cached same URL. Tests mocking fetch MUST call \`useTestConfigDir()\` + \`setAuthToken()\` + \`resetCacheState()\` + \`disableResponseCache()\` + \`resetAuthenticatedFetch()\` in beforeEach. -* **Sentry CLI resolve-target cascade has 5 priority levels with env var support**: (architecture) Resolve-target cascade: (1) CLI flags, (2) SENTRY\_ORG/SENTRY\_PROJECT env vars, (3) SQLite defaults, (4) DSN auto-detection, (5) directory name inference. SENTRY\_PROJECT supports \`org/project\` combo — SENTRY\_ORG ignored if set. Malformed combos discarded. \`resolveFromEnvVars()\` injected into all four resolution functions. Schema v13 merged \`defaults\` table into \`metadata\` KV with keys \`defaults.{org,project,telemetry,url}\`; getters/setters in \`src/lib/db/defaults.ts\`. Prefer dedicated SQLite tables + migrations over \`metadata\` KV for non-trivial caches — dedicated tables give clearer schema, proper indexes, simpler bulk-clear. \`metadata\` KV fine for small scalars. Example: \`issue\_org\_cache\` (v15) replaced \`metadata\` keys. +* **Sentry CLI resolve-target cascade has 5 priority levels with env var support**: (architecture) Resolve-target cascade: (1) CLI flags, (2) SENTRY\_ORG/SENTRY\_PROJECT env vars, (3) SQLite defaults, (4) DSN auto-detection, (5) directory name inference. SENTRY\_PROJECT supports \`org/project\` combo — SENTRY\_ORG ignored if set. Malformed combos discarded. \`resolveFromEnvVars()\` injected into all four resolution functions. Schema v13 merged \`defaults\` table into \`metadata\` KV with keys \`defaults.{org,project,telemetry,url}\`; getters/setters in \`src/lib/db/defaults.ts\`. Prefer dedicated SQLite tables + migrations over \`metadata\` KV for non-trivial caches — dedicated tables give clearer schema, proper indexes, simpler bulk-clear. \`metadata\` KV fine for small scalars. Example: \`issue\_org\_cache\` (v15) replaced \`metadata\` keys. Hidden global \`--org\`/\`--project\` flags: \`mergeGlobalFlags()\` in command.ts injects hidden flag shapes, \`applyOrgProjectFlags()\` writes to \`SENTRY\_ORG\`/\`SENTRY\_PROJECT\` before auth guard. No short aliases (\`-p\` conflicts). * **Sentry log IDs are UUIDv7 — enables deterministic retention checks**: (architecture) Sentry log IDs are UUIDv7 — enables deterministic retention checks. \`decodeUuidV7Timestamp()\` and \`ageInDaysFromUuidV7()\` in \`src/lib/hex-id.ts\` return null for non-v7, safe to call unconditionally. \`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\`) live in \`hex-id.ts\`. Three Sentry span APIs: (1) \`/trace/{traceId}/\` — hierarchical tree with \`additional\_attributes\`. (2) \`/projects/{org}/{project}/trace-items/{itemId}/\` — single span with ALL attributes. (3) \`/events/?dataset=spans\` — list/search. \`meta.fields\` order is non-deterministic — derive column order from user's \`--field\` list via \`orderFieldNames()\` in \`explore.ts\`. - -* **Sentry SDK uses @sentry/node-core/light instead of @sentry/bun to avoid OTel overhead**: (architecture) Sentry SDK uses \`@sentry/node-core/light\` (not \`@sentry/bun\`) to avoid OpenTelemetry overhead (~150ms, 24MB). \`@sentry/core\` barrel patched via \`bun patch\` to remove ~32 unused exports. \`LightNodeClient\` hardcodes \`runtime:{name:'node'}\` AFTER spreading options — fix by patching \`client.getOptions().runtime\` post-init. Transport uses Node \`http\` instead of native \`fetch\`. Shell completions set \`SENTRY\_CLI\_NO\_TELEMETRY=1\` before imports, skipping SDK load (~85ms). Timing queued to \`completion\_telemetry\_queue\` SQLite table; normal runs drain via \`DELETE...RETURNING\`. Always import from \`@sentry/node-core/light\`; root barrel pulls uninstalled @opentelemetry/instrumentation. When bumping SDK: remove patches, install, patch, edit, commit. - * **Sentry token formats: only sntrys\_ embeds host claim, and it's unsigned**: (architecture) Sentry token formats: \`sntryu\_\\` (user auth, no claims); \`sntrys\_\\_\\` (org auth, \*\*unsigned\*\*/plaintext — UX hint only, not verifiable); \`sntrya\_\`/\`sntryi\_\` — random hex; OAuth — random, no prefix. \`classifySentryToken()\` returns \`'org-auth-token'\`/\`'user-auth-token'\`/\`'oauth-or-legacy'\`. \`parseSntrysClaim\` requires exactly 2 underscores, 2KB cap, fail-open. Two consumers: (1) \`captureEnvTokenHost\` — claim url first for \`sntrys\_\` (defends against \`$GITHUB\_ENV\` poisoning); env wins for \`sntryu\_\`/OAuth. (2) \`prepareHeaders\` — refuses bearer attach if request origin doesn't match claim url. \`auth.host\` column \[\[019dc168-adb2-7bed-900e-cab5d3716099]] is strictly stronger than token claims. @@ -57,11 +66,20 @@ * **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\`. + +* **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. + ### Gotcha * **Biome noUselessUndefined also rejects () => {} empty arrow callbacks**: (gotcha) Biome lint traps (run \`bun run lint\` not \`lint:fix\` before pushing): (1) \`noUselessUndefined\`+\`noEmptyBlockStatements\` reject \`()=>undefined\` and \`()=>{}\` — use \`function noop():void{}\`. (2) \`noExcessiveCognitiveComplexity\` caps at 15 — extract helpers. (3) \`noPrecisionLoss\` on int >2^53 — use \`Number(string)\`. (4) \`noIncrementDecrement\` — use \`i+=1\`. (5) \`useYield\` on \`async \*func()\` needs \`biome-ignore\`. (6) Plugin forbids raw \`metadata\` table queries — use \`getMetadata\`/\`setMetadata\`/\`clearMetadata\`. (7) \`noMisplacedAssertion\` fires on helper functions — use inline \`biome-ignore\` above each \`expect()\`, NOT file-level. (8) \`AuthError(reason, message?)\` — easy to swap args; correct: \`new AuthError("expired", "Token expired")\`. Tests aren't type-checked but ARE lint-checked. Node polyfill (\`script/node-polyfills.ts\`) is INCOMPLETE — prefer \`node:fs/promises\` for file ops; \`execSync\` for shell. + +* **dist/bin.cjs runtime Node version check must match engines.node**: dist/bin.cjs runtime Node check: engines.node >=22.12 matches Astro 6 floor. Node 20 is EOL — do not add feature-detection workarounds for it. CI builds \`\["22","24"]\`; E2E job must use \`actions/setup-node\` with Node 22 (ubuntu-latest defaults to Node 20 which is EOL and unsupported). Don't use \`parseInt(node\_version) < 22\` — it misses 22.0.0–22.11.x. Use: \`let v=process.versions.node.split('.').map(Number);if(v\[0]<22||(v\[0]===22&\&v\[1]<12))\`. Update BIN\_WRAPPER in lockstep. + + +* **MastraClient has no dispose API — use AbortController for cleanup**: MastraClient has no \`close()\`/\`dispose()\` API — cleanup via \`ClientOptions.abortSignal\` (constructor) or per-prompt \`signal\`. Without explicit abort, Bun's fetch dispatcher keep-alive sockets hold the event loop alive past natural exit. Pattern in \`src/lib/init/wizard-runner.ts\`: create \`AbortController\` per \`runWizard\`, pass \`abortSignal: controller.signal\` to \`new MastraClient(...)\`, abort via \`using \_ = { \[Symbol.dispose]: () => controller.abort() }\`. Custom \`fetch\` wrapper must preserve \`init.signal\` via spread. Tests capture \`ClientOptions\` via \`spyOn(MastraClient.prototype, 'getWorkflow').mockImplementation(function() { capturedOpts.push(this.options); ... })\`. + * **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 … \ * **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. + +* **ubuntu-latest defaults to Node 20 (EOL) — E2E jobs must use actions/setup-node**: Trap: \`ubuntu-latest\` GitHub Actions runner ships Node 20 (EOL). E2E jobs that run the npm bundle via \`node -e "..."\` silently use Node 20 unless \`actions/setup-node\` is explicitly added. This looks fine because the job succeeds, but it violates \`engines.node >=22.12\`. Fix: add \`actions/setup-node\` with \`node-version: 22\` to any job that invokes \`node\` directly. The \`build-npm\` job already does this correctly; E2E jobs need the same treatment. Never add feature-detection workarounds for Node 20 — it's EOL and unsupported. + + +* **whichSync must use 'command -v' not 'which' for PATH-restricted lookups**: (pattern) Bun→Node.js API replacements: \`Bun.which(cmd, {PATH})\` → \`whichSync()\` from \`src/lib/which.ts\` (uses \`command -v\` shell builtin, NOT \`which\` binary — \`which\` fails when PATH is restricted to a test dir). \`Bun.spawn\` → \`spawn\` from \`node:child\_process\`; wrap \`.exited\` as \`new Promise(r => proc.on('close', code => r(code ?? 1)))\`. \*\*CRITICAL: always attach \`proc.on('error', () => {})\` — Node crashes on unhandled spawn errors; Bun did not.\*\* \`Bun.spawnSync\` → \`spawnSync\`. \`Bun.sleep(ms)\` → \`import { setTimeout as sleepMs } from 'node:timers/promises'\`. \`new Bun.Glob(p).match(i)\` → \`picomatch(p, { dot: true })(i)\` — pre-compile matchers outside hot loops. \`Bun.randomUUIDv7()\` → \`uuidv7()\`. \`Bun.semver.order()\` → \`compare()\` from \`semver\`. Only change actual API call sites — never comments. Update type annotations. + * **Whole-buffer matchAll slower than split+test when aggregated over many files**: Grep/scan traps in \`src/lib/scan/\`: (1) Whole-buffer \`regex.exec\` 12× faster per-file but ~1.6× SLOWER over 10k files — early-exit at \`maxResults\` via \`mapFilesConcurrent.onResult\` wins. (2) Literal prefilter is FILE-LEVEL gate (\`indexOf\`→skip); per-line verify breaks cross-newline patterns and Unicode length-changing \`toLowerCase\` (Turkish \`İ\`→\`i̇\`). (3) Extractor \`hasTopLevelAlternation\`+\`skipGroup\` must call \`skipCharacterClass\` (PCRE \`\[]abc]\` ≠ JS empty class). (4) Wake-latch race: naive \`let notify=null; await new Promise(r=>notify=r)\` loses signals — use latched \`pendingWake\` flag. (5) \`mapFilesConcurrent\` filters \`null\` but NOT \`\[]\` — return \`null\` for no-op files. (6) \`collectGlob\`/\`collectGrep\` must NOT forward \`maxResults\` to iterator; drain uncapped, set \`truncated=true\`. @@ -80,13 +104,30 @@ * **Grouped widget --limit auto-default via applyGroupLimitAutoDefault helper**: Dashboard widget flag normalization: (1) Dataset aliases (errors→error-events) normalize ONCE at top of \`func()\` via \`normalizeDataset()\` in \`src/commands/dashboard/resolve.ts\`. In \`edit.ts\`, pass \`normalizedFlags\` to \`buildReplacement\` — \`validateAggregateNames\` reads \`flags.dataset\` and rejects valid aggregates like \`failure\_rate\` if it sees raw alias. (2) Grouped widgets need \`limit\` (API rejects). \`applyGroupLimitAutoDefault\` defaults to \`DEFAULT\_GROUP\_BY\_LIMIT=5\` only when user passed \`--group-by\` without \`--limit\`; skip for auto-defaulted columns like \`\["issue"]\`. (3) Tests asserting \`--limit\` >10 survives into PUT body must use \`display: "line"\` — \`prepareWidgetQueries\` clamps bar/table to max=10. -* **Merging mock.module() test files with static-import counterparts**: (pattern) Bun test mocking traps: (1) \`mock.module()\` for CJS built-ins needs \`default\` re-export + named exports, declared top-level BEFORE \`await import()\`. (2) Convert code-under-test to \`await import()\` when merging mocks — static imports won't re-bind. (3) \`Bun.mmap()\` always PROT\_WRITE — use \`new Uint8Array(await Bun.file(path).arrayBuffer())\` for read-only. (4) \`mock.module()\` pollutes registry — use \`test/isolated/\`. (5) \`buildCommand\` wrapper: \`cmd.loader()\` returns wrapped async fn; call \`func.call(ctx, flags, ...args)\` as a promise. Auth guard runs first; \`test/preload.ts\` sets fake \`SENTRY\_AUTH\_TOKEN\`. (6) Test glob \`test:unit\` only picks up \`test/lib\`, \`test/commands\`, \`test/types\` — tests under \`test/fixtures/\`, \`test/scripts/\`, \`test/script/\` NOT run by CI. (7) Tests mocking fetch MUST call \`useTestConfigDir()\` + \`setAuthToken()\` + \`resetCacheState()\` + \`disableResponseCache()\` + \`resetAuthenticatedFetch()\` in beforeEach — filesystem cache will serve prior test responses otherwise. +* **Merging mock.module() test files with static-import counterparts**: (pattern) Bun test mocking traps: (1) \`mock.module()\` for CJS built-ins needs \`default\` re-export + named exports, declared top-level BEFORE \`await import()\`. (2) Convert code-under-test to \`await import()\` when merging mocks — static imports won't re-bind. (3) \`Bun.mmap()\` always PROT\_WRITE — use \`new Uint8Array(await Bun.file(path).arrayBuffer())\` for read-only. (4) \`mock.module()\` pollutes registry — use \`test/isolated/\`. (5) \`buildCommand\` wrapper: \`cmd.loader()\` returns wrapped async fn; call \`func.call(ctx, flags, ...args)\` as a promise. Auth guard runs first; \`test/preload.ts\` sets fake \`SENTRY\_AUTH\_TOKEN\`. (6) Test glob \`test:unit\` only picks up \`test/lib\`, \`test/commands\`, \`test/types\` — tests under \`test/fixtures/\`, \`test/scripts/\`, \`test/script/\` NOT run by CI. (7) Tests mocking fetch MUST call \`useTestConfigDir()\` + \`setAuthToken()\` + \`resetCacheState()\` + \`disableResponseCache()\` + \`resetAuthenticatedFetch()\` in beforeEach — filesystem cache will serve prior test responses otherwise. Symptom: test expects fresh mock value, receives prior test's value. TTL tiers: stable=5min, volatile=60s, immutable=24h. -* **Shared pagination infrastructure: buildPaginationContextKey and parseCursorFlag**: (pattern) Pagination infrastructure + org flag injection: 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. \`paginationHint()\` builds nav strings. Critical: \`resolveCursor()\` must be called INSIDE \`org-all\` override closures, not before \`dispatchOrgScopedList\`. Hidden global \`--org\`/\`--project\` flags accept old \`sentry-cli\` syntax via \`mergeGlobalFlags()\` in command.ts; \`applyOrgProjectFlags()\` writes to \`SENTRY\_ORG\`/\`SENTRY\_PROJECT\` before auth guard. No short aliases (\`-p\` conflicts). Issue list \`--limit\` is global total: \`fetchWithBudget\` Phase 1 divides evenly, Phase 2 redistributes surplus. \`trimWithProjectGuarantee\` ensures ≥1 issue per project. Compound cursor (pipe-separated) enables \`-c last\` for multi-target pagination. +* **Shared pagination infrastructure: buildPaginationContextKey and parseCursorFlag**: (pattern) Pagination infrastructure + org flag injection: 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. \`paginationHint()\` builds nav strings. Critical: \`resolveCursor()\` must be called INSIDE \`org-all\` override closures, not before \`dispatchOrgScopedList\`. \`issue list --limit\` is global total: \`fetchWithBudget\` Phase 1 divides evenly, Phase 2 redistributes surplus. \`trimWithProjectGuarantee\` ensures ≥1 issue per project. Compound cursor (pipe-separated) enables \`-c last\` for multi-target pagination. JSON output wraps in \`{ data, hasMore }\` with optional \`errors\` array. -* **Telemetry instrumentation pattern: withTracingSpan + captureException for handled errors**: (pattern) Telemetry instrumentation + command bypass: Use \`withTracingSpan\` from \`src/lib/telemetry.ts\` for child spans (\`onlyIfParent: true\` — no-op without active transaction) and \`captureException\` from \`@sentry/bun\` (named import — Biome forbids namespace imports) with \`level: 'warning'\` for non-fatal errors. Several commands bypass telemetry by importing \`buildCommand\` from \`@stricli/core\` directly instead of \`../../lib/command.js\`. \`ENV\_VAR\_REGISTRY\` in \`src/lib/env-registry.ts\` is the single source for all honored env vars; \`topLevel: true\` + \`briefDescription\` surfaces in \`--help\`. Add new env vars here with \`installOnly: true\` if install-script-only. Opt-out priority: (1) \`SENTRY\_CLI\_NO\_TELEMETRY=1\`, (2) \`DO\_NOT\_TRACK=1\`, (3) \`metadata.defaults.telemetry\`, (4) default on. \`computeTelemetryEffective()\` returns resolved source for display. +* **Telemetry instrumentation pattern: withTracingSpan + captureException for handled errors**: (pattern) Telemetry instrumentation + command bypass: Use \`withTracingSpan\` from \`src/lib/telemetry.ts\` for child spans (\`onlyIfParent: true\`) and \`captureException\` from \`@sentry/bun\` (named import) with \`level: 'warning'\` for non-fatal errors. Several commands bypass telemetry by importing \`buildCommand\` from \`@stricli/core\` directly. \`ENV\_VAR\_REGISTRY\` in \`src/lib/env-registry.ts\` is the single source for all honored env vars; \`topLevel: true\` + \`briefDescription\` surfaces in \`--help\`. Add new env vars with \`installOnly: true\` if install-script-only. Opt-out priority: (1) \`SENTRY\_CLI\_NO\_TELEMETRY=1\`, (2) \`DO\_NOT\_TRACK=1\`, (3) \`metadata.defaults.telemetry\`, (4) default on. \`computeTelemetryEffective()\` returns resolved source for display. -* **Test helpers for host-scoping security tests**: (pattern) Test helpers for host-scoping security tests: \`test/helpers.ts\` provides shared utilities. \`useEnvSandbox(keys)\` saves+clears+restores env keys (do NOT use in tests depending on preload's \`SENTRY\_AUTH\_TOKEN\`). \`resetHostScopingState()\` bundles \`resetEnvTokenHostForTesting\` + \`resetLoginTrustAnchorForTesting\` + \`resetTrustedRegionUrlsForTesting\` (always reset together). \`mintSntrysToken(payload)\` produces \`sntrys\_\\_\\` test tokens (rstrip \`=\`). \`extractFetchUrl(input)\` for fetch-mock assertions. \`useTestConfigDir\` handles config-dir isolation. Tests mocking fetch with non-SaaS URLs must pass \`{host}\` to \`setAuthToken\` — token defaults to SaaS via \`captureEnvTokenHost\`. For \`assertRcUrlTrusted\`: sequence is \`resetEnvTokenHostForTesting()\` → delete env vars → \`captureEnvTokenHost()\` → \`applySentryCliRcEnvShim()\` → \`assertRcUrlTrusted()\`. E2E: \`createE2EContext\` parent must \`setAuthToken(token, ttl, {host: serverUrl})\`; multi-region tests need \`registerTrustedRegionUrls\`. +* **Test helpers for host-scoping security tests**: (pattern) Test helpers for host-scoping security tests: \`useEnvSandbox(keys)\` saves+clears+restores env keys (do NOT use in tests depending on preload's \`SENTRY\_AUTH\_TOKEN\`). \`resetHostScopingState()\` bundles \`resetEnvTokenHostForTesting\` + \`resetLoginTrustAnchorForTesting\` + \`resetTrustedRegionUrlsForTesting\` (always reset together). \`mintSntrysToken(payload)\` produces \`sntrys\_\\_\\` test tokens (rstrip \`=\`). \`extractFetchUrl(input)\` for fetch-mock assertions. Tests mocking fetch with non-SaaS URLs must pass \`{host}\` to \`setAuthToken\`. For \`assertRcUrlTrusted\`: sequence is \`resetEnvTokenHostForTesting()\` → delete env vars → \`captureEnvTokenHost()\` → \`applySentryCliRcEnvShim()\` → \`assertRcUrlTrusted()\`. E2E: \`createE2EContext\` parent must \`setAuthToken(token, ttl, {host: serverUrl})\`; multi-region tests need \`registerTrustedRegionUrls\`. + + +* **Token-type classification via literal prefix match (classifySentryToken)**: (pattern) Token-type classification: \`src/lib/token-type.ts\` \`classifySentryToken(token)\` returns \`'org-auth-token'\` (\`sntrys\_\` prefix), \`'user-auth-token'\` (\`sntryu\_\` prefix), or \`'oauth-or-legacy'\`. Case-sensitive \`startsWith\`. Use to short-circuit commands where a token type is semantically invalid (e.g. \`whoami\` with org token — \`/auth/\` rejects \`sntrys\_\`). \`getAuthToken()\` from \`db/auth\` returns the effective token. Non-essential DB cache writes (e.g., \`setUserInfo()\`, \`setInstallInfo()\`) must wrap in try-catch so broken/read-only DB doesn't crash a successful command. Pattern: \`try { setInstallInfo(...) } catch { log.debug(...) }\`. Exception: \`getUserRegions()\` failure should \`clearAuth()\` and fail hard. + +### Preference + + +* **Always investigate multiple related files thoroughly before proposing or implementing changes**: Deep investigation before implementation: Always investigate multiple related files thoroughly before proposing changes — read complete files, check SDK/type definitions, grep for existing usage, map all call sites. Surface all findings (including what is NOT present) before moving to a fix. Before merging any PR, perform a critical self-review categorizing findings by severity (CRITICAL, MEDIUM, LOW); fix all CRITICAL and MEDIUM issues before pushing. When reporting issues, provide exact file paths, function names, and line number ranges. Write migration plans to \`.opencode/plans/\` with full codebase exploration first; call \`plan\_exit\` after writing. + + +* **Always provide explicit replacement rules before migrating APIs**: When requesting API migrations (e.g., Bun → Node.js), the user always specifies exact replacement mappings upfront: which source API maps to which target API, which files to modify, what to skip, and any special-case handling rules (e.g., 'do NOT modify comments', 'check existing imports before adding new ones', 'this call has special stdout/stderr handling — read carefully'). Follow these rules precisely without improvising alternatives. Never modify excluded items. Always extend existing imports rather than duplicating them. Treat the user's explicit mapping list as the authoritative spec for the entire migration. After migration, always check for missing \`proc.on('error')\` listeners on every spawn site — Node crashes on unhandled spawn errors. + + +* **Always provide root cause context and specific file/line locations when requesting code review or investigation**: When asking for code review or bug investigation, the user consistently pre-loads the conversation with: (1) the root cause already identified or strongly suspected, (2) exact file paths and line numbers to examine, (3) the specific symptoms and expected vs. actual behavior, and (4) any relevant constraints (SDK type limitations, API behavior, etc.). The assistant should treat these upfront details as authoritative ground truth, skip re-investigating already-stated facts, and focus review/analysis effort on the specific locations and concerns the user has already scoped. Do not re-derive what the user has already explained. + + +* **Always request critical PR reviews before merging, focusing on correctness, error handling, and edge cases**: When working on PRs (especially large refactors or migrations), the user consistently requests thorough code reviews before merging. Reviews should focus on: correctness of API replacements, error handling gaps (e.g., missing event listeners on spawned processes), type safety, security issues (e.g., shell injection), edge cases, and accuracy of PR descriptions/comments. The user expects the reviewer to read critical files in full, identify CRITICAL vs MEDIUM severity findings, and then immediately proceed to fix all identified issues — not just report them. After fixes, all tests and lint must pass before the PR is pushed. diff --git a/README.md b/README.md index 4bb25d2f5..265293cfa 100644 --- a/README.md +++ b/README.md @@ -77,7 +77,7 @@ Credentials are stored in `~/.sentry/` with restricted permissions (mode 600). ## Library Usage -Use Sentry CLI programmatically in Node.js (≥22.12) or Bun without spawning a subprocess: +Use Sentry CLI programmatically in Node.js (≥22.15) or Bun without spawning a subprocess: ```typescript diff --git a/package.json b/package.json index 5d6e44e13..16ea6e9ef 100644 --- a/package.json +++ b/package.json @@ -63,7 +63,7 @@ }, "description": "Sentry CLI - A command-line interface for using Sentry built by robots and humans for robots and humans", "engines": { - "node": ">=22.12" + "node": ">=22.15" }, "files": [ "dist/bin.cjs", diff --git a/script/bundle.ts b/script/bundle.ts index 9477b6386..21b9f895e 100644 --- a/script/bundle.ts +++ b/script/bundle.ts @@ -242,7 +242,7 @@ const result = await build({ // Write the CLI bin wrapper (tiny — shebang + version check + dispatch). // Version floor must track `engines.node` in package.json. const BIN_WRAPPER = `#!/usr/bin/env node -{let v=process.versions.node.split(".").map(Number);if(v[0]<22||(v[0]===22&&v[1]<12)){console.error("Error: sentry requires Node.js 22.12 or later (found "+process.version+").\\n\\nEither upgrade Node.js, or install the standalone binary instead:\\n curl -fsSL https://cli.sentry.dev/install | bash\\n");process.exit(1)}} +{let v=process.versions.node.split(".").map(Number);if(v[0]<22||(v[0]===22&&v[1]<15)){console.error("Error: sentry requires Node.js 22.15 or later (found "+process.version+").\\n\\nEither upgrade Node.js, or install the standalone binary instead:\\n curl -fsSL https://cli.sentry.dev/install | bash\\n");process.exit(1)}} {let e=process.emit;process.emit=function(n,...a){return n==="warning"?!1:e.apply(this,[n,...a])}} require('./index.cjs')._cli().catch(()=>{process.exitCode=1}); `; diff --git a/src/commands/cli/upgrade.ts b/src/commands/cli/upgrade.ts index cac820f45..427276bc4 100644 --- a/src/commands/cli/upgrade.ts +++ b/src/commands/cli/upgrade.ts @@ -431,9 +431,9 @@ async function spawnWithRetry( stdio: ["ignore", "inherit", "inherit"], env, }); - return await new Promise((resolve) => { + return await new Promise((resolve, reject) => { proc.on("close", (code) => resolve(code ?? 1)); - proc.on("error", () => resolve(1)); + proc.on("error", (err) => reject(err)); }); } catch (error) { // Translate the opaque Bun "Executable not found" error into an diff --git a/src/lib/api/sourcemaps.ts b/src/lib/api/sourcemaps.ts index 78ac361de..41768a107 100644 --- a/src/lib/api/sourcemaps.ts +++ b/src/lib/api/sourcemaps.ts @@ -24,8 +24,11 @@ import { open, readFile, stat, unlink } from "node:fs/promises"; import { tmpdir } from "node:os"; import { join } from "node:path"; import { promisify } from "node:util"; -// biome-ignore lint/performance/noNamespaceImport: needed for feature-detected zstd access -import * as zlib from "node:zlib"; +import { + gzip as gzipCb, + constants as zlibConstants, + zstdCompress as zstdCompressCb, +} from "node:zlib"; import pLimit from "p-limit"; import { z } from "zod"; import { ApiError } from "../errors.js"; @@ -35,22 +38,8 @@ import { getSdkConfig } from "../sentry-client.js"; import { type ZipCompression, ZipWriter } from "../sourcemap/zip.js"; import { apiRequestToRegion } from "./infrastructure.js"; -const gzipAsync = promisify(zlib.gzip); -// zstdCompress is available in Node 22.15+. Feature-detect to avoid crashing -// the npm bundle on older Node versions (e.g., CI runners with Node 20). -// zstdCompress is available in Node 22.15+. Feature-detect to avoid crashing -// the npm bundle on older Node versions (e.g., CI runners with Node 20). -// biome-ignore lint/suspicious/noExplicitAny: zstd types unavailable on older @types/node -const zstdCompressFn = (zlib as any).zstdCompress as - | ((...args: unknown[]) => unknown) - | undefined; -const zstdCompressAsync = - typeof zstdCompressFn === "function" - ? (promisify(zstdCompressFn) as ( - buf: Buffer, - opts?: unknown - ) => Promise) - : undefined; +const gzipAsync = promisify(gzipCb); +const zstdCompressAsync = promisify(zstdCompressCb); const log = logger.withTag("api.sourcemaps"); // ── Schemas ───────────────────────────────────────────────────────── @@ -219,13 +208,13 @@ export async function encodeChunk( buf: Buffer, encoding: UploadEncoding | undefined ): Promise { - if (encoding === "zstd" && zstdCompressAsync) { + if (encoding === "zstd") { // L3 is libzstd's default; passed explicitly for self-documenting // code. L9+ trades ~14% size for 4x compress time and forces the // server's decoder to allocate 15-30 MiB of window state -- not // worth it once decode cost is counted. return await zstdCompressAsync(buf, { - params: { [zlib.constants.ZSTD_c_compressionLevel]: 3 }, + params: { [zlibConstants.ZSTD_c_compressionLevel]: 3 }, }); } if (encoding === "gzip") { diff --git a/src/lib/db/sqlite.ts b/src/lib/db/sqlite.ts index 7b50c5782..c752266bd 100644 --- a/src/lib/db/sqlite.ts +++ b/src/lib/db/sqlite.ts @@ -139,7 +139,11 @@ export class Database { this.db.exec("COMMIT"); return result; } catch (error) { - this.db.exec("ROLLBACK"); + try { + this.db.exec("ROLLBACK"); + } catch (rollbackError) { + log.debug("ROLLBACK failed after transaction error", rollbackError); + } throw error; } }; diff --git a/src/lib/delta-upgrade.ts b/src/lib/delta-upgrade.ts index cf7d0ed18..991210914 100644 --- a/src/lib/delta-upgrade.ts +++ b/src/lib/delta-upgrade.ts @@ -21,7 +21,7 @@ import { unlinkSync } from "node:fs"; // biome-ignore lint/performance/noNamespaceImport: Sentry SDK recommends namespace import import * as Sentry from "@sentry/node-core/light"; -import { compare as semverCompare } from "semver"; +import { compare as semverCompare, valid as semverValid } from "semver"; import { GITHUB_RELEASES_URL, @@ -460,10 +460,17 @@ export function filterAndSortChainTags( currentVersion: string, targetVersion: string ): string[] { + if (!(semverValid(currentVersion) && semverValid(targetVersion))) { + return []; + } + const chainTags: { tag: string; version: string }[] = []; for (const tag of allTags) { const version = tag.slice(PATCH_TAG_PREFIX.length); + if (!semverValid(version)) { + continue; + } // Include tags where: currentVersion < version <= targetVersion if ( semverCompare(version, currentVersion) === 1 && diff --git a/src/lib/shell.ts b/src/lib/shell.ts index 728d82b15..5980f42f6 100644 --- a/src/lib/shell.ts +++ b/src/lib/shell.ts @@ -8,8 +8,11 @@ import { existsSync } from "node:fs"; import { access, readFile, writeFile } from "node:fs/promises"; import { basename, delimiter, join } from "node:path"; +import { logger } from "./logger.js"; import { whichSync } from "./which.js"; +const log = logger.withTag("shell"); + /** Supported shell types */ export type ShellType = "bash" | "zsh" | "fish" | "sh" | "ash" | "unknown"; @@ -299,7 +302,12 @@ export async function addToGitHubPath( let content = ""; try { content = await readFile(env.GITHUB_PATH, "utf-8"); - } catch { + } catch (error) { + const code = (error as NodeJS.ErrnoException).code; + if (code !== "ENOENT") { + log.debug(`Failed to read GITHUB_PATH (${code}):`, error); + return false; + } // File doesn't exist yet — start with empty content } @@ -310,7 +318,8 @@ export async function addToGitHubPath( await writeFile(env.GITHUB_PATH, newContent, "utf-8"); } return true; - } catch { + } catch (error) { + log.debug("Failed to update GITHUB_PATH:", error); return false; } } diff --git a/src/lib/telemetry/zstd-transport.ts b/src/lib/telemetry/zstd-transport.ts index ce3fb1b2e..632fe2316 100644 --- a/src/lib/telemetry/zstd-transport.ts +++ b/src/lib/telemetry/zstd-transport.ts @@ -32,8 +32,11 @@ import * as http from "node:http"; import * as https from "node:https"; import { Readable } from "node:stream"; import { promisify } from "node:util"; -// biome-ignore lint/performance/noNamespaceImport: needed for feature-detected zstd access -import * as zlib from "node:zlib"; +import { + gzip as gzipCb, + constants as zlibConstants, + zstdCompress as zstdCompressCb, +} from "node:zlib"; import { createTransport, suppressTracing, @@ -78,22 +81,8 @@ const ZSTD_THRESHOLD = 1024; */ const GZIP_THRESHOLD = 1024 * 32; -const gzipAsync = promisify(zlib.gzip); -// zstdCompress is available in Node 22.15+. Feature-detect to avoid crashing -// the npm bundle on older Node versions (e.g., CI runners with Node 20). -// zstdCompress is available in Node 22.15+. Feature-detect to avoid crashing -// the npm bundle on older Node versions (e.g., CI runners with Node 20). -// biome-ignore lint/suspicious/noExplicitAny: zstd types unavailable on older @types/node -const zstdCompressFn = (zlib as any).zstdCompress as - | ((...args: unknown[]) => unknown) - | undefined; -const zstdCompressAsync = - typeof zstdCompressFn === "function" - ? (promisify(zstdCompressFn) as ( - buf: Buffer, - opts?: unknown - ) => Promise) - : undefined; +const gzipAsync = promisify(gzipCb); +const zstdCompressAsync = promisify(zstdCompressCb); /** * Factory for the SDK's `Sentry.init({ transport })` option. @@ -288,9 +277,9 @@ export async function maybeCompress( return { payload: buf, encodingApplied: "none" }; } - if (encoding === "zstd" && zstdCompressAsync) { + if (encoding === "zstd") { const out = await zstdCompressAsync(buf, { - params: { [zlib.constants.ZSTD_c_compressionLevel]: ZSTD_LEVEL }, + params: { [zlibConstants.ZSTD_c_compressionLevel]: ZSTD_LEVEL }, }); return { payload: Buffer.from(out.buffer, out.byteOffset, out.byteLength), @@ -304,7 +293,7 @@ export async function maybeCompress( /** Feature-detect zstd support on the current runtime. */ export function hasZstdSupport(): boolean { - return zstdCompressAsync !== undefined; + return typeof zstdCompressCb === "function"; } /** diff --git a/src/lib/upgrade.ts b/src/lib/upgrade.ts index 8bdd70fd7..3797d3ec5 100644 --- a/src/lib/upgrade.ts +++ b/src/lib/upgrade.ts @@ -572,7 +572,24 @@ async function streamDecompressToFile( if (writeError) { break; } - writer.write(chunk); + const ok = writer.write(chunk); + if (!(ok || writeError)) { + // Race drain against error — an I/O failure (ENOSPC) while the + // buffer is full would never emit 'drain', causing a hang. + // Clean up the unused listener to avoid MaxListenersExceededWarning. + await new Promise((resolve) => { + const onDrain = (): void => { + writer.removeListener("error", onError); + resolve(); + }; + const onError = (): void => { + writer.removeListener("drain", onDrain); + resolve(); + }; + writer.once("drain", onDrain); + writer.once("error", onError); + }); + } } } finally { await new Promise((resolve, reject) => { diff --git a/src/lib/which.ts b/src/lib/which.ts index 881b2e9de..b7868b2a2 100644 --- a/src/lib/which.ts +++ b/src/lib/which.ts @@ -8,6 +8,9 @@ import { execFileSync } from "node:child_process"; +/** Matches CRLF or LF line endings. Used to split `where.exe` output on Windows. */ +const NEWLINE_RE = /\r?\n/; + /** * Synchronously find the full path to a command in the system PATH. * @@ -58,7 +61,7 @@ export function whichSync( ); } - return stdout.trim().split("\n")[0] || null; + return stdout.trim().split(NEWLINE_RE)[0] || null; } catch { return null; }