diff --git a/.lore.md b/.lore.md index 60ccf6b17..8079ec021 100644 --- a/.lore.md +++ b/.lore.md @@ -14,7 +14,7 @@ * **Auth token env var override pattern: SENTRY\_AUTH\_TOKEN > SENTRY\_TOKEN > SQLite**: Auth token precedence in \`src/lib/db/auth.ts\`: \`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. \`runInteractiveLogin\` catches OAuth flow errors internally and returns falsy on failure; login command sets \`process.exitCode = 1\` and returns normally (does NOT reject). Tests expecting \`rejects.toThrow()\` will fail — assert via fetch-call inspection instead. \`requestDeviceCode\` requires \`SENTRY\_CLIENT\_ID\` env var. -* **Binary build pipeline: esbuild → fossilize → Node SEA (replacing Bun.build compile)**: Binary build pipeline: \`src/bin.ts → \[esbuild CJS, node24] → dist-build/bin.js → \[fossilize --no-bundle --hole-punch] → Node SEA binary → gzip\`. CRITICAL ORDER: hole-punch BEFORE signing — hole-punching after signing invalidates macOS code signature (AMFI SIGKILL). fossilize 0.8.0+ runs hole-punch via \`--hole-punch\` between chmod and sign+notarize. Strip debug symbols handled inside fossilize (v0.7.0+). macOS: \`strip -x\` on unsigned copy; cross-strip from Linux silently fails. UPX RULED OUT — destroys ELF notes. ALL\_TARGETS: darwin-arm64/x64, linux-arm64/x64, win32-x64 + musl variants. Post-process: rename \`sentry-win-x64.exe\`→\`sentry-windows-x64.exe\`. \`FOSSILIZE\_SIGN=y\` on push to main/release. \`useSnapshot:true\` BROKEN; \`useCodeCache:true\` ~15% startup improvement. fossilize 0.8.1 fixes cross-compile strip crash. Runtime modes: (1) SEA binary — \`process.execPath\` IS the sentry binary; (2) npm package — \`process.execPath\` is Node.js, \`process.argv\[1]\` is script in node\_modules, bin field: \`'sentry': './dist/bin.cjs'\`. +* **Binary build pipeline: esbuild → fossilize → Node SEA (replacing Bun.build compile)**: Binary build pipeline: \`src/bin.ts → \[esbuild CJS, node24 target] → dist-build/bin.js → \[fossilize --no-bundle] → Node SEA binary → \[binpunch ICU hole-punch] → gzip\`. Strip debug symbols handled INSIDE fossilize (as of fossilize 0.7.0) — fossilize strips the copied binary BEFORE postject injection. Strip MUST happen before injection — after SEA injection, \`strip\` fails ('section .text can't be allocated in segment 2'). macOS: \`strip -x\` on unsigned copy; cross-strip from Linux silently fails (caught). Windows: skipped (no debug symbols). NODE\_VERSION='lts'. ALL\_TARGETS: darwin-arm64, darwin-x64, linux-arm64, linux-x64, win32-x64 + musl variants. Post-process: rename \`sentry-win-x64.exe\`→\`sentry-windows-x64.exe\`. UPX RULED OUT — destroys ELF notes. \`FOSSILIZE\_SIGN=y\` on push to main/release. Gzip only when \`RELEASE\_BUILD=1\`. \`stripCachedNodeBinaries()\` removed from \`script/build.ts\` — superseded by fossilize 0.7.0. * **Binary size breakdown: 94.5% is Node.js runtime — bundled code is ~6.3 MiB**: Binary composition (linux-x64, Node 24 LTS): Node.js runtime=121 MiB (ships with debug symbols). \`strip --strip-unneeded\` → 99 MiB (-17 MiB raw, -4 MiB compressed). Strip built into fossilize 0.7.0 — happens on the copied binary BEFORE postject injection. After strip+SEA+binpunch: ~108 MiB raw, ~30 MiB gzip (vs 125 MiB / 34 MiB unstripped). .rodata=52.5 MB: V8 snapshot ~12 MB, ICU full-icu data ~28 MB. UPX compresses to 25 MiB but DESTROYS ELF notes — ruled out. \`--with-intl=small-icu\` saves ~26-28 MiB (biggest win from custom build); \`--without-lief\` BREAKS SEA; \`--without-sqlite\` BREAKS CLI; \`--disable-single-executable-application\` BREAKS EVERYTHING. Custom build deferred — poor cost/benefit (~3.5h build vs 5min fossilize). Final vs Bun: download 30 MiB (Bun: 32 MiB), \`--version\` ~1.0s (Bun: ~1.9s), completions ~150ms (Bun: ~180ms). @@ -35,7 +35,7 @@ * **collapse=lifetime in issue list: LIFETIME\_FIELDS, buildListApiOptions, and API gotcha**: \`src/commands/issue/list.ts\` \`LIFETIME\_FIELDS = new Set(\['count','userCount','firstSeen','lastSeen'])\` — fields stripped by \`collapse=lifetime\` on the list endpoint. \`buildListApiOptions(json, fields)\`: \`collapseLifetime\` only true when \`json && fields !== undefined && fields.length > 0 && !fields.some(f => LIFETIME\_FIELDS.has(f))\`. Human output NEVER collapses lifetime. \`buildIssueListCollapse()\` always starts with \`\['filtered','unhandled']\`, conditionally adds \`'lifetime'\` then \`'stats'\`. \`ISSUE\_DETAIL\_COLLAPSE\` safely includes \`'lifetime'\` — detail endpoint preserves top-level fields regardless. \`IssueViewOutputSchema\` in \`src/types/sentry.ts\` extends \`SentryIssueSchema\` with enrichment fields (\`event\`, \`org\`, \`replayIds\`, \`trace\`) added by \`jsonTransformIssueView\`. Wired via \`schema: IssueViewOutputSchema\` on output config in \`view.ts\`. NOTE: \`count\`/\`userCount\`/\`firstSeen\`/\`lastSeen\` always present on \`issue view\` (detail endpoint) — only potentially absent on \`issue list\` when collapse=lifetime is active. -* **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. Telemetry invariants (\`src/lib/telemetry.ts\`): (1) \`initSentry()\` ALWAYS removes \`currentBeforeExitHandler\` before registering new one. (2) \`isOwnedByRoot()\` returns \`false\` immediately on Windows. (3) NEVER block CLI — all drains are best-effort, wrapped in try/catch. (4) \`SENSITIVE\_ARGV\_FLAGS\` (\`token\`, \`auth-token\`) NEVER sent — \`redactArgv()\` handles both \`--flag=value\` and \`--flag \\` forms. \`runCompletion()\` sets \`SENTRY\_CLI\_NO\_TELEMETRY=1\`. Opt-out priority: (1) \`SENTRY\_CLI\_NO\_TELEMETRY=1\`, (2) \`DO\_NOT\_TRACK=1\`, (3) \`metadata.defaults.telemetry\`, (4) default on. \`ENV\_VAR\_REGISTRY\` in \`src/lib/env-registry.ts\` is single source for all honored env vars; \`topLevel: true\` + \`briefDescription\` surfaces in \`--help\`. +* **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. Telemetry opt-out priority: (1) \`SENTRY\_CLI\_NO\_TELEMETRY=1\`, (2) \`DO\_NOT\_TRACK=1\`, (3) \`metadata.defaults.telemetry\`, (4) default on. Shell completions set \`SENTRY\_CLI\_NO\_TELEMETRY=1\` in \`bin.ts\` before imports. Timing queued to \`completion\_telemetry\_queue\` SQLite table; normal runs drain via \`DELETE ... RETURNING\`. \`ENV\_VAR\_REGISTRY\` in \`src/lib/env-registry.ts\` is single source for all honored env vars; \`topLevel: true\` + \`briefDescription\` surfaces in \`--help\`. Add install-script-only vars with \`installOnly: true\`. * **Custom CA loading: priority, caching, TLS error detection, and SaaS warning**: Custom CA in \`src/lib/custom-ca.ts\`: Priority: (1) \`sentry cli defaults ca-cert\` (SQLite), (2) \`NODE\_EXTRA\_CA\_CERTS\`. Cached per-process via module-level vars (\`hasResolved\` flag). \`resolve()\` concatenates custom PEM with \`rootCertificates\` (additive — Bun replaces Mozilla bundle otherwise). \`tryReadPem()\` NEVER throws — missing CA file logs warn and returns \`undefined\`. \`injectIntoNodeTls()\` uses \`tls.setDefaultCACertificates()\` (Node 24+ only; no-op on Node 22). \`TLS\_ERROR\_PATTERNS\`: 5 patterns (local issuer, verify first cert, UNABLE\_TO\_VERIFY\_LEAF\_SIGNATURE, DEPTH\_ZERO\_SELF\_SIGNED\_CERT, SELF\_SIGNED\_CERT\_IN\_CHAIN) — explicitly excludes \`CERT\_HAS\_EXPIRED\` and \`ERR\_TLS\_CERT\_ALTNAME\_INVALID\`. \`getTlsCertErrorMessage()\` walks \`error.cause\` chain with cycle detection. SaaS target + env-sourced CA → one-time warning; stored default silences it. \`\_\_resetForTests()\` resets all cached state. @@ -102,12 +102,18 @@ * **CLI source-tier: bundle-sources first, print-sources deferred pending source enumeration API**: Chose to implement \`bundle-sources\` first over \`print-sources\` because \`SourceBundleWriter.writeObject()\` in \`@sentry/symbolic@13.4.0\` provides exactly the right shape (callback provider reads from disk). \`print-sources\` deferred because \`ObjectFile\` has no \`sourceFiles()\` enumeration method in 13.4.0 — blocked on Dav1dde shipping source enumeration in a future \`@sentry/symbolic\` release. WASM debug-files track is otherwise complete: symbolic PRs #988–#993 merged, \`@sentry/symbolic@13.4.0\` published, CLI PR #1124 merged. + +* **Decided to use job ID instead**: decided to use job ID instead. + * **Migrated to node**: Migrated to Node.js (from Bun). Migration complete as of fossilize 0.7.0 update. Stack: pnpm, vitest, tsx, Node SEA via fossilize. PRs #1017, #1018, #1019 on getsentry/cli. Benchmarks vs v0.34.0 (Bun): download 32MB→30MB, startup ~1s both, shell completions 180ms→150ms. All macOS binaries signed+notarized. fossilize handles SEA builds with V8 code cache on linux+macOS, strips debug symbols automatically. \`bun.lock\` deleted, \`vitest.config.ts\` added, all test files migrated to vitest. \`script/build.ts\` uses fossilize (\`--no-bundle\`) with esbuild for bundling — does NOT use \`Bun.build({ compile: true })\`. * **Node.js slim build flags for SEA binary size reduction**: Node.js configure.py size-reduction flags for SEA builds: \`--with-intl=small-icu\` (English-only ICU, saves ~26-28 MiB — biggest win; CLI uses hardcoded en-US/sv-SE locales, safe); \`--with-intl=none\` (saves ~28-30 MiB but breaks \`Intl.NumberFormat\`/\`String.normalize()\` — NOT safe for this CLI); \`--without-inspector\` saves ~2-4 MiB; \`--without-amaro\` saves ~0.5 MiB; \`--v8-disable-maglev\` saves ~1-2 MiB; \`--enable-lto\` saves ~3-5 MiB. AVOID: \`--without-ssl\` (breaks HTTPS), \`--without-lief\` (BREAKS SEA), \`--without-sqlite\` (BREAKS CLI — uses node:sqlite), \`--disable-single-executable-application\` (BREAKS EVERYTHING), \`--v8-lite-mode\` (10x slower). Custom build deferred indefinitely — requires 5 native CI runners, ~3.5h cold build vs 5min fossilize. Cross-compilation from Linux to darwin NOT officially supported. + +* **pnpm overrides for cached lockfile deps need --force to re-resolve**: When adding a pnpm override for a package already resolved in lockfile (e.g. vite transitively via vitest), the override entry appears in lockfile but pnpm doesn't trigger re-resolution — the cached version remains. Fix: run \`pnpm install --force\` after adding new overrides. \`pnpm dedupe\` may time out on large monorepos. Exact version pinning (\`"vite": "8.0.16"\`) is more reliable than range syntax. Discovered while fixing vite@8.0.13→8.0.16 override. + * **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. @@ -116,12 +122,27 @@ ### Gotcha + +* **@sentry/symbolic wasm-pack test never exercises the shipped artifact — use npm pack smoke test instead**: Trap: \`wasm-pack test\` looks like the right way to test \`@sentry/symbolic\` — it builds and runs tests. But it builds its own glue code and never loads the \`--target web\` \`symbolic.js\` + \`initSync\` + package \`exports\`/\`files\` that consumers actually import. Fix: use a packaged-artifact smoke test: \`npm pack\` → install tarball into tmp dir → \`import '@sentry/symbolic'\` → assert via \`node:test\`. This catches exports-map/resolution breakage and runtime errors (e.g., the \`Object\` naming bug) that \`wasm-pack test\` misses entirely. Test files live in the package dir but are excluded from \`files\[]\`; run via \`cd symbolic-wasm/npm && npm test\`. + + +* **@sentry/symbolic: naming wasm export class \`Object\` shadows JS global — breaks Object.create and Object.getPrototypeOf**: Trap: wasm-bindgen generates a JS class named after the Rust struct — naming it \`Object\` looks fine in Rust. But it shadows the JS global \`Object\` for the entire module: \`Object.create\` (used in \`objects()\`) and \`Object.getPrototypeOf\` (used in \`initSync\`) break with \`TypeError: Object.getPrototypeOf is not a function\`. Fix (PR #993): rename the export to \`ObjectFile\` via \`#\[wasm\_bindgen(js\_name = "ObjectFile")]\` — any name that isn't a JS global works. Discovered via packaged-artifact smoke test, not \`wasm-pack test\`. + + +* **@stricli/core -H alias patch: pin version to 1.2.7, never remove aliases**: Trap: When Stricli throws on \`-H\` aliases used for \`--header\` or \`--host\`, removing the aliases from command files looks like the simple fix. But the project intentionally uses \`-H\` for curl-style API usage. Fix: the in-repo patch for @stricli/core (targeting 1.2.7) removes \`-H\` from the reserved list. Pin version to \`1.2.7\` (not \`^1.2.8\`) so the patch applies. Never remove \`-H\` alias usages from command files. Added in commit \`78c9b04a5\`. Cursor Bugbot and Seer both flag \`-H\` removal as blocking. + -* **acquireLock ENOENT: parent directory may not exist — must mkdir before writeFileSync**: Trap: \`acquireLock(lockPath)\` calls \`writeFileSync(lockPath, pid, { flag: 'wx' })\` — looks safe because the lock path is derived from the install path. But the parent directory (e.g. \`~/.sentry/bin\` or a stale \`/tmp/sentry-test-install/\`) may not exist, causing ENOENT (Sentry issue CLI-1E1). Fix: call \`mkdirSync(dirname(lockPath), { recursive: true, mode: 0o755 })\` BEFORE the try block in \`acquireLock\` (\`src/lib/binary.ts\`). CRITICAL: keep mkdir OUTSIDE the try block — if inside, mkdir's EEXIST error routes into \`handleExistingLock\`, obscuring root cause (though infinite recursion claim in comment is inaccurate: actual path is EEXIST→handleExistingLock→readFileSync→ENOTDIR, single throw). The directory creation is idempotent — \`recursive: true\` is a no-op if it already exists. +* **acquireLock ENOENT: parent directory may not exist — must mkdir before writeFileSync**: Trap: \`acquireLock(lockPath)\` calls \`writeFileSync(lockPath, pid, { flag: 'wx' })\` — looks safe because the lock path is derived from the install path. But the parent directory (e.g. \`~/.sentry/bin\` or a stale \`/tmp/sentry-test-install/\`) may not exist, causing ENOENT (Sentry issue CLI-1E1). Fix: call \`mkdirSync(dirname(lockPath), { recursive: true, mode: 0o755 })\` BEFORE the try block in \`acquireLock\` (\`src/lib/binary.ts\`). CRITICAL: keep mkdir OUTSIDE the try block — if inside, mkdir's EEXIST error routes into \`handleExistingLock\`, obscuring root cause (comment claiming infinite recursion is inaccurate: actual path is EEXIST→handleExistingLock→readFileSync→ENOTDIR, single throw). The directory creation is idempotent — \`recursive: true\` is a no-op if it already exists. Merged in PR #1125 (squash \`ee768454c\`). * **batch-queue.ts: 404 from upstream treated as transient — provider never disabled**: Trap: \`BatchProvider.submit()\` returns \`null\` for any non-401/403 HTTP error, including 404. \`submitBatch()\` treats \`null\` as transient and falls back — no disable happens. For providers that don't implement \`/v1/messages/batches\` (e.g. MiniMax), this causes a wasted HTTP round-trip every 30s forever. Fix: add \`"not-found"\` return value for 404 in both Anthropic and OpenAI submit methods. In \`submitBatch()\`, handle \`"not-found"\` with provider-level disable: add \`disabledBatchProviders: Set\\` (keyed by provider name), persist to \`kv\_meta\` via \`setKV()\`, restore on startup. Add fast-path bypass in both \`flush()\` and \`prompt()\`. Provider-level (not per-session) because the URL is baked in at construction — one provider per process. \`groupKey()\` = \`authFingerprint(cred)|providerID\`; per-credential disable was removed in favor of per-session historically. + +* **Biome noParameterProperties: never use TypeScript constructor parameter properties in class definitions**: Trap: TypeScript parameter properties (\`constructor(private readonly handle: FileHandle)\`) look like idiomatic shorthand and compile fine. But Biome enforces \`noParameterProperties\` and will error. Fix: always declare explicit class fields and assign in constructor body. Applies to all new classes in \`src/lib/\*\*/\*.ts\`. Caught during \`FileOldReader\`/\`MemoryOldReader\` addition in \`bspatch.ts\` — 4 Biome errors at lines 281, 310, 311, 312. + + +* **bundle-sources.ts exit code 1 for no-sources relies on cli.ts never resetting exitCode — fragile invariant**: Trap: using \`this.process.exitCode = 1\` directly (at \`bundle-sources.ts:145\`) instead of throwing \`OutputError\`(=60) looks like a valid shortcut. It works today because \`cli.ts:622-649\` never resets \`exitCode\` on a clean return. But this is a fragile invariant — if \`cli.ts\` ever resets \`exitCode\`, the no-sources case silently exits 0. The \`OutputError\`(=60) idiom is the correct pattern. The \`exitCode=1\` approach was kept for consistency with \`check.ts\` pattern; a comment was added explaining the choice. Consider migrating to \`OutputError\` in a future cleanup. + * **check:fragments only validates file existence — not subcommand coverage depth**: RESOLVED in PR #1024. \`script/check-fragments.ts\` now has Check 5: for each route with >1 command, verifies the fragment mentions each subcommand via a heading (outside fenced code blocks) or \`sentry \ \\` code reference. Default commands (e.g., \`local serve\`) are handled — if the fragment contains \`sentry \\` bare, the default command is considered covered. Default commands detected from route map \`defaultCommand\` field. Fenced code block content stripped before heading scan to avoid bash-comment false positives. Warnings (not errors) by default; \`--strict\` flag makes them errors. @@ -132,7 +153,7 @@ * **dashboard revisions/restore and issue events subcommands are undocumented in fragment files**: RESOLVED in PR #1024. \`docs/src/fragments/commands/dashboard.md\` now documents \`revisions\` and \`restore\`. \`docs/src/fragments/commands/issue.md\` now documents \`events\` and \`@latest\`/\`@most\_frequent\` selectors. \`docs/src/fragments/commands/cli.md\` now documents \`defaults\` and \`import\`. \`check:fragments\` (Check 5) now validates subcommand coverage within fragment files — not just file existence. When adding new subcommands, always update the corresponding fragment in \`docs/src/fragments/commands/\` AND run \`pnpm run check:fragments\` to verify coverage. -* **delta-upgrade intermediate files: always alternate .patching.a/.b — never write to source path**: delta-upgrade intermediate files: always alternate .patching.a/.b — never write to source path: Trap: writing patch output to the same path as input looks like a simple in-place update. Fix: \`applyPatchesSequentially()\` alternated between \`${destPath}.patching.a\` and \`${destPath}.patching.b\` because the old binary is mmap'd for reading — writing to the source path truncates it and corrupts output. \*\*Superseded by in-memory chaining\*\*: \`applyPatchChainInMemory(oldPath, patches\[], destPath)\` keeps intermediate buffers in memory (no disk writes, no intermediate files). \`applyPatchToMemory(oldFile, patchData)→Uint8Array\` for intermediate hops; \`applyPatchToFile(oldPath, patchData, destPath)→SHA-256\` for final hop. \`loadOldBinary\` still copies running binary to temp file (COPYFILE\_FICLONE) — called once per chain, not per-hop. Peak memory: ~200MB for multi-hop chains (acceptable). \`cleanupIntermediates()\` and \`PatchResources\` removed. \`applyPatchChain()\` does NOT set executable permissions — caller (\`downloadBinaryToTemp\`) sets \`chmod 0o755\`. +* **delta-upgrade intermediate files: always alternate .patching.a/.b — never write to source path**: Trap: writing patch output to the same path as input looks like a simple in-place update. Fix: \`applyPatchesSequentially()\` alternates between \`${destPath}.patching.a\` and \`${destPath}.patching.b\` because the old binary is mmap'd for reading — writing to the source path truncates it and corrupts output. Single-patch chains apply old→dest directly (safe, different paths). \`cleanupIntermediates()\` called in \`finally\` block removes both intermediates via \`unlinkSync\` (ignoring errors) — always runs even on failure. This is a standing directive: 'Always clean up intermediate files, even on failure' and 'never target the same path.' * **DEVELOPMENT.md hand-written prose is not covered by any staleness check**: RESOLVED in PR #1024. \`DEVELOPMENT.md\` hand-written prose is now wrapped in \`GENERATED:START/END\` markers: \`dev-prereq\` (lines 4-7) and \`build-toolchain\` (lines 91-97). These sections are now auto-generated from \`package.json\` by \`generate-docs-sections.ts\` and validated by \`check:docs-sections --check\`. The only remaining non-generated prose in \`DEVELOPMENT.md\` is the OAuth app setup instructions and architecture description — these don't reference toolchain versions. \`generate-docs-sections.ts\` no longer contains any Bun references; \`extractPnpmVersion()\` and \`extractNodeVersion()\` throw on mismatch. @@ -149,6 +170,9 @@ * **getCurlInstallPaths trusts stale stored install path — must guard with existsSync(dirname)**: Trap: \`getCurlInstallPaths()\` in \`src/lib/upgrade.ts\` reads the stored install path from SQLite and uses it directly — looks correct because the path was valid at install time. But macOS cleans \`/tmp\` on reboot, and users may delete test install dirs, leaving a stale DB entry. Fix: guard the stored-path branch with \`existsSync(dirname(stored.path))\` before trusting it; fall back to \`process.execPath\` startsWith-match against \`KNOWN\_CURL\_DIRS\` (\`\['.local/bin','bin','.sentry/bin']\`), then \`~/.sentry/bin\` default. Conservative: only add the guard — do NOT prefer \`execPath\` over stored path (breaks npm→nightly migration flow). NFS edge case is self-resolving: if binary runs from NFS mount, mount must be active so \`existsSync\` passes. + +* **git add -A during rebase sweeps in stray untracked files and .lore.md conflicts**: Trap: \`git add -A\` looks like a safe 'stage everything' shortcut during rebase conflict resolution. But it stages untracked stray files (e.g. \`sentry-lightning-talk.md\`) and auto-stages \`.lore.md\` conflict markers, polluting the commit. Fix: always stage specific file paths explicitly (e.g. \`git add src/commands/debug-files/read-file.ts\`) during rebase resolution. If the rebase is complex, abort with \`git rebase --abort\`, reset to \`origin/main\`, and re-apply edits manually. + * **GitHub CI skips pull\_request workflows entirely when PR has merge conflicts**: Trap: missing CI jobs on a PR look like a workflow trigger bug or branch filter issue — easy to spend time investigating ci.yml triggers. Fix: check \`mergeable\`/\`mergeStateStatus\` first. When a PR is \`CONFLICTING\` (GitHub cannot compute the merge ref), ALL \`pull\_request\`-triggered workflows are silently skipped — only \`pull\_request\_target\` and CodeQL/external checks still run. Confirmed on sentry-cli (TypeScript) PR #1123: full Build/lint/test suite absent because \`mergeStateStatus: DIRTY\`. Resolution: rebase or resolve conflicts, then CI triggers normally. @@ -161,6 +185,9 @@ * **loadOldBinary: new Uint8Array(readFile()) double-allocates — return Buffer directly**: Trap: \`new Uint8Array(await readFile(tempCopy))\` looks like a safe type conversion. But \`readFile()\` already returns a \`Buffer\` which IS a \`Uint8Array\` — wrapping it in \`new Uint8Array()\` copies the entire ~100MB binary, causing a transient double-allocation peak. Fix: return the \`Buffer\` directly from \`loadOldBinary\` with no wrapper. Zero tradeoff — Buffer is already a Uint8Array subclass and works everywhere Uint8Array is expected. + +* **Local tarball paths in package.json break CI with pnpm install --frozen-lockfile**: Trap: \`file:/tmp/opencode/sentry-symbolic-new.tgz\` in dependencies looks harmless locally — \`pnpm install\` works fine. But CI runs \`pnpm install --frozen-lockfile\`, which exits 254 when the tarball path doesn't exist. The lockfile also changes the specifier from published version to path, causing diffs on every install. Fix: always keep published version specifiers (e.g. \`13.4.0\`) in package.json. Use \`npm pack\` + separate install for local testing, or pnpm overrides. + * **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); ... })\`. @@ -176,6 +203,9 @@ * **pnpm nested script invocation loses TTY — inline tsx to fix**: Trap: \`"cli": "pnpm tsx src/bin.ts"\` creates nested pnpm invocations (pnpm → /bin/sh → pnpm → /bin/sh → tsx → node). Each inner pnpm layer pipes stdio, so \`process.stdin.isTTY\` and \`process.stdout.isTTY\` are \`undefined\` in the final Node process. This breaks \`sentry init\` at three gates: \`isNonInteractiveContext()\` (init.ts:182), \`isInteractiveTerminal()\` (factory.ts:62), and the wizard preamble (wizard-runner.ts:376). Fix: inline tsx directly — \`"cli": "tsx --import ./script/require-shim.mjs src/bin.ts"\` and same for \`dev\`. Single-layer \`pnpm run\` uses \`stdio: 'inherit'\`; nested pnpm does not. Approach B (\`node --import tsx/esm ...\`) rejected as fragile (tsx internal API). Approach C (shell wrapper) rejected as non-portable. Keep the \`tsx\` alias for non-interactive scripts. + +* **pnpm test runs generate:docs + generate:sdk before vitest — too slow for direct invocation**: Trap: \`pnpm test\` (or \`pnpm run test:unit\`) runs \`generate:docs && generate:sdk\` before vitest, making the full suite take minutes even for small changes — looks like a normal test command. Fix: invoke vitest directly to skip doc/SDK generation: \`vitest run test/lib test/commands test/types\`. This drops runtime from minutes to ~2-10s. Use \`pnpm run test:unit\` only when you need the generated artifacts to be fresh (e.g., testing doc generation itself). + * **process.stdin.isTTY unreliable in Bun — use isatty(0) and backfill for clack**: \`process.stdin.isTTY\` unreliable — use \`isatty(0)\` from \`node:tty\`. Bun's single-file binary can leave \`process.stdin.isTTY === undefined\` on TTY fds. \`@clack/core\` gates \`setRawMode(true)\` on \`input.isTTY\`, silently disabling raw mode. Fix: backfill \`process.stdin.isTTY = true\` when \`isatty(0)\` confirms. Debugging: \`src/lib/init/tty-diagnostics.ts\` \`dumpTtyDiagnostics(label)\` — no-op unless \`SENTRY\_INIT\_DIAGNOSTICS=1\`. @@ -186,7 +216,7 @@ * **SQLite transaction() ROLLBACK can throw, discarding original error**: (gotcha) SQLite transaction ROLLBACK error-swallowing trap: In \`src/lib/db/sqlite.ts\`, \`transaction()\` catches errors and runs \`this.db.exec('ROLLBACK')\`. If ROLLBACK itself throws, the original error is lost. Fix: \`const origErr = e; try { this.db.exec('ROLLBACK'); } catch (rbErr) { log.debug(...); } throw origErr;\` -* **streamDecompressToFile: never emit 'drain' on ENOSPC — race drain against error to avoid hang**: streamDecompressToFile: never emit 'drain' on ENOSPC — race drain against error to avoid hang: Trap: \`writer.write()\` returning false normally means wait for 'drain' before continuing. But on ENOSPC/EIO, the error fires while the buffer is full — 'drain' never fires, causing a permanent hang. Fix: \`streamDecompressToFile()\` races 'drain' against 'error' listeners so whichever fires first unblocks the loop; \`onDrain\` removes \`onError\` listener and vice versa to avoid \`MaxListenersExceededWarning\`. Same pattern applies to \`downloadStableToPath()\` which uses \`arrayBuffer()\` + \`writeFile()\` (not streaming) as a fallback due to the Bun event-loop GC bug (https://github.com/oven-sh/bun/issues/13237). Package managers always need network to fetch and install packages — offline fallback is only supported for \`method === 'curl'\` installs. +* **streamDecompressToFile: never emit 'drain' on ENOSPC — race drain against error to avoid hang**: Trap: \`writer.write()\` returning false normally means wait for 'drain' before continuing. But on ENOSPC/EIO, the error fires while the buffer is full — 'drain' never fires, causing a permanent hang. Fix: \`streamDecompressToFile()\` races 'drain' against 'error' listeners so whichever fires first unblocks the loop. This is a standing directive: 'never emit drain' when an I/O failure occurs while the buffer is full. Same pattern applies to \`downloadStableToPath()\` which uses \`arrayBuffer()\` + \`writeFile()\` (not streaming) as a fallback due to the Bun event-loop GC bug (https://github.com/oven-sh/bun/issues/13237). * **strip fails on Node SEA binaries — must strip BEFORE fossilize injection**: Strip debug symbols must happen BEFORE fossilize SEA injection. Trap: \`strip --strip-unneeded\` on a plain Node binary saves ~17 MiB and still runs — looks like it should work on the final SEA binary too. But after postject injects the SEA blob, \`strip\` fails: 'section .text can't be allocated in segment 2'. Fix: as of fossilize 0.7.0, stripping is built into fossilize itself — it strips the copied binary (already unsigned for macOS/Windows) BEFORE calling postject. Cross-strip from Linux to macOS silently fails (caught); native macOS runners strip correctly with \`strip -x\`. Windows skipped (no debug symbols). \`stripCachedNodeBinaries()\` was removed from \`script/build.ts\` in fossilize 0.7.0 update — fossilize handles it natively. @@ -200,9 +230,6 @@ * **useTestConfigDir afterEach: never delete CONFIG\_DIR\_ENV\_VAR — always restore previous value**: Trap: deleting \`process.env.SENTRY\_CONFIG\_DIR\` in \`afterEach\` looks like proper cleanup. But \`preload.ts\` always sets \`SENTRY\_CONFIG\_DIR\`, so \`savedConfigDir\` is always defined — deleting it causes subsequent test files' module-level code or \`beforeEach\` hooks to read \`undefined\`. Fix: always restore the previous value, never delete. The \`else { delete process.env\[CONFIG\_DIR\_ENV\_VAR] }\` branch is intentionally omitted in \`test/helpers.ts\` \`useTestConfigDir\`. Same principle applies in \`test/fixture.ts\` \`setAuthToken()\` finally block — the delete there is acceptable only because it's a scoped try/finally restore, not a test lifecycle hook. - -* **wasm-bindgen: exporting a class named \`Object\` shadows JS global Object — use js\_name**: Trap: naming a \`#\[wasm\_bindgen]\` Rust struct \`Object\` looks fine in Rust, and \`wasm-pack test --node\` passes because it never loads the \`--target web\` glue via \`initSync\`. Fix: the generated \`symbolic.js\` emits \`export class Object\` which shadows the JS global \`Object\` throughout the module. \`initSync\` calls \`Object.getPrototypeOf(module)\` — resolves to the exported class, throws \`TypeError: Object.getPrototypeOf is not a function\`. Also breaks \`Object.create\` in \`objects()\`. Consumer doing \`import \* as symbolic from '@sentry/symbolic'; symbolic.initSync({ module })\` gets this error. Fix: add \`#\[wasm\_bindgen(js\_name = "ObjectFile")]\` on the struct and \`#\[wasm\_bindgen(js\_class = "ObjectFile")]\` on the impl block. Only artifact smoke tests (loading actual \`symbolic.js\` via \`initSync\`) catch this — \`wasm-pack test --node\` misses it entirely. - * **wasm-pack test --node never catches js\_name/js\_class binding bugs like Object shadowing**: Trap: \`wasm-pack test --node\` looks like a complete test of the WASM package — it runs Rust tests compiled to WASM. But it builds its own JS glue and never loads the \`--target web\` artifact. So \`export class Object\` shadowing the JS global \`Object\` passes all wasm-pack tests. Fix: use the two-layer approach — (1) \`wasm\_bindgen\_test\` + \`wasm-pack test\` for bulk behavior, (2) artifact smoke test that does \`npm pack\` → install into temp dir → \`import "@sentry/symbolic"\` → assert API loads. The smoke test catches packaging regressions that wasm-pack misses. Fix for Object shadowing: \`#\[wasm\_bindgen(js\_name = "ObjectFile")]\` + \`#\[wasm\_bindgen(js\_class = "ObjectFile")]\`; Rust struct name \`Object\` unchanged. @@ -210,7 +237,7 @@ * **wasm-pack test never loads the published package — builds its own glue instead**: Trap: \`wasm-pack test\` looks like the right way to test the published \`@sentry/symbolic\` package (it builds WASM and runs tests). Fix: \`wasm-pack test\` builds its own glue (\`--target web\`, \`symbolic.js\`, \`initSync\`) and never loads what the package actually ships via \`exports\`/\`files\`. Testing the published package requires loading it through the actual package entry points, not wasm-pack's test harness. Confirmed by Burak Yigit Kaya on Jun 22 2026. -* **Whole-buffer matchAll slower than split+test when aggregated over many files**: Grep/scan traps in \`src/lib/scan/\`: (1) Literal prefilter is FILE-LEVEL gate; per-line verify breaks cross-newline patterns. (2) \`hasTopLevelAlternation\`+\`skipGroup\` must call \`skipCharacterClass\`. (3) Wake-latch race: use latched \`pendingWake\` flag. (4) \`mapFilesConcurrent\` filters \`null\` but NOT \`\[]\` — return \`null\` for no-op files. (5) \`collectGlob\`/\`collectGrep\` must NOT forward \`maxResults\` to iterator. Worker pool: lazy singleton, size \`min(8, max(2, availableParallelism()))\`. Matches encoded as \`Uint32Array\` quads (~40% faster). \`new Worker(new URL(...))\` HANGS in SEA binaries — use Blob+URL.createObjectURL. \`ref()\`/\`unref()\` idempotent — only unref when \`inflight\` drops to 0. Disable via \`SENTRY\_SCAN\_DISABLE\_WORKERS=1\`. +* **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\`. (3) Extractor \`hasTopLevelAlternation\`+\`skipGroup\` must call \`skipCharacterClass\`. (4) Wake-latch race: use latched \`pendingWake\` flag, not \`let notify=null; await new Promise(r=>notify=r)\`. (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\`. Worker pool: lazy singleton, size \`min(8, max(2, availableParallelism()))\`. Matches encoded as \`Uint32Array\` quads transferred via \`postMessage\` (~40% faster). \`new Worker(new URL(...))\` HANGS in SEA binaries — use Blob+URL.createObjectURL. FIFO \`pending\` queue per worker. \`ref()\`/\`unref()\` idempotent — only unref when \`inflight\` drops to 0. Disable via \`SENTRY\_SCAN\_DISABLE\_WORKERS=1\`. * **worktree node\_modules symlink causes esbuild host/binary version mismatch**: Trap: when running multiple agent worktrees in parallel, \`node\_modules\` may be a symlink to a sibling worktree's install — looks fine until esbuild runs. Error: 'Host version X does not match binary version Y' crashes all generate scripts and typecheck. Fix: remove the symlink and run \`pnpm install\` in the worktree root to get an independent \`node\_modules\`. The symlink is created by mutation-test workflows that intentionally share \`node\_modules\` for speed — but only safe when both worktrees are on the same branch/lockfile. @@ -229,9 +256,24 @@ * **@sentry/symbolic wasm-bindgen instances: auto-freed via FinalizationRegistry — no explicit .free() needed**: wasm-bindgen instances (\`Archive\`, \`ObjectFile\`) created in \`src/lib/dif/index.ts\` are never explicitly \`.free()\`'d — this is acceptable. \`symbolic.js\` registers \`Archive\` in its constructor (\`:45\`) and each \`ObjectFile\` in \`\_\_wrap\` (\`:136\`) with a \`FinalizationRegistry\` (\`:633-647\`) that calls \`.free()\` on GC. For a one-shot CLI that exits after parsing one file, no manual disposal is needed. Only the \`.wasm\` subpath (\`@sentry/symbolic/symbolic\_bg.wasm\`) is marked \`external\` in both esbuild configs (\`build.ts:137\`, \`bundle.ts:226\`); the JS glue exporting \`Archive\` is bundled. + +* **@sentry/symbolic: object.has\_sources() only reports embedded sources — use debug\_session.files() to detect any sources**: In \`@sentry/symbolic\` (mirroring \`print\_sources.rs\` in legacy Rust sentry-cli): \`object.has\_sources()\` only reports \*embedded\* sources, NOT referenced files. To detect whether an object has any sources at all, use \`debug\_session.files().next().is\_none()\`. Core enumeration pattern: \`Archive::parse(\&data)\` → \`archive.objects()\` → \`object.debug\_session()?.files()\` → \`FileEntry.abs\_path\_str()\` → \`debug\_session.source\_by\_path(abs\_path)\` → \`SourceFileDescriptor\` (fields: \`contents()\`, \`url()\`, \`debug\_id()\`, \`source\_mapping\_url()\`). PE with embedded PDB: also handle via \`pe.embedded\_ppdb()\`. + + +* **bundle-sources exit-code convention: manual exitCode=1 vs OutputError for no-sources path**: In \`src/commands/debug-files/bundle-sources.ts\`, the no-sources-found path uses \`this.process.exitCode = 1\` (not \`OutputError\`). This is a known Medium deviation from the framework-blessed pattern: \`OutputError\` (exit 60) is robust against framework finalization resetting exitCode; manual assignment is fragile. The \`-o\` flag also does NOT create parent directories (unlike \`bundle-jvm\`), so \`writeFile\` throws raw ENOENT if the parent doesn't exist. Both are accepted as-is in PR #1126 — do not 'fix' them without a follow-up PR discussion. Exit code map: ValidationError→21, success→0, no-sources→1. + * **clack-utils.ts filename preserved intentionally — rename deferred to next cleanup PR**: \`src/lib/init/clack-utils.ts\` filename kept (not renamed to \`wizard-utils.ts\`) to keep PR 4 diff focused on clack removal. No clack references remain in the file. \`WizardCancelledError\` lives here. \`abortIfCancelled\()\` return type uses \`Exclude\\` to narrow union types. \`FEATURE\_DISPLAY\_ORDER\` and \`CANONICAL\_STEP\_ORDER\` (12 steps) also defined here. Rename is intentionally deferred. + +* **createSourceBundle: object selection, sync provider contract, writer lifecycle**: \`createSourceBundle(data, objectName, readSource)\` in \`src/lib/dif/index.ts\`: selects \`objects.find(o => o.hasDebugInfo) ?? objects\[0]\`; returns \`{bundle:null, debugId:null, fileCount:0}\` if no objects. \`SourceBundleWriter.writeObject\` is synchronous — provider/filter callbacks must be sync (\`readFileSync\` in bundle-sources.ts). Writer is single-use: \`writeObject\` calls \`\_\_destroy\_into\_raw()\` (zeroes ptr, unregisters FinalizationRegistry). Provider returning \`null\` signals skip (WASM glue checks \`arg0 == null\`). \`bundle === null || fileCount === 0\` correctly catches manifest-only ZIPs with zero source files. + + +* **debug-files shared readDebugFile extracted to read-file.ts — not inlined per command**: \`src/commands/debug-files/read-file.ts\` is the shared async helper for reading DIF files. Throws \`ValidationError\` on ENOENT (\`File '${path}' does not exist.\`), EISDIR (\`Path '${path}' is a directory...\`), and generic read failures. Both \`check.ts\` and \`bundle-sources.ts\` import from \`./read-file.js\`. Pattern: extract shared file-reading logic here rather than duplicating in each debug-files subcommand. + + +* **debug-files/bundle-sources: SourceBundleWriter is one-shot/consuming — create fresh per call**: \`SourceBundleWriter.writeObject\` in \`@sentry/symbolic\` consumes the writer via \`\_\_destroy\_into\_raw()\` — a single instance cannot be reused. \`createSourceBundle(data: Uint8Array)\` in \`src/lib/dif/index.ts\` creates a fresh writer per call. Returns \`SourceBundleResult | null\` (null = empty bundle, i.e. \`writeObject\` returned \`undefined\`). Wasm boundary masks original JS error messages thrown inside provider callbacks — errors do propagate but message is not preserved; tests should use \`.toThrow()\` without message assertion for provider-error cases. + * **Dedupe resolved entity IDs in batch operations before API call**: Batch issue merge (\`src/commands/issue/merge.ts\`): (1) Dedupe by resolved numeric ID after \`Promise.all(args.map(resolveIssue))\` — users may pass same entity as \`CLI-K9\`, \`my-org/CLI-K9\`, or \`123\`. Throw \`ValidationError\` if \`new Set(ids).size < 2\`. (2) Reject \`undefined\` orgs in cross-org check — bare numeric IDs without DSN/config resolve with \`org: undefined\`. (3) Pass \`--into\` through \`resolveIssue()\`; compare by numeric \`id\`, not \`shortId\`. (4) Sentry bulk merge API picks canonical parent by event count — \`--into\` is preference only; warn when API's \`parent\` differs. @@ -266,7 +308,7 @@ * **sensitive argv flags must never reach telemetry — redactArgv() in cli.ts**: \`SENSITIVE\_ARGV\_FLAGS = new Set(\['token', 'auth-token'])\` in \`src/cli.ts\`. \`redactArgv()\` replaces values of these flags with \`\[REDACTED]\` before any telemetry call. This is an absolute invariant — never pass raw \`process.argv\` to telemetry without running through \`redactArgv()\` first. -* **Sentry SDK tree-shaking patches must be regenerated via bun patch workflow**: Sentry SDK tree-shaking via pnpm patch: \`patchedDependencies\` in \`package.json\` under \`pnpm\` config block strips unused exports from \`@sentry/core\` and \`@sentry/node-core\`. Always import from \`@sentry/node-core/light\`. Bumping SDK: remove old patches, \`pnpm patch @sentry/core\`, edit, \`pnpm patch-commit\`; repeat for node-core. \`check:patches\` validates version alignment AND content. \`@stricli/core\` patch targets \`dist/index.js\` (ESM) only — \`dist/index.cjs\` intentionally unpatched. When bumping \`@stricli/core\`, patch line numbers shift — always verify offsets. KNOWN: Stricli \`-H\` removal patch is fragile — any Stricli version bump will break it; upstream fix will NOT be accepted. Chunk-upload wire format: zstd → \`Content-Encoding: zstd\` + \`file\` field; gzip → \`file\_gzip\` field, NO \`Content-Encoding\` header (servers reject \`Content-Encoding: gzip\` + \`file\_gzip\` with 400); plain → \`file\` field, no encoding header. NEVER emit \`Content-Encoding: gzip\` alongside \`file\_gzip\`. +* **Sentry SDK tree-shaking patches must be regenerated via bun patch workflow**: Sentry SDK tree-shaking via bun patch: \`patchedDependencies\` in \`package.json\` strips unused exports from \`@sentry/core\` and \`@sentry/node-core\`. Non-light root of \`@sentry/node-core\` pulls uninstalled \`@opentelemetry/instrumentation\` — \*\*always import from \`@sentry/node-core/light\`\*\* (subpaths: \`.\`, \`./light\`, \`./light/otlp\`, \`./init\`, \`./loader\`, \`./import\`). No supported import for \`HttpsProxyAgent\`. Bumping SDK: remove old patches, \`rm -rf ~/.bun/install/cache/@sentry\`, \`bun install\`, \`bun patch @sentry/core\`, edit, \`bun patch --commit\`; repeat for node-core. Preserved: \`\_INTERNAL\_safeUnref\`, \`\_INTERNAL\_safeDateNow\`, \`nodeRuntimeMetricsIntegration\`. Before stripping any core export, grep \`node-core/build/{cjs,esm}/light/sdk.js\` for runtime usage (e.g. \`spanStreamingIntegration\` when \`traceLifecycle === 'stream'\`). Remove \`.bun-tag-\*\` hunks from generated patches. Manual \`git diff\` patches fail. * **sentry-cli debug-files: @sentry/symbolic usage locations for API migration**: \`@sentry/symbolic\` consumed in sentry-cli (TypeScript) at \`src/lib/dif/index.ts\`: imports \`initSync\`, \`parse\_debug\_file as wasmParseDebugFile\`, \`peek\_format as wasmPeekFormat\`. \`DIF\_WASM\_ASSET\_KEY = 'dist-build/symbolic\_bg.wasm'\` (SEA asset key). \`SYMBOLIC\_WASM\_SUBPATH = '@sentry/symbolic/symbolic\_bg.wasm'\` (dev resolution, marked external in esbuild). 3-path WASM loader: SEA \`node:sea.getRawAsset\`, sibling \`./vendor/symbolic\_bg.wasm\`, dev \`\_require.resolve(SYMBOLIC\_WASM\_SUBPATH)\`. Public API: \`parseDebugFile(data: Uint8Array): DifArchiveInfo\`, \`peekFormat(data: Uint8Array): string\`. Internal \`RawArchiveInfo\` (snake\_case Rust) mapped to camelCase \`DifArchiveInfo\`. Migration to \`Archive\`/\`ObjectFile\` class-based API on branch \`byk/feat/debug-files-new-symbolic-api\` (commit \`3052a6e2d\`). Consumer: \`src/commands/debug-files/check.ts\` imports \`DifArchiveInfo\`, \`parseDebugFile\`. @@ -275,7 +317,7 @@ * **setup.ts bestEffort() wrapper: post-install steps must never crash setup**: \`src/commands/cli/setup.ts\` \`bestEffort(stepName, fn)\` wraps non-essential post-install steps (recording install info, shell completions, agent skills) in try/catch. On failure: calls \`warn(stepName, error)\` + \`captureException(error, { level: 'warning', tags: { 'setup.step': stepName } })\`. These steps must NEVER crash setup — enforced by \`bestEffort()\`. \`runConfigurationSteps()\` applies \`bestEffort()\` independently to all 4 steps. Install dir priority: (1) \`$SENTRY\_INSTALL\_DIR\`, (2) \`~/.local/bin\` if exists+in PATH, (3) \`~/bin\` if exists+in PATH, (4) \`~/.sentry/bin\` fallback. Welcome message only on fresh install (not upgrades). -* **Shared pagination infrastructure: buildPaginationContextKey and parseCursorFlag**: 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. 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. Hidden global \`--org\`/\`--project\` flags: defined in \`GLOBAL\_FLAGS\`, \`mergeGlobalFlags()\` injects hidden flag shapes, \`applyOrgProjectFlags()\` writes to \`SENTRY\_ORG\`/\`SENTRY\_PROJECT\` before auth guard. \`applyGroupLimitAutoDefault\` helper keeps \`buildCommand\` under Biome's cognitive complexity limit of 15. +* **Shared pagination infrastructure: buildPaginationContextKey and parseCursorFlag**: 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. 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. \`sort\` flag is resolved once in \`func()\` before dispatch — never re-derived by infra. \`handleOrgAllIssues\` returns server order (no client-side sort). \`isMultiProject\` guard gates client-side sort at list.ts:1144-1148. * **Stricli optional flags with runtime-resolved defaults: omit static default, resolve in func()**: When a flag's default depends on runtime state (e.g., host type), Stricli static defaults cannot be used. Pattern: mark flag \`optional: true\` with no \`default\` in the flag definition; in \`func()\`, immediately resolve: \`const flags = { ...rawFlags, sort: rawFlags.sort ?? defaultIssueSort() }\`. All downstream code receives the concrete resolved type. \`ListFlagsInput\` uses \`Omit\ & { readonly sort?: SortValue }\` as the \`func()\` parameter type; \`ListFlags\` (with required sort) is used everywhere else. This avoids Stricli's static-default limitation while keeping type safety. @@ -284,7 +326,7 @@ * **Telemetry instrumentation pattern: withTracingSpan + captureException for handled errors**: For graceful-fallback operations, use \`withTracingSpan\` from \`src/lib/telemetry.ts\` for child spans and \`captureException\` from \`@sentry/bun\` (named import — Biome forbids namespace imports) with \`level: 'warning'\` for non-fatal errors. \`withTracingSpan\` uses \`onlyIfParent: true\` — no-op without active transaction. User-visible fallbacks use \`log.warn()\` not \`log.debug()\`. Several commands bypass telemetry by importing \`buildCommand\` from \`@stricli/core\` directly instead of \`../../lib/command.js\` (trace/list, trace/view, log/view, api.ts, help.ts). -* **Testing Stricli command func() bodies via spyOn mocking**: Testing Stricli command func() bodies: \`const func = await cmd.loader(); func.call(mockContext, flags, ...args)\` with mock \`stdout\`, \`stderr\`, \`cwd\`, \`setContext\`. \`.call()\` LSP false-positives pass \`tsc --noEmit\`. When API functions are renamed, update both spy target AND mock return shape. \`normalizeSlug\` replaces \`\_\`→\`-\` but does NOT lowercase. Vitest: use \`vi.spyOn\` / mock fetch via \`globalThis.fetch\`. \`mock.module()\` pollutes module registry — put in \`test/isolated/\`. ALL test files MUST import from \`'vitest'\` — NEVER \`'bun:test'\`. Variadic flag pattern: \`kind: 'parsed', parse: String, variadic: true, optional: true\`. Aliases: \`aliases: { s: 'scope' }\` — place INSIDE \`parameters\` (sibling to \`flags\`), NOT at top-level of \`buildCommand\` options (causes TS2353). \`-s\` is free in \`auth/login.ts\`. Biome formatter enforces specific line-break style for \`func.call(ctx, { ... }, arg)\` — run \`biome format --write\` after generating test files. Bun→Node.js: \`Bun.spawn\`→\`node:child\_process\`, \`bun:sqlite\`→\`node:sqlite\`, \`bun run\`→\`pnpm run\`/\`tsx\`, \`Bun.randomUUIDv7()\`→\`uuidv7\`. \`new Worker(new URL(...))\` HANGS in SEA — use Blob+URL.createObjectURL. +* **Testing Stricli command func() bodies via spyOn mocking**: Testing Stricli command func() bodies: (1) \`const func = await cmd.loader(); func.call(mockContext, flags, ...args)\` with mock \`stdout\`, \`stderr\`, \`cwd\`, \`setContext\`. \`loader()\` return type union causes \`.call()\` LSP false-positives that pass \`tsc --noEmit\`. (2) When API functions are renamed, update both spy target AND mock return shape. (3) \`normalizeSlug\` replaces \`\_\`→\`-\` but does NOT lowercase. (4) Bun \`mockFetch()\` replaces \`globalThis.fetch\` — use one unified mock dispatching by URL. (5) \`mock.module()\` pollutes module registry for ALL subsequent files — put in \`test/isolated/\` and run via \`test:isolated\`. (6) For \`Bun.spawn\`, use direct property assignment in \`beforeEach\`/\`afterEach\`. * **wizard-runner.ts: large shared context via initialState, not inputData — D1 row size limit**: In \`wizard-runner.ts\`, large shared context (\`dirListing\`, \`fileCache\`, \`existingSentry\`) travels via \`initialState\` (not \`inputData\`) to avoid D1 per-row size overflow (see getsentry/cli-init-api#98). \`MAX\_RESUME\_RETRIES = 3\`, \`RETRY\_BACKOFF\_MS = \[2000, 4000, 8000]\`. \`resumeWithRetry()\` handles stale-step recovery via \`tryRecoverCurrentRunState()\` when \`isStepAlreadyAdvancedError()\` detects 'was not suspended' 500. @@ -295,7 +337,7 @@ * **Always add new check scripts to both package.json and CI workflow**: When introducing a new check or validation script, the user expects it to be registered in two places simultaneously: (1) as a named script in \`package.json\` alongside other \`check:\*\` scripts, and (2) as a \`- run: pnpm run \\` step in \`.github/workflows/ci.yml\`. Never add to only one location. This applies to any new linting, validation, or verification script added to the project. -* **Always address bot review comments (Cursor Bugbot, Sentry bot) by applying fixes before merging**: When a PR has review comments, the user expects every comment to be investigated and resolved before merging — including bot comments (Cursor Bugbot, Seer) and human reviewer comments. This includes: removing flagged unnecessary code/comments, adding missing CI steps identified by reviewers, and replying to each comment after fixing. The user also investigates ambiguous reviewer references (e.g., 'image.yml' when no such file exists) to find the actual intended target. Only after all comments are addressed and CI passes does the user proceed to merge. +* **Always address bot review comments (Cursor Bugbot, Sentry bot) by applying fixes before merging**: When automated bots (Cursor Bugbot, Sentry/Warden) post review comments on a PR, the user consistently reads and acts on each finding — either applying a fix or explicitly acknowledging it as stale/already-addressed. The user does not dismiss bot findings as noise; even medium/low severity issues get investigated and fixed if valid. After fixing, the user typically replies to the bot comment referencing the fix commit. This applies to all bot sources: Cursor Bugbot, Sentry bot, and Warden. When multiple bots flag the same location, the user treats it as higher-priority confirmation of a real bug. * **Always branch off main for focused fixes, never commit unrelated changes to feature branches**: When starting a focused fix or chore (security patch, bug fix, etc.), the user consistently creates a fresh branch off origin/main rather than committing to an existing feature branch. They explicitly scope the branch to only the relevant changes, excluding unrelated modifications (e.g., .lore.md updates, auto-regenerated files, OTel work). When the working tree has uncommitted changes on a feature branch, they stash, back up, or otherwise isolate those changes before branching from main. The new branch name reflects the specific fix (e.g., \`chore/security-deps-bump\`, \`byk/fix/upgrade-missing-install-dir\`). Follow this pattern by always proposing a clean branch from main for any fix that is independent of ongoing feature work. @@ -303,6 +345,9 @@ * **Always clarify that the repo uses plain git (not jj) when jj commands fail**: When a jj command fails with 'no jj repo in .', the user consistently clarifies that the repo is a plain git repo and that jj's 'never fails on conflict' behavior is being referenced conceptually — meaning conflicts should be recorded/resolved rather than aborting operations. The agent should: (1) fall back to git commands immediately without retrying jj, (2) handle merge conflicts by stashing, pulling, and resolving (e.g., \`git checkout --theirs\` for files like \`.lore.md\`), and (3) not attempt \`jj git init\` or any jj initialization. This pattern appears at the start of every build session. + +* **Always commit and push when local tests pass, relying on CI for full test matrix**: When the full local test suite is too slow or times out, the user accepts targeted test runs (covering only the changed files/units) plus passing typecheck and lint as sufficient local verification. They then commit and push, trusting CI to run the complete test matrix. The assistant should not block on full local suite completion when targeted tests, typecheck, and lint all pass — proceed to commit and push, noting that CI will validate the rest. + * **Always compare PR branch against main before reviewing changes**: When reviewing a PR, the user consistently wants to understand exactly what changed in the PR branch versus main before diving into the content. This means fetching the remote branch if not available locally, running \`git log main..origin/\\` to see commits, and \`git diff\` (with stat) to understand the scope of changes. The user explicitly frames this as needing to know 'what changes were made vs what actually exists on main.' Always establish this baseline diff context first before analyzing or discussing PR content. @@ -318,6 +363,12 @@ * **Always confirm and send notifications to David (Dav1dde) when PR milestones are reached**: When a significant PR milestone is reached (merge, readiness for handoff, or status update), the user explicitly confirms sending a notification to David (Dav1dde/dav1d) via Beeper chat 557079. The user gives a short confirmation ('yup', explicit instruction) and expects the assistant to open the Beeper chat and send the message. This applies to: merged PRs that unblock David's work, status summaries of completed tracks, and follow-up replies to David's messages. The assistant should draft the message, confirm with the user, then send it via Beeper /open/557079. + +* **Always coordinate task ownership with collaborators before starting implementation**: When working on a shared codebase with other contributors (e.g., dav1d), the user explicitly checks who is implementing what before starting work, to avoid parallel duplicate effort. They confirm ownership verbally (via chat/Beeper), claim the task, and only then begin implementation. This applies especially when a collaborator has signaled they might draft something themselves. The user also communicates task boundaries proactively — e.g., 'I'll take the debug-session wasm API so you don't need to draft it.' When coordinating, the user opens the relevant chat tool and sends a message to claim the work before branching or writing code. + + +* **Always coordinate with David (dav1d) via Beeper before proceeding on blocked tasks**: When a task is blocked on an external dependency owned by David (dav1d), the user consistently reaches out to him via Beeper (chat \`/open/557079\`) to unblock progress — sharing relevant code references, asking about API availability, or confirming implementation details. The user expects the assistant to draft or send these messages proactively, using BYK's voice, and to treat David's responses as authoritative signals for what to build next. Always use the Beeper deep-link \`/open/557079\` to open David's chat when sending messages. + * **Always create a dedicated branch when updating fossilize versions**: When a new version of fossilize is released, always create a branch named \`chore/fossilize-{version}\` tracking origin/main, update the dependency, remove any functionality now handled natively by fossilize (e.g., \`stripCachedNodeBinaries()\` removed in 0.7.0), verify the build succeeds, then commit with \`chore: update fossilize to X.Y.Z\`. Follow this exact pattern: branch → update dep → remove superseded code → build verify → commit → PR. @@ -342,6 +393,9 @@ * **Always enforce read-only adversarial review mode with strict branch isolation**: When starting a code review session, the user consistently issues explicit system-level directives: (1) act as adversarial READ-ONLY reviewer — never modify files, never switch branches, never touch the working tree; (2) never return an empty report; (3) specify the exact repo, working dir, current checkout branch (which must not be switched), and PR branch to review. The user reviews PRs while on a \*different\* parallel branch and accesses PR content exclusively via \`gh pr diff\`, \`gh pr view\`, and \`git show origin/\:\\`. Follow these constraints strictly without prompting for confirmation. + +* **Always enforce strict read-only constraints during adversarial PR code reviews**: When the user issues an adversarial code review task, they consistently impose hard read-only constraints: never modify, stage, or commit any tracked files in the working repo; never touch \`.lore.md\`; and place any throwaway test files only inside existing vitest include directories (never \`/tmp\`). They also require mutation testing directives to validate test coverage (e.g., introduce specific bugs and confirm tests fail). The review must never return an empty report. The assistant should treat these constraints as non-negotiable and apply them automatically whenever a PR review is framed as 'adversarial' or 'read-only'. + * **Always explicitly confirm and narrow scope before implementation begins**: When presented with a problem or plan that contains multiple potential fix paths or related issues, the user explicitly confirms which items are in scope and which are deferred/out of scope before work proceeds. The user treats scope confirmation as a required gate before implementation. When the user marks something as out of scope, the assistant should update the plan to reflect this clearly (e.g., label deferred items explicitly) and not revisit them unless the user re-opens them. Do not assume adjacent issues are in scope just because they are related. @@ -351,6 +405,9 @@ * **Always explore e2e test infrastructure thoroughly before debugging or modifying tests**: When approaching e2e test work, always explore the full infrastructure before making changes: \`test/e2e/\` (14 files: api, auth, bundle, completion, delta-upgrade, event, issue, library, log, multiregion, project, skill-eval, telemetry-exit, trace), \`test/fixture.ts\` (getCliCommand, runCli, createE2EContext), \`test/helpers.ts\` (useTestConfigDir, useEnvSandbox, resetHostScopingState, mintSntrysToken, extractFetchUrl), \`test/mocks/\` (server.ts, routes.js, multiregion.ts), \`src/bin.ts\`, \`src/cli.ts\`. Key: \`getCliCommand()\` returns \`\[SENTRY\_CLI\_BINARY]\` if set, else \`\[process.execPath, 'run', 'src/bin.ts']\`. \`createE2EContext.run()\` sets \`SENTRY\_AUTH\_TOKEN: ''\`, \`SENTRY\_TOKEN: ''\`, \`SENTRY\_CLI\_NO\_TELEMETRY: '1'\`. \`test:e2e\` runs without \`--isolate --parallel\`. Map full infrastructure before proposing fixes. + +* **Always fetch security advisories and Dependabot alerts before creating a fix plan**: The user consistently directs you to run two \`gh api\` commands: one for security advisories and one for Dependabot alerts, using headers \`Accept: application/vnd.github+json\` and \`X-GitHub-Api-Version: 2022-11-28\`. After retrieving the data, you must analyze the vulnerabilities and create a detailed plan for fixing them. This sequence recurs across multiple sessions, demonstrating a preferred workflow for data-driven security remediation planning. + * **Always flag import framework mismatches as blocking CI issues in PR reviews**: When reviewing PRs, the user consistently identifies test files using the wrong import framework (e.g., \`bun:test\` instead of \`vitest\`) as a BLOCKING issue, not a non-blocking suggestion. This applies when the project has migrated frameworks and all other test files use the new one. The user expects the reviewer/assistant to explicitly label it as blocking (B1, B2, etc.) and distinguish it from non-blocking issues (N1, N2, etc.), using a clear severity classification system in PR review feedback. @@ -372,6 +429,9 @@ * **Always keep work-in-progress repos at permanent paths under ~/Code/getsentry/**: When working on a repository across sessions, the user expects it to live at a permanent location under \`~/Code/getsentry/\\` (e.g., \`~/Code/getsentry/symbolic\`, \`~/Code/getsentry/cli\`), not in ephemeral directories like \`/tmp\`. If a clone exists in \`/tmp\`, move it to the permanent location and remove the temporary one. This applies to any \`getsentry\` org repo being actively developed on. Always use the permanent path when referencing or operating on the repo in subsequent steps. + +* **Always merge PRs despite transient CI failures when root cause is confirmed external**: When all CI failures are confirmed to be caused by external/transient issues (e.g., Anthropic outage, provider unavailability) rather than code problems, the user proceeds to merge the PR without waiting for those checks to go green. The user investigates the failure root cause first, confirms it's unrelated to the code changes, then merges — even if the merge state shows UNSTABLE due to failing checks. This applies specifically when: (1) the failing checks have no code findings, (2) the failures are clearly transient/infrastructure issues, and (3) all code-related checks pass. + * **Always migrate Bun-specific APIs and tooling to Node.js equivalents**: Migrating from Bun to Node.js/pnpm. Replace Bun-specific APIs: \`Bun.spawn\`→\`node:child\_process\`, \`Bun.sleep\`→\`node:timers/promises\`, \`bun:sqlite\`→\`node:sqlite\`, \`bun run\`→\`pnpm run\`/\`tsx\`, \`bun.lock\`→pnpm lockfile. All packages in \`devDependencies\` (never \`dependencies\`). Exception: \`script/build.ts\` uses fossilize (not \`Bun.build\`) and stays on Bun for the build-binary CI job. \`script/bundle.ts\` (npm bundle) uses esbuild via tsx and is Node-native. \`packageManager\` field in package.json: \`pnpm@10.11.0\`. After each migration phase, ensure lint and tests pass before committing. Migration is complete as of main branch (bun.lock deleted, vitest.config.ts added, all test files migrated to vitest). @@ -396,6 +456,9 @@ * **Always read source files thoroughly before asking questions or making changes**: The user consistently reads full source files (often 400-900+ lines) and traces complete data flow pipelines across multiple modules before taking action. They examine types, constants, function signatures, and cross-module dependencies in depth. They do not ask clarifying questions upfront — instead they investigate the codebase themselves to build a complete mental model. When helping this user, proactively read all relevant files in full, trace imports and data flows end-to-end, and present comprehensive findings rather than asking what they want to know. Assume they want the full picture, not a summary. + +* **Always record detailed post-merge state in session observations including squash commit hash, adjacent commits, and completed todo items**: After a PR is merged, the user consistently logs: (1) the squash commit hash and full commit message, (2) the names of adjacent commits on main at merge time, (3) confirmation that all todo/task items are completed with their descriptions, and (4) a technical summary of what the fix actually does. This pattern appears in session observations marked with 🟡 timestamps. When assisting with PR workflows, ensure post-merge summaries capture the squash SHA, neighboring commits on main, task completion status, and a concise fix description — not just a simple 'merged' confirmation. + * **Always record detailed session observations with timestamped bullet points**: The user consistently documents work sessions as structured observation logs with emoji status indicators (🟡 in-progress, 🔴 decision/blocker), timestamps in (HH:MM) format, and granular bullet points covering: design decisions made and alternatives rejected, exact file paths and line numbers changed, compile/test results with timing, and explicit rationale for choices. These logs appear to be maintained as a persistent record across sessions. When working, always produce or update this style of log — capturing not just what was done but why alternatives were rejected, exact technical details (commit hashes, line numbers, timing), and outcome verification (test counts, build success). @@ -414,17 +477,23 @@ * **Always request critical self-review of PRs before considering them ready to merge**: PR review workflow: (1) After completing a PR, always do a critical self-review (subagent for objectivity) before merge. Structure: severity levels (Critical/High/Medium/Low), actionable findings, cover correctness/safety/CI wiring/docs/changelog. (2) Address all bot/automated review comments (Cursor Bugbot, Sentry bot) as actionable work items — enumerate, investigate, fix, verify no regressions before merge. (3) Verify PR merge status (MERGED state, all checks passed, remote branch deleted) before starting follow-up work. (4) When reviewing PRs, verify factual claims (e.g. 'no existing code changed') with concrete tool-based evidence (git diff, file reads) — never accept at face value. (5) After requesting a merge, confirm merge commit OID, mergedAt, mergedBy, and final state. When blocked, explain why (ruleset vs. branch protection, self-approval restrictions). Post-merge: run verification tests, post results as PR comment, then merge. \`gh pr view --json merged\` field unavailable — use \`mergeCommit\`, \`mergedAt\`, \`mergedBy\`, \`state\`. + +* **Always request READ-ONLY critical code reviews with explicit focus areas**: When asking for a code review, the user consistently specifies: (1) it is READ-ONLY — no file modifications; (2) it is 'critical' or 'objective'; (3) the exact repo path, branch, and base branch; (4) the specific files under review; (5) explicit focus areas that CI cannot catch (e.g., correctness, memory management, format compatibility, error handling, convention compliance). Always treat these reviews as advisory-only, never modify files, and structure feedback around the user-specified focus areas rather than general style or formatting concerns. + -* **Always require a critical self-review before merging any PR**: Before merging any PR, the user consistently requests a final, objective, read-only code review — typically delegated to a subagent for impartiality. The review covers: behavior parity, error handling, type correctness, memory/resource management, test passage (vitest run), typecheck (tsc --noEmit), absence of stale references (grep), and PR description accuracy. The verdict must explicitly classify findings by severity (Critical/High/Medium/Low) before a merge recommendation is given. This pattern applies to all PRs regardless of apparent simplicity. +* **Always require a critical self-review before merging any PR**: Before merging any PR, the user consistently requires a thorough critical self-review of both the code and the PR description, explicitly preferring a subagent for objectivity/adversarial perspective. The subagent must be read-only (no file modifications, no branch switching). The review should cover: correctness of logic, accuracy of PR description claims, edge cases, regression test adequacy, and any stale/aspirational statements. The review must always produce a report (never empty). Issues found should be fixed before merging. This is a non-negotiable pre-merge gate, not optional. * **Always research technical approaches thoroughly before implementation**: When facing a significant technical decision or migration, the user consistently requests deep research into multiple approaches before writing any code. This includes: fetching specific upstream documentation/source files (e.g., BUILDING.md, configure.py), identifying concrete flags/options, estimating build times, and evaluating cross-compilation feasibility. The user wants tradeoffs between paths laid out explicitly. Only after research is complete does implementation begin. When presenting research, include specific flags, URLs, estimated costs (time/size), and platform constraints. + +* **Always respond to adversarial/bot review findings by applying concrete fixes and re-running mutation/regression tests to verify**: When a code review (adversarial subagent, Cursor Bugbot, Seer, or similar) identifies a vacuous test, inaccurate comment, or swallowed error, the user expects: (1) the specific finding to be fixed immediately (strengthen the test assertion, correct the comment, add a tradeoff note), (2) mutation testing or targeted re-runs to confirm the fix now catches the previously-uncaught mutation, and (3) a full test+lint+typecheck pass before committing. The user also expects replies posted to bot review comments acknowledging the fix with the commit SHA. Do not defer or mark findings as 'low priority' — apply all SHOULD-FIX items before committing. + * **Always review drafted messages before sending, then explicitly approve sending**: When the assistant drafts a Slack/Beeper message on the user's behalf, the user expects it to be pre-filled or staged for review — not auto-sent. The user then explicitly approves sending (e.g., selecting an option, confirming intent). The assistant should never auto-send communications without a clear send instruction from the user. Once the user gives explicit approval (e.g., 'send it', selecting a send option), the assistant should proceed to send immediately. - -* **Always run vitest directly instead of via test:unit script to avoid slow doc/SDK generation**: When running tests, the user consistently bypasses the \`pnpm run test:unit\` (or \`pnpm test\`) script because it runs \`generate:docs\` and \`generate:sdk\` as a prelude, making it too slow or causing timeouts. Instead, invoke vitest directly (e.g., \`pnpm exec vitest run \\`) to skip the generation steps. This applies whenever running specific test files or a subset of tests during development. Only use the full test script when a complete suite run with doc/SDK generation is explicitly required. + +* **Always run lint and typecheck after code changes, then fix all errors before proceeding**: After implementing or modifying code, the user consistently runs the full lint (Biome) and typecheck (tsc) pipeline and expects all errors to be resolved before moving on. When lint errors are found (e.g., \`noParameterProperties\`, complexity violations), the user applies fixes immediately and re-runs checks to confirm a clean exit. The pattern is: implement → lint → fix lint errors → re-lint to confirm clean → typecheck → confirm clean → run tests. The user does not consider a task complete until lint, tsc, and tests all pass with zero errors. * **Always sequence npm publication before CLI integration (no vendored blobs)**: Always sequence npm publication before CLI integration (no vendored blobs): Publish upstream packages to npm first through the proper release pipeline before integrating into the CLI. Correct sequence: (1) create and merge full upstream PR including CI, build, and craft/npm targets, (2) wait for npm publish, (3) open/merge CLI PR consuming it as a proper npm dependency. CLI PR #1109 (MERGED): \`@sentry/symbolic@13.3.0\` consumed as devDependency (pinned exactly — no \`^\` or \`~\` for Sentry-scoped packages). 3-path wasm loader: (1) SEA: \`node:sea.getRawAsset(DIF\_WASM\_ASSET\_KEY)\`, (2) npm bundle: sibling \`dist/vendor/symbolic\_bg.wasm\` via \`existsSync(new URL('./vendor/symbolic\_bg.wasm', import.meta.url))\`, (3) dev: \`readFileSync(\_require.resolve(SYMBOLIC\_WASM\_SUBPATH))\`. \`@sentry/symbolic/symbolic\_bg.wasm\` marked \`external\` in esbuild (both \`build.ts\` and \`bundle.ts\`). Vendored crate \`crates/dif-wasm/\` and \`src/lib/dif/vendor/\` fully deleted. @@ -456,12 +525,18 @@ * **Always track project state through structured session observations before acting**: The user consistently works across multiple sessions on long-running, multi-PR projects (e.g., symbolic-wasm, debug-files CLI). Before taking action, they review prior session context and orient around the current state of PRs, branches, and design decisions. They expect the assistant to fetch and summarize PR/code details proactively, record decisions and findings as structured observations, and maintain continuity across sessions. When resuming work, always re-establish context (branch state, open PRs, pending items) before proceeding with implementation or review tasks. + +* **Always use Beeper deep-link /open/557079 to contact David**: When the user needs to send a message to David (dav1d/Dav1dde), they consistently use the Beeper deep-link command \`/open/557079\` to open the DM chat. This applies whenever coordinating on API availability, confirming implementation details, sharing PR status, or asking blocking questions. The assistant should use this exact deep-link path without prompting the user for contact details or confirmation of how to reach David. + * **Always use specific file paths when staging git commits, never \`git add -A\`**: When staging files for a commit, the user explicitly lists individual file paths rather than using \`git add -A\` or \`git add .\`. This avoids accidentally including untracked files (e.g., stray markdown files, \`.lore.md\` conflicts) or unintended changes. When a rebase or merge goes wrong due to bulk staging, the user aborts and re-applies changes manually with targeted \`git add \\` calls. Always recommend staging by explicit file path, and warn against broad staging commands that could sweep in unrelated files. * **Always validate changes with local builds and tests before committing**: Before committing or finalizing a branch, the user consistently runs local validation: native tests (cargo test), target-specific builds (e.g., wasm32-unknown-unknown), fmt checks, and clippy. This applies especially after non-trivial changes like dependency swaps, cfg-gating, or CI workflow additions. The assistant should proactively run these checks (or prompt the user to) before declaring work done, and report pass/fail counts and timing. Only after all checks pass should files be committed. + +* **Always validate published package artifacts via pack-install smoke tests, not just unit/wasm-pack tests**: When working on WASM/npm packages, the user consistently insists on validating the actual shipped artifact (via \`npm pack\` → install tarball → import → \`node:test\`) rather than relying on \`wasm-pack test\` or similar build-time tests. The user understands that \`wasm-pack test\` builds its own glue and never exercises the \`--target web\` \`symbolic.js\` + \`initSync\` + exports map that consumers actually load. When bugs are found (e.g., class name shadowing JS globals), the user attributes the miss to this gap and adds a packaged-artifact smoke test as a permanent fixture. Always recommend or implement pack-install smoke tests for npm/WASM packages, not just wasm-pack or unit tests. + * **Always verify all tasks are complete before committing, then commit with descriptive conventional commit messages**: Before committing, the user reviews a task checklist to confirm all items are completed or in-progress. They stage all relevant files, then commit with a conventional commit message (e.g., 'docs: fix stale Bun references and add systemic doc checks') that summarizes the scope of changes. The commit message reflects the primary theme of the work session. The user expects the assistant to help verify task completion status, check git status, and confirm the commit succeeds with a summary of files changed and insertions/deletions. @@ -480,6 +555,12 @@ * **Always verify code claims against actual file contents before accepting them as true**: When evaluating PRs, documentation, or assertions about code behavior, the user systematically cross-checks every claim against the actual source files at specific line numbers. They expect the assistant to read the real files, confirm exact line locations, quote the relevant code/comments, and flag discrepancies between what is claimed and what the code actually does. The user marks confirmed findings with 🟡 (verified) and actionable directives with 🔴 (user assertion/directive). Never accept a PR description or assertion at face value — always ground-truth it against the codebase with precise line references. + +* **Always verify code comments that assert invariants**: The user consistently reads and evaluates code comments that make absolute claims such as 'always', 'never', or 'must'. They check whether the implementation actually upholds these claims and may flag discrepancies. During code review or implementation, the assistant should proactively identify such comments, verify their correctness, and ensure they accurately reflect the code behavior to prevent misleading future readers. + + +* **Always verify compilation after applying code changes**: After making code changes (edits, refactors, new impls, or fixes), the user consistently runs a compile/check command (e.g., \`cargo check\`, \`cargo check --target wasm32-unknown-unknown\`, \`cargo check -p \ --all-features\`) to confirm the changes compile cleanly before proceeding. If compilation fails, the user investigates the root cause, applies a fix, and re-runs the check until it passes. This applies to both library crates and wasm targets. The assistant should proactively suggest or run the appropriate compile check after any code modification, and report the result (pass/fail, duration, any warnings or errors). + * **Always verify lint and typecheck pass after code changes before committing**: Always verify lint and typecheck pass after code changes before committing: Run Biome lint and typecheck after implementing or modifying code. When lint errors appear, verify whether pre-existing (use git stash) vs. newly introduced — fix only newly introduced issues. Expect clean lint output (0 errors) and passing tests before committing or pushing. Use \`npmx.dev\` (not npmjs.com or npmx.com) when searching for existing npm packages. @@ -504,9 +585,15 @@ * **Bot review triage: distinguish real bugs from SDK-mirroring false positives**: When Sentry Seer or Cursor Bugbot flags 'unusual' code that intentionally mirrors upstream SDK behavior (e.g., \`http\_proxy\` as last-resort fallback for HTTPS URLs — deliberate in \`@sentry/node-core\` \`applyNoProxyOption\`), decline with a written rationale referencing the SDK source rather than silently changing behavior. Verify against \`node\_modules/@sentry/node-core/build/esm/transports/http.js\`, post a reply explaining the precedent, and resolve the thread. Real bugs (uppercase env var support, whitespace trimming, wildcard handling) get fixed; SDK-mirroring 'bugs' get explained and dismissed. Removing the mirror creates a divergence where users get different proxy semantics from our transport vs. the SDK default. + +* **Create dedicated branch for dependabot/security fixes tracking origin/main**: When fixing Dependabot alerts or security advisories, create a dedicated branch named \`chore/fix-\-\\` tracking origin/main (not a feature branch). Include all fix changes in a single focused commit. Push, create PR, and address CI failures immediately. This ensures security fixes are isolated from feature work and can be merged independently. Branch name example: \`chore/fix-dependabot-alerts-2026-06-23\`. + * **Follow the established git workflow (branch, PR, review)**: Behavioral pattern detected across 6 sessions (action: enforced-workflow). The user consistently demonstrates this behavior. + +* **Never uses form-data at runtime — only dev transitive dependency via @types/node-fetch**: User stated never uses form-data at runtime — only dev transitive dependency via @types/node-fetch. + * **Prefers Bun-native APIs; use buildCommand from lib/command.js (never @stricli/core directly); use buildRouteMap from lib/route-map.js; silent catch blocks prohibited; every new src/lib/\*\*/\*.ts must start with module-level JSDoc; test isolation via useTestConfigDir(); prefer property-based and model-based tests over unit tests; DEFAULT\_NUM\_RUNS = 50; architecture tree documented; error exit code ranges: 1x=auth**: Project conventions (AGENTS.md): use \`pnpm run\`/\`pnpm install\`/\`pnpm add -D\` (NOT bun for package management); use buildCommand from lib/command.js (never @stricli/core directly); use buildRouteMap from lib/route-map.js; silent catch blocks prohibited — every catch must re-throw, log with log.debug()/log.warn(), or return fallback WITH a log.debug() call; every new src/lib/\*\*/\*.ts must start with module-level JSDoc; test isolation via useTestConfigDir(); prefer property-based and model-based tests over unit tests; DEFAULT\_NUM\_RUNS=50; error exit code ranges: 1x=auth, 2x=input/config, 3x=API/network, 4x=feature/billing, 5x=operations, 6x=command-specific. Testing: vitest + fast-check (NEVER bun:test). All packages in devDependencies (CI enforces via \`pnpm run check:deps\`). NEVER merge a PR if CI is failing unless explicitly told to ignore. Always use \`pnpm add -D \\` — never add to \`dependencies\`. diff --git a/docs/src/content/docs/contributing.md b/docs/src/content/docs/contributing.md index 24e37aad0..8a07ef96f 100644 --- a/docs/src/content/docs/contributing.md +++ b/docs/src/content/docs/contributing.md @@ -58,7 +58,7 @@ cli/ │ │ ├── code-mappings/# upload │ │ ├── dart-symbol-map/# upload │ │ ├── dashboard/ # add, create, delete, edit, list, restore, revisions, view -│ │ ├── debug-files/ # bundle-jvm, bundle-sources, check +│ │ ├── debug-files/ # bundle-jvm, bundle-sources, check, print-sources │ │ ├── event/ # list, send, view │ │ ├── issue/ # archive, events, explain, list, merge, plan, resolve, unresolve, view │ │ ├── local/ # run, serve diff --git a/docs/src/fragments/commands/debug-files.md b/docs/src/fragments/commands/debug-files.md index 641a6b54a..1056862ff 100644 --- a/docs/src/fragments/commands/debug-files.md +++ b/docs/src/fragments/commands/debug-files.md @@ -9,6 +9,10 @@ sentry debug-files check ./libexample.so sentry debug-files check MyApp.dSYM/Contents/Resources/DWARF/MyApp sentry debug-files check ./app.pdb --json +# List the source files a debug file references (and whether they're available) +sentry debug-files print-sources ./libexample.so +sentry debug-files print-sources ./app.pdb --json + # Bundle a debug file's referenced source files (run on the build machine) sentry debug-files bundle-sources ./libexample.so sentry debug-files bundle-sources ./app.pdb --output ./app.src.zip @@ -25,12 +29,16 @@ sentry debug-files bundle-jvm --output ./out --debug-id --json ./src ## Important Notes -- `check`, `bundle-sources`, and `bundle-jvm` are **local-only** — they make no - network requests. They parse object files in-process (Mach-O/dSYM, ELF, - PE/PDB, Portable PDB, WebAssembly, Breakpad, source bundles) via a bundled - `symbolic` WASM module. +- `check`, `print-sources`, `bundle-sources`, and `bundle-jvm` are **local-only** + — they make no network requests. They parse object files in-process + (Mach-O/dSYM, ELF, PE/PDB, Portable PDB, WebAssembly, Breakpad, source bundles) + via a bundled `symbolic` WASM module. - `check` exits non-zero if the file is not usable for symbolication (no debug id or no useful features). +- `print-sources` lists the source files each object references, reporting for + each whether the source is embedded in the debug file, available via a source + link, or present on the local disk. It is a read-only preview of what + `bundle-sources` would collect and always exits zero on a parseable file. - `bundle-sources` reads source files from the paths recorded in the debug info, so it is normally run on the build machine right after compiling. Referenced files that are not present locally are skipped; it exits non-zero (writing diff --git a/package.json b/package.json index 8d89ff0a5..58962e2f9 100644 --- a/package.json +++ b/package.json @@ -16,7 +16,7 @@ "@sentry/core": "10.50.0", "@sentry/node-core": "10.50.0", "@sentry/sqlish": "^1.0.1", - "@sentry/symbolic": "13.4.0", + "@sentry/symbolic": "13.5.0", "@spotlightjs/spotlight": "^4.11.7", "@stricli/auto-complete": "^1.2.8", "@stricli/core": "1.2.7", diff --git a/plugins/sentry-cli/skills/sentry-cli/SKILL.md b/plugins/sentry-cli/skills/sentry-cli/SKILL.md index 2adf1cac4..dc3bfc948 100644 --- a/plugins/sentry-cli/skills/sentry-cli/SKILL.md +++ b/plugins/sentry-cli/skills/sentry-cli/SKILL.md @@ -411,8 +411,9 @@ Work with Dart/Flutter symbol maps Work with debug information files - `sentry debug-files check ` — Inspect a debug information file -- `sentry debug-files bundle-jvm ` — Create a JVM source bundle for source context +- `sentry debug-files print-sources ` — List the source files a debug file references - `sentry debug-files bundle-sources ` — Bundle a debug file's source files for source context +- `sentry debug-files bundle-jvm ` — Create a JVM source bundle for source context → Full flags and examples: `references/debug-files.md` diff --git a/plugins/sentry-cli/skills/sentry-cli/references/debug-files.md b/plugins/sentry-cli/skills/sentry-cli/references/debug-files.md index d1a09061d..d442fea7d 100644 --- a/plugins/sentry-cli/skills/sentry-cli/references/debug-files.md +++ b/plugins/sentry-cli/skills/sentry-cli/references/debug-files.md @@ -15,14 +15,9 @@ Work with debug information files Inspect a debug information file -### `sentry debug-files bundle-jvm ` +### `sentry debug-files print-sources ` -Create a JVM source bundle for source context - -**Flags:** -- `-o, --output - Output directory for the bundle ZIP` -- `-d, --debug-id - Debug ID (UUID) to stamp on the bundle` -- `-e, --exclude ... - Additional directory names to exclude (repeatable)` +List the source files a debug file references ### `sentry debug-files bundle-sources ` @@ -31,6 +26,15 @@ Bundle a debug file's source files for source context **Flags:** - `-o, --output - Output path for the source bundle ZIP (default: .src.zip)` +### `sentry debug-files bundle-jvm ` + +Create a JVM source bundle for source context + +**Flags:** +- `-o, --output - Output directory for the bundle ZIP` +- `-d, --debug-id - Debug ID (UUID) to stamp on the bundle` +- `-e, --exclude ... - Additional directory names to exclude (repeatable)` + **Examples:** ```bash @@ -39,6 +43,10 @@ sentry debug-files check ./libexample.so sentry debug-files check MyApp.dSYM/Contents/Resources/DWARF/MyApp sentry debug-files check ./app.pdb --json +# List the source files a debug file references (and whether they're available) +sentry debug-files print-sources ./libexample.so +sentry debug-files print-sources ./app.pdb --json + # Bundle a debug file's referenced source files (run on the build machine) sentry debug-files bundle-sources ./libexample.so sentry debug-files bundle-sources ./app.pdb --output ./app.src.zip diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index 2722c9938..2e4c33bbd 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -54,8 +54,8 @@ importers: specifier: ^1.0.1 version: 1.0.1(react@19.2.7) '@sentry/symbolic': - specifier: 13.4.0 - version: 13.4.0 + specifier: 13.5.0 + version: 13.5.0 '@spotlightjs/spotlight': specifier: ^4.11.7 version: 4.11.7(@opentelemetry/core@2.8.0(@opentelemetry/api@1.9.1))(hono-rate-limiter@0.4.2(hono@4.12.27)) @@ -809,8 +809,8 @@ packages: react: optional: true - '@sentry/symbolic@13.4.0': - resolution: {integrity: sha512-38pUc0+jmRNHPdMEyUr+wUHB9lJLfbIigppfzZs2M7GLvUbCeqV9QzcLjhNyuDRR2K+Db3yMIVFy+EeuVw/MFg==} + '@sentry/symbolic@13.5.0': + resolution: {integrity: sha512-92buXpApwcFs5TsglryoPudQJBBQxF9ASOU//iJQSHlGjAVmk38KsqW0xshe//TTXjFdrDkXCQosR7eUV+1Z5Q==} '@sindresorhus/merge-streams@4.0.0': resolution: {integrity: sha512-tlqY9xq5ukxTUZBmoOp+m61cqwQD5pHJtFY3Mn8CA8ps6yghLH/Hw8UPdqg4OLmFW3IFlcXnQNmo/dh8HzXYIQ==} @@ -3276,7 +3276,7 @@ snapshots: optionalDependencies: react: 19.2.7 - '@sentry/symbolic@13.4.0': {} + '@sentry/symbolic@13.5.0': {} '@sindresorhus/merge-streams@4.0.0': {} diff --git a/src/commands/debug-files/index.ts b/src/commands/debug-files/index.ts index 579cfdf68..be40a4aa8 100644 --- a/src/commands/debug-files/index.ts +++ b/src/commands/debug-files/index.ts @@ -8,12 +8,14 @@ import { buildRouteMap } from "../../lib/route-map.js"; import { bundleJvmCommand } from "./bundle-jvm.js"; import { bundleSourcesCommand } from "./bundle-sources.js"; import { checkCommand } from "./check.js"; +import { printSourcesCommand } from "./print-sources.js"; export const debugFilesRoute = buildRouteMap({ routes: { check: checkCommand, - "bundle-jvm": bundleJvmCommand, + "print-sources": printSourcesCommand, "bundle-sources": bundleSourcesCommand, + "bundle-jvm": bundleJvmCommand, }, docs: { brief: "Work with debug information files", diff --git a/src/commands/debug-files/print-sources.ts b/src/commands/debug-files/print-sources.ts new file mode 100644 index 000000000..60c574433 --- /dev/null +++ b/src/commands/debug-files/print-sources.ts @@ -0,0 +1,209 @@ +/** + * sentry debug-files print-sources + * + * List the source files referenced by a debug information file (Mach-O/dSYM, + * ELF, PE/PDB, Portable PDB, Breakpad, source bundles). For each referenced + * file it reports whether the source is embedded, available via a source link, + * or present on the local disk. + * + * This mirrors the legacy `sentry-cli difutil print-sources` and complements + * `bundle-sources`. Local-only — no API calls. Parsing happens in-process via + * the bundled `symbolic` WASM module (see `src/lib/dif/`). + */ + +import { existsSync } from "node:fs"; +import type { SentryContext } from "../../context.js"; +import { buildCommand } from "../../lib/command.js"; +import { + type DifSourcesInfo, + listSources, + selectBundledObject, +} from "../../lib/dif/index.js"; +import { ValidationError } from "../../lib/errors.js"; +import { + colorTag, + renderMarkdown, + safeCodeSpan, +} from "../../lib/formatters/markdown.js"; +import { CommandOutput } from "../../lib/formatters/output.js"; +import { logger } from "../../lib/logger.js"; +import { readDebugFile } from "./read-file.js"; + +const USAGE_HINT = "sentry debug-files print-sources "; + +const log = logger.withTag("debug-files.print-sources"); + +/** A referenced source file augmented with local-disk availability. */ +type PrintSourcesFile = DifSourcesInfo["objects"][number]["files"][number] & { + /** Whether the file exists on the local disk; `null` when embedded or linked. */ + availableLocally: boolean | null; +}; + +/** Per-object referenced sources for the command output. */ +type PrintSourcesObject = { + debugId: string; + fileFormat: string; + /** + * Whether this object carries debug info. Exposed so `--json` consumers can + * apply the same `selectBundledObject` rule `bundle-sources` uses to pick the + * single slice it bundles. + */ + hasDebugInfo: boolean; + /** Error message if enumerating this object's sources failed, else `null`. */ + enumerationError: string | null; + files: PrintSourcesFile[]; +}; + +/** Structured result yielded by the command (and serialized to JSON). */ +type PrintSourcesResult = { + path: string; + objects: PrintSourcesObject[]; +}; + +/** Short, human-readable description of where a referenced source lives. */ +function describeSource(file: PrintSourcesFile): string { + if (file.url !== null) { + return colorTag("muted", `source link: ${file.url}`); + } + if (file.resolved) { + return colorTag("muted", "embedded"); + } + if (file.availableLocally) { + return colorTag("muted", "not embedded, available locally"); + } + return colorTag("muted", "not embedded, not available locally"); +} + +/** Human formatter: one section per object, listing each referenced source. */ +function formatPrintSources(data: PrintSourcesResult): string { + if (data.objects.length === 0) { + return renderMarkdown(colorTag("muted", "No objects found in the file.")); + } + + const sections = data.objects.map((object) => { + const header = `${object.fileFormat} ${safeCodeSpan(object.debugId)}`; + if (object.enumerationError !== null) { + return `${header} — ${colorTag("muted", `could not read sources: ${object.enumerationError}`)}`; + } + if (object.files.length === 0) { + return `${header} — ${colorTag("muted", "no referenced sources")}`; + } + const lines = object.files.map( + (file) => `- ${safeCodeSpan(file.path)} — ${describeSource(file)}` + ); + const count = object.files.length; + return `${header} — ${count} source file${count === 1 ? "" : "s"}:\n\n${lines.join("\n")}`; + }); + + return renderMarkdown(sections.join("\n\n")); +} + +export const printSourcesCommand = buildCommand({ + // Local-only: parses + enumerates in-process, no API calls. + auth: false, + docs: { + brief: "List the source files a debug file references", + fullDescription: + "List the source files referenced by a debug information file. For each " + + "referenced file it reports whether the source is embedded in the file, " + + "available via a source link, or present on the local disk — useful for " + + "checking what `bundle-sources` would include. Supports Mach-O/dSYM, " + + "ELF, PE/PDB, Portable PDB, and Breakpad.\n\n" + + "The format is auto-detected. This command is local-only and makes no " + + "network requests.\n\n" + + "Usage:\n" + + " sentry debug-files print-sources ./libexample.so\n" + + " sentry debug-files print-sources ./app.pdb --json", + }, + output: { + human: formatPrintSources, + }, + parameters: { + positional: { + kind: "tuple", + parameters: [ + { + brief: "Path to the debug information file", + parse: String, + placeholder: "path", + }, + ], + }, + flags: {}, + }, + async *func( + this: SentryContext, + _flags: Record, + path: string + ) { + const content = await readDebugFile(path); + + let info: DifSourcesInfo; + try { + info = listSources(new Uint8Array(content)); + } catch (err) { + const msg = err instanceof Error ? err.message : String(err); + throw new ValidationError( + `'${path}' is not a recognized debug information file: ${msg}`, + "path" + ); + } + + const objects: PrintSourcesObject[] = info.objects.map((object) => ({ + debugId: object.debugId, + fileFormat: object.fileFormat, + hasDebugInfo: object.hasDebugInfo, + enumerationError: object.enumerationError, + files: object.files.map((file) => ({ + ...file, + // Local availability only matters for files that aren't embedded/linked. + availableLocally: file.resolved ? null : existsSync(file.path), + })), + })); + + // Surface objects whose source enumeration failed: their empty file list is + // a degraded result, not a genuine "no sources" — warn so it isn't read as + // the latter (the command still exits zero since the file itself parsed). + const failed = objects.filter((object) => object.enumerationError !== null); + for (const object of failed) { + log.warn( + `Could not enumerate sources for ${object.debugId} in '${path}': ${object.enumerationError}` + ); + } + + // All slices are listed for inspection, but `bundle-sources` only bundles + // one of them. Warn (mirroring `bundle-sources`) so the preview does not + // imply every listed source would end up in the bundle. + const bundled = selectBundledObject(info.objects); + if (info.objects.length > 1 && bundled) { + log.warn( + `'${path}' contains ${info.objects.length} objects; ` + + `\`bundle-sources\` would bundle sources for ${bundled.debugId} only. ` + + "Other slices are not included." + ); + } + + yield new CommandOutput({ path, objects }); + + if (objects.length === 0) { + return { hint: `No objects found in '${path}'. Try: ${USAGE_HINT}` }; + } + const total = objects.reduce((sum, object) => sum + object.files.length, 0); + if (total === 0) { + // If any slice failed to enumerate, "no sources" is not a safe + // conclusion — even when other slices genuinely reference none — so the + // hint must not contradict the per-slice enumeration warnings. + if (failed.length > 0) { + return { + hint: `Could not read referenced sources from '${path}'. Re-run with --log-level=debug for details.`, + }; + } + return { + hint: `No referenced sources found in '${path}'. Try: ${USAGE_HINT}`, + }; + } + return { + hint: `Bundle these into a source bundle with: sentry debug-files bundle-sources ${path}`, + }; + }, +}); diff --git a/src/lib/dif/index.ts b/src/lib/dif/index.ts index a00b5707d..89bf67158 100644 --- a/src/lib/dif/index.ts +++ b/src/lib/dif/index.ts @@ -157,6 +157,24 @@ export function peekFormat(data: Uint8Array): string { return Archive.peek(data) ?? "unknown"; } +/** + * Select the single object that source-bundling operates on: the first object + * that carries debug info, falling back to the first object in the archive. + * + * Fat archives (e.g. universal Mach-O) contain one object per arch slice, but + * {@link createSourceBundle} bundles only one slice. Both the bundler and the + * `print-sources` preview share this rule so they stay consistent about which + * slice is the canonical one. + * + * @param objects - Objects in the archive (in archive order). + * @returns The selected object, or `undefined` if the archive has no objects. + */ +export function selectBundledObject( + objects: readonly T[] +): T | undefined { + return objects.find((object) => object.hasDebugInfo) ?? objects[0]; +} + /** Result of building a source bundle from a debug information file. */ export type SourceBundleResult = { /** The source bundle ZIP bytes, or `null` if the bundle would be empty. */ @@ -178,9 +196,10 @@ export type SourceBundleResult = { * `readSource` to skip a path that isn't available locally. The bundle is built * entirely in memory; nothing is read from disk by this function itself. * - * The bundle is built for the first object that carries debug info (falling back - * to the first object), which matches the single-object debug files this is used - * for; fat archives with multiple debug-info slices are not split here. + * The bundle is built for the single object chosen by {@link selectBundledObject} + * (first object with debug info, falling back to the first object), which matches + * the single-object debug files this is used for; fat archives with multiple + * debug-info slices are not split here. * * @param data - The full contents of the debug information file. * @param objectName - Name stamped on the bundle (typically the input file name). @@ -199,7 +218,7 @@ export function createSourceBundle( const archive = new Archive(data); const objects = archive.objects(); const objectCount = objects.length; - const object = objects.find((o) => o.hasDebugInfo) ?? objects[0]; + const object = selectBundledObject(objects); if (!object) { return { bundle: null, debugId: null, fileCount: 0, objectCount }; } @@ -221,3 +240,107 @@ export function createSourceBundle( writer.writeObject(object, objectName, filter, provider) ?? null; return { bundle, debugId: object.debugId, fileCount, objectCount }; } + +/** A source file referenced by an object, with any resolved descriptor metadata. */ +export type DifSourceFile = { + /** Absolute path recorded in the debug info. */ + path: string; + /** Whether the path resolved to a descriptor (embedded contents or a source link). */ + resolved: boolean; + /** + * The descriptor's source-file type (e.g. `source`, `minified_source`, + * `source_map`), or `null` when the path did not resolve to a descriptor. + */ + type: string | null; + /** Source link URL carried by the descriptor, if any. */ + url: string | null; + /** Debug id associated with the source, if any. */ + debugId: string | null; + /** Source map URL reference, if any. */ + sourceMappingUrl: string | null; +}; + +/** The source files referenced by a single object within a debug file. */ +export type DifObjectSources = { + /** The object's debug identifier. */ + debugId: string; + /** The object file format (e.g. `elf`, `pdb`). */ + fileFormat: string; + /** + * Whether the object carries debug info. Mirrors the slice-selection used by + * {@link createSourceBundle} so callers can preview exactly the object that + * `bundle-sources` would bundle. + */ + hasDebugInfo: boolean; + /** Source files referenced by the object's debug info. */ + files: DifSourceFile[]; + /** + * Error message if opening the object's debug session or enumerating its + * source files failed, otherwise `null`. When set, `files` is empty because + * enumeration aborted — this is distinct from an object that genuinely + * references no sources (where this stays `null`). + */ + enumerationError: string | null; +}; + +/** All objects and the source files they reference, for a debug file. */ +export type DifSourcesInfo = { + objects: DifObjectSources[]; +}; + +/** + * Enumerate the source files referenced by a debug information file. + * + * For each object, opens a debug session and lists every referenced source + * path, resolving each to its descriptor (embedded contents or a source link) + * when available. Nothing is read from the local filesystem here. + * + * @param data - The full contents of the debug information file. + * @returns Per-object lists of referenced source files with descriptor metadata. + * @throws If the buffer cannot be parsed. + */ +export function listSources(data: Uint8Array): DifSourcesInfo { + ensureInitialized(); + const archive = new Archive(data); + const objects = archive.objects().map((object) => { + const files: DifSourceFile[] = []; + let enumerationError: string | null = null; + try { + const session = object.debugSession(); + for (const file of session.files()) { + const path = file.abs_path_str; + const descriptor = session.sourceByPath(path); + // Only cheap descriptor metadata is read here. Reading `contents` + // would copy the full source text across the wasm/JS boundary — and + // re-encode the Rust UTF-8 string to a JS UTF-16 string — for every + // referenced file, which listing references never needs. + files.push({ + path, + resolved: descriptor !== undefined, + type: descriptor?.type ?? null, + url: descriptor?.url ?? null, + debugId: descriptor?.debugId ?? null, + sourceMappingUrl: descriptor?.sourceMappingUrl ?? null, + }); + } + } catch (err) { + // One slice failing to enumerate must not abort the whole listing, but + // the failure is recorded so callers can distinguish it from an object + // that simply references no sources. Discard any entries collected before + // the error so `files` stays empty when enumeration aborted (per the + // DifObjectSources contract) — a partial list paired with an error would + // mislead consumers and be hidden by the human formatter. + files.length = 0; + enumerationError = err instanceof Error ? err.message : String(err); + log.debug(`Failed to enumerate sources for ${object.debugId}`, err); + } + return { + debugId: object.debugId, + fileFormat: object.fileFormat, + hasDebugInfo: object.hasDebugInfo, + files, + enumerationError, + }; + }); + return { objects }; +} diff --git a/test/commands/debug-files/print-sources.test.ts b/test/commands/debug-files/print-sources.test.ts new file mode 100644 index 000000000..1d91e8f8e --- /dev/null +++ b/test/commands/debug-files/print-sources.test.ts @@ -0,0 +1,145 @@ +/** + * Tests for `sentry debug-files print-sources`. + * + * Uses Breakpad symbol files (a deterministic text format) whose `FILE` record + * points at a path under the test's temp dir, so local-availability reporting + * can be exercised without committed binaries. + */ + +import { mkdtemp, rm, writeFile } from "node:fs/promises"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; +import { run } from "@stricli/core"; +import { afterEach, beforeEach, describe, expect, test } from "vitest"; +import { app } from "../../../src/app.js"; +import type { SentryContext } from "../../../src/context.js"; +import { useTestConfigDir } from "../../helpers.js"; + +useTestConfigDir("debug-files-print-sources-"); + +let tempDir: string; + +beforeEach(async () => { + tempDir = await mkdtemp(join(tmpdir(), "df-print-sources-test-")); +}); + +afterEach(async () => { + await rm(tempDir, { recursive: true, force: true }); +}); + +const KNOWN_DEBUG_ID = "0f13a5da-412a-fbf7-c866-2048f3294f3d"; + +/** Build a Breakpad symbol file that references a single source file. */ +function breakpadReferencing(sourcePath: string): string { + return [ + "MODULE Linux x86_64 0F13A5DA412AFBF7C8662048F3294F3D0 example", + "INFO CODE_ID DAA5130F2A41F7FBC8662048F3294F3D439CA7FF", + `FILE 0 ${sourcePath}`, + "FUNC 1000 10 0 main", + "1000 10 42 0", + ].join("\n"); +} + +/** A Breakpad symbol file that references no source files. */ +const BREAKPAD_NO_SOURCES = [ + "MODULE Linux x86_64 0F13A5DA412AFBF7C8662048F3294F3D0 example", + "INFO CODE_ID DAA5130F2A41F7FBC8662048F3294F3D439CA7FF", + "FUNC 1000 10 0 main", + "PUBLIC 2000 0 some_symbol", +].join("\n"); + +/** Run `debug-files print-sources` and capture stdout + exit code. */ +async function runPrintSources( + args: string[] +): Promise<{ output: string; exitCode: number | undefined }> { + let output = ""; + const mockContext: SentryContext = { + process: { ...process, exitCode: undefined } as typeof process, + env: process.env, + cwd: process.cwd(), + homeDir: "/tmp", + configDir: "/tmp", + stdout: { + write(data: string | Uint8Array) { + output += + typeof data === "string" ? data : new TextDecoder().decode(data); + return true; + }, + }, + stderr: { write: () => true }, + stdin: process.stdin, + }; + + await run(app, ["debug-files", "print-sources", ...args], mockContext); + return { output, exitCode: mockContext.process.exitCode }; +} + +describe("sentry debug-files print-sources", () => { + test("lists a referenced source that exists locally", async () => { + const sourcePath = join(tempDir, "example.c"); + await writeFile(sourcePath, "int main(void) { return 0; }\n"); + const debugPath = join(tempDir, "example.sym"); + await writeFile(debugPath, breakpadReferencing(sourcePath)); + + const { output, exitCode } = await runPrintSources([debugPath]); + + expect(exitCode).toBe(0); + expect(output).toContain(KNOWN_DEBUG_ID); + expect(output).toContain(sourcePath); + expect(output).toContain("available locally"); + }); + + test("reports a referenced source that is missing locally", async () => { + const debugPath = join(tempDir, "example.sym"); + await writeFile(debugPath, breakpadReferencing(join(tempDir, "missing.c"))); + + const { output, exitCode } = await runPrintSources([debugPath]); + + expect(exitCode).toBe(0); + expect(output).toContain("not available locally"); + }); + + test("reports when there are no referenced sources", async () => { + const debugPath = join(tempDir, "example.sym"); + await writeFile(debugPath, BREAKPAD_NO_SOURCES); + + const { output, exitCode } = await runPrintSources([debugPath]); + + expect(exitCode).toBe(0); + expect(output.toLowerCase()).toContain("no referenced sources"); + }); + + test("emits structured JSON with --json", async () => { + const sourcePath = join(tempDir, "example.c"); + await writeFile(sourcePath, "int main(void) { return 0; }\n"); + const debugPath = join(tempDir, "example.sym"); + await writeFile(debugPath, breakpadReferencing(sourcePath)); + + const { output } = await runPrintSources([debugPath, "--json"]); + const parsed = JSON.parse(output); + + expect(parsed.objects).toHaveLength(1); + expect(parsed.objects[0].debugId).toBe(KNOWN_DEBUG_ID); + // Exposed so JSON consumers can apply the same bundled-slice rule as + // `bundle-sources` and distinguish a failed read from "no sources". + expect(parsed.objects[0].hasDebugInfo).toBe(true); + expect(parsed.objects[0].enumerationError).toBeNull(); + expect(parsed.objects[0].files[0].path).toBe(sourcePath); + expect(parsed.objects[0].files[0].availableLocally).toBe(true); + }); + + test("fails with a validation error for a path that does not exist", async () => { + const { exitCode } = await runPrintSources([ + join(tempDir, "does-not-exist.sym"), + ]); + expect(exitCode).toBe(21); + }); + + test("fails with a validation error for a non-debug file", async () => { + const path = join(tempDir, "garbage.bin"); + await writeFile(path, "not an object file"); + + const { exitCode } = await runPrintSources([path]); + expect(exitCode).toBe(21); + }); +}); diff --git a/test/lib/dif/index.test.ts b/test/lib/dif/index.test.ts index 77418e26c..c493be2c1 100644 --- a/test/lib/dif/index.test.ts +++ b/test/lib/dif/index.test.ts @@ -9,8 +9,10 @@ import { describe, expect, test } from "vitest"; import { createSourceBundle, + listSources, parseDebugFile, peekFormat, + selectBundledObject, } from "../../../src/lib/dif/index.js"; /** A minimal, valid Breakpad symbol file with a known debug id + code id. */ @@ -151,3 +153,56 @@ describe("createSourceBundle", () => { ).toThrow(); }); }); + +describe("listSources", () => { + test("lists the source files an object references", () => { + const info = listSources(toBytes(BREAKPAD_WITH_SOURCE)); + expect(info.objects).toHaveLength(1); + + const object = info.objects[0]; + expect(object?.debugId).toBe("0f13a5da-412a-fbf7-c866-2048f3294f3d"); + expect(object?.fileFormat).toBe("breakpad"); + expect(object?.hasDebugInfo).toBe(true); + expect(object?.enumerationError).toBeNull(); + expect(object?.files).toHaveLength(1); + + const file = object?.files[0]; + expect(file?.path).toBe("/src/example.c"); + // Breakpad references files but embeds no source content. + expect(file?.resolved).toBe(false); + expect(file?.type).toBeNull(); + }); + + test("returns an empty file list for an object with no referenced sources", () => { + const info = listSources(toBytes(BREAKPAD_FIXTURE)); + expect(info.objects).toHaveLength(1); + expect(info.objects[0]?.files).toHaveLength(0); + }); + + test("throws on unrecognized data", () => { + expect(() => listSources(toBytes("not an object file"))).toThrow(); + }); +}); + +describe("selectBundledObject", () => { + test("prefers the first object that carries debug info", () => { + const objects = [ + { hasDebugInfo: false, id: "a" }, + { hasDebugInfo: true, id: "b" }, + { hasDebugInfo: true, id: "c" }, + ]; + expect(selectBundledObject(objects)?.id).toBe("b"); + }); + + test("falls back to the first object when none carry debug info", () => { + const objects = [ + { hasDebugInfo: false, id: "a" }, + { hasDebugInfo: false, id: "b" }, + ]; + expect(selectBundledObject(objects)?.id).toBe("a"); + }); + + test("returns undefined for an empty archive", () => { + expect(selectBundledObject([])).toBeUndefined(); + }); +});