Skip to content

Commit 0d0266b

Browse files
committed
fix(dsn): prevent silent exit during uncached DSN auto-detection (#411)
Replace Bun.Glob.scan() async iterators with readdir() in DSN detection paths. The async iterators don't ref-count the event loop, causing the process to exit with code 0 and no output while scanning is still in progress on first run (uncached). Changes: - bin.ts: Add setInterval keepalive cleared via .finally() to prevent premature event loop drain (can't use top-level await due to CJS bundle) - project-root.ts: Replace anyGlobMatches() Bun.Glob.scan() with readdir() + synchronous Bun.Glob.match() for language marker detection - env-file.ts: Replace Bun.Glob.scan('*') with readdir() in detectFromMonorepoEnvFiles() for listing monorepo package directories
1 parent fb38a16 commit 0d0266b

File tree

4 files changed

+82
-54
lines changed

4 files changed

+82
-54
lines changed

AGENTS.md

Lines changed: 40 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -628,39 +628,58 @@ mock.module("./some-module", () => ({
628628

629629
### Architecture
630630

631-
<!-- lore:019cbeba-e4d3-748c-ad50-fe3c3d5c0a0d -->
632-
* **Auth token env var override pattern: SENTRY\_AUTH\_TOKEN > SENTRY\_TOKEN > SQLite**: Auth in \`src/lib/db/auth.ts\` follows layered precedence: \`SENTRY\_AUTH\_TOKEN\` > \`SENTRY\_TOKEN\` > SQLite OAuth token. \`getEnvToken()\` trims env vars (empty/whitespace = unset). \`AuthSource\` tracks provenance. \`ENV\_SOURCE\_PREFIX = "env:"\` — use \`.length\` not hardcoded 4. Env tokens bypass refresh/expiry. \`isEnvTokenActive()\` guards auth commands. Logout must NOT clear stored auth when env token active. These functions stay in \`db/auth.ts\` despite not touching DB because they're tightly coupled with token retrieval.
631+
<!-- lore:019cafbb-24ad-75a3-b037-5efbe6a1e85d -->
632+
* **DSN org prefix normalization in arg-parsing.ts**: Sentry DSN hosts encode org IDs as \`oNNNNN\` (e.g., \`o1081365.ingest.us.sentry.io\`). The Sentry API rejects the \`o\`-prefixed form. \`stripDsnOrgPrefix()\` in \`src/lib/arg-parsing.ts\` uses \`/^o(\d+)$/\` to strip the prefix — safe for slugs like \`organic\`. Applied in \`parseOrgProjectArg()\` and \`parseWithSlash()\`, covering all API call paths consuming \`parsed.org\`.
633633
634-
<!-- lore:019cbaa2-e4a2-76c0-8f64-917a97ae20c5 -->
635-
* **Consola chosen as CLI logger with Sentry createConsolaReporter integration**: Consola is the CLI logger with Sentry \`createConsolaReporter\` integration. Two reporters: FancyReporter (stderr) + Sentry structured logs. Level via \`SENTRY\_LOG\_LEVEL\`. \`buildCommand\` injects hidden \`--log-level\`/\`--verbose\` flags. \`withTag()\` creates independent instances; \`setLogLevel()\` propagates via registry. All user-facing output must use consola, not raw stderr. \`HandlerContext\` intentionally omits stderr.
634+
<!-- lore:019cb38b-e327-7ec5-8fb0-9e635b2bac48 -->
635+
* **GHCR versioned nightly tags for delta upgrade support**: GHCR nightly distribution uses three tag types: \`:nightly\` (rolling), \`:nightly-\<version>\` (immutable), \`:patch-\<version>\` (delta manifest). Delta patches use zig-bsdiff TRDIFF10 (zstd-compressed), ~50KB vs ~29MB full. Client bspatch via \`Bun.zstdDecompressSync()\`. N-1 patches only, full download fallback, SHA-256 verify, 60% size threshold. npm/Node excluded. Test mocks: use \`mockGhcrNightlyVersion()\` helper.
636636
637-
<!-- lore:019cce8d-f2c5-726e-8a04-3f48caba45ec -->
638-
* **Input validation layer: src/lib/input-validation.ts guards CLI arg parsing**: Four validators in \`src/lib/input-validation.ts\` guard against agent-hallucinated inputs: \`rejectControlChars\` (ASCII < 0x20), \`rejectPreEncoded\` (%XX), \`validateResourceId\` (rejects ?, #, %, whitespace), \`validateEndpoint\` (rejects \`..\` traversal). Applied in \`parseSlashOrgProject\`, bare-slug path in \`parseOrgProjectArg\`, \`parseIssueArg\`, and \`normalizeEndpoint\` (api.ts). NOT applied in \`parseSlashSeparatedArg\` for no-slash plain IDs — those may contain structural separators (newlines for log view batch IDs) that callers split downstream. Validation targets user-facing parse boundaries only; env vars and DB cache values are trusted.
637+
<!-- lore:a1f33ceb-6116-4d29-b6d0-0dc9678e4341 -->
638+
* **Issue list auto-pagination beyond API's 100-item cap**: Sentry API silently caps \`limit\` at 100 per request. \`listIssuesAllPages()\` auto-paginates using Link headers, bounded by MAX\_PAGINATION\_PAGES (50). \`API\_MAX\_PER\_PAGE\` constant is shared across all paginated consumers. \`--limit\` means total results everywhere (max 1000, default 25). Org-all mode uses \`fetchOrgAllIssues()\`; explicit \`--cursor\` does single-page fetch to preserve cursor chain.
639639
640-
<!-- lore:019cd2d1-aa47-7fc1-92f9-cc6c49b19460 -->
641-
* **Magic @ selectors resolve issues dynamically via sort-based list API queries**: Magic \`@\` selectors (\`@latest\`, \`@most\_frequent\`) in \`parseIssueArg\` are detected early (before \`validateResourceId\`) because \`@\` is not in the forbidden charset. \`SELECTOR\_MAP\` provides case-insensitive matching with common variations (\`@mostfrequent\`, \`@most-frequent\`). Resolution in \`resolveSelector\` (issue/utils.ts) maps selectors to \`IssueSort\` values (\`date\`, \`freq\`), calls \`listIssuesPaginated\` with \`perPage: 1\` and \`query: 'is:unresolved'\`. Supports org-prefixed form: \`sentry/@latest\`. Unrecognized \`@\`-prefixed strings fall through to suffix-only parsing (not an error). The \`ParsedIssueArg\` union includes \`{ type: 'selector'; selector: IssueSelector; org?: string }\`.
640+
<!-- lore:019cb950-9b7b-731a-9832-b7f6cfb6a6a2 -->
641+
* **Self-hosted OAuth device flow requires Sentry 26.1.0+ and SENTRY\_CLIENT\_ID**: Self-hosted OAuth device flow requires Sentry 26.1.0+ and both \`SENTRY\_URL\` and \`SENTRY\_CLIENT\_ID\` env vars. Users must create a public OAuth app in Settings → Developer Settings. The client ID is NOT optional for self-hosted. Fallback for older instances: \`sentry auth login --token\`. \`getSentryUrl()\` and \`getClientId()\` in \`src/lib/oauth.ts\` read lazily (not at module load) so URL parsing from arguments can set \`SENTRY\_URL\` after import.
642642
643-
### Decision
643+
<!-- lore:019ca9c3-989c-7c8d-bcd0-9f308fd2c3d7 -->
644+
* **Sentry CLI markdown-first formatting pipeline replaces ad-hoc ANSI**: Formatters build CommonMark strings; \`renderMarkdown()\` renders to ANSI for TTY or raw markdown for non-TTY. Key helpers: \`colorTag()\`, \`mdKvTable()\`, \`mdRow()\`, \`mdTableHeader()\` (\`:\` suffix = right-aligned), \`renderTextTable()\`. \`isPlainOutput()\` checks \`SENTRY\_PLAIN\_OUTPUT\` > \`NO\_COLOR\` > \`!isTTY\`. Batch path: \`formatXxxTable()\`. Streaming path: \`StreamingTable\` (TTY) or raw markdown rows (plain). Both share \`buildXxxRowCells()\`.
644645
645-
<!-- lore:019cc2ef-9be5-722d-bc9f-b07a8197eeed -->
646-
* **All view subcommands should use \<target> \<id> positional pattern**: All \`\* view\` subcommands should follow a consistent \`\<target> \<id>\` positional argument pattern where target is the optional \`org/project\` specifier. During migration, use opportunistic argument swapping with a stderr warning when args are in wrong order. This is an instance of the broader CLI UX auto-correction pattern: safe when input is already invalid, correction is unambiguous, warning goes to stderr. Normalize at command level, keep parsers pure. Model after \`gh\` CLI conventions.
646+
<!-- lore:019cd2b7-bb98-730e-a0d3-ec25bfa6cf4c -->
647+
* **Sentry issue stats field: time-series controlled by groupStatsPeriod**: The \`stats\` field on issues is \`{ '24h': \[\[ts, count], ...] }\`. Key depends on \`groupStatsPeriod\` param (\`""\`, \`"14d"\`, \`"24h"\`, \`"auto"\`). \`statsPeriod\` controls time window; \`groupStatsPeriod\` controls stats key. \*\*Critical\*\*: \`count\` is period-scoped — \`lifetime.count\` is the true lifetime total. Issue list table uses \`groupStatsPeriod: 'auto'\` for sparkline data. Column order: SHORT ID, ISSUE, SEEN, AGE, TREND, EVENTS, USERS, TRIAGE. TREND auto-hidden when terminal < 100 cols. \`--compact\` tri-state: explicit overrides; \`undefined\` triggers \`shouldAutoCompact(rowCount)\` — compact if \`3N + 3 > termHeight\`, false for non-TTY. Height is \`3N + 3\` (not \`3N + 4\`) because last data row has no trailing separator.
648+
649+
<!-- lore:019ca9c3-98a2-7a81-9db7-d36c2e71237c -->
650+
* **Sentry trace-logs API is org-scoped, not project-scoped**: The Sentry trace-logs endpoint (\`/organizations/{org}/trace-logs/\`) is org-scoped, so \`trace logs\` uses \`resolveOrg()\` not \`resolveOrgAndProject()\`. The endpoint is PRIVATE in Sentry source, excluded from the public OpenAPI schema — \`@sentry/api\` has no generated types. The hand-written \`TraceLogSchema\` in \`src/types/sentry.ts\` is required until Sentry makes it public.
651+
652+
<!-- lore:019cbf3f-6dc2-727d-8dca-228555e9603f -->
653+
* **withAuthGuard returns discriminated Result type, not fallback+onError**: \`withAuthGuard\<T>(fn)\` in \`src/lib/errors.ts\` returns a discriminated Result: \`{ ok: true, value: T } | { ok: false, error: unknown }\`. AuthErrors always re-throw (triggers bin.ts auto-login). All other errors are captured. Callers inspect \`result.ok\` to degrade gracefully. Used across 12+ files.
647654
648655
### Gotcha
649656
650-
<!-- lore:019cd379-4c6a-7a93-beae-b1d5b4df69b1 -->
651-
* **Dot-notation field filtering is ambiguous for keys containing dots**: The \`filterFields\` function in \`src/lib/formatters/json.ts\` uses dot-notation to address nested fields (e.g., \`metadata.value\`). This means object keys that literally contain dots are ambiguous and cannot be addressed. Property-based tests for this function must generate field name arbitraries that exclude dots — use a restricted charset like \`\[a-zA-Z0-9\_]\` in fast-check arbitraries. Counterexample found by fast-check: \`{"a":{".":false}}\` with path \`"a."\` splits into \`\["a", ""]\` and fails to resolve.
657+
<!-- lore:019c9994-d161-783e-8b3e-79457cd62f42 -->
658+
* **Biome lint: Response.redirect() required, nested ternaries forbidden**: Biome lint rules that frequently trip up this codebase: (1) \`useResponseRedirect\`: use \`Response.redirect(url, status)\` not \`new Response\`. (2) \`noNestedTernary\`: use \`if/else\`. (3) \`noComputedPropertyAccess\`: use \`obj.property\` not \`obj\["property"]\`. (4) Max cognitive complexity 15 per function — extract helpers to stay under.
659+
660+
<!-- lore:019c8c31-f52f-7230-9252-cceb907f3e87 -->
661+
* **Bugbot flags defensive null-checks as dead code — keep them with JSDoc justification**: Cursor Bugbot and Sentry Seer repeatedly flag two false positives: (1) defensive null-checks as "dead code" — keep them with JSDoc explaining why the guard exists for future safety, especially when removing would require \`!\` assertions banned by \`noNonNullAssertion\`. (2) stderr spinner output during \`--json\` mode — always a false positive since progress goes to stderr, JSON to stdout. Reply explaining the rationale and resolve.
652662
653-
<!-- lore:019cbe0d-d03e-716c-b372-b09998c07ed6 -->
654-
* **Stricli rejects unknown flags — pre-parsed global flags must be consumed from argv**: Stricli's arg parser is strict: any \`--flag\` not registered on a command throws \`No flag registered for --flag\`. Global flags (parsed before Stricli in bin.ts) MUST be spliced out of argv. \`--log-level\` was correctly consumed but \`--verbose\` was intentionally left in (for the \`api\` command's own \`--verbose\`). This breaks every other command. Also, \`argv.indexOf('--flag')\` doesn't match \`--flag=value\` form — must check both space-separated and equals-sign forms when pre-parsing. A Biome \`noRestrictedImports\` lint rule in \`biome.jsonc\` now blocks \`import { buildCommand } from "@stricli/core"\` at error level — only \`src/lib/command.ts\` is exempted. Other \`@stricli/core\` exports (\`buildRouteMap\`, \`run\`, etc.) are allowed.
663+
<!-- lore:019cc3e6-0cdd-7a53-9eb7-a284a3b4eb78 -->
664+
* **Bun mock.module for node:tty requires default export and class stubs**: Bun testing gotchas: (1) \`mock.module()\` for CJS built-ins requires a \`default\` re-export plus all named exports. Missing any causes \`SyntaxError: Export named 'X' not found\`. Always check the real module's full export list. (2) \`Bun.mmap()\` always opens with PROT\_WRITE — macOS SIGKILL on signed Mach-O, Linux ETXTBSY. Fix: use \`new Uint8Array(await Bun.file(path).arrayBuffer())\` in bspatch.ts. (3) Wrap \`Bun.which()\` with optional \`pathEnv\` param for deterministic testing without mocks.
655665
656666
### Pattern
657667
658-
<!-- lore:019cce8d-f2d6-7862-9105-7a0048f0e993 -->
659-
* **Property-based tests for input validators use stringMatching for forbidden char coverage**: In \`test/lib/input-validation.property.test.ts\`, forbidden-character arbitraries are built with \`stringMatching\` targeting specific regex patterns (e.g., \`/^\[^\x00-\x1f]\*\[\x00-\x1f]\[^\x00-\x1f]\*$/\` for control chars). This ensures fast-check generates strings that always contain the forbidden character while varying surrounding content. The \`biome-ignore lint/suspicious/noControlCharactersInRegex\` suppression is needed on the control char regex constant in \`input-validation.ts\`.
668+
<!-- lore:dbd63348-2049-42b3-bb99-d6a3d64369c7 -->
669+
* **Branch naming and commit message conventions for Sentry CLI**: Branch naming: \`feat/\<short-description>\` or \`fix/\<issue-number>-\<short-description>\` (e.g., \`feat/ghcr-nightly-distribution\`, \`fix/268-limit-auto-pagination\`). Commit message format: \`type(scope): description (#issue)\` (e.g., \`fix(issue-list): auto-paginate --limit beyond 100 (#268)\`, \`feat(nightly): distribute via GHCR instead of GitHub Releases\`). Types seen: fix, refactor, meta, release, feat. PRs are created as drafts via \`gh pr create --draft\`. Implementation plans are attached to commits via \`git notes add\` rather than in PR body or commit message.
670+
671+
<!-- lore:019cc3e6-0cf5-720d-beb7-97c9c9901295 -->
672+
* **Codecov patch coverage only counts test:unit and test:isolated, not E2E**: CI coverage merges \`test:unit\` (\`test/lib test/commands test/types --coverage\`) and \`test:isolated\` (\`test/isolated --coverage\`) into \`coverage/merged.lcov\`. E2E tests (\`test/e2e\`) are NOT included in coverage reports. So func tests that spy on exports (e.g., \`spyOn(apiClient, 'getLogs')\`) give zero coverage to the mocked function's body. To cover \`api-client.ts\` function bodies in unit tests, mock \`globalThis.fetch\` + \`setOrgRegion()\` + \`setAuthToken()\` and call the real function.
673+
674+
<!-- lore:019c90f5-913b-7995-8bac-84289cf5d6d9 -->
675+
* **Pagination contextKey must include all query-varying parameters with escaping**: Pagination \`contextKey\` must encode every query-varying parameter (sort, query, period) with \`escapeContextKeyValue()\` (replaces \`|\` with \`%7C\`). Always provide a fallback before escaping since \`flags.period\` may be \`undefined\` in tests despite having a default: \`flags.period ? escapeContextKeyValue(flags.period) : "90d"\`.
676+
677+
<!-- lore:019c8a8a-64ee-703c-8c1e-ed32ae8a90a7 -->
678+
* **PR review workflow: reply, resolve, amend, force-push**: PR review workflow: (1) Read unresolved threads via GraphQL, (2) make code changes, (3) run lint+typecheck+tests, (4) create a SEPARATE commit per review round (not amend) for incremental review, (5) push normally, (6) reply to comments via REST API, (7) resolve threads via GraphQL \`resolveReviewThread\`. Only amend+force-push when user explicitly asks or pre-commit hook modified files.
660679
661-
<!-- lore:019cd379-4c71-7477-9cc6-3c0dfc7fb597 -->
662-
* **Shared flag constants in list-command.ts for cross-command consistency**: \`src/lib/list-command.ts\` exports shared Stricli flag definitions (\`FIELDS\_FLAG\`, \`FRESH\_FLAG\`, \`FRESH\_ALIASES\`) reused across all commands. When adding a new global-ish flag to multiple commands, define it once here as a const satisfying Stricli's flag shape, then spread into each command's \`flags\` object. The \`--fields\` flag is \`{ kind: 'parsed', parse: String, brief: '...', optional: true }\`. \`parseFieldsList()\` in \`formatters/json.ts\` handles comma-separated parsing with trim/dedup. \`writeJson()\` accepts an optional \`fields\` array and calls \`filterFields()\` before serialization.
680+
<!-- lore:019cdd9b-330a-784f-9487-0abf7b80be3c -->
681+
* **Stricli optional boolean flags produce tri-state (true/false/undefined)**: Stricli boolean flags with \`optional: true\` (no \`default\`) produce \`boolean | undefined\` in the flags type. \`--flag\`\`true\`, \`--no-flag\`\`false\`, omitted → \`undefined\`. This enables auto-detect patterns: explicit user choice overrides, \`undefined\` triggers heuristic. Used by \`--compact\` on issue list. The flag type must be \`readonly field?: boolean\` (not \`readonly field: boolean\`). This differs from \`default: false\` which always produces a defined boolean.
663682
664-
<!-- lore:019cbe44-7687-7288-81a2-662feefc28ea -->
665-
* **SKILL.md generator must filter hidden Stricli flags**: \`script/generate-skill.ts\` introspects Stricli's route tree to auto-generate \`plugins/sentry-cli/skills/sentry-cli/SKILL.md\`. The \`FlagDef\` type must include \`hidden?: boolean\` and \`extractFlags\` must propagate it to \`FlagInfo\`. The filter in \`generateCommandDoc\` must exclude \`f.hidden\` alongside \`help\`/\`helpAll\`. Without this, hidden flags injected by \`buildCommand\` (like \`--log-level\`, \`--verbose\`) appear on every command in the AI agent skill file. Global flags should instead be documented once in \`docs/src/content/docs/commands/index.md\` Global Options section, which the generator pulls into SKILL.md via \`loadCommandsOverview\`.
683+
<!-- lore:019cc325-d322-7e6e-86cc-93010b71abee -->
684+
* **Testing Stricli command func() bodies via spyOn mocking**: Stricli/Bun test patterns: (1) Command func tests: \`const func = await cmd.loader()\`, then \`func.call(mockContext, flags, ...args)\`. \`loader()\` return type union causes LSP errors — false positives that pass \`tsc\`. File naming: \`\*.func.test.ts\`. (2) ESM prevents \`vi.spyOn\` on Node built-in exports. Workaround: test subclass that overrides the method calling the built-in. (3) Follow-mode uses \`setTimeout\`-based scheduling; test with \`interceptSigint()\` helper. \`Bun.sleep()\` has no AbortSignal so \`setTimeout\`/\`clearTimeout\` required.
666685
<!-- End lore-managed section -->

src/bin.ts

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -204,4 +204,11 @@ async function main(): Promise<void> {
204204
}
205205
}
206206

207-
main();
207+
// Keep the event loop alive until main() settles. Without this, the process
208+
// can exit with code 0 while async work (e.g., DSN auto-detection) is still
209+
// pending, because some async iterators don't ref-count the event loop.
210+
// We can't use top-level `await` here because the npm bundle uses CJS format.
211+
const KEEPALIVE_MS = 0x40_00_00_00; // ~12 days, effectively infinite
212+
// biome-ignore lint/suspicious/noEmptyBlockStatements: intentional no-op keepalive
213+
const keepalive = setInterval(() => {}, KEEPALIVE_MS);
214+
main().finally(() => clearInterval(keepalive));

src/lib/dsn/env-file.ts

Lines changed: 22 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@
88
* to find DSNs in individual packages/apps.
99
*/
1010

11-
import { stat } from "node:fs/promises";
11+
import { readdir, stat } from "node:fs/promises";
1212
import { join } from "node:path";
1313
import { createDetectedDsn } from "./parser.js";
1414
import { scanSpecificFiles } from "./scanner.js";
@@ -168,39 +168,38 @@ export async function detectFromMonorepoEnvFiles(
168168
): Promise<EnvFileScanResult> {
169169
const dsns: DetectedDsn[] = [];
170170
const sourceMtimes: Record<string, number> = {};
171-
const pkgGlob = new Bun.Glob("*");
172171

173172
for (const monorepoRoot of MONOREPO_ROOTS) {
174173
const rootDir = join(cwd, monorepoRoot);
175174

175+
let entries: string[];
176176
try {
177-
// Scan for subdirectories (each is a potential package/app)
178-
for await (const pkgName of pkgGlob.scan({
179-
cwd: rootDir,
180-
onlyFiles: false,
181-
})) {
182-
const pkgDir = join(rootDir, pkgName);
177+
entries = await readdir(rootDir);
178+
} catch {
179+
// Directory doesn't exist, skip this monorepo root
180+
continue;
181+
}
182+
183+
for (const pkgName of entries) {
184+
const pkgDir = join(rootDir, pkgName);
183185

184-
// Only process directories, not files
185-
try {
186-
const stats = await stat(pkgDir);
187-
if (!stats.isDirectory()) {
188-
continue;
189-
}
190-
} catch {
186+
// Only process directories, not files
187+
try {
188+
const stats = await stat(pkgDir);
189+
if (!stats.isDirectory()) {
191190
continue;
192191
}
192+
} catch {
193+
continue;
194+
}
193195

194-
const packagePath = `${monorepoRoot}/${pkgName}`;
196+
const packagePath = `${monorepoRoot}/${pkgName}`;
195197

196-
const result = await detectDsnInPackage(pkgDir, packagePath);
197-
if (result.dsn) {
198-
dsns.push(result.dsn);
199-
Object.assign(sourceMtimes, result.sourceMtimes);
200-
}
198+
const result = await detectDsnInPackage(pkgDir, packagePath);
199+
if (result.dsn) {
200+
dsns.push(result.dsn);
201+
Object.assign(sourceMtimes, result.sourceMtimes);
201202
}
202-
} catch {
203-
// Directory doesn't exist or scan failed, skip this monorepo root
204203
}
205204
}
206205

0 commit comments

Comments
 (0)