Skip to content

Commit 27b35bb

Browse files
committed
chore: address adversarial review findings for monorepo pre-shape
Follow-ups from the PR #1254 review (no behavior change to shipped CLI): - check-patches.ts: harden resolvePackageFile() to derive the package root from the FIRST `<pkgName>/` after the last `node_modules/` boundary, instead of `lastIndexOf('/<pkgName>/')`. A package that vendors a nested copy of itself would otherwise truncate at the wrong inner segment. Also refresh the stale `node-linker=hoisted` comment. - biome.jsonc / packages/cli/.gitignore: reconcile the contradictory comments about which .gitignore Biome reads. Biome honors the workspace-root .gitignore via `vcs.root: "../.."`; the package-level .gitignore is for plain git only. Drop the dead `!docs` ignore (no packages/cli/docs after the move). - build.ts / bundle.ts: fix comments that still claimed deps are "hoisted to the workspace-root node_modules" — under node-linker=isolated they live in the isolated store; the code (module resolution / `pnpm exec`) is unchanged and correct. - packages/cli/AGENTS.md: replace stale Bun-era guidance with the current pnpm/Node/Vitest reality — command reference, "Use Node.js APIs" section, test-runner isolation notes (VITEST_POOL_ID), vitest imports and `vi.mock`, moved docs path (apps/cli-docs), and removed the dangling `.cursor/rules/bun-cli.mdc` reference. Verified: typecheck, lint, all check:* scripts, build (+ binary smoke).
1 parent 410c3e9 commit 27b35bb

8 files changed

Lines changed: 99 additions & 77 deletions

File tree

.lore.md

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -435,6 +435,9 @@
435435
<!-- lore:019f6507-7efb-70c8-b243-5f87f6234fbc -->
436436
* **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/\<scope>-\<issue>\`), 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.
437437

438+
<!-- lore:019f668c-39f6-72c9-b8b7-43a1f61ecb8d -->
439+
* **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.
440+
438441
<!-- lore:019f6660-35d8-77f5-a523-b722e9b32c98 -->
439442
* **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.
440443

@@ -445,7 +448,7 @@
445448
* **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.
446449

447450
<!-- lore:019f6648-b59d-70e9-b626-40db33aae377 -->
448-
* **Always Monitor CI Status**: The user consistently checks and monitors the status of CI (Continuous Integration) runs for their pull requests. This includes tracking the progress of various checks, such as linting, testing, and code analysis, across multiple repositories like \`getsentry/cli\` and \`getsentry/symbolic\`. The user seems to be interested in ensuring that all checks pass before proceeding with the merge or further actions.
451+
* **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.
449452

450453
<!-- lore:019f666a-ac76-7581-82f8-8d2af3f3e428 -->
451454
* **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.

AGENTS.md

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
<!-- This section is maintained by the coding agent via lore (https://github.com/BYK/loreai) -->
2+
## Long-term Knowledge
3+
4+
For long-term knowledge entries managed by [lore](https://github.com/BYK/loreai) (gotchas, patterns, decisions, architecture), see [`.lore.md`](.lore.md) in the project root.
5+
<!-- End lore-managed section -->

packages/cli/.gitignore

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,7 @@
1-
# Package-level ignores (see the workspace-root .gitignore for shared rules).
2-
# Duplicated here because Biome resolves useIgnoreFile relative to biome.jsonc.
1+
# Package-level ignores for git. The workspace-root .gitignore mirrors these
2+
# with `packages/cli/`-prefixed patterns (that root file is what Biome reads,
3+
# since biome.jsonc sets `vcs.root: "../.."`). This file exists so plain `git`
4+
# ignores build artifacts within the package regardless of the root file.
35

46
# dependencies
57
node_modules

packages/cli/AGENTS.md

Lines changed: 55 additions & 57 deletions
Original file line numberDiff line numberDiff line change
@@ -4,15 +4,15 @@ Guidelines for AI agents working in this codebase.
44

55
## Project Overview
66

7-
**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/).
7+
**Sentry CLI** is a command-line interface for [Sentry](https://sentry.io), built with Node.js, [pnpm](https://pnpm.io), and [Stricli](https://bloomberg.github.io/stricli/). It lives in `packages/cli` of a pnpm workspace; the docs site lives in `apps/cli-docs`.
88

99
### Goals
1010

1111
- **Zero-config experience** - Auto-detect project context from DSNs in source code and env files
1212
- **AI-powered debugging** - Integrate Seer AI for root cause analysis and fix plans
1313
- **Developer-friendly** - Follow `gh` CLI conventions for intuitive UX
1414
- **Agent-friendly** - JSON output and predictable behavior for AI coding agents
15-
- **Fast** - Native binaries via Bun, SQLite caching for API responses
15+
- **Fast** - Native binaries via Node SEA (fossilize), SQLite caching for API responses
1616

1717
### Key Features
1818

@@ -28,86 +28,83 @@ Guidelines for AI agents working in this codebase.
2828

2929
Before working on this codebase, read the Cursor rules:
3030

31-
- **`.cursor/rules/bun-cli.mdc`** - Bun API usage, file I/O, process spawning, testing
3231
- **`.cursor/rules/ultracite.mdc`** - Code style, formatting, linting rules
3332

3433
## Quick Reference: Commands
3534

36-
> **Note**: Always check `package.json` for the latest scripts.
35+
> **Note**: Always check `package.json` for the latest scripts. All commands run
36+
> from `packages/cli` (or via `pnpm --filter sentry run <script>` from the repo root).
3737
3838
```bash
3939
# Development
40-
bun install # Install dependencies
41-
bun run dev # Run CLI in dev mode
42-
bun run --env-file=.env.local src/bin.ts # Dev with env vars
40+
pnpm install # Install dependencies (from repo root)
41+
pnpm run dev # Run CLI in dev mode
42+
pnpm run cli -- <args> # Run the CLI with arguments
4343

4444
# Build
45-
bun run build # Build for current platform
46-
bun run build:all # Build for all platforms
45+
pnpm run build # Build for current platform
46+
pnpm run build:all # Build for all platforms
4747

4848
# Type Checking
49-
bun run typecheck # Check types
49+
pnpm run typecheck # Check types (tsc)
5050

5151
# Linting & Formatting
52-
bun run lint # Check for issues
53-
bun run lint:fix # Auto-fix issues (run before committing)
54-
55-
# Testing
56-
bun test # Run all tests
57-
bun test path/to/file.test.ts # Run single test file
58-
bun test --watch # Watch mode
59-
bun test --filter "test name" # Run tests matching pattern
60-
bun run test:unit # Run unit tests only
61-
bun run test:e2e # Run e2e tests only
52+
pnpm run lint # Check for issues (Biome)
53+
pnpm run lint:fix # Auto-fix issues (run before committing)
54+
55+
# Testing (Vitest)
56+
pnpm run test:unit # Run unit tests
57+
pnpm run test:e2e # Run e2e tests
58+
pnpm exec vitest run test/lib/foo.test.ts # Run a single test file (skips generate steps)
59+
pnpm exec vitest # Watch mode
6260
```
6361

6462
## Rules: No Runtime Dependencies
6563

66-
**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`.
64+
**CRITICAL**: All packages must be in `devDependencies`, never `dependencies`. Everything is bundled at build time via esbuild. CI enforces this with `pnpm run check:deps`.
6765

68-
When adding a package, always use `bun add -d <package>` (the `-d` flag).
66+
When adding a package, always use `pnpm add -D <package>` (the `-D` flag).
6967

7068
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`.
7169

72-
## Rules: Use Bun APIs
70+
## Rules: Use Node.js APIs
7371

74-
**CRITICAL**: This project uses Bun as runtime. Always prefer Bun-native APIs over Node.js equivalents.
72+
**CRITICAL**: This project runs on Node.js. Use Node.js standard-library
73+
(`node:*`) APIs; there is no Bun runtime. (This project was migrated from Bun to
74+
Node — do not reintroduce `Bun.*` globals, `bun:test`, or `bun` CLI commands.)
7575

76-
Read the full guidelines in `.cursor/rules/bun-cli.mdc`.
76+
### Quick API Reference
7777

78-
**Bun Documentation**: https://bun.sh/docs - Consult these docs when unsure about Bun APIs.
78+
| Task | Use This |
79+
|------|----------|
80+
| Read file | `await readFile(path, "utf-8")` (`node:fs/promises`) |
81+
| Write file | `await writeFile(path, content)` (`node:fs/promises`) |
82+
| Check file exists | `existsSync(path)` (`node:fs`) |
83+
| Spawn process | `spawn()` / `execFile()` (`node:child_process`) |
84+
| Shell commands | `execSync()` (`node:child_process`) |
85+
| Find executable | `which` helper in `src/lib/which.ts` |
86+
| Glob patterns | the scanner in `src/lib/scan/` |
87+
| Sleep | `await setTimeout(ms)` (`node:timers/promises`) |
88+
| Parse JSON file | `JSON.parse(await readFile(path, "utf-8"))` |
7989

80-
### Quick Bun API Reference
81-
82-
| Task | Use This | NOT This |
83-
|------|----------|----------|
84-
| Read file | `await Bun.file(path).text()` | `fs.readFileSync()` |
85-
| Write file | `await Bun.write(path, content)` | `fs.writeFileSync()` |
86-
| Check file exists | `await Bun.file(path).exists()` | `fs.existsSync()` |
87-
| Spawn process | `Bun.spawn()` | `child_process.spawn()` |
88-
| Shell commands | `Bun.$\`command\`` ⚠️ | `child_process.exec()` |
89-
| Find executable | `Bun.which("git")` | `which` package |
90-
| Glob patterns | `new Bun.Glob()` | `glob` / `fast-glob` packages |
91-
| Sleep | `await Bun.sleep(ms)` | `setTimeout` with Promise |
92-
| Parse JSON file | `await Bun.file(path).json()` | Read + JSON.parse |
93-
94-
**Exception**: Use `node:fs` for directory creation with permissions:
90+
Use `node:fs` for directory creation with permissions:
9591
```typescript
9692
import { mkdirSync } from "node:fs";
9793
mkdirSync(dir, { recursive: true, mode: 0o700 });
9894
```
9995

100-
**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:
96+
For shell commands, prefer `execFileSync` with array args over `execSync` with a
97+
string when passing user-controlled values, to avoid shell injection:
10198
```typescript
102-
import { execSync } from "node:child_process";
103-
const result = execSync("id -u username", { encoding: "utf-8", stdio: ["pipe", "pipe", "ignore"] });
99+
import { execFileSync } from "node:child_process";
100+
const result = execFileSync("id", ["-u", "username"], { encoding: "utf-8", stdio: ["pipe", "pipe", "ignore"] });
104101
```
105102

106103
## Architecture
107104

108105
The full project-structure tree — including the live command/subcommand list and the
109106
domain API modules — is generated from the route tree and lives in
110-
[`docs/src/content/docs/contributing.md`](docs/src/content/docs/contributing.md)
107+
[`apps/cli-docs/src/content/docs/contributing.md`](apps/cli-docs/src/content/docs/contributing.md)
111108
(the `project-structure` block produced by `script/generate-docs-sections.ts`). It is
112109
kept in sync automatically, so it is **not** duplicated here to avoid drift. For the
113110
current command list run `ls src/commands/` or `sentry --help`.
@@ -127,8 +124,9 @@ Top-level layout:
127124
- **`src/types/`** — TypeScript types and Zod schemas.
128125
- **`test/`** — tests mirroring `src/` (unit, `*.property.test.ts`,
129126
`*.model-based.test.ts`, `e2e/`, `fixtures/`, `mocks/`).
130-
- **`docs/`** — documentation site (Astro + Starlight); **`script/`** — build/utility
131-
scripts; **`.cursor/rules/`** — Cursor AI rules; **`biome.jsonc`** — lint config.
127+
- **`../../apps/cli-docs/`** — documentation site (Astro + Starlight);
128+
**`script/`** — build/utility scripts; **`.cursor/rules/`** — Cursor AI rules;
129+
**`biome.jsonc`** — lint config.
132130

133131
## Key Patterns
134132

@@ -409,12 +407,12 @@ Use `"date"` for timestamp-based sort (not `"time"`). Export sort types from the
409407

410408
### Generated Docs & Skills
411409

412-
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.
410+
All command docs and skill files are generated via `pnpm run generate:docs` (which runs `generate:command-docs` then `generate:skill`). This runs automatically as part of `dev`, `build`, `typecheck`, and `test` scripts.
413411

414412
- **Command docs** (`docs/src/content/docs/commands/*.md`) are **gitignored** and generated from CLI metadata + hand-written fragments in `docs/src/fragments/commands/`.
415413
- **Skill files** (`plugins/sentry-cli/skills/sentry-cli/`) are **committed** (consumed by external plugin systems) and auto-committed by CI when stale.
416414
- Edit fragments in `docs/src/fragments/commands/` for custom examples and guides.
417-
- `bun run check:fragments` validates fragment ↔ route consistency.
415+
- `pnpm run check:fragments` validates fragment ↔ route consistency.
418416
- Positional `placeholder` values must be descriptive: `"org/project/trace-id"` not `"args"`.
419417

420418
### Zod Schemas for Validation
@@ -510,7 +508,7 @@ CliError (base, exitCode=1)
510508
- Pass `alternatives: []` when defaults are irrelevant (e.g., for missing Trace ID, Event ID)
511509
- Use `" and "` in `resource` for plural grammar: `"Trace ID and span ID"` → "are required"
512510

513-
**CI enforcement:** `bun run check:errors` scans for `ContextError` with multiline commands, `CliError` with ad-hoc "Try:" strings, and silent `catch` blocks (advisory).
511+
**CI enforcement:** `pnpm run check:errors` scans for `ContextError` with multiline commands, `CliError` with ad-hoc "Try:" strings, and silent `catch` blocks (advisory).
514512

515513
```typescript
516514
// Usage examples
@@ -554,7 +552,7 @@ catch (error) {
554552

555553
Use `logger.withTag("command-name")` for tagged logging in command files.
556554

557-
**CI enforcement:** `bun run check:errors` includes a silent-catch scan that flags
555+
**CI enforcement:** `pnpm run check:errors` includes a silent-catch scan that flags
558556
`catch` blocks which are empty, comment-only, or return-only without surfacing the
559557
error. It is currently **advisory** (warns, does not fail CI) because of a pre-existing
560558
backlog; run with `SENTRY_STRICT_SILENT_CATCH=1` to enforce. Do not add new silent
@@ -700,7 +698,7 @@ await deleteUserData(userId)
700698
### Goal
701699
Minimal comments, maximum clarity. Comments explain **intent and reasoning**, not syntax.
702700

703-
## Testing (bun:test + fast-check)
701+
## Testing (Vitest + fast-check)
704702

705703
**Prefer property-based and model-based testing** over traditional unit tests. These approaches find edge cases automatically and provide better coverage with less code.
706704

@@ -734,7 +732,7 @@ Tests that need a database or config directory **must** use `useTestConfigDir()`
734732
- `const baseDir = process.env[CONFIG_DIR_ENV_VAR]!` at module scope — This captures a value that may be stale
735733
- Manual `beforeEach`/`afterEach` that sets/deletes `SENTRY_CONFIG_DIR`
736734

737-
**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.
735+
**Why**: Vitest runs each test file in a worker pool (see `test:unit` in `package.json`), so each file runs in its own 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 `VITEST_POOL_ID` in `test/constants.ts` so parallel workers don't wipe each other's temp state during preload.
738736

739737
```typescript
740738
// CORRECT: Use the helper
@@ -757,7 +755,7 @@ afterEach(() => { delete process.env.SENTRY_CONFIG_DIR; }); // BUG!
757755
Use property-based tests when verifying invariants that should hold for **any valid input**.
758756

759757
```typescript
760-
import { describe, expect, test } from "bun:test";
758+
import { describe, expect, test } from "vitest";
761759
import { constantFrom, assert as fcAssert, property, tuple } from "fast-check";
762760
import { DEFAULT_NUM_RUNS } from "../model-based/helpers.js";
763761

@@ -805,7 +803,7 @@ describe("property: myFunction", () => {
805803
Use model-based tests for **stateful systems** where sequences of operations should maintain invariants.
806804

807805
```typescript
808-
import { describe, expect, test } from "bun:test";
806+
import { describe, expect, test } from "vitest";
809807
import {
810808
type AsyncCommand,
811809
asyncModelRun,
@@ -933,7 +931,7 @@ When adding property tests for a function that already has unit tests, **remove
933931
```
934932

935933
```typescript
936-
import { describe, expect, test, mock } from "bun:test";
934+
import { describe, expect, test, vi } from "vitest";
937935

938936
describe("feature", () => {
939937
test("should return specific value", async () => {
@@ -942,7 +940,7 @@ describe("feature", () => {
942940
});
943941

944942
// Mock modules when needed
945-
mock.module("./some-module", () => ({
943+
vi.mock("./some-module", () => ({
946944
default: () => "mocked",
947945
}));
948946
```

packages/cli/biome.jsonc

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,10 @@
11
{
22
"$schema": "./node_modules/@biomejs/biome/configuration_schema.json",
33
"extends": ["ultracite/core"],
4-
// The workspace .gitignore lives at the monorepo root (two levels up from this
5-
// package). Point Biome's VCS integration there so `useIgnoreFile` resolves it.
4+
// The .gitignore Biome honors lives at the monorepo root (two levels up).
5+
// Point Biome's VCS integration there so `useIgnoreFile` reads the root file,
6+
// whose patterns are `packages/cli/`-prefixed. (The package-level .gitignore
7+
// is for plain git only; Biome does not read it.)
68
"vcs": {
79
"enabled": true,
810
"clientKind": "git",
@@ -23,7 +25,6 @@
2325
// on the node:tls module graph — an internal Biome bug that surfaces
2426
// non-deterministically as error vs warning. See biome issue tracker.
2527
"includes": [
26-
"!docs",
2728
"!test/init-eval/templates",
2829
"!dist-build",
2930
// The codemod package is a self-contained Codemod (jssg) project with

0 commit comments

Comments
 (0)