diff --git a/.craft.yml b/.craft.yml index ffac3629a8..692ccd3bba 100644 --- a/.craft.yml +++ b/.craft.yml @@ -3,8 +3,8 @@ changelog: policy: auto versioning: policy: auto -preReleaseCommand: node --experimental-strip-types script/bump-version.ts --pre -postReleaseCommand: node --experimental-strip-types script/bump-version.ts --post +preReleaseCommand: bash -c 'cd packages/cli && node --experimental-strip-types script/bump-version.ts --pre' +postReleaseCommand: bash -c 'cd packages/cli && node --experimental-strip-types script/bump-version.ts --post' artifactProvider: name: github config: diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index c2a3520547..042d99ec58 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -61,24 +61,24 @@ jobs: with: filters: | skill: - - 'src/**' - - 'docs/**' - - 'package.json' - - 'README.md' - - 'DEVELOPMENT.md' - - 'script/generate-skill.ts' - - 'script/generate-command-docs.ts' - - 'script/generate-docs-sections.ts' - - 'script/eval-skill.ts' - - 'test/skill-eval/**' + - 'packages/cli/src/**' + - 'apps/cli-docs/**' + - 'packages/cli/package.json' + - 'packages/cli/README.md' + - 'packages/cli/DEVELOPMENT.md' + - 'packages/cli/script/generate-skill.ts' + - 'packages/cli/script/generate-command-docs.ts' + - 'packages/cli/script/generate-docs-sections.ts' + - 'packages/cli/script/eval-skill.ts' + - 'packages/cli/test/skill-eval/**' code: - - 'src/**' - - 'test/**' - - 'script/**' - - 'patches/**' - - 'docs/**' - - 'plugins/**' - - 'package.json' + - 'packages/cli/src/**' + - 'packages/cli/test/**' + - 'packages/cli/script/**' + - 'packages/cli/patches/**' + - 'apps/cli-docs/**' + - 'packages/cli/plugins/**' + - 'packages/cli/package.json' - 'pnpm-lock.yaml' - '.github/workflows/ci.yml' codemod: @@ -113,7 +113,7 @@ jobs: if: github.ref == 'refs/heads/main' && github.event_name == 'push' run: | TS=$(date -d "$COMMIT_TIMESTAMP" +%s) - CURRENT=$(jq -r .version package.json) + CURRENT=$(jq -r .version packages/cli/package.json) VERSION=$(echo "$CURRENT" | sed "s/-dev\.[0-9]*$/-dev.${TS}/") echo "version=${VERSION}" >> "$GITHUB_OUTPUT" echo "Nightly version: ${VERSION}" @@ -163,8 +163,11 @@ jobs: - uses: actions/cache@v5 id: cache with: - path: node_modules - key: node-modules-${{ hashFiles('pnpm-lock.yaml', '.npmrc', 'patches/**') }} + path: | + node_modules + packages/*/node_modules + apps/*/node_modules + key: node-modules-${{ hashFiles('pnpm-lock.yaml', '.npmrc', 'packages/cli/patches/**') }} - if: steps.cache.outputs.cache-hit != 'true' run: pnpm install --frozen-lockfile - name: Generate API Schema @@ -176,7 +179,7 @@ jobs: - name: Check skill files id: check-skill run: | - if git diff --quiet plugins/sentry-cli/skills/sentry-cli/; then + if git diff --quiet packages/cli/plugins/sentry-cli/skills/sentry-cli/; then echo "Skill files are up to date" else echo "stale=true" >> "$GITHUB_OUTPUT" @@ -185,7 +188,7 @@ jobs: - name: Check docs sections id: check-sections run: | - if git diff --quiet README.md DEVELOPMENT.md docs/src/content/docs/contributing.md docs/src/content/docs/self-hosted.md docs/src/content/docs/getting-started.mdx; then + if git diff --quiet packages/cli/README.md packages/cli/DEVELOPMENT.md apps/cli-docs/src/content/docs/contributing.md apps/cli-docs/src/content/docs/self-hosted.md apps/cli-docs/src/content/docs/getting-started.mdx; then echo "Docs sections are up to date" else echo "stale=true" >> "$GITHUB_OUTPUT" @@ -196,7 +199,7 @@ jobs: run: | git config user.name "github-actions[bot]" git config user.email "github-actions[bot]@users.noreply.github.com" - git add plugins/sentry-cli/skills/sentry-cli/ README.md DEVELOPMENT.md docs/src/content/docs/contributing.md docs/src/content/docs/self-hosted.md docs/src/content/docs/getting-started.mdx + git add packages/cli/plugins/sentry-cli/skills/sentry-cli/ packages/cli/README.md packages/cli/DEVELOPMENT.md apps/cli-docs/src/content/docs/contributing.md apps/cli-docs/src/content/docs/self-hosted.md apps/cli-docs/src/content/docs/getting-started.mdx git diff --cached --quiet || (git commit -m "chore: regenerate docs" && git push) - name: Fail for fork PRs with stale generated files if: (steps.check-skill.outputs.stale == 'true' || steps.check-sections.outputs.stale == 'true') && steps.token.outcome != 'success' @@ -218,8 +221,11 @@ jobs: - uses: actions/cache@v5 id: cache with: - path: node_modules - key: node-modules-${{ hashFiles('pnpm-lock.yaml', '.npmrc', 'patches/**') }} + path: | + node_modules + packages/*/node_modules + apps/*/node_modules + key: node-modules-${{ hashFiles('pnpm-lock.yaml', '.npmrc', 'packages/cli/patches/**') }} - if: steps.cache.outputs.cache-hit != 'true' run: pnpm install --frozen-lockfile - run: pnpm run generate:schema @@ -249,8 +255,11 @@ jobs: - uses: actions/cache@v5 id: cache with: - path: node_modules - key: node-modules-${{ hashFiles('pnpm-lock.yaml', '.npmrc', 'patches/**') }} + path: | + node_modules + packages/*/node_modules + apps/*/node_modules + key: node-modules-${{ hashFiles('pnpm-lock.yaml', '.npmrc', 'packages/cli/patches/**') }} - if: steps.cache.outputs.cache-hit != 'true' run: pnpm install --frozen-lockfile - name: Generate API Schema @@ -284,8 +293,11 @@ jobs: - uses: actions/cache@v5 id: cache with: - path: node_modules - key: node-modules-${{ matrix.os }}-${{ hashFiles('pnpm-lock.yaml', '.npmrc', 'patches/**') }} + path: | + node_modules + packages/*/node_modules + apps/*/node_modules + key: node-modules-${{ matrix.os }}-${{ hashFiles('pnpm-lock.yaml', '.npmrc', 'packages/cli/patches/**') }} - name: Install dependencies if: steps.cache.outputs.cache-hit != 'true' shell: bash @@ -331,8 +343,8 @@ jobs: if: needs.changes.outputs.nightly-version != '' shell: bash run: | - jq --arg v "${{ needs.changes.outputs.nightly-version }}" '.version = $v' package.json > package.json.tmp - mv package.json.tmp package.json + jq --arg v "${{ needs.changes.outputs.nightly-version }}" '.version = $v' packages/cli/package.json > packages/cli/package.json.tmp + mv packages/cli/package.json.tmp packages/cli/package.json - name: Build env: # Environment-scoped (production) — must be set at step level to @@ -351,9 +363,9 @@ jobs: shell: bash run: | if [[ "${{ matrix.target }}" == "windows-x64" ]]; then - ./dist-bin/sentry-windows-x64.exe --help + ./packages/cli/dist-bin/sentry-windows-x64.exe --help else - ./dist-bin/sentry-${{ matrix.target }} --help + ./packages/cli/dist-bin/sentry-${{ matrix.target }} --help fi - name: Smoke test (deep — SQLite, telemetry, auth DB) if: matrix.can-test @@ -363,9 +375,9 @@ jobs: SENTRY_TOKEN: "" run: | if [[ "${{ matrix.target }}" == "windows-x64" ]]; then - BIN=./dist-bin/sentry-windows-x64.exe + BIN=./packages/cli/dist-bin/sentry-windows-x64.exe else - BIN=./dist-bin/sentry-${{ matrix.target }} + BIN=./packages/cli/dist-bin/sentry-${{ matrix.target }} fi # auth status without a token exercises SQLite init, schema # migrations, telemetry lazy import, and the CJS require chain. @@ -391,7 +403,7 @@ jobs: (github.ref_name == 'main' || startsWith(github.ref_name, 'release/')) shell: bash run: | - BIN=./dist-bin/sentry-${{ matrix.target }} + BIN=./packages/cli/dist-bin/sentry-${{ matrix.target }} echo "Verifying code signature on $BIN..." rcodesign verify "$BIN" - name: Upload binary artifact @@ -399,15 +411,15 @@ jobs: with: name: sentry-${{ matrix.target }} path: | - dist-bin/sentry-* - !dist-bin/*.gz + packages/cli/dist-bin/sentry-* + !packages/cli/dist-bin/*.gz - name: Upload compressed artifact if: github.event_name != 'pull_request' uses: actions/upload-artifact@v7 with: name: sentry-${{ matrix.target }}-gz - path: dist-bin/*.gz + path: packages/cli/dist-bin/*.gz generate-patches: name: Generate Delta Patches @@ -748,8 +760,11 @@ jobs: - uses: actions/cache@v5 id: cache with: - path: node_modules - key: node-modules-${{ hashFiles('pnpm-lock.yaml', '.npmrc', 'patches/**') }} + path: | + node_modules + packages/*/node_modules + apps/*/node_modules + key: node-modules-${{ hashFiles('pnpm-lock.yaml', '.npmrc', 'packages/cli/patches/**') }} - if: steps.cache.outputs.cache-hit != 'true' run: pnpm install --frozen-lockfile - name: Download Linux binary @@ -790,8 +805,11 @@ jobs: - uses: actions/cache@v5 id: cache with: - path: node_modules - key: node-modules-${{ hashFiles('pnpm-lock.yaml', '.npmrc', 'patches/**') }} + path: | + node_modules + packages/*/node_modules + apps/*/node_modules + key: node-modules-${{ hashFiles('pnpm-lock.yaml', '.npmrc', 'packages/cli/patches/**') }} - if: steps.cache.outputs.cache-hit != 'true' run: pnpm install --frozen-lockfile - name: Bundle @@ -800,7 +818,7 @@ jobs: SENTRY_AUTH_TOKEN: ${{ secrets.SENTRY_AUTH_TOKEN }} run: pnpm run bundle - name: Smoke test (Node.js) - run: node dist/bin.cjs --help + run: node packages/cli/dist/bin.cjs --help - name: Smoke test (Node.js — deep) shell: bash env: @@ -810,7 +828,7 @@ jobs: # auth status without a token exercises SQLite init, schema # migrations, telemetry lazy import, and the CJS require chain. # Expected: exit 10 (AUTH_NOT_AUTHENTICATED), NOT a crash/syntax error. - OUTPUT=$(node dist/bin.cjs auth status 2>&1) && EXIT_CODE=$? || EXIT_CODE=$? + OUTPUT=$(node packages/cli/dist/bin.cjs auth status 2>&1) && EXIT_CODE=$? || EXIT_CODE=$? if [[ $EXIT_CODE -ne 10 ]]; then echo "::error::Expected exit code 10 (not authenticated), got $EXIT_CODE" echo "$OUTPUT" @@ -822,12 +840,13 @@ jobs: exit 1 fi - run: npm pack + working-directory: packages/cli - name: Upload artifact if: matrix.node == '22' uses: actions/upload-artifact@v7 with: name: npm-package - path: "*.tgz" + path: "packages/cli/*.tgz" build-docs: name: Build Docs @@ -853,13 +872,16 @@ jobs: - uses: actions/cache@v5 id: cache with: - path: node_modules - key: node-modules-${{ hashFiles('pnpm-lock.yaml', '.npmrc', 'patches/**') }} + path: | + node_modules + packages/*/node_modules + apps/*/node_modules + key: node-modules-${{ hashFiles('pnpm-lock.yaml', '.npmrc', 'packages/cli/patches/**') }} - if: steps.cache.outputs.cache-hit != 'true' run: pnpm install --frozen-lockfile - name: Get CLI version id: version - run: echo "version=$(node -p 'require("./package.json").version')" >> "$GITHUB_OUTPUT" + run: echo "version=$(node -p 'require("./packages/cli/package.json").version')" >> "$GITHUB_OUTPUT" - name: Download compiled CLI binary uses: actions/download-artifact@v8 with: @@ -870,7 +892,7 @@ jobs: - name: Generate docs content run: pnpm run generate:schema && pnpm run generate:docs - name: Build Docs - working-directory: docs + working-directory: apps/cli-docs env: PUBLIC_SENTRY_ENVIRONMENT: production SENTRY_RELEASE: ${{ steps.version.outputs.version }} @@ -887,18 +909,18 @@ jobs: SENTRY_ORG: sentry SENTRY_PROJECT: cli-website run: | - ./dist-bin/sentry-linux-x64 sourcemap inject docs/dist/ - ./dist-bin/sentry-linux-x64 sourcemap upload docs/dist/ \ + ./dist-bin/sentry-linux-x64 sourcemap inject apps/cli-docs/dist/ + ./dist-bin/sentry-linux-x64 sourcemap upload apps/cli-docs/dist/ \ --release "${{ steps.version.outputs.version }}" \ --url-prefix "~/" # Remove .map files — they were uploaded to Sentry but shouldn't # be deployed to production. - name: Remove sourcemaps from output - run: find docs/dist -name '*.map' -delete + run: find apps/cli-docs/dist -name '*.map' -delete - name: Package Docs run: | - cp .nojekyll docs/dist/ - cd docs/dist && zip -r ../../gh-pages.zip . + cp .nojekyll apps/cli-docs/dist/ + cd apps/cli-docs/dist && zip -r "$GITHUB_WORKSPACE/gh-pages.zip" . - name: Upload docs artifact uses: actions/upload-artifact@v7 with: diff --git a/.github/workflows/docs-preview.yml b/.github/workflows/docs-preview.yml index 5df7f0cb36..cf8313474d 100644 --- a/.github/workflows/docs-preview.yml +++ b/.github/workflows/docs-preview.yml @@ -4,11 +4,11 @@ on: push: branches: [main] paths: - - 'docs/**' - - 'src/**' - - 'script/generate-command-docs.ts' - - 'script/generate-skill.ts' - - 'install' + - 'apps/cli-docs/**' + - 'packages/cli/src/**' + - 'packages/cli/script/generate-command-docs.ts' + - 'packages/cli/script/generate-skill.ts' + - 'packages/cli/install' - '.github/workflows/docs-preview.yml' pull_request: # No paths filter here: the 'closed' event must ALWAYS fire so @@ -48,11 +48,11 @@ jobs: with: filters: | docs: - - 'docs/**' - - 'src/**' - - 'script/generate-command-docs.ts' - - 'script/generate-skill.ts' - - 'install' + - 'apps/cli-docs/**' + - 'packages/cli/src/**' + - 'packages/cli/script/generate-command-docs.ts' + - 'packages/cli/script/generate-skill.ts' + - 'packages/cli/install' - '.github/workflows/docs-preview.yml' # Central gate. `build` = produce the site (push, or a docs PR being @@ -93,8 +93,11 @@ jobs: id: cache if: steps.gate.outputs.build == 'true' with: - path: node_modules - key: node-modules-${{ hashFiles('pnpm-lock.yaml', '.npmrc', 'patches/**') }} + path: | + node_modules + packages/*/node_modules + apps/*/node_modules + key: node-modules-${{ hashFiles('pnpm-lock.yaml', '.npmrc', 'packages/cli/patches/**') }} - if: steps.gate.outputs.build == 'true' && steps.cache.outputs.cache-hit != 'true' run: pnpm install --frozen-lockfile @@ -102,7 +105,7 @@ jobs: - name: Get CLI version id: version if: steps.gate.outputs.build == 'true' - run: echo "version=$(node -p 'require("./package.json").version')" >> "$GITHUB_OUTPUT" + run: echo "version=$(node -p 'require("./packages/cli/package.json").version')" >> "$GITHUB_OUTPUT" - name: Generate docs content if: steps.gate.outputs.build == 'true' @@ -110,7 +113,7 @@ jobs: - name: Build Docs for Preview if: steps.gate.outputs.build == 'true' - working-directory: docs + working-directory: apps/cli-docs env: DOCS_BASE_PATH: ${{ github.event_name == 'push' && '/_preview/pr-main' @@ -129,15 +132,15 @@ jobs: SENTRY_ORG: sentry SENTRY_PROJECT: cli-website run: | - pnpm run cli sourcemap inject docs/dist/ - pnpm run cli sourcemap upload docs/dist/ \ + pnpm run cli sourcemap inject "$GITHUB_WORKSPACE/apps/cli-docs/dist/" + pnpm run cli sourcemap upload "$GITHUB_WORKSPACE/apps/cli-docs/dist/" \ --release "${{ steps.version.outputs.version }}" \ --url-prefix "~/" # Remove .map files — uploaded to Sentry but shouldn't be deployed. - name: Remove sourcemaps from output if: steps.gate.outputs.build == 'true' - run: find docs/dist -name '*.map' -delete + run: find apps/cli-docs/dist -name '*.map' -delete - name: Ensure .nojekyll at gh-pages root # Runs whenever we deploy (build or a 'closed' cleanup), but never for @@ -180,7 +183,7 @@ jobs: if: steps.gate.outputs.deploy == 'true' uses: rossjrw/pr-preview-action@v1 with: - source-dir: docs/dist/ + source-dir: apps/cli-docs/dist/ preview-branch: gh-pages umbrella-dir: _preview pages-base-url: cli.sentry.dev diff --git a/.github/workflows/eval-skill-fork.yml b/.github/workflows/eval-skill-fork.yml index 7edffa694c..60718b94c5 100644 --- a/.github/workflows/eval-skill-fork.yml +++ b/.github/workflows/eval-skill-fork.yml @@ -54,8 +54,11 @@ jobs: - uses: actions/cache@v5 id: cache with: - path: node_modules - key: node-modules-${{ hashFiles('pnpm-lock.yaml', '.npmrc', 'patches/**') }} + path: | + node_modules + packages/*/node_modules + apps/*/node_modules + key: node-modules-${{ hashFiles('pnpm-lock.yaml', '.npmrc', 'packages/cli/patches/**') }} - if: steps.cache.outputs.cache-hit != 'true' run: pnpm install --frozen-lockfile diff --git a/.gitignore b/.gitignore index 858ee3ec52..3f638f09d4 100644 --- a/.gitignore +++ b/.gitignore @@ -43,10 +43,10 @@ report.[0-9]_.[0-9]_.[0-9]_.[0-9]_.json .turbo .mastra -# docs -docs/dist -docs/node_modules -docs/.astro +# docs (now under apps/cli-docs) +apps/cli-docs/dist +apps/cli-docs/node_modules +apps/cli-docs/.astro # local planning notes (not for version control) .plans @@ -58,20 +58,20 @@ docs/.astro .DS_Store # Generated files (rebuilt at build time) -src/generated/ -src/sdk.generated.ts -src/sdk.generated.d.cts +packages/cli/src/generated/ +packages/cli/src/sdk.generated.ts +packages/cli/src/sdk.generated.d.cts # ...except the sixel banner: it's small, deterministic (baked from the committed # assets/banner-mask.png), and statically imported by src/lib/sixel.ts, so it must # exist on a fresh checkout before any build step regenerates it. -!src/generated/banner-sixel.ts +!packages/cli/src/generated/banner-sixel.ts # Generated docs pages (rebuilt from fragments + CLI introspection / env registry) -docs/src/content/docs/commands/ -docs/src/content/docs/configuration.md +apps/cli-docs/src/content/docs/commands/ +apps/cli-docs/src/content/docs/configuration.md # Generated discovery manifest (rebuilt by generate:skill, served via symlinked skill files) -docs/public/.well-known/skills/index.json +apps/cli-docs/public/.well-known/skills/index.json # OpenCode .opencode/ @@ -79,7 +79,7 @@ opencode.json* # Throwaway documentation audit reports (bot-generated, not real docs) DOCUMENTATION_GAPS.md -docs/DOCUMENTATION_GAPS.md +apps/cli-docs/DOCUMENTATION_GAPS.md DOCUMENTATION_GAP_REPORT.md # Throwaway planning / migration notes (agent- or human-authored scratch docs, diff --git a/.lore.md b/.lore.md index be3af6a137..4ca8695381 100644 --- a/.lore.md +++ b/.lore.md @@ -108,12 +108,18 @@ * **bspatch.ts: no mmap — Node has no native mmap API and native addons break SEA bundling**: True mmap not viable: Node core has no \`fs.mmap\` (Buffer is always heap-backed); native addons (\`mmap-io\` etc.) break two hard project rules — no runtime dependencies and must bundle into Node SEA binary (esbuild can't bundle \`.node\` addons). \`Bun.mmap\` would have worked but project migrated off Bun. Alternative \`pread\` via \`fs.read(fd, buf, 0, len, pos)\` saves ~100MB JS heap for base binary (bytes stay in OS page cache) but cannot help intermediate hops — those buffers must stay in memory for in-memory chaining. Also: one big sequential \`readFile\` is faster than many small \`pread\`s. pread deferred as optional future optimization for single-patch case only. + +* **Chose Cloudflare for scalable website deployment over Vercel**: User prefers using Cloudflare for scalable website deployment over Vercel. + * **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. + +* **Merge CLI and MCP repositories**: Chose to merge CLI and MCP repositories into a new repository named \`toolkit\`. + * **Migrated to node**: migrated from Bun to Node. @@ -126,23 +132,17 @@ * **Raw markdown output for non-interactive terminals, rendered for TTY**: Markdown-first output pipeline: custom renderer in \`src/lib/formatters/markdown.ts\` walks \`marked\` tokens to produce ANSI-styled output. Commands build CommonMark using helpers (\`mdKvTable()\`, \`mdRow()\`, \`colorTag()\`, \`escapeMarkdownCell()\`, \`safeCodeSpan()\`) and pass through \`renderMarkdown()\`. \`isPlainOutput()\` precedence: \`SENTRY\_PLAIN\_OUTPUT\` > \`NO\_COLOR\` > \`FORCE\_COLOR\` > \`!isTTY\`. \`--json\` always outputs JSON. Colors defined in \`COLORS\` object in \`colors.ts\`. Tests run non-TTY so assertions match raw CommonMark; use \`stripAnsi()\` helper for rendered-mode assertions. - -* **sentry-cli v3→v4 codemod: replaced jscodeshift with jssg/codemod.com registry package**: Chose jssg (codemod.com engine) over jscodeshift for the v3→v4 migration codemod. jscodeshift's large loosely-typed AST API caused LLM-invented method failures; jssg's small direct API (\`find\`, \`findAll\`, \`replace\`, \`text()\`) reduces that failure mode. Package lives at \`codemods/sentry-v3-to-v4/\` (isolated from root typecheck/biome/check-no-deps). Old \`codemods/sentry-v3-to-v4.cjs\` and \`test/codemods/sentry-v3-to-v4.test.ts\` deleted. Migration guide updated to reference \`npx codemod @sentry/cli-v3-to-v4\`. biome.jsonc force-ignore entry for the deleted \`.cjs\` file also removed. - * **Sixel banner approach deferred — block-art 56×8 wordmark chosen for now**: User explicitly chose the 56×8 quadrant block-art wordmark over a sixel-based approach: 'Love the 56x8 let's roll with that. We can revisit sixel later.' Sixel is deferred as a future follow-up, not rejected permanently. + +* **Switched from Python to TypeScript for the project (no longer using Python)**: switched from Python to TypeScript for the project (no longer using Python) + * **symbolic-wasm API scope: general-purpose base, not CLI-specific shortcuts**: Agreed with Dav1dde (getsentry/symbolic maintainer): \`symbolic-wasm\` may live in the symbolic repo only if it exposes a full general-purpose API base (analogous to the Python package), not CLI-focused shortcuts. CLI-specific logic (e.g. \`collect\_il2cpp\` orchestration, source bundle writing with CLI semantics) must NOT live in the symbolic repo — move to getsentry/cli. Rationale: symbolic is a general-purpose library; CLI concerns create unwanted coupling. PR C (\`feat/wasm-api-classes\`): class-based API — \`Archive\` owns \`Rc\>\`, caches metadata; \`Object\` caches fields at construction, re-reads debug session on demand for \`source\_files()\`/\`create\_source\_bundle()\`. Callback-based wasm API uses \`js\_sys::Function\` (getSource(path) → Uint8Array | null). Free functions \`list\_source\_files\`/\`create\_source\_bundle\` removed; \`parse\_debug\_file\`/\`peek\_format\` kept for back-compat. ### 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")]\` and \`#\[wasm\_bindgen(js\_class = "ObjectFile")]\` — Rust struct name \`Object\` unchanged. Discovered via packaged-artifact smoke test, not \`wasm-pack test\`. Similarly, \`ObjectDebugSession\` needed \`AsSelf\` impl (PR #997) — check all new wasm-exposed types for missing \`AsSelf\` impls before using \`derived\_from\_cell!\`. - * **@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. @@ -150,10 +150,7 @@ * **\`--require-all\` false negatives for fat binaries — scan all matched objects**: Trap: \`missingRequestedIds\` computed \`foundIds\` from only the primary object's \`debugId\` per file. Fat Mach-O with multiple slices causes non-primary slice IDs to be reported missing and exits 1 even though they were found. Fix: compute \`foundIds\` from all matched objects (\`.objects\` array), not just \`selectBundledObject()?.debugId\`. Applies to \`src/commands/debug-files/upload.ts\`. -* **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\`). - - -* **Atomic-write test tautology: 'no temp files' test passes even without atomic behavior**: Trap: a test that checks 'no \`.tmp\` files left behind after install' looks like it guards atomic write behavior. But if the non-atomic path (\`writeFile\` directly) never creates \`.tmp\` files in the first place, the \`leftovers\` filter is trivially empty and the test passes with the atomic code removed entirely — confirmed via mutation test on \`byk/fix/agent-skills-atomic-write\`. Fix: the test must exercise the overwrite/update path (where a concurrent reader could observe truncation) AND inject a fault (e.g., mock \`rename\` to throw) to verify cleanup. A test that only runs the happy-path fresh-install never touches the race condition the feature guards against. Per red-green directive: if the test passes with the guard deleted, it isn't testing the guard. +* **acquireLock ENOENT: parent directory may not exist — must mkdir before writeFileSync**: acquireLock ENOENT: parent directory may not exist — must mkdir before writeFileSync * **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. @@ -173,39 +170,18 @@ * **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. - * **check.ts hasId() uses !== null but ObjectFile.codeId is string|undefined — null guard mismatch**: Trap: \`check.ts\` \`hasId()\` guards \`o.codeId !== null && o.codeId.length > 0\`. Looks correct because old \`DifObjectInfo.codeId\` was \`string | null\`. But \`ObjectFile.codeId\` in \`@sentry/symbolic\` 13.4.0 is \`string | undefined\` — the getter returns \`undefined\` (not \`null\`) when Rust returns \`None\`. If \`DifObjectInfo.codeId\` is mapped as \`string | null\` (via \`?? null\`), the \`!== null\` guard works. If mapped as \`string | undefined\` (via \`?? undefined\`), the guard silently passes \`undefined\` through. Fix: ensure \`parseDebugFile\` maps \`codeId\` to \`string | null\` (using \`obj.codeId ?? null\`) so \`check.ts\`'s existing \`!== null\` guard remains correct without touching \`check.ts\`. - -* **chunk-upload gzip wire format: never emit Content-Encoding: gzip with file\_gzip field**: Trap: gzip-compressed chunks look like they should use \`Content-Encoding: gzip\` alongside the \`file\_gzip\` multipart field — standard HTTP compression convention. But zstd-aware Sentry servers reject that combination with 400 to avoid ambiguity. Fix: gzip path uses ONLY the \`file\_gzip\` multipart field with NO \`Content-Encoding\` header (legacy protocol). Zstd path uses \`Content-Encoding: zstd\` + \`file\` field (requires server opt-in via getsentry/sentry#113760+). This is a standing directive from code comments in \`src/lib/api/chunk-upload.ts\`. - * **ci.yml NODE\_VERSION pins must be exact patch — floating major silently reuses buggy cached patch**: Trap: using a floating major version like \`22.x\` in ci.yml looks safe and auto-updates. But GitHub Actions caches the resolved binary — a CVE fix (e.g. CVE-2026-48931 requiring 22.23.1/24.18.0) won't be picked up until the cache expires, silently running the vulnerable patch. Fix: always pin exact patch versions in ci.yml env vars (\`NODE\_VERSION\_22: "22.23.1"\`, \`NODE\_VERSION\_24: "24.18.0"\`). Update pins explicitly when a CVE fix requires a new patch. - -* **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. - - -* **debug-files upload: partial size-drop silently exits 0 — doUpload must receive oversizedCount**: Trap: \`doNothingToUpload\` (all-dropped path) correctly calls \`setExitCode(1)\` when \`oversizedCount > 0\`, establishing the contract that oversized files → non-zero exit. But \`doUpload\` (partial-drop path) did not receive \`oversizedCount\`/\`maxFileSize\` — so uploading 3 files where 1 was oversized exited 0 silently. Fix (PR #1146): pass \`oversizedCount\` and \`maxFileSize\` to \`doUpload\`; append scan-oversize message to failures hint; call \`setExitCode(1)\` when \`oversizedCount > 0\` even with no upload failures. Warden finding 9LL-87A on PR #1140. - - -* **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. +* **DEVELOPMENT.md hand-written prose is not covered by any staleness check**: DEVELOPMENT.md hand-written prose is not covered by any staleness check * **difFromCandidateBuffer: PE entries must be allowed through format filter when portablepdb is requested**: Trap: the format filter in \`difFromCandidateBuffer\` rejects PE files when \`--type portablepdb\` is specified, because \`peekFormat\` returns \`'pe'\` not \`'portablepdb'\`. But embedded PPDBs live inside PE files — blocking the PE at the format-filter stage means the PPDB is never extracted. Fix: when \`filters.formats\` includes \`'portablepdb'\`, also allow \`'pe'\` through the format gate so the parser can inspect the PE for an embedded PPDB. The PE itself is still dropped after extraction if it has no features. - -* **embeddedPpdbDif: oversized extracted PPDB dropped silently — oversizedCount not incremented**: Trap: \`embeddedPpdbDif()\` (scan.ts:628-633) returns \`null\` when the decompressed PPDB exceeds \`maxFileSize\`, but does NOT increment \`oversizedCount\`. Looks correct because the PPDB is a derived artifact, not a scanned file. But the user gets no signal that a PPDB was found and dropped — silent data loss. The existing directive (PR #1146) requires \`oversizedCount > 0\` → non-zero exit and scan-oversize message. Fix: increment \`oversizedCount\` (or return a sentinel) when an extracted PPDB is size-gated out, so the caller can report it. - - -* **error-reporting.ts: three independent 4xx classifiers must stay in sync — isUserError, classifySilenced, isUserApiError**: Trap: \`classifySilenced\` (Sentry capture gate), \`isUserError\` in \`errors.ts\` (upgrade nudge via \`getErrorUpdateNotification\`), and \`isUserApiError\` in \`telemetry.ts\` (span attribution) all classify 4xx errors independently. Adding a new silence rule to only one leaves the others out of sync — e.g., query-parse 400s were silenced in \`classifySilenced\` but still triggered the upgrade nudge until \`isUserError\` was updated. Fix: whenever a new error classification is added (e.g., \`isSearchQueryParseError\`, \`isNetworkError\`), update all three classifiers. Export shared predicates from \`errors.ts\` so all three import the same function. - * **existsSync+realpathSync TOCTOU: catch ENOENT instead**: Trap: \`if (!existsSync(p)) return resolve(p); return realpathSync(p)\` looks safe but has a TOCTOU race. Also: \`realpathSync\` inside async is inconsistent. Fix: call \`await realpath(p)\` (node:fs/promises) directly; catch \`ENOENT\` to fall back to \`resolve(p)\`; log non-ENOENT errors via \`logger.debug(msg, error)\` before falling back. When mocking in vitest, mock \`node:fs/promises\` not \`node:fs\`. RELATED: In cleanup/unlink catch blocks, only log non-ENOENT errors — \`ENOENT\` during cleanup is expected. Pattern: \`if ((error as NodeJS.ErrnoException).code !== 'ENOENT') logger.debug(msg, error)\`. Pre-existing silent \`catch { // Ignore }\` blocks must be fixed to log non-ENOENT errors. Confirmed fixed in PR #1046 (\`fix/install-binary-symlink-self-copy\`). @@ -213,7 +189,7 @@ * **Frontify brand portal cannot be accessed programmatically — requires auth**: Trap: \`https://brand.getsentry.com/share/wLssCFiQ5ZzmQmKCWym4\` returns HTTP 200 and looks like a static asset server. It's a JS-rendered SPA — no static asset URLs are extractable from HTML. API probes (\`/api/share/…\`, \`/api/shares/…\`) return 404 or HTML. CDN URLs found embedded in the SPA shell (\`media.ffycdn.net\`) are portal chrome assets (OG illustration, favicon), not the actual brand files. Fix: use the pre-signed download endpoint \`https://brand.getsentry.com/api/screen/download/\\` — token must be extracted from the authenticated session or provided by the user. Always ask the user to provide Frontify asset URLs or download tokens directly. -* **generate-docs-sections.ts still references Bun — extractBunVersion() silently falls back to hardcoded '1.3'**: RESOLVED in PR #1024. \`generate-docs-sections.ts\` previously had \`BUN\_VERSION\_RE\` and \`extractBunVersion()\` that silently returned hardcoded \`'1.3'\` when \`packageManager\` was \`pnpm@10.11.0\`. Fixed: replaced with \`extractPnpmVersion()\` and \`extractNodeVersion()\` that \*\*throw on mismatch\*\* instead of silently falling back. \`generateDevPrereq()\`, \`generateDevPrereqContributing()\`, \`generateLibraryPrereq()\` now reference Node.js + pnpm. \`DEVELOPMENT.md\` lines 5 and 91 are now wrapped in \`GENERATED:START/END\` markers so they can't drift again. No Bun references remain in the script. +* **generate-docs-sections.ts still references Bun — extractBunVersion() silently falls back to hardcoded '1.3'**: generate-docs-sections.ts still references Bun — extractBunVersion() silently falls back to hardcoded '1.3' * **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. @@ -224,9 +200,6 @@ * **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. - -* **isNetworkError must NOT match ApiError(status=0) — TLS cert errors share status 0**: Trap: \`ApiError\` with \`status === 0\` looks like a network failure (no HTTP response received) and is tempting to include in \`isNetworkError()\`. But TLS certificate errors are also wrapped as \`ApiError(status=0)\` — once wrapped, \`isTlsCertError()\` cannot reliably re-detect them. Silencing all status-0 errors would suppress actionable TLS config issues. Fix: \`isNetworkError()\` matches ONLY \`error instanceof TypeError && error.message === 'fetch failed'\` (the unambiguous undici network signature). \`ApiError(status=0)\` is handled separately via an explicit \`error.status === 0\` branch in \`isUserError()\` with a comment explaining the TLS overlap. Callers that want status-0 treated as user-env check it explicitly. - * **issue list --sort recommended: client-side re-sort must be skipped for single-project results**: Trap: applying \`getComparator('recommended')\` to single-project results looks like consistent behavior. Fix: the \`isMultiProject\` guard at list.ts:1144-1148 is the ONLY client-side issue sort — it's intentionally skipped for single-project results because re-sorting would silently replace the server's recommended ranking with a \`lastSeen\` fallback. \`getComparator('recommended')\` returns \`compareDates(a.lastSeen, b.lastSeen)\` — same as \`date\` sort — so applying it to single-project results would corrupt the server's relevance ranking without any visible error. @@ -267,7 +240,7 @@ * **parseWithHash short-circuits before the main validateResourceId guard — must self-validate (CLI-1G1)**: GitHub-style \`org/project#SHORTID\` issue identifiers handled by \`parseWithHash()\` in \`src/lib/arg-parsing.ts\`, inserted in \`parseIssueArg\` AFTER the \`@\`-selector block and BEFORE the \`validateResourceId(input.replace(/\\//g,''))\` guard (line ~1115, which rejects \`#\`). Because it runs before that guard, \`parseWithHash\` MUST validate BOTH the project prefix AND the fragment itself. \`validateResourceId\` permits \`:\`, so \`:\` mixed with \`#\` is rejected explicitly. Semantics: \`org/project#ID\` → delegates to \`parseWithSlash('org/project/ID')\`; \`project#ID\` → \`project-search\` via \`parseProjectIdentifier\`; \`#ID\` → bare identifier via \`parseBareIssueIdentifier\`. \`parseProjectIdentifier\` is shared with \`parseWithColon\`. BEHAVIORAL CHANGE: \`CLI-G#anchor\` went from \`ValidationError\` → \`project-search{projectSlug:'cli-g', suffix:'ANCHOR'}\`. Test at \`arg-parsing.test.ts\` injection-hardening block updated accordingly. -* **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 nested script invocation loses TTY — inline tsx to fix**: Trap: 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. Fix: inline tsx directly — \`"cli": "tsx --import ./script/require-shim.mjs src/bin.ts"\` and same for \`dev\`. * **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). @@ -282,14 +255,11 @@ * **prepareZipDifs oversizedCount must NOT feed exit-driving counter — format unknown pre-decompression**: Trap: zip entry oversized warnings look like they should increment \`oversizedCount\` in \`prepareDifs\`, just like on-disk file oversize does — both are 'too large' signals. But a compressed entry's format is unknown until inflated, so it can't be attributed to the requested \`--type\`. Counting it would turn an unrelated oversized asset inside a \`.zip\` into a false 'all matched files too large' failure and wrong exit code. Fix: \`prepareZipDifs\` returns \`PreparedDif\[]\` (not \`{ prepared, oversizedCount }\`); oversized zip entries warn per-entry inside \`readZipDifEntries\` only. The \`oversizedCount\` counter in \`prepareDifs\` is exclusively for format-accurate on-disk files. -* **print-sources must never claim to preview bundle-sources while listing sources bundle-sources would skip**: Trap: \`print-sources\` iterates ALL objects in an archive (full inspection value), while \`bundle-sources\` only bundles the first object with debug info (\`selectBundledObject\`). Listing all objects then showing a \`bundle-sources\` hint looks like a preview of that command — but it isn't. Fix: add multi-object warning naming the exact slice that would be bundled (via \`selectBundledObject\`), add \`hasDebugInfo\` to JSON output so consumers can apply the same selection rule, and ensure footer hint distinguishes no-objects / enumeration-failure / no-sources. 🔴 Directive: never claim to preview \`bundle-sources\` while collecting sources \`bundle-sources\` would never collect. +* **print-sources must never claim to preview bundle-sources while listing sources bundle-sources would skip**: Trap: \`print-sources\` iterates ALL objects in an archive (full inspection value), while \`bundle-sources\` only bundles the first object with debug info (\`selectBundledObject\`). Listing all objects then showing a \`bundle-sources\` hint looks like a preview of that command — but it isn't. Fix: add multi-object warning naming the exact slice that would be bundled (via \`selectBundledObject\`), add \`hasDebugInfo\` to JSON output so consumers can apply the same selection rule, and ensure footer hint distinguishes no-objects / enumeration-failure / no-sources. * **ruzstd partial decompression: must validate output size explicitly**: Trap: \`ruzstd::StreamingDecoder\` (unlike \`zstd::bulk::decompress\`) silently returns a partial result when passed a too-small \`size\` — it does NOT error. Fix: read \`size + 1\` bytes into the output buffer, then assert \`decompressed.len() == size\`; return \`None\` on mismatch. This matches \`zstd::bulk::decompress\` error-on-mismatch semantics. Confirmed via test: exact(560)→Some(560)✓, toosmall(550)→None✓, toolarge(570)→None✓. - -* **scan.ts: oversized PE early-return must not skip embedded PPDB extraction**: Trap: in \`prepareFileDif\` (src/lib/dif/scan.ts), gating on a PE's on-disk size vs \`maxFileSize\` looks like a safe early-exit optimization — but it isn't PPDB-aware. A large assembly can contain a small embedded .pdb well within the limit, yet the pre-fix code returned before \`embeddedPpdbDif\` ever ran, silently dropping the extractable PDB (Bugbot finding, PR #1163, bug 47). Fix: \`embeddedPpdbDif\` cheap-peeks only the first \`PEEK\_HEADER\_BYTES\` (4096) via \`peekFormat()\` to confirm PE format before doing a second full \`Archive\` parse, so oversized-container gating never blocks embedded-content extraction. The oversized signal instead threads end-to-end: \`embeddedPpdbDif\` → \`difFromBuffer\` → \`readMatchedDif\` → \`prepareFileDif\` → \`prepareDifs\` (\`oversizedCount+=1\`), and is only counted when \`portablepdb\` format is actually requested via \`formatMatches\`. - * **sentry-cli SKILL.md has non-standard frontmatter fields (version, requires) — safe for OpenCode but worth knowing**: SKILL.md frontmatter includes \`version\` and \`requires: {bins: \["sentry"], auth: true}\` — fields not in the OpenCode skill spec. OpenCode's \`isSkillFrontmatter()\` only validates \`name: string\` and optional \`description: string\`; extra fields are silently ignored. gray-matter (js-yaml) parses the nested \`requires\` object without error. Trap: nested objects in frontmatter look like they'd cause a YAML parse failure or schema rejection. They don't — confirmed via gray-matter test and zero "failed to load skill" log entries. The skill loads correctly; absence from a session is always a stale-snapshot issue, not a parse issue. @@ -306,7 +276,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**: 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). +* **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 * **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. @@ -327,7 +297,7 @@ * **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 handles (Archive/ObjectFile/PeFile) must be freed — new DIF functions multiply live handles**: Trap: \`extractEmbeddedPpdb\` and \`createIl2cppLineMapping\` create \`Archive\`/\`ObjectFile\`/\`PeFile\` instances (all expose \`free()\`/\`\[Symbol.dispose]\()\`) but never release them. PRs #1163/#1164 multiply live handles per PE up to ~4 \`Archive\`s (\`matchedDif\`, \`extractEmbeddedPpdb\`, \`createIl2cppLineMapping\`, \`createSourceBundle\`). Looks safe because JS GC eventually collects — but WASM linear memory is not GC'd; handles accumulate until process exit. Fix: use \`using archive = new Archive(...)\` (explicit resource management) in all new functions that create WASM objects. +* **WASM handles (Archive/ObjectFile/PeFile) must be freed — new DIF functions multiply live handles**: Gotcha: scan.ts: oversized PE early-return must not skip embedded PPDB extraction — Trap: in \`prepareFileDif\` (src/lib/dif/scan.ts), gating on a PE's on-disk size vs \`maxFileSize\` looks like a safe early-exit optimization — but it isn't PPDB-aware. A large assembly can contain a small embedded .pdb well within the limit, yet the pre-fix code returned before \`embeddedPpdbDif\` ever ran, silently dropping the extractable PDB. Fix: \`embeddedPpdbDif\` cheap-peeks only the first \`PEEK\_HEADER\_BYTES\` (4096) via \`peekFormat()\` to confirm PE format before doing a second full \`Archive\` parse, so oversized-container gating never blocks embedded-content extraction. * **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. @@ -361,9 +331,6 @@ * **atomicWriteFile in agent-skills.ts: same-dir temp + rename guarantees no partial reads**: \`atomicWriteFile(destPath, content)\` at \`src/lib/agent-skills.ts:72\`: writes to \`.\.\.\.tmp\` in the same directory as \`destPath\`, then calls \`rename()\` into place. Same-directory placement guarantees same filesystem → POSIX atomic rename. Concurrent readers never observe a truncated or partially-written file. Temp file is cleaned up on error. Used by \`writeSkillFiles()\` (replacing in-place \`writeFile\`). Skills are written on every version — write-if-changed optimization was explicitly rejected as unnecessary. - -* **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. - * **CI Node version pinning: centralized env block per workflow file, ternary for matrix jobs**: Node CVE-2026-48931 fix: \`NODE\_VERSION\_22="22.23.1"\`, \`NODE\_VERSION\_24="24.18.0"\` (22.23.0 had the vulnerability; fix landed in 22.23.1 via nodejs/node#64004). Pattern: add top-level \`env:\` block to each workflow file (ci.yml, release.yml, sentry-release.yml, docs-preview.yml) with both constants + rationale comment. Reference via \`${{ env.NODE\_VERSION\_22 }}\`. Matrix jobs (build-npm) use ternary: \`${{ matrix.node == '24' && env.NODE\_VERSION\_24 || env.NODE\_VERSION\_22 }}\` — matrix labels stay as bare majors (\`\["22","24"]\`) for job naming. Gotcha: \`eval-skill-fork.yml\` has no \`setup-node\` step at all — must add one explicitly \[\[019f03bb-f9cf-7208-a183-d4f0074480f9]]. @@ -403,9 +370,6 @@ * **idle.ts eviction: upstream uses per-function cleanup in idle.ts, not centralized evictSession in pipeline.ts**: Upstream (main branch) puts session eviction logic directly in \`idle.ts\` rather than a centralized \`evictSession()\` in \`pipeline.ts\`. \`idle.ts\` imports cleanup functions individually: \`evictSession as evictGradientSession\` from \`@loreai/core\`; also \`deleteSessionAuth\`, \`clearAuthStale\` from \`./auth\`; \`deleteSessionCosts\` from \`./cost-tracker\`; \`deleteBillingPrefix\` from \`./cch\`; \`clearWarmupAuthDisabled\` from \`./cache-warmer\`. The \`startIdleScheduler\` signature uses \`onEvict?: (sessionID: string) => void\` (upstream) vs \`onEvictSession?: (sessionID: string) => boolean\` (branch). Upstream inline \`onEvict\` in \`pipeline.ts\` cleans 5 Maps: \`headerSessionIndex\`, \`ltmSessionCache\`, \`ltmPinnedText\`, \`stableLtmCache\`, \`cwdWarned\`. When merging, adopt upstream's per-function approach and add any missing cleanup calls. - -* **jssg codemod authoring conventions for sentry-cli**: jssg transform signature: \`export default (root: SgRoot\) => string | null | undefined\`. Always import: \`\`\`ts import type { SgRoot, SgNode, Edit } from "codemod:ast-grep"; import type TSX from "codemod:ast-grep/langs/tsx"; \`\`\` Key gotchas: (1) ESM import patterns are quote-sensitive — \`import $NAME from "@sentry/cli"\` (double-quote literal) does NOT match single-quoted source; use \`require($SRC)\` metavar for CJS (quote-agnostic). (2) Always include explicit null checks before accessing node properties (\`node.field("parameters")?.child(0)\`). (3) \`pair\` kind = key-value pair; \`shorthand\_property\_identifier\` = shorthand \`{ foo }\`. (4) Batch all edits before \`commitEdits\`. Test via \`npx codemod jssg test -l typescript ./scripts/codemod.ts\`; update snapshots with \`-u\`. - * **Node version pinning convention: workflow-level env vars NODE\_VERSION\_22 / NODE\_VERSION\_24**: As of PR #1145, all GitHub Actions workflows in sentry-cli (TypeScript) centralize Node version pins as workflow-level \`env\` vars: \`NODE\_VERSION\_22: "22.23.1"\` and \`NODE\_VERSION\_24: "24.18.0"\`. All \`actions/setup-node\` steps reference \`${{ env.NODE\_VERSION\_22 }}\` or \`${{ env.NODE\_VERSION\_24 }}\` — no bare \`"22"\`/\`"24"\` strings. Matrix jobs use ternary: \`${{ matrix.node == '24' && env.NODE\_VERSION\_24 || env.NODE\_VERSION\_22 }}\`. Motivation: Node 24.17.0/22.23.0 shipped \`ERR\_STREAM\_PREMATURE\_CLOSE\` regression (CVE-2026-48931 http.Agent fix); fixed in 24.18.0/22.23.1 (nodejs/node#64004). When bumping Node, update the \`env\` block in each workflow file. @@ -425,7 +389,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 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 SDK tree-shaking patches must be regenerated via bun patch workflow**: Sentry SDK tree-shaking patches must be regenerated via pnpm patch workflow. Regenerate \`@sentry/core\` and \`@sentry/node-core\` patches after bumping to catalog version 10.54.0. * **sentry-cli banner local test commands (isTTY-gated)**: Banner only renders in interactive terminals (\`if (process.stdout.isTTY)\`). Test commands: \`pnpm cli help\` or \`pnpm cli\` (fastest, tsx via src/bin.ts, no build); \`pnpm cli init\` (Ink wizard banner path). Quick sanity check without CLI: \`FORCE\_COLOR=3 pnpm exec tsx -e "import('./src/lib/banner.ts').then(m => console.log(m.formatBanner()))"\`. Env vars: \`FORCE\_COLOR=3\` forces color output; \`NO\_COLOR\` or \`SENTRY\_PLAIN\_OUTPUT\` strips gradient. @@ -450,54 +414,45 @@ ### Preference - -* **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 all bot review comments (BugBot/Seer/cursor\[bot]) before proceeding**: When working on PRs, the user consistently expects all bot-generated review comments (from cursor\[bot] BugBot, sentry\[bot] Seer, or similar automated reviewers) to be explicitly reviewed and addressed before merging or moving on. This includes triaging each finding as valid or false positive, applying fixes for valid bugs, and not treating a 'Bugbot: pass' CI status as sufficient — the user expects the assistant to read and act on the actual comment content. The user also expects this audit to happen proactively across all open PRs, not just the one currently being worked on. - * **Always audit PR bot/reviewer feedback and triage each finding before merging**: After opening or updating a PR, the user routinely asks the assistant to check for and address feedback from automated bots (cursor\[bot], sentry\[bot], github-actions\[bot], Codecov) as well as human reviewers across one or more PRs before proceeding further. When following this pattern: fetch all inline comments and reviews via the correct API endpoints, list each finding with its ID, severity, and file/line location, investigate root cause for each flagged issue (don't just trust the bot's suggested fix — validate it against actual code semantics), and decide whether to apply a fix, add a clarifying TODO/comment, or reject the suggestion as unsafe/incorrect. Summarize findings per-PR and per-reviewer, and confirm the repo state (e.g., git status) before/after making changes. Always distinguish valid bugs from false positives before acting. + +* **Always available in Node 22**: User stated always available in Node 22. + * **Always checks PR status and review comments before merging**: The user consistently checks the status of Pull Requests (PRs) and review comments before merging them. This includes verifying that all checks have passed (e.g., CI status, linting, typechecking, unit tests), that review comments have been addressed, and that the merge state is clean. The user also ensures that all review threads are resolved or replied to, and that any outstanding issues are clarified or confirmed as addressed. - -* **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 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. - - -* **Always conduct thorough PR reviews with severity-classified findings**: PR review standards: (1) Compare branch vs main first (\`git log main..origin/\\`, \`git diff --stat\`). (2) Verify every PR description claim against actual source files at specific line numbers — never trust PR metadata. (3) Classify findings as BLOCKING vs NON-BLOCKING with file paths and line numbers. (4) Flag LLM-generated planning artifacts (e.g., DOCS-AUDIT.md) as blocking violations of repo conventions. (5) Investigate root causes — check bundle output, trace esbuild variable renaming, identify silent regressions. (6) Run relevant check scripts and grep codebase directly rather than reasoning from PR metadata. - - -* **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. - * **Always Directs to Resolve Merge Conflicts and Ensure Compatibility**: The user consistently directs the assistant to resolve merge conflicts, ensure compatibility, and verify code changes before merging. This includes rebasing branches, resolving conflicts, and verifying code changes. The user also emphasizes the importance of backwards and forwards compatibility during transitions and releases. - -* **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 ensures merge readiness before merging**: The user consistently seeks to ensure that all prerequisites are met before merging, including assessing the impact of changes, verifying CI checks, evaluating test coverage, and confirming the correctness of the code and its documentation. The user also tends to consider the user-facing aspects and potential issues that may arise after merging. * **Always fetchable from the base repo with github**: User stated always fetchable from the base repo with github. - -* **Always fix root causes and add preventive safeguards together**: When the user identifies a recurring or accumulated problem (e.g., 61 stale PR previews bloating gh-pages), they direct: (1) fix the immediate root cause, (2) port or add automation to prevent recurrence (e.g., on-PR-close cleanup), and (3) add a scheduled safety-net sweep as a belt-and-suspenders measure. Similarly with code bugs: they want the bug fixed AND the underlying gap addressed. Never just patch the symptom — always pair the fix with structural prevention. This applies to CI/CD workflows, dependency issues, and code correctness problems alike. + +* **Always follow a standard workflow for PR creation and merging**: The user consistently enforces a standard workflow for creating and merging Pull Requests (PRs). This workflow includes creating a branch with a specific naming convention (e.g., \`fix/\-\\`), committing changes with a \`.lore.md\` file, creating a PR, and waiting for CI checks and bot reviews to complete before merging. The user also emphasizes the importance of proper issue tracking, cross-linking tasks, and regenerating patches when necessary. - -* **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. + +* **Always inject debug IDs and sourcemaps for local builds**: The user consistently requires debug IDs to be injected into JavaScript bundles during local builds, even when no authentication token is present. This ensures local development always has debug information available. The user also mandates that sourcemaps are never shipped to end users in server-side deployments, maintaining security while preserving debuggability during development. + + +* **Always investigate build configurations and dependencies when researching Node version compatibility**: The user consistently researches build configurations (esbuild, bundle scripts) and dependency management when evaluating Node version compatibility. They specifically examine how external dependencies like SQLite modules are handled, including import patterns, externalization in bundlers, and fallback mechanisms. The user wants to understand the complete dependency chain and build pipeline implications when supporting different Node versions. * **Always investigate root cause by tracing through multiple specific code layers before accepting a fix**: When facing a runtime bug (especially undefined values from framework internals), the user consistently demands thorough investigation across multiple layers — framework source code (node\_modules), wrapper utilities, bundler config, and call sites — before accepting any fix. The user explicitly rejects surface-level explanations and pushes for tracing the exact code path that produces the unexpected value. Only after exhausting the investigation does the user accept a defensive fix strategy. When directing investigation, the user specifies concrete areas to search (e.g., 4 specific code locations). Always read and analyze the relevant framework internals, not just application code. - -* **Always makes fresh requests**: User stated always makes fresh requests. - * **Always migrate Bun-specific APIs and tooling to Node.js equivalents**: 🔴 Directive (repeated 25+ sessions): ALWAYS prefer editing existing files in the codebase. NEVER write new files unless explicitly required. NEVER proactively create documentation files (\*.md) or README files — only create documentation files if explicitly requested by the user. + +* **Always Monitor CI Status**: The user consistently checks and verifies the status of CI checks, including Validate generated files, CodeQL, and other workflows. They also investigate and address failures, often by identifying root causes and applying fixes. Additionally, the user monitors and confirms the resolution of transient issues and ensures that all required checks pass before proceeding with the PR. + + +* **Always perform rigorous, read-only code reviews with specific focus areas**: The user consistently conducts critical, adversarial code reviews in a read-only manner, focusing on specific technical areas. They provide detailed context about the repository, branch, and changes under review, and specify exact focus areas like path correctness, dependency management, build configurations, and security implications. The user maintains strict read-only discipline, prohibiting any modifications to source files, and often works within isolated worktrees. They expect thorough verification of technical changes against documented requirements and prior decisions. + * **Always perform thorough codebase exploration before designing or implementing fixes**: When investigating a bug or feature, the user consistently requests comprehensive upfront exploration across multiple files before any code changes. This includes: reading relevant command and API files completely, searching for all references to key terms/parameters, checking type definitions in SDK/node\_modules, and understanding the full data flow from flags to API calls. The user expects the assistant to map out the entire call chain, identify misleading comments, and surface all related code paths before proposing a solution. Do not jump to fixes — first read all relevant files thoroughly and report findings. @@ -507,9 +462,18 @@ * **Always plan systemic fixes with structured multi-problem breakdowns before implementation**: When the user identifies documentation or tooling issues, they consistently organize them as numbered problems with precise file locations, line numbers, and root causes before any code is written. They expect the assistant to engage at the planning level first — proposing detection strategies, fix approaches, and tradeoffs — and to consolidate related problems (e.g., merging overlapping tasks) rather than treating each in isolation. Plans are written to files and iterated on. Implementation only follows after the plan is agreed upon. The user prefers systemic/automated fixes (e.g., derive patterns from package.json) over one-off patches. + +* **Always plans for testing or verification of tasks**: The user consistently implies a need for testing or verification of tasks, as seen in multiple instances where they either directly request tests or imply the necessity for verification. This pattern is observed across various sessions where the user is planning and executing tasks, indicating a preference for ensuring that tasks are validated or tested. + * **Always prefer systemic/automated solutions over one-off fixes**: When the user identifies errors, gaps, or problems, they explicitly direct the assistant to create or fix systems that prevent the entire class of errors in the future, rather than applying isolated one-off fixes. This applies especially when evaluating code quality, reviewing PRs, or addressing bugs. The user wants automated checks (e.g., CI steps, lint rules, scripts) and general solutions that scale, not patches that only address the immediate symptom. When planning or executing fixes, always ask: 'Can this be automated or systematized?' and prefer that approach. + +* **Always prefers or chooses practical, immediate solutions with minimal effort over ideal but more costly alternatives**: The user consistently selects or acquiesces to options that provide immediate relief or unblock tasks with the least effort, even if they are not the most optimal or long-term solutions. This is evident in instances where the assistant presents multiple options for resolving issues, such as shipping a codemod or addressing documentation. The user tends to favor quicker, albeit potentially less ideal, solutions like using 'npx jscodeshift' over more involved approaches like porting to 'jssg' or setting up registry packages. This pattern suggests the user prioritizes expediency and practicality in their decision-making process. + + +* **Always prefers pnpm over Turbo for task orchestration in monorepos**: The user consistently expresses a preference for using pnpm-native orchestration over Turbo for task management in monorepos. In multiple sessions, the user has indicated a dislike for manual and fragile configuration of Turbo and has sought automated configuration derived from pnpm workspace dependencies. The user has explicitly stated that they want to get rid of Turbo for task orchestration in the \`getsentry/toolkit\` monorepo and instead use pnpm-native orchestration. + * **Always re-check PR feedback and fix Cursor/Bugbot findings before proceeding**: User repeatedly asks the assistant to re-check all outstanding review feedback (from cursor\[bot], sentry\[bot], and human reviewers) on the same PR (e.g. #1201) for the sentry-v3-to-v4 codemod/migration docs. For each finding: (1) fetch full comment text/detail via the correct API endpoint, (2) investigate root cause in the actual code (codemod.ts, migrating-from-v3.md), (3) distinguish genuinely new findings from persistent/already-resolved re-posted ones, (4) apply a real fix or, if the bot's suggested fix is unsafe/ambiguous, add a TODO comment instead of guessing, (5) add/update tests to cover the fix, (6) verify with test run and lint, then commit/push and reply to or summarize resolution on the PR thread. Always prioritize correctness of argument/flag mapping between v3 and v4 semantics, and avoid touching unrelated APIs. Continue this feedback-triage loop each time new bot comments appear. @@ -526,20 +490,26 @@ * **Always requests a thorough review before merging PRs**: The user consistently requests detailed reviews of pull requests (PRs) before merging them. This includes scrutinizing changes, verifying git status is clean, and checking test coverage. The user also focuses on specific areas such as error handling, edge cases, and code correctness. Additionally, the user often performs multiple review passes, requesting objective and critical reviews to ensure high-quality code changes. -* **Always requests exhaustive and detailed research**: The user consistently requests thorough and precise investigations into specific aspects of the codebase, infrastructure, or related tools. They emphasize the need for exhaustive research, often specifying that no code should be written and that findings should be detailed, including file paths, line numbers, and config values. This pattern is evident across multiple sessions, where the user asks for detailed inventories, code reviews, and investigations into various components such as release infrastructure, build processes, and versioning mechanisms. +* **Always requests exhaustive and detailed research**: The user consistently demonstrates a preference for thorough planning and exhaustive research before making changes to the codebase or repository structure. This includes creating detailed migration plans, researching repository structures, build processes, release infrastructure, and deployment processes. The user also values precision and accuracy in the information provided, often requesting exhaustive and precise inventories of repository structures, build processes, and other relevant details. + + +* **Always requests exhaustive and precise information**: The user consistently requests detailed and thorough information about repository structures, build, release, deployment, and website infrastructure. They emphasize the need for precision, specifying requirements such as file paths, package names, versions, and config values. The user also often prohibits writing any code, indicating that the focus is on research and reporting. This pattern is observed across multiple sessions, including instances where the user requests information about the sentry-mcp and getsentry/cli repositories. + + +* **Always requests tests after implementation**: The user consistently requests tests or verification after each implementation or code change. This is observed in multiple sessions where the user asks for tests or implies the need for testing/verification after tasks are executed or code is modified. + + +* **Always research distribution architecture before implementation**: The user consistently performs deep architectural research on distribution mechanisms before making implementation decisions. This includes examining package.json (bin, files, exports, engines), build scripts, polyfills, runtime detection, CI/CD workflows, and documented policies. The research is read-only and focuses on understanding how the software is built, bundled, and distributed across different platforms and environments. * **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 resolve merge conflicts manually in .lore.md**: User stated always resolve merge conflicts manually in . + +* **Always resolve a string (or change the type definition)**: User stated always resolve a string (or change the type definition). * **Always restores the prior tty state**: Always restores the prior tty state. This is a critical behavior for handling terminal interactions. - -* **Always run lint, typecheck, and tests after code changes**: The user consistently runs tests after making changes to the code. This is evident from the multiple instances of test runs observed across different sessions, often with a focus on specific test suites or files that have been touched by recent changes. The user appears to prioritize verifying the correctness of their code through testing. - * **Always stage all modified files before committing, not just already-staged ones**: When preparing to commit, the user reviews git status and expects ALL modified files to be staged together — not just files already in the index. If unstaged modified files exist alongside staged ones, the user treats this as an incomplete commit state that needs to be resolved before proceeding. The user reviews the full list of changed files (staged + unstaged) as a checklist against completed tasks, and expects the commit to encompass all related changes from the session as a single coherent unit. @@ -558,21 +528,27 @@ * **Always track pre-existing failures separately from introduced regressions**: When running tests, the user consistently distinguishes between failures that existed before their changes and failures caused by their changes. They verify pre-existing failures by checking out main/stashing changes and confirming the same failures reproduce. Only new failures introduced by the current branch are treated as actionable. When reporting test results, always clarify which failures are pre-existing (with evidence) versus newly introduced, and never treat pre-existing failures as blockers for the current fix. - -* **Always verifies generated assets**: The user consistently verifies generated assets, such as images and banners, to ensure their correctness. This includes visual verification of transparency, gradients, and other visual properties. The user also checks the integrity of generated files by recording SHA256 checksums and confirming that they match expected values. + +* **Always update configuration files systematically**: The user consistently updates configuration files in a methodical and systematic way, focusing on specific sections or blocks. This pattern is observed in multiple sessions where the user or assistant updates files like \`docs-preview.yml\`, \`ci.yml\`, and others, starting with specific sections such as paths-filter globs and version-read, and proceeding with a detailed examination and edits of the file contents. + + +* **Always use absolute language when describing system behavior**: The user consistently uses absolute terms like 'always' and 'never' when describing system behavior, especially regarding code execution paths, API availability, and module usage. This indicates a preference for precise, unambiguous language when documenting or discussing technical implementation details. The assistant should mirror this communication style by using definitive language when describing system behavior. * **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. - -* **Always verify CI/merge status before closing out a task or session**: At the end of a coding session or task, the user consistently checks that all CI checks are green and the PR mergeState is CLEAN before considering the work done. This applies to both single PRs and batches of PRs. The user runs tools to confirm merge state, reviews bot/warden findings, and expects the assistant to provide a final wrap-up summary confirming everything is clear. Do not declare a task complete without explicitly confirming CI status and mergeState via tooling. + +* **Always Verify and Publish Code Changes with Documentation**: The user consistently requests to publish code changes and file issues for them. They also verify the documentation and ensure it is correct before publishing. The user checks the configuration files, such as configuration.md, which is generated by a script, and ensures that the documentation is up-to-date and accurate. The user also confirms that the CI builds are correct and that the documentation is valid. + + +* **Always verify and update file/directory paths in CI/CD configurations after repository restructuring**: The user consistently demonstrates a pattern of thoroughly checking and updating file and directory paths in CI/CD configuration files (ci.yml, docs-preview.yml, eval-skill-fork.yml) following repository restructuring. This includes verifying paths for build jobs, cache configurations, test commands, documentation generation, and artifact handling. The user provides specific line numbers and path details, confirming that paths are updated to reflect new locations (e.g., packages/cli/, apps/cli-docs/) and that absolute paths are used where necessary to avoid ambiguity. + + +* **Always Verify and Validate Code Correctness**: The user consistently verifies and validates code correctness across multiple sessions. This includes checking the correctness of functions, ensuring proper scoping of catch blocks, verifying variable destructuring, and confirming the integrity of specific code elements. The user also tends to ask questions about code behavior, compatibility, and potential issues, demonstrating a thorough approach to code review and validation. * **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 edits with typecheck, lint, and tests before finishing**: After applying code edits, always run a verification sequence: typecheck, then lint, then the relevant test suite (e.g., for affected commands/modules), before considering the change complete or moving to the next issue/comment. When adding a bug-fix or new behavior, follow TDD discipline by first confirming the test fails without the fix (or catches the bug), then reapplying the fix to confirm the test passes. Test edge cases and variations of command inputs (e.g., with/without prefixes, subcommand stripping, return codes) rather than just the happy path. Only proceed to unrelated tasks (like addressing review comments) once all checks pass cleanly. - * **Always verify PR claims against actual codebase before accepting changes**: When reviewing a PR, the user consistently directs the assistant to check each stated claim against the real source files on the main branch rather than trusting the PR description or commit messages. This applies especially to documentation PRs: the user wants specific file paths, line numbers, and code excerpts cited as evidence. The user also cross-checks automated tooling (scripts, CI configs) against what they actually produce. When a PR introduces fixes, the user wants confirmation that the underlying problem genuinely existed and that the fix is correct — not just that the PR author says so. Always run the relevant check scripts and grep the codebase directly rather than reasoning from PR metadata alone. @@ -582,56 +558,59 @@ * **Always work from a structured plan file before executing multi-step tasks**: When tackling multi-step or multi-file changes, the user consistently creates a formal plan file (e.g., \`.opencode/plans/\-\.md\`) during a planning phase before any edits are made. The plan enumerates discrete numbered tasks with priorities and target files. Execution only begins after the user explicitly approves the plan. During execution, tasks are marked in\_progress and completed sequentially. The user expects this plan-then-execute workflow to be followed strictly — no file edits during planning, and tasks tracked against the approved plan. - -* **Always write tests alongside implementation**: Behavioral pattern detected across 3 sessions (action: requested-tests). The user consistently demonstrates this behavior. + +* **Always write tests alongside implementation**: Behavioral pattern detected across 8 sessions (action: requested-tests). The user consistently demonstrates this behavior. + + +* **Call plan\_exit to indicate planning done**: Always call plan\_exit to indicate that planning is done. - -* **Deep-dive into getsentry/cli codebase architecture before writing docs or code**: Before implementing features, writing documentation, or answering questions about the getsentry/cli (Sentry CLI TypeScript rewrite) project, the user consistently has the assistant thoroughly explore the existing codebase structure first: reading src/app.ts route definitions, command subdirectories (auth, release, debug-files, proguard, etc.), backward-compatibility aliases, plural-to-singular mappings, lib/db internals, and related config files. The user values exhaustive, precise inventories of existing commands, flags, file structures, and naming conventions (e.g., old sentry-cli vs new sentry package/binary names) before any new artifact (docs, migration guides, code) is produced. When asked to create something (like a migration guide), first verify current state via file reads and grep/search across the actual source tree rather than assuming or guessing structure. Cross-reference package.json, README, and app.ts for accurate naming and command listings. + +* **Command execution security**: User stated commands should 'never interpolated' for security, indicating a preference for safe command execution practices. - -* **Designer Steven Lewis directs Dammit Sans for headings, Rubik for body**: Steven Lewis is the designer for the sentry-cli docs site. His direction: use Dammit Sans for headers, Rubik for body. Dammit Sans will eventually be scoped mainly to larger headings (h1 + some h2s). Dammit Sans missing glyphs: \`/ \* \ | ^ \_ \\\`\` plus \`#\` and \`~\` — fall back to Rubik Variable. + +* **Debug ID injection behavior**: User stated debug IDs should 'always runs (even without auth token) so local builds get' and 'Always inject debug IDs (even without auth token); upload is gated inside the plugin', indicating a preference for consistent debug ID injection regardless of authentication status. - -* **Enforce full verification pipeline and clean branch scoping before committing**: Before finalizing any change, the user consistently expects: (1) full verification — typecheck (tsc), Biome lint (including auto-fix and unsafe-fix passes), and the full/relevant test suite all passing clean; (2) Biome lint issues (complexity, parameter properties, top-level regex, formatting) are fixed immediately rather than ignored, including applying suggested/unsafe fixes when appropriate; (3) changes are kept scoped to the correct branch/PR — if work gets mixed onto the wrong branch (e.g., stacked incorrectly, or docs/skill files pollute an unrelated feature), the user expects the assistant to stash, create a fresh branch off main, and restore only the relevant source files rather than reusing contaminated generated files; (4) auto-generated docs (contributing.md, SKILL.md, references/\*.md) must be regenerated fresh to match the actual branch scope, not carried over stale. Always verify lint+typecheck+tests before considering work done, and proactively fix branch/scope hygiene issues rather than leaving mixed changes. + +* **Debug ID replacement in JS bundles**: User stated to 'Replace the placeholder UUID with the real debug ID in the JS bundle.', indicating a preference for proper debug ID handling in JavaScript bundles. + + +* **Follow consistent code style conventions**: Behavioral pattern detected across 3 sessions (action: corrected-style). The user consistently demonstrates this behavior. * **Follow the established git workflow (branch, PR, review)**: Behavioral pattern detected across 6 sessions (action: enforced-workflow). The user consistently demonstrates this behavior. - -* **Never block on a prompt**: User stated never block on a prompt. - - -* **Never block on or interleave with the org/project picker**: User stated never block on or interleave with the org/project picker. + +* **Ink import behavior**: User stated code should 'never calls \`import("ink")\` at runtime —', indicating a preference against runtime imports of Ink library. - -* **Never exceed the terminal width (it would wrap into a broken layout otherwise)**: Never exceed the terminal width (it would wrap into a broken layout otherwise). This is a standing directive to ensure readable output. + +* **Monorepo setup for toolkit**: Always resolve merge conflicts manually in .lore.md — applies to CLI and MCP -* **Never target the same path**: User stated never target the same path. +* **Never target the same path**: Never target the same path. -* **Never throws - errors are caught and reported to Sentry**: User stated Never throws - errors are caught and reported to Sentry. +* **Never throws - errors are caught and reported to Sentry**: Never throws - errors are caught and reported to Sentry. - -* **Never treat incomplete operations as successful — always surface silent failures**: When reviewing code, the user explicitly asserts that incomplete operations must never report success. Examples from \`debug-files upload\`: \`not\_found\` after deadline is a failure (exit 1), not success (exit 0). Symlink cycles must not hang silently. Missing requested IDs must be surfaced. Any failure path that silently exits 0 or produces no diagnostic is a blocker. + +* **Never use node\_modules/**: User stated never to use 'node\_modules/.'. * **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 freshly-created location over existing**: \`installAgentSkills()\` in \`src/lib/agent-skills.ts\` returns \`results.find(location => location.created) ?? results\[0] ?? null\` — prefers freshly-created location over existing. Installs to \`~/.agents/skills/sentry-cli/\` and \`~/.claude/skills/sentry-cli/\` only. Detection: \`detectClaudeCode()\` checks \`existsSync(join(homeDir, '.claude'))\`; installer never creates top-level agent roots — presence of root dir is the detection signal. OpenCode is detected via \`OPENCODE\_CLIENT\` env var in \`detect-agent.ts\` for telemetry only — NOT for skill installation. No OpenCode skill install path exists anywhere in the repo. + +* **Prefers Bun-native APIs over Node**: prefer Bun-native APIs over Node. - -* **Prioritize pragmatic parity over full backwards compatibility**: When porting/reimplementing legacy CLI commands (e.g., react-native gradle/xcode) into the new tool, the user prefers a pragmatic parity approach rather than exhaustive 100% backwards compatibility. Focus effort on closing concrete, high-value functional gaps (e.g., real command behavior for gradle/xcode), while treating minor divergences—deprecated commands, legacy aliases, cosmetic output format differences—as acceptable intentional trade-offs not worth fixing. When given a choice of how deep to go into replicating legacy edge cases, default to 'Pragmatic parity (Recommended)' rather than chasing full fidelity. Also follow the established workflow: implement changes on a properly stacked/independent branch, run the full test suite before committing, open a PR, and once the base PR merges, rebase dependent branches onto main to remove stacking before continuing. + +* **Respect explicitly rejected approaches**: Behavioral pattern detected across 13 sessions (action: rejected-approach). The user consistently demonstrates this behavior. - -* **Request critical, objective, read-only code reviews of PRs before/after merge**: User consistently requests thorough, adversarial code reviews of pull requests (often via a subagent) rather than direct code edits. When requesting these reviews, user expects: (1) findings organized by severity (Critical/High/Medium/Low/Nit) with file:line references and concrete fixes, (2) explicit instruction that the assistant should NOT write code during review — findings only, (3) a defined checklist of files/functions to read fully (e.g., core logic, related tests, shared utilities), (4) verification of specific technical concerns like dangling references, regressions, behavior-neutrality of refactors, shell-out safety, and no-silent-catch conventions, and (5) a clear final verdict (e.g., no regressions found, follow-up needed) plus confirmation of read-only discipline (e.g., clean git status). Apply this pattern whenever asked to review a PR: scope the review precisely, avoid code changes, and report structured, severity-ranked findings with verification evidence. + +* **Review code before committing**: Behavioral pattern detected across 9 sessions (action: requested-review). The user consistently demonstrates this behavior. - -* **Verify CI status via GitHub API/logs after pushing changes**: After pushing commits or opening a PR, the user consistently checks CI run status by querying the GitHub Actions API (runs, jobs, logs) rather than waiting for notifications. When a job fails or its status is unclear, the user persists through multiple strategies — querying by run ID, job ID, job name, or downloading full raw logs — to locate the specific failing test or assertion, especially when initial log output is truncated or only shows warnings/passing tests. The user pays close attention to distinguishing real regressions from flaky/isolation-related failures (e.g., re-running a test in isolation to confirm) and cross-references CI Status step logic (upstream job cascades) to determine root cause. When assisting, proactively fetch full logs, identify actual failure lines (not just warnings), and check whether a failure is a known flake versus a real issue before concluding investigation. + +* **Server-side feature shipping**: User stated certain features are 'never shipped to users) for server-side', indicating a preference for keeping some functionality server-side only. - -* **Verify TypeScript CLI ports against both legacy Rust source and live server implementation**: User is porting Sentry's legacy Rust CLI to a new TypeScript CLI and consistently prioritizes verifying command-for-command and behavioral parity between the two before considering work complete. This includes: comparing command structures to find missing commands/subcommands, fetching and analyzing specific legacy Rust source files (e.g., debug\_files/\*.rs, vcs.rs, ci.rs) to understand exact original logic, identifying native Rust dependencies (e.g., symbolic, odiff) that block direct ports and researching JS/WASM alternatives, and explicitly asking whether the new CLI is '100% backwards compatible' with the old one. When assisting, proactively cross-reference legacy Rust implementation details against the TypeScript port, flag any functional gaps or behavioral differences, and suggest alternatives for native-dependency features rather than assuming parity by default. + +* **SQLite usage research for Node 20 support**: User is researching Node 20 support for Sentry CLI (getsentry/cli) and needs to understand how SQLite is used. Specifically investigating the lack of built-in node:sqlite module in Node 20 and exploring alternatives. - -* **Wait for all CI checks to reach CLEAN mergeState before merging**: The user requires that PRs only be merged once mergeStateStatus is CLEAN — meaning all required checks (Unit Tests, E2E Tests, Build Docs, Seer Code Review, Codecov, etc.) have passed, with no pending or blocked checks. When a check fails, the assistant should verify if it's flaky by re-running before assuming failure. Do not merge while any check is BLOCKED or pending. Before finalizing, perform a final sweep to check for new bot findings/comments on the latest commit and ensure all review replies are posted. Use squash merges with conventional PR title format (including PR number) when merging, consistent with repo conventions. Only proceed with merge once mergeState is confirmed CLEAN and mergeable is MERGEABLE. + +* **Understanding SQLite usage patterns**: User repeatedly asks to understand HOW SQLite is used across multiple sessions, indicating a strong preference for deep technical understanding of database usage patterns. diff --git a/.npmrc b/.npmrc index d67f374883..1aa498c4e2 100644 --- a/.npmrc +++ b/.npmrc @@ -1 +1 @@ -node-linker=hoisted +node-linker=isolated diff --git a/AGENTS.md b/AGENTS.md index 798b0250b9..3316fa78e2 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -1,1004 +1,3 @@ -# AGENTS.md - -Guidelines for AI agents working in this codebase. - -## Project Overview - -**Sentry CLI** is a command-line interface for [Sentry](https://sentry.io), built with [Bun](https://bun.sh) and [Stricli](https://bloomberg.github.io/stricli/). - -### Goals - -- **Zero-config experience** - Auto-detect project context from DSNs in source code and env files -- **AI-powered debugging** - Integrate Seer AI for root cause analysis and fix plans -- **Developer-friendly** - Follow `gh` CLI conventions for intuitive UX -- **Agent-friendly** - JSON output and predictable behavior for AI coding agents -- **Fast** - Native binaries via Bun, SQLite caching for API responses - -### Key Features - -- **DSN Auto-Detection** - Scans `.env` files and source code (JS, Python, Go, Java, Ruby, PHP) to find Sentry DSNs -- **Project Root Detection** - Walks up from CWD to find project boundaries using VCS, language, and build markers -- **Directory Name Inference** - Fallback project matching using bidirectional word boundary matching -- **Multi-Region Support** - Automatic region detection with fan-out to regional APIs (us.sentry.io, de.sentry.io) -- **Monorepo Support** - Generates short aliases for multiple projects -- **Seer AI Integration** - `issue explain` and `issue plan` commands for AI analysis -- **OAuth Device Flow** - Secure authentication without browser redirects - -## Cursor Rules (Important!) - -Before working on this codebase, read the Cursor rules: - -- **`.cursor/rules/bun-cli.mdc`** - Bun API usage, file I/O, process spawning, testing -- **`.cursor/rules/ultracite.mdc`** - Code style, formatting, linting rules - -## Quick Reference: Commands - -> **Note**: Always check `package.json` for the latest scripts. - -```bash -# Development -bun install # Install dependencies -bun run dev # Run CLI in dev mode -bun run --env-file=.env.local src/bin.ts # Dev with env vars - -# Build -bun run build # Build for current platform -bun run build:all # Build for all platforms - -# Type Checking -bun run typecheck # Check types - -# Linting & Formatting -bun run lint # Check for issues -bun run lint:fix # Auto-fix issues (run before committing) - -# Testing -bun test # Run all tests -bun test path/to/file.test.ts # Run single test file -bun test --watch # Watch mode -bun test --filter "test name" # Run tests matching pattern -bun run test:unit # Run unit tests only -bun run test:e2e # Run e2e tests only -``` - -## Rules: No Runtime Dependencies - -**CRITICAL**: All packages must be in `devDependencies`, never `dependencies`. Everything is bundled at build time via esbuild. CI enforces this with `bun run check:deps`. - -When adding a package, always use `bun add -d ` (the `-d` flag). - -When the `@sentry/api` SDK provides types for an API response, import them directly from `@sentry/api` instead of creating redundant Zod schemas in `src/types/sentry.ts`. - -## Rules: Use Bun APIs - -**CRITICAL**: This project uses Bun as runtime. Always prefer Bun-native APIs over Node.js equivalents. - -Read the full guidelines in `.cursor/rules/bun-cli.mdc`. - -**Bun Documentation**: https://bun.sh/docs - Consult these docs when unsure about Bun APIs. - -### Quick Bun API Reference - -| Task | Use This | NOT This | -|------|----------|----------| -| Read file | `await Bun.file(path).text()` | `fs.readFileSync()` | -| Write file | `await Bun.write(path, content)` | `fs.writeFileSync()` | -| Check file exists | `await Bun.file(path).exists()` | `fs.existsSync()` | -| Spawn process | `Bun.spawn()` | `child_process.spawn()` | -| Shell commands | `Bun.$\`command\`` ⚠️ | `child_process.exec()` | -| Find executable | `Bun.which("git")` | `which` package | -| Glob patterns | `new Bun.Glob()` | `glob` / `fast-glob` packages | -| Sleep | `await Bun.sleep(ms)` | `setTimeout` with Promise | -| Parse JSON file | `await Bun.file(path).json()` | Read + JSON.parse | - -**Exception**: Use `node:fs` for directory creation with permissions: -```typescript -import { mkdirSync } from "node:fs"; -mkdirSync(dir, { recursive: true, mode: 0o700 }); -``` - -**Exception**: `Bun.$` (shell tagged template) has no shim in `script/node-polyfills.ts` and will crash on the npm/node distribution. Until a shim is added, use `execSync` from `node:child_process` for shell commands that must work in both runtimes: -```typescript -import { execSync } from "node:child_process"; -const result = execSync("id -u username", { encoding: "utf-8", stdio: ["pipe", "pipe", "ignore"] }); -``` - -## Architecture - -The full project-structure tree — including the live command/subcommand list and the -domain API modules — is generated from the route tree and lives in -[`docs/src/content/docs/contributing.md`](docs/src/content/docs/contributing.md) -(the `project-structure` block produced by `script/generate-docs-sections.ts`). It is -kept in sync automatically, so it is **not** duplicated here to avoid drift. For the -current command list run `ls src/commands/` or `sentry --help`. - -Top-level layout: - -- **`src/bin.ts`** — entry point; **`src/app.ts`** — Stricli application setup; - **`src/context.ts`** — dependency-injection context. -- **`src/commands/`** — one directory per command group (`auth`, `cli`, `dashboard`, - `event`, `issue`, `log`, `org`, `project`, `release`, `replay`, `repo`, `sourcemap`, - `span`, `team`, `trace`, `trial`, `local`, …) plus standalone command files - (`api.ts`, `explore.ts`, `help.ts`, `init.ts`, `schema.ts`). -- **`src/lib/`** — shared utilities. Key subtrees: `api/` (domain API modules), - `db/` (SQLite layer), `dsn/` (DSN detection, with per-language extractors under - `dsn/languages/`), and `formatters/` (output formatting). See the file-locations - table below and the JSDoc in each module for details. -- **`src/types/`** — TypeScript types and Zod schemas. -- **`test/`** — tests mirroring `src/` (unit, `*.property.test.ts`, - `*.model-based.test.ts`, `e2e/`, `fixtures/`, `mocks/`). -- **`docs/`** — documentation site (Astro + Starlight); **`script/`** — build/utility - scripts; **`.cursor/rules/`** — Cursor AI rules; **`biome.jsonc`** — lint config. - -## Key Patterns - -### CLI Commands (Stricli) - -Commands use [Stricli](https://bloomberg.github.io/stricli/docs/getting-started/principles) wrapped by `src/lib/command.ts`. - -**CRITICAL**: Import `buildCommand` from `../../lib/command.js`, **NEVER** from `@stricli/core` directly — the wrapper adds telemetry, `--json`/`--fields` injection, and output rendering. - -Pattern: - -```typescript -import { buildCommand } from "../../lib/command.js"; -import type { SentryContext } from "../../context.js"; -import { CommandOutput } from "../../lib/formatters/output.js"; - -export const myCommand = buildCommand({ - docs: { - brief: "Short description", - fullDescription: "Detailed description", - }, - output: { - human: formatMyData, // (data: T) => string - jsonTransform: jsonTransformMyData, // optional: (data: T, fields?) => unknown - jsonExclude: ["humanOnlyField"], // optional: strip keys from JSON - }, - parameters: { - flags: { - limit: { kind: "parsed", parse: Number, brief: "Max items", default: 10 }, - }, - }, - async *func(this: SentryContext, flags) { - const data = await fetchData(); - yield new CommandOutput(data); - return { hint: "Tip: use --json for machine-readable output" }; - }, -}); -``` - -**Key rules:** -- Functions are `async *func()` generators — yield `new CommandOutput(data)`, return `{ hint }`. -- `output.human` receives the same data object that gets serialized to JSON — no divergent-data paths. -- The wrapper auto-injects `--json` and `--fields` flags. Do NOT add your own `json` flag. -- Do NOT use `stdout.write()` or `if (flags.json)` branching — the wrapper handles it. - -### Command File Structure - -Command files in `src/commands/` should focus on three concerns: -1. **Argument parsing** — positional args, flags, URL detection -2. **API orchestration** — fetching data, error handling, enrichment -3. **Output dispatch** — `yield new CommandOutput(data)` - -Formatting and rendering logic belongs in `src/lib/formatters/.ts`. If a command file exceeds ~400 lines, extract formatting helpers into a dedicated formatter module. - -Reference: `src/lib/formatters/replay.ts` (extracted from `replay/view.ts`), `src/lib/formatters/trace.ts`, `src/lib/formatters/human.ts`. - -Lint enforcement: `stderr.write()` is banned in command files (GritQL rule). Use `logger` for diagnostics and `CommandOutput` for data output. - -### Route Maps (Stricli) - -Route groups use Stricli's `buildRouteMap` wrapped by `src/lib/route-map.ts`. - -**CRITICAL**: Import `buildRouteMap` from `../../lib/route-map.js`, **NEVER** from `@stricli/core` directly — the wrapper auto-injects standard subcommand aliases based on which route keys exist: - -| Route | Auto-aliases | -|----------|----------------| -| `list` | `ls` | -| `view` | `show` | -| `delete` | `remove`, `rm` | -| `create` | `new` | - -Manually specified aliases in `aliases` are merged with (and take precedence over) auto-generated ones. Do NOT manually add aliases that are already in the standard set above. - -```typescript -import { buildRouteMap } from "../../lib/route-map.js"; - -export const myRoute = buildRouteMap({ - routes: { - list: listCommand, - view: viewCommand, - create: createCommand, - }, - defaultCommand: "view", - // No need for aliases — ls, show, and new are auto-injected. - // Only add aliases for non-standard mappings: - // aliases: { custom: "list" }, - docs: { - brief: "Manage my resources", - }, -}); -``` - -### Positional Arguments - -Use `parseSlashSeparatedArg` from `src/lib/arg-parsing.ts` for the standard `[//]` pattern. Required identifiers (trace IDs, span IDs) should be **positional args**, not flags. - -```typescript -import { parseSlashSeparatedArg, parseOrgProjectArg } from "../../lib/arg-parsing.js"; - -// "my-org/my-project/abc123" → { id: "abc123", targetArg: "my-org/my-project" } -const { id, targetArg } = parseSlashSeparatedArg(first, "Trace ID", USAGE_HINT); -const parsed = parseOrgProjectArg(targetArg); -// parsed.type: "auto-detect" | "explicit" | "project-search" | "org-all" -``` - -Reference: `span/list.ts`, `trace/view.ts`, `event/view.ts` - -### Markdown Rendering - -All non-trivial human output must use the markdown rendering pipeline: - -- Build markdown strings with helpers: `mdKvTable()`, `colorTag()`, `escapeMarkdownCell()`, `renderMarkdown()` -- **NEVER** use raw `muted()` / chalk in output strings — use `colorTag("muted", text)` inside markdown -- Tree-structured output (box-drawing characters) that can't go through `renderMarkdown()` should use the `plainSafeMuted` pattern: `isPlainOutput() ? text : muted(text)` -- `isPlainOutput()` precedence: `SENTRY_PLAIN_OUTPUT` > `NO_COLOR` > `FORCE_COLOR` (TTY only) > `!isTTY` -- `isPlainOutput()` lives in `src/lib/formatters/plain-detect.ts` (re-exported from `markdown.ts` for compat) - -Reference: `formatters/trace.ts` (`formatAncestorChain`), `formatters/human.ts` (`plainSafeMuted`) - -### Create & Delete Command Standards - -Mutation (create/delete) commands use shared infrastructure from `src/lib/mutate-command.ts`, -paralleling `list-command.ts` for list commands. - -**Delete commands** MUST use `buildDeleteCommand()` instead of `buildCommand()`. It: -1. Auto-injects `--yes`, `--force`, `--dry-run` flags with `-y`, `-f`, `-n` aliases -2. Runs a non-interactive safety guard before `func()` — refuses to proceed if - stdin is not a TTY and `--yes`/`--force` was not passed (dry-run bypasses) -3. Options to skip specific injections (`noForceFlag`, `noDryRunFlag`, `noNonInteractiveGuard`) - -```typescript -import { buildDeleteCommand, confirmByTyping, isConfirmationBypassed, requireExplicitTarget } from "../../lib/mutate-command.js"; - -export const deleteCommand = buildDeleteCommand({ - // Same args as buildCommand — flags/aliases auto-injected - async *func(this: SentryContext, flags, target) { - requireExplicitTarget(parsed, "Entity", "sentry entity delete "); - if (flags["dry-run"]) { yield preview; return; } - if (!isConfirmationBypassed(flags)) { - if (!await confirmByTyping(expected, promptMessage)) return; - } - await doDelete(); - }, -}); -``` - -**Create commands** import `DRY_RUN_FLAG` and `DRY_RUN_ALIASES` for consistent dry-run support: - -```typescript -import { DRY_RUN_FLAG, DRY_RUN_ALIASES } from "../../lib/mutate-command.js"; - -// In parameters: -flags: { "dry-run": DRY_RUN_FLAG, team: { ... } }, -aliases: { ...DRY_RUN_ALIASES, t: "team" }, -``` - -**Key utilities** in `mutate-command.ts`: -- `isConfirmationBypassed(flags)` — true if `--yes` or `--force` is set -- `guardNonInteractive(flags)` — throws in non-interactive mode without `--yes` -- `confirmByTyping(expected, message)` — type-out confirmation prompt -- `requireExplicitTarget(parsed, entityType, usage)` — blocks auto-detect for safety -- `DESTRUCTIVE_FLAGS` / `DESTRUCTIVE_ALIASES` — spreadable bundles for manual use - -### List Command Pagination - -All list commands with API pagination MUST use the shared cursor-stack -infrastructure for **bidirectional** pagination (`-c next` / `-c prev`): - -```typescript -import { LIST_CURSOR_FLAG } from "../../lib/list-command.js"; -import { - buildPaginationContextKey, resolveCursor, - advancePaginationState, hasPreviousPage, -} from "../../lib/db/pagination.js"; - -export const PAGINATION_KEY = "my-entity-list"; - -// In buildCommand: -flags: { cursor: LIST_CURSOR_FLAG }, -aliases: { c: "cursor" }, - -// In func(): -const contextKey = buildPaginationContextKey("entity", `${org}/${project}`, { - sort: flags.sort, q: flags.query, -}); -const { cursor, direction } = resolveCursor(flags.cursor, PAGINATION_KEY, contextKey); -const { data, nextCursor } = await listEntities(org, project, { cursor, ... }); -advancePaginationState(PAGINATION_KEY, contextKey, direction, nextCursor); -const hasPrev = hasPreviousPage(PAGINATION_KEY, contextKey); -const hasMore = !!nextCursor; -``` - -**Cursor stack model:** The DB stores a JSON array of page-start cursors -plus a page index. Each entry is an opaque string — plain API cursors, -compound cursors (issue list), or extended cursors with mid-page bookmarks -(dashboard list). `-c next` increments the index, `-c prev` decrements it, -`-c first` resets to 0. The stack truncates on back-then-forward to avoid -stale entries. `"last"` is a silent alias for `"next"`. - -**Hint rules:** Show `-c prev` when `hasPreviousPage()` returns true. -Show `-c next` when `hasMore` is true. Include both `nextCursor` and -`hasPrev` in the JSON envelope. - -**Navigation hint generation:** Use `paginationHint()` from -`src/lib/list-command.ts` to build bidirectional navigation strings. -Pass it pre-built `prevHint`/`nextHint` command strings and it returns -the combined `"Prev: X | Next: Y"` string (or single-direction, or `""`). -Do NOT assemble `navParts` arrays manually — the shared helper ensures -consistent formatting across all list commands. - -```typescript -import { paginationHint } from "../../lib/list-command.js"; - -const nav = paginationHint({ - hasPrev, - hasMore, - prevHint: `sentry entity list ${org}/ -c prev`, - nextHint: `sentry entity list ${org}/ -c next`, -}); -if (items.length === 0 && nav) { - hint = `No entities on this page. ${nav}`; -} else if (hasMore) { - header = `Showing ${items.length} entities (more available)\n${nav}`; -} else if (nav) { - header = `Showing ${items.length} entities\n${nav}`; -} -``` - -**Three abstraction levels for list commands** (prefer the highest level -that fits your use case): - -1. **`buildOrgListCommand`** (team/repo list) — Fully automatic. Pagination - hints, cursor management, JSON envelope, and human formatting are all - handled internally. New simple org-scoped list commands should use this. - -2. **`dispatchOrgScopedList` with overrides** (project/issue list) — Automatic - for most modes; custom `"org-all"` override calls `resolveCursor` + - `advancePaginationState` + `paginationHint` manually. - -3. **`buildListCommand` with manual pagination** (trace/span/dashboard list) — - Command manages its own pagination loop. Must call `resolveCursor`, - `advancePaginationState`, `hasPreviousPage`, and `paginationHint` directly. - -**Auto-pagination for large limits:** - -When `--limit` exceeds `API_MAX_PER_PAGE` (100), list commands MUST transparently -fetch multiple pages to fill the requested limit. Cap `perPage` at -`Math.min(flags.limit, API_MAX_PER_PAGE)` and loop until `results.length >= limit` -or pages are exhausted. This matches the `listIssuesAllPages` pattern. - -```typescript -const perPage = Math.min(flags.limit, API_MAX_PER_PAGE); -for (let page = 0; page < MAX_PAGINATION_PAGES; page++) { - const { data, nextCursor } = await listPaginated(org, { perPage, cursor }); - results.push(...data); - if (results.length >= flags.limit || !nextCursor) break; - cursor = nextCursor; -} -``` - -Never pass a `per_page` value larger than `API_MAX_PER_PAGE` to the API — the -server silently caps it, causing the command to return fewer items than requested. - -Reference template: `trace/list.ts`, `span/list.ts`, `dashboard/list.ts` - -### ID Validation - -Use shared validators from `src/lib/hex-id.ts`: -- `validateHexId(value, label)` — 32-char hex IDs (trace IDs, log IDs). Auto-strips UUID dashes. -- `validateSpanId(value)` — 16-char hex span IDs. Auto-strips dashes. -- `validateTraceId(value)` — thin wrapper around `validateHexId` in `src/lib/trace-id.ts`. - -All normalize to lowercase. Throw `ValidationError` on invalid input. - -### Sort Convention - -Use `"date"` for timestamp-based sort (not `"time"`). Export sort types from the API layer (e.g., `SpanSortValue` from `api/traces.ts`), import in commands. This matches `issue list`, `trace list`, and `span list`. - -### Generated Docs & Skills - -All command docs and skill files are generated via `bun run generate:docs` (which runs `generate:command-docs` then `generate:skill`). This runs automatically as part of `dev`, `build`, `typecheck`, and `test` scripts. - -- **Command docs** (`docs/src/content/docs/commands/*.md`) are **gitignored** and generated from CLI metadata + hand-written fragments in `docs/src/fragments/commands/`. -- **Skill files** (`plugins/sentry-cli/skills/sentry-cli/`) are **committed** (consumed by external plugin systems) and auto-committed by CI when stale. -- Edit fragments in `docs/src/fragments/commands/` for custom examples and guides. -- `bun run check:fragments` validates fragment ↔ route consistency. -- Positional `placeholder` values must be descriptive: `"org/project/trace-id"` not `"args"`. - -### Zod Schemas for Validation - -All config and API types use Zod schemas: - -```typescript -import { z } from "zod"; - -export const MySchema = z.object({ - field: z.string(), - optional: z.number().optional(), -}); - -export type MyType = z.infer; - -// Validate data -const result = MySchema.safeParse(data); -if (result.success) { - // result.data is typed -} -``` - -### Type Organization - -- Define Zod schemas alongside types in `src/types/*.ts` -- Key type files: `sentry.ts` (API types), `config.ts` (configuration), `oauth.ts` (auth flow), `seer.ts` (Seer AI) -- Re-export from `src/types/index.ts` -- Use `type` imports: `import type { MyType } from "../types/index.js"` - -### SQL Utilities - -Use the `upsert()` helper from `src/lib/db/utils.ts` to reduce SQL boilerplate: - -```typescript -import { upsert, runUpsert } from "../db/utils.js"; - -// Generate UPSERT statement -const { sql, values } = upsert("table", { id: 1, name: "foo" }, ["id"]); -db.query(sql).run(...values); - -// Or use convenience wrapper -runUpsert(db, "table", { id: 1, name: "foo" }, ["id"]); - -// Exclude columns from update -const { sql, values } = upsert( - "users", - { id: 1, name: "Bob", created_at: now }, - ["id"], - { excludeFromUpdate: ["created_at"] } -); -``` - -### Error Handling - -All CLI errors extend the `CliError` base class from `src/lib/errors.ts`: - -```typescript -// Error hierarchy in src/lib/errors.ts -// Exit codes are defined in the EXIT constant object — use EXIT.* constants -// when constructing errors, never hardcode numeric exit codes outside errors.ts. -CliError (base, exitCode=1) -├── HostScopeError (exitCode=13) -├── ApiError (exitCode=30 — HTTP/API failures) -├── AuthError (exitCode=10–12 by reason — 'not_authenticated' | 'expired' | 'invalid') -├── ConfigError (exitCode=20 — configuration/DSN) -├── OutputError (exitCode=60 — data rendered, but operation failed) -├── ContextError (exitCode=22 — missing context) -├── ResolutionError (exitCode=23 — value provided but not found) -├── ValidationError (exitCode=21 — input validation) -├── DeviceFlowError (exitCode=51 — OAuth flow) -├── SeerError (exitCode=40–42 by reason — 'not_enabled' | 'no_budget' | 'ai_disabled') -├── TimeoutError (exitCode=31 — operation timed out) -├── UpgradeError (exitCode=50 — upgrade failures) -└── WizardError (exitCode=61–64 by workflow step — init wizard error) -``` - -> Exit code ranges: 1x=auth, 2x=input/config, 3x=API/network, 4x=feature/billing, -> 5x=operations, 6x=command-specific. See `EXIT` in `src/lib/errors.ts` and -> https://cli.sentry.dev/exit-codes/ for the full reference. - -**Choosing between ContextError, ResolutionError, and ValidationError:** - -| Scenario | Error Class | Example | -|----------|-------------|---------| -| User **omitted** a required value | `ContextError` | No org/project provided | -| User **provided** a value that wasn't found | `ResolutionError` | Project 'cli' not found | -| User input is **malformed** | `ValidationError` | Invalid hex ID format | - -**ContextError rules:** -- `command` must be a **single-line** CLI usage example (e.g., `"sentry org view "`) -- Constructor throws if `command` contains `\n` (catches misuse in tests) -- Pass `alternatives: []` when defaults are irrelevant (e.g., for missing Trace ID, Event ID) -- Use `" and "` in `resource` for plural grammar: `"Trace ID and span ID"` → "are required" - -**CI enforcement:** `bun run check:errors` scans for `ContextError` with multiline commands, `CliError` with ad-hoc "Try:" strings, and silent `catch` blocks (advisory). - -```typescript -// Usage examples -throw new ContextError("Organization", "sentry org view "); -throw new ContextError("Trace ID", "sentry trace view ", []); // no alternatives -throw new ResolutionError("Project 'cli'", "not found", "sentry issue list /cli", [ - "No project with this slug found in any accessible organization", -]); -throw new ValidationError("Invalid trace ID format", "traceId"); -``` - -**Fuzzy suggestions in resolution errors:** - -When a user-provided name/title doesn't match any entity, use `fuzzyMatch()` from -`src/lib/fuzzy.ts` to suggest similar candidates instead of listing all entities -(which can be overwhelming). Show at most 5 fuzzy matches. - -Reference: `resolveDashboardId()` in `src/commands/dashboard/resolve.ts`. - -### Catch Block Logging - -Silent `catch` blocks are prohibited in `src/` production code. Biome's `noEmptyBlockStatements` catches syntactically empty `catch {}` blocks, but blocks with only a `return` statement and no logging are equally problematic — errors vanish silently, making debugging impossible. - -Every `catch` block must either: -1. Re-throw the error -2. Log with `log.debug()` or `log.warn()` for diagnostic visibility -3. Return a fallback value **with** a `log.debug()` call explaining the suppression - -```typescript -// WRONG — error vanishes silently -try { data = await fetchOptionalData(); } -catch { return []; } - -// RIGHT — error is visible in debug logs -try { data = await fetchOptionalData(); } -catch (error) { - log.debug("Failed to fetch optional data", error); - return []; -} -``` - -Use `logger.withTag("command-name")` for tagged logging in command files. - -**CI enforcement:** `bun run check:errors` includes a silent-catch scan that flags -`catch` blocks which are empty, comment-only, or return-only without surfacing the -error. It is currently **advisory** (warns, does not fail CI) because of a pre-existing -backlog; run with `SENTRY_STRICT_SILENT_CATCH=1` to enforce. Do not add new silent -catches — they will appear in the scan output during review. - -### Auto-Recovery for Wrong Entity Types - -When a user provides the wrong type of identifier (e.g., an issue short ID -where a trace ID is expected), commands should **auto-recover** when the -user's intent is unambiguous: - -1. **Detect** the actual entity type using helpers like `looksLikeIssueShortId()`, - `SPAN_ID_RE`, `HEX_ID_RE`, or non-hex character checks. -2. **Resolve** the input to the correct type (e.g., issue → latest event → trace ID). -3. **Warn** via `log.warn()` explaining what happened. -4. **Show** the result with a return `hint` nudging toward the correct command. - -When recovery is **ambiguous or impossible**, keep the existing error but add -entity-aware suggestions (e.g., "This looks like a span ID"). - -**Detection helpers:** -- `looksLikeIssueShortId(value)` — uppercase dash-separated (e.g., `CLI-G5`) -- `SPAN_ID_RE.test(value)` — 16-char hex (span ID) -- `HEX_ID_RE.test(value)` — 32-char hex (trace/event/log ID) -- `/[^0-9a-f]/.test(normalized)` — non-hex characters → likely a slug/name - -**Reference implementations:** -- `event/view.ts` — issue short ID → latest event redirect -- `span/view.ts` — `traceId/spanId` slash format → auto-split -- `trace/view.ts` — issue short ID → issue's trace redirect -- `hex-id.ts` — entity-aware error hints in `validateHexId`/`validateSpanId` - -### Async Config Functions - -All config operations are async. Always await: - -```typescript -const token = await getAuthToken(); -const isAuth = await isAuthenticated(); -await setAuthToken(token, expiresIn); -``` - -### Adding New Utility Files - -Before creating a new `src/lib/*.ts` utility file, check whether existing shared modules already cover your use case: - -| If you need... | Check first... | -|----------------|---------------| -| Duration formatting | `src/lib/formatters/time-utils.ts` (`formatDurationCompact`, `formatDurationVerbose`) | -| Hex ID validation/normalization | `src/lib/hex-id.ts` (`validateHexId`, `tryNormalizeHexId`, `normalizeHexId`) | -| Relative time display | `src/lib/formatters/time-utils.ts` (`formatRelativeTime`) | -| Table/markdown output | `src/lib/formatters/` directory | -| Pagination | `src/lib/db/pagination.ts`, `src/lib/list-command.ts` | -| Error classes | `src/lib/errors.ts` (never create ad-hoc error types) | -| Search query building | `src/lib/search-query.ts`, `src/lib/arg-parsing.ts` | - -If an existing module covers ≥80% of what you need, extend it with new exported functions rather than creating a new file. New files are appropriate when the domain is genuinely new (e.g., `replay-search.ts` for replay-specific field resolution). - -Every new `src/lib/**/*.ts` file must start with a module-level JSDoc comment describing the module's purpose. - -### Imports - -- Use `.js` extension for local imports (ESM requirement) -- Group: external packages first, then local imports -- Use `type` keyword for type-only imports - -```typescript -import { z } from "zod"; -import { buildCommand } from "../../lib/command.js"; -import type { SentryContext } from "../../context.js"; -import { getAuthToken } from "../../lib/config.js"; -``` - -### List Command Infrastructure - -Two abstraction levels exist for list commands: - -1. **`src/lib/list-command.ts`** — `buildOrgListCommand` factory + shared Stricli parameter constants (`LIST_TARGET_POSITIONAL`, `LIST_JSON_FLAG`, `LIST_CURSOR_FLAG`, `buildListLimitFlag`). Use this for simple entity lists like `team list` and `repo list`. - -2. **`src/lib/org-list.ts`** — `dispatchOrgScopedList` with `OrgListConfig` and a 4-mode handler map: `auto-detect`, `explicit`, `org-all`, `project-search`. Complex commands (`project list`, `issue list`) call `dispatchOrgScopedList` with an `overrides` map directly instead of using `buildOrgListCommand`. - -Key rules when writing overrides: -- Each mode handler receives a `HandlerContext` with the narrowed `parsed` plus shared I/O (`stdout`, `cwd`, `flags`). Access parsed fields via `ctx.parsed.org`, `ctx.parsed.projectSlug`, etc. — no manual `Extract<>` casts needed. -- Commands with extra fields (e.g., `stderr`, `setContext`) spread the context and add them: `(ctx) => handle({ ...ctx, flags, stderr, setContext })`. Override `ctx.flags` with the command-specific flags type when needed. -- `resolveCursor()` must be called **inside** the `org-all` override closure, not before `dispatchOrgScopedList`, so that `--cursor` validation errors fire correctly for non-org-all modes. -- `handleProjectSearch` errors must use `"Project"` as the `ContextError` resource, not `config.entityName`. -- Always set `orgSlugMatchBehavior` on `dispatchOrgScopedList` to declare how bare-slug org matches are handled. Use `"redirect"` for commands where listing all entities in the org makes sense (e.g., `project list`, `team list`, `issue list`). Use `"error"` for commands where org-all redirect is inappropriate. The pre-check uses cached orgs to avoid N API calls — when the cache is cold, the handler's own org-slug check serves as a safety net (throws `ResolutionError` with a hint). - -3. **Standalone list commands** (e.g., `span list`, `trace list`) that don't use org-scoped dispatch wire pagination directly in `func()`. See the "List Command Pagination" section above for the pattern. - -### Project Filtering in API Calls - -Different Sentry API endpoints use different project filtering mechanisms. Never apply both simultaneously: - -| API Endpoint | Project filter | Helper | -|-------------|---------------|--------| -| Discover/Events (`queryEvents`) | `project:` in query string | `buildProjectQuery()` | -| Replay index (`listReplays`) | `projectSlugs` parameter | Direct parameter | -| Issue index (`listIssuesPaginated`) | `project` parameter or query string | Varies by mode | - -When adding a new dataset to `explore`, verify which filtering mechanism the underlying API expects and handle it in `resolveDatasetConfig`. The `explore` command centralizes dataset-specific behavior (sort, query, fetch, field validation) in `resolveDatasetConfig` — add new datasets there rather than scattering `if (dataset === ...)` checks through the `func` body. - -## Commenting & Documentation (JSDoc-first) - -### Default Rule -- **Prefer JSDoc over inline comments.** -- Code should be readable without narrating what it already says. - -### Required: JSDoc -Add JSDoc comments on: -- **Every exported function, class, and type** (and important internal ones). -- **Types/interfaces**: document each field/property (what it represents, units, allowed values, meaning of `null`, defaults). - -Include in JSDoc: -- What it does -- Key business rules / constraints -- Assumptions and edge cases -- Side effects -- Why it exists (when non-obvious) - -### Inline Comments (rare) -Inline comments are **allowed only** when they add information the code cannot express: -- **"Why"** - business reason, constraint, historical context -- **Non-obvious behavior** - surprising edge cases -- **Workarounds** - bugs in dependencies, platform quirks -- **Hardcoded values** - why hardcoded, what would break if changed - -Inline comments are **NOT allowed** if they just restate the code: -```typescript -// Bad: -if (!person) // if no person -i++ // increment i -return result // return result - -// Good: -// Required by GDPR Article 17 - user requested deletion -await deleteUserData(userId) -``` - -### Prohibited Comment Styles -- **ASCII art section dividers** - Do not use decorative box-drawing characters like `─────────` to create section headers. Use standard JSDoc comments or simple `// Section Name` comments instead. - -### Goal -Minimal comments, maximum clarity. Comments explain **intent and reasoning**, not syntax. - -## Testing (bun:test + fast-check) - -**Prefer property-based and model-based testing** over traditional unit tests. These approaches find edge cases automatically and provide better coverage with less code. - -**fast-check Documentation**: https://fast-check.dev/docs/core-blocks/arbitraries/ - -### Testing Hierarchy (in order of preference) - -1. **Model-Based Tests** - For stateful systems (database, caches, state machines) -2. **Property-Based Tests** - For pure functions, parsing, validation, transformations -3. **Unit Tests** - Only for trivial cases or when properties are hard to express - -### Test File Naming - -| Type | Pattern | Location | -|------|---------|----------| -| Property-based | `*.property.test.ts` | `test/lib/` | -| Model-based | `*.model-based.test.ts` | `test/lib/db/` | -| Unit tests | `*.test.ts` | `test/` (mirrors `src/`) | -| E2E tests | `*.test.ts` | `test/e2e/` | - -### Test Environment Isolation (CRITICAL) - -Tests that need a database or config directory **must** use `useTestConfigDir()` from `test/helpers.ts`. This helper: -- Creates a unique temp directory in `beforeEach` -- Sets `SENTRY_CONFIG_DIR` to point at it -- **Restores** (never deletes) the env var in `afterEach` -- Closes the database and cleans up temp files - -**NEVER** do any of these in test files: -- `delete process.env.SENTRY_CONFIG_DIR` — This pollutes other test files that load after yours -- `const baseDir = process.env[CONFIG_DIR_ENV_VAR]!` at module scope — This captures a value that may be stale -- Manual `beforeEach`/`afterEach` that sets/deletes `SENTRY_CONFIG_DIR` - -**Why**: Bun's test runner uses `--isolate --parallel` (see `test:unit` in `package.json`), so each test file runs in a fresh global environment within a worker process. That bounds most cross-file leaks to a single worker, but `process.env` is still shared within a file's lifecycle — if your `afterEach` deletes the env var, the next describe/test's module-level code (or a beforeEach that re-reads env) gets `undefined`, causing `TypeError: The "paths[0]" property must be of type string`. Also, `TEST_TMP_DIR` is namespaced by `BUN_TEST_WORKER_ID` in `test/constants.ts` so parallel workers don't wipe each other's temp state during preload. - -```typescript -// CORRECT: Use the helper -import { useTestConfigDir } from "../helpers.js"; - -const getConfigDir = useTestConfigDir("my-test-prefix-"); - -// If you need the directory path in a test: -test("example", () => { - const dir = getConfigDir(); -}); - -// WRONG: Manual env var management -beforeEach(() => { process.env.SENTRY_CONFIG_DIR = tmpDir; }); -afterEach(() => { delete process.env.SENTRY_CONFIG_DIR; }); // BUG! -``` - -### Property-Based Testing - -Use property-based tests when verifying invariants that should hold for **any valid input**. - -```typescript -import { describe, expect, test } from "bun:test"; -import { constantFrom, assert as fcAssert, property, tuple } from "fast-check"; -import { DEFAULT_NUM_RUNS } from "../model-based/helpers.js"; - -// Define arbitraries (random data generators) -const slugArb = array(constantFrom(..."abcdefghijklmnopqrstuvwxyz0123456789".split("")), { - minLength: 1, - maxLength: 15, -}).map((chars) => chars.join("")); - -describe("property: myFunction", () => { - test("is symmetric", () => { - fcAssert( - property(slugArb, slugArb, (a, b) => { - // Properties should always hold regardless of input - expect(myFunction(a, b)).toBe(myFunction(b, a)); - }), - { numRuns: DEFAULT_NUM_RUNS } - ); - }); - - test("round-trip: encode then decode returns original", () => { - fcAssert( - property(validInputArb, (input) => { - const encoded = encode(input); - const decoded = decode(encoded); - expect(decoded).toEqual(input); - }), - { numRuns: DEFAULT_NUM_RUNS } - ); - }); -}); -``` - -**Good candidates for property-based testing:** -- Parsing functions (DSN, issue IDs, aliases) -- Encoding/decoding (round-trip invariant) -- Symmetric operations (a op b = b op a) -- Idempotent operations (f(f(x)) = f(x)) -- Validation functions (valid inputs accepted, invalid rejected) - -**See examples:** `test/lib/dsn.property.test.ts`, `test/lib/alias.property.test.ts`, `test/lib/issue-id.property.test.ts` - -### Model-Based Testing - -Use model-based tests for **stateful systems** where sequences of operations should maintain invariants. - -```typescript -import { describe, expect, test } from "bun:test"; -import { - type AsyncCommand, - asyncModelRun, - asyncProperty, - commands, - assert as fcAssert, -} from "fast-check"; -import { createIsolatedDbContext, DEFAULT_NUM_RUNS } from "../../model-based/helpers.js"; - -// Define a simplified model of expected state -type DbModel = { - entries: Map; -}; - -// Define commands that operate on both model and real system -class SetCommand implements AsyncCommand { - constructor(readonly key: string, readonly value: string) {} - - check = () => true; - - async run(model: DbModel, real: RealDb): Promise { - // Apply to real system - await realSet(this.key, this.value); - - // Update model - model.entries.set(this.key, this.value); - } - - toString = () => `set("${this.key}", "${this.value}")`; -} - -class GetCommand implements AsyncCommand { - constructor(readonly key: string) {} - - check = () => true; - - async run(model: DbModel, real: RealDb): Promise { - const realValue = await realGet(this.key); - const expectedValue = model.entries.get(this.key); - - // Verify real system matches model - expect(realValue).toBe(expectedValue); - } - - toString = () => `get("${this.key}")`; -} - -describe("model-based: database", () => { - test("random sequences maintain consistency", () => { - fcAssert( - asyncProperty(commands(allCommandArbs), async (cmds) => { - const cleanup = createIsolatedDbContext(); - try { - await asyncModelRun( - () => ({ model: { entries: new Map() }, real: {} }), - cmds - ); - } finally { - cleanup(); - } - }), - { numRuns: DEFAULT_NUM_RUNS } - ); - }); -}); -``` - -**Good candidates for model-based testing:** -- Database operations (auth, caches, regions) -- Stateful caches with invalidation -- Systems with cross-cutting invariants (e.g., clearAuth also clears regions) - -**See examples:** `test/lib/db/model-based.test.ts`, `test/lib/db/dsn-cache.model-based.test.ts` - -### Test Helpers - -Use `test/model-based/helpers.ts` for shared utilities: - -```typescript -import { createIsolatedDbContext, DEFAULT_NUM_RUNS } from "../model-based/helpers.js"; - -// Create isolated DB for each test run (prevents interference) -const cleanup = createIsolatedDbContext(); -try { - // ... test code -} finally { - cleanup(); -} - -// Use consistent number of runs across tests -fcAssert(property(...), { numRuns: DEFAULT_NUM_RUNS }); // 50 runs -``` - -### When to Use Unit Tests - -Use traditional unit tests only when: -- Testing trivial logic with obvious expected values -- Properties are difficult to express or would be tautological -- Testing error messages or specific output formatting -- Integration with external systems (E2E tests) - -### Avoiding Unit/Property Test Duplication - -When a `*.property.test.ts` file exists for a module, **do not add unit tests that re-check the same invariants** with hardcoded examples. Before adding a unit test, check whether the companion property file already generates random inputs for that invariant. - -**Unit tests that belong alongside property tests:** -- Edge cases outside the property generator's range (e.g., self-hosted DSNs when the arbitrary only produces SaaS ones) -- Specific output format documentation (exact strings, column layouts, rendered vs plain mode) -- Concurrency/timing behavior that property tests cannot express -- Integration tests exercising multiple functions together (e.g., `writeJsonList` envelope shape) - -**Unit tests to avoid when property tests exist:** -- "returns true for valid input" / "returns false for invalid input" — the property test already covers this with random inputs -- Basic round-trip assertions — property tests check `decode(encode(x)) === x` for all `x` -- Hardcoded examples of invariants like idempotency, symmetry, or subset relationships - -When adding property tests for a function that already has unit tests, **remove the unit tests that become redundant**. Add a header comment to the unit test file noting which invariants live in the property file: - -```typescript -/** - * Note: Core invariants (round-trips, validation, ordering) are tested via - * property-based tests in foo.property.test.ts. These tests focus on edge - * cases and specific output formatting not covered by property generators. - */ -``` - -```typescript -import { describe, expect, test, mock } from "bun:test"; - -describe("feature", () => { - test("should return specific value", async () => { - expect(await someFunction("input")).toBe("expected output"); - }); -}); - -// Mock modules when needed -mock.module("./some-module", () => ({ - default: () => "mocked", -})); -``` - -## File Locations - -| What | Where | -|------|-------| -| Add new command | `src/commands//` | -| Add API types | `src/types/sentry.ts` | -| Add config types | `src/types/config.ts` | -| Add Seer types | `src/types/seer.ts` | -| Add utility | `src/lib/` | -| Add DSN language support | `src/lib/dsn/languages/` | -| Add DB operations | `src/lib/db/` | -| Build scripts | `script/` | -| Add property tests | `test/lib/.property.test.ts` | -| Add model-based tests | `test/lib/db/.model-based.test.ts` | -| Add unit tests | `test/` (mirror `src/` structure) | -| Add E2E tests | `test/e2e/` | -| Test helpers | `test/model-based/helpers.ts` | -| Add documentation | `docs/src/content/docs/` | -| Hand-written command doc content | `docs/src/fragments/commands/` | - -## Automated Fix PRs (BugBot / agents) - -Automated bug-fix PRs (e.g. Cursor BugBot) must follow these rules to avoid the -duplication and staleness that caused five overlapping PRs to pile up: - -1. **Check for existing work first.** Before opening a PR, search open PRs and - recently-closed PRs/issues for the same file + symbol: - ```bash - gh pr list --state open --search "in:title " - gh issue list --state all --search "" - ``` - If an open PR already touches the target function, **comment on it** or extend - it instead of opening a duplicate. Multiple BugBot PRs independently re-fixed - the same `JSON.parse` guard, `withTTY` helper, and pagination code. - -2. **Rebase before review.** A PR that is many commits behind `main` may fail CI - on unrelated drift (e.g. a lint error in a file the PR never touched) and its - fix may already be superseded. Rebase onto `main` and re-verify the bug still - exists before requesting review. Verify against current `main`, not the - snapshot the PR was generated from. - -3. **Separate correctness fixes from opinion.** A real bug (wrong output, crash, - skipped data) is in scope. A subjective UX change (different hint wording, - different default) is **not** a bug — `main`'s current behavior is often - deliberate. Do not bundle UX opinions into bug-fix PRs; they waste review - cycles and are usually dropped. - -4. **Prefer shared helpers over re-deriving fixes.** If a correct implementation - already exists (e.g. `autoPaginate()` for pagination, `safeParseJson()` for - cached JSON), use it rather than hand-rolling a one-off fix. The recurring - pagination-overshoot and parse-crash bugs were classes solved once centrally. - ## Long-term Knowledge diff --git a/README.md b/README.md index 3118cfc309..a201255c5b 100644 --- a/README.md +++ b/README.md @@ -1,166 +1,27 @@ -

- Sentry CLI -

+# Sentry Toolkit (pre-shape) -

- The command-line interface for Sentry. Built for developers and AI agents. -

+This repository is being reshaped into a pnpm-workspace monorepo ahead of merging +the Sentry CLI and Sentry MCP server into a single `getsentry/toolkit` monorepo. -

- Documentation | - Getting Started | - Commands -

+## Workspace layout ---- - -## Installation - -### Install Script (Recommended) - -```bash -curl https://cli.sentry.dev/install -fsS | bash -``` - -### Homebrew - -```bash -brew install getsentry/tools/sentry -``` - -### Package Managers - -```bash -npm install -g sentry -pnpm add -g sentry -yarn global add sentry -bun add -g sentry -``` - -> The npm/pnpm/yarn packages require Node.js 22.15+. - -### Run Without Installing - -```bash -npx sentry@latest -pnpm dlx sentry --help -yarn dlx sentry --help -bunx sentry --help -``` - -## Quick Start - -```bash -# Authenticate with Sentry -sentry auth login - -# List issues (auto-detects project from your codebase) -sentry issue list - -# Get AI-powered root cause analysis -sentry issue explain PROJ-ABC - -# Generate a fix plan -sentry issue plan PROJ-ABC -``` - -## Features - -- **DSN Auto-Detection** - Automatically detects your project from `.env` files or source code. No flags needed. -- **Seer AI Integration** - Get root cause analysis and fix plans directly in your terminal. -- **Monorepo Support** - Works with multiple projects, generates short aliases for easy navigation. -- **JSON Output** - All commands support `--json` for scripting and pipelines. -- **Open in Browser** - Use `-w` flag to open any resource in your browser. - -## Commands - -Run `sentry --help` to see all available commands, or browse the [command reference](https://cli.sentry.dev/commands/). - -## Configuration - -Credentials are stored in `~/.sentry/` with restricted permissions (mode 600). - -## Library Usage - - -Use Sentry CLI programmatically in Node.js (≥22.15) without spawning a subprocess: - - -```typescript -import createSentrySDK from "sentry"; - -const sdk = createSentrySDK({ token: "sntrys_..." }); - -// Typed methods for every CLI command -const orgs = await sdk.org.list(); -const issues = await sdk.issue.list({ orgProject: "acme/frontend", limit: 5 }); -const issue = await sdk.issue.view({ issue: "ACME-123" }); - -// Nested commands -await sdk.dashboard.widget.add({ display: "line", query: "count" }, "my-org/my-dashboard"); - -// Escape hatch for any CLI command -const version = await sdk.run("--version"); -const text = await sdk.run("issue", "list", "-l", "5"); -``` - -Options (all optional): -- `token` — Auth token. Falls back to `SENTRY_AUTH_TOKEN` / `SENTRY_TOKEN` env vars. -- `url` — Sentry instance URL for self-hosted (e.g., `"sentry.example.com"`). -- `org` — Default organization slug (avoids passing it on every call). -- `project` — Default project slug. -- `text` — Return human-readable string instead of parsed JSON (affects `run()` only). -- `cwd` — Working directory for DSN auto-detection. Defaults to `process.cwd()`. -- `signal` — `AbortSignal` to cancel streaming commands (`--follow`, `--refresh`). - -Streaming commands return `AsyncIterable` — use `for await...of` and `break` to stop. - -Errors are thrown as `SentryError` with `.exitCode` and `.stderr`. - ---- +- [`packages/cli/`](./packages/cli) — the Sentry CLI (npm package `sentry`, + binary `sentry`). See [packages/cli/README.md](./packages/cli/README.md). +- [`apps/cli-docs/`](./apps/cli-docs) — the CLI documentation site + (Astro + Starlight, published to `cli.sentry.dev`). ## Development -### Prerequisites - - -- [Node.js](https://nodejs.org) v22.15+ and [pnpm](https://pnpm.io) v10.11+ - - -### Setup - -```bash -git clone https://github.com/getsentry/cli.git -cd cli -pnpm install -``` - -### Running Locally +This is a [pnpm workspace](https://pnpm.io/workspaces). Common tasks are exposed +as delegating scripts at the root and forwarded to the relevant package: -```bash -# Run CLI in development mode -pnpm run cli -- --help - -# With environment variables (create .env.local first, see DEVELOPMENT.md) -pnpm run cli -- --help -``` - -### Scripts - - -```bash -pnpm run build # Build for current platform -pnpm run typecheck # Type checking -pnpm run lint # Check for issues -pnpm run lint:fix # Auto-fix issues -pnpm run test:unit # Run unit tests -pnpm run test:e2e # Run end-to-end tests -pnpm run generate:docs # Regenerate command docs and skills +```sh +pnpm install # install all workspace packages +pnpm run build # build the CLI package +pnpm run typecheck # typecheck the CLI package +pnpm run lint # lint +pnpm run test # run the CLI unit tests ``` - - -See [DEVELOPMENT.md](DEVELOPMENT.md) for detailed setup and [CONTRIBUTING.md](CONTRIBUTING.md) for contribution guidelines. - -## License -[FSL-1.1-Apache-2.0](LICENSE.md) +To work directly within a package, use pnpm filters, e.g. +`pnpm --filter sentry run