diff --git a/AGENTS.md b/AGENTS.md index 7a1ab4b53..a36c3ec76 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -276,6 +276,19 @@ export const myCommand = buildCommand({ - 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`. @@ -619,6 +632,30 @@ When a user-provided name/title doesn't match any entity, use `fuzzyMatch()` fro 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. + ### Auto-Recovery for Wrong Entity Types When a user provides the wrong type of identifier (e.g., an issue short ID @@ -656,6 +693,24 @@ 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) @@ -686,6 +741,18 @@ Key rules when writing overrides: 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 @@ -1001,81 +1068,89 @@ mock.module("./some-module", () => ({ ### Architecture - -* **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. + +* **Issue resolve --in grammar: release + @next + @commit sentinels**: \`sentry issue resolve --in\` grammar: (a) omitted→immediate resolve, (b) \`\\`→\`inRelease\` (monorepo \`spotlight@1.2.3\` pass-through), (c) \`@next\`→\`inNextRelease\`, (d) \`@commit\`→auto-detect git HEAD + match Sentry repos, (e) \`@commit:\@\\`→explicit. Sentinel matching case-insensitive; unknown \`@\`-prefixed tokens throw \`ValidationError\`. \`parseResolveSpec\` splits on LAST \`@\` to handle scoped names like \`@acme/web\`. \`resolveCommitSpec\` uses \`getHeadCommit\`/\`getRepositoryName\` from \`src/lib/git.ts\`, matching Sentry repo \`externalSlug\` or \`name\` via \`listRepositoriesCached\`. API requires \`statusDetails.inCommit: {commit, repository}\` — not bare SHA. - -* **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. + +* **npm bundle requires Node.js >= 22 due to node:sqlite polyfill**: The npm package (dist/bin.cjs) requires Node.js >= 22 because the bun:sqlite polyfill uses \`node:sqlite\`. A runtime version guard in the esbuild banner catches this early. When writing esbuild banner strings in TS template literals, double-escape: \`\\\\\\\n\` in TS → \`\\\n\` in output → newline at runtime. Single \`\\\n\` produces a literal newline inside a JS string, causing SyntaxError. - -* **DSN cache invalidation uses two-level mtime tracking (sourceMtimes + dirMtimes)**: DSN cache invalidation — two-level mtime tracking: \`sourceMtimes\` (DSN-bearing files, catches in-place edits) + \`dirMtimes\` (every walked dir, catches new files) + root mtime fast-path + 24h TTL. Dropping either map is a correctness regression. Walker emits mtimes via \`onDirectoryVisit\` hook + \`recordMtimes\` option; DSN scanner uses \`grepFiles({pattern: DSN\_PATTERN, recordMtimes: true, onDirectoryVisit})\` (~20% faster than walkFiles). \`scanCodeForFirstDsn\` stays on direct walker loop (worker init ~20ms dominates single-DSN). Invariants: \`processMatch\` must record mtime for EVERY file with host-validated DSN via \`fileHadValidDsn\` flag independent of \`seen.has(raw)\`. \`scanDirectory\` catch MUST return empty \`dirMtimes: {}\`, NOT partial map (would silently bless unvisited dirs); \`ConfigError\` re-throws. + +* **repo\_cache SQLite table for offline Sentry repo lookups**: Schema v14 adds \`repo\_cache\` table in \`src/lib/db/schema.ts\` + helpers in \`src/lib/db/repo-cache.ts\` (7-day TTL). \`listAllRepositories(org)\` in \`src/lib/api/repositories.ts\` paginates through \`listRepositoriesPaginated\` using \`API\_MAX\_PER\_PAGE\` and \`MAX\_PAGINATION\_PAGES\` — never use the unpaginated \`listRepositories\` for cache-backed lookups (silently caps at ~25). \`listRepositoriesCached(org)\` wraps it with cache-first lookup and a try/catch around \`setCachedRepos\` so read-only databases (macOS \`sudo brew install\`) don't crash commands whose API fetch already succeeded. Used by \`@commit\` resolver to match git origin \`owner/repo\` against Sentry repo \`externalSlug\` or \`name\`. - -* **Grep worker pool: binary-transferable matches + streaming dispatch in src/lib/scan/**: Grep worker pool (\`src/lib/scan/worker-pool.ts\` + \`grep-worker.js\`): lazy singleton, size \`min(8, max(2, availableParallelism()))\`. Matches encoded as \`Uint32Array\` quads \`\[pathIdx, lineNum, lineOffset, lineLength]\` + \`linePool\` string, transferred via \`postMessage(msg, \[ints.buffer])\` (~40% faster than structuredClone). Worker imported via \`with { type: 'text' }\` → \`Blob\` + \`URL.createObjectURL\`; \`new Worker(new URL(...))\` HANGS in \`bun build --compile\` binaries. FIFO \`pending\` queue per worker — per-dispatch \`addEventListener\` causes wrong-request resolution. \`ref()\`/\`unref()\` idempotent booleans, NOT refcounted — only unref when \`inflight\` drops to 0; spawn unref'd. Disable via \`SENTRY\_SCAN\_DISABLE\_WORKERS=1\`. Track dispatched/failed batches with \`Promise.allSettled\`; throw if all failed so DSN cache doesn't persist false-negatives. + +* **Response cache hit invisibility — synthetic Response carries no marker**: Response cache hit invisibility — synthetic Response from \`getCachedResponse()\` in \`src/lib/response-cache.ts\` is indistinguishable from network. Solved via module-level \`lastCacheHitAgeMs\`: set on hit, cleared at top of \`authenticatedFetch()\` per-call (single-process CLI = race-free). \`src/lib/cache-hint.ts\` provides \`formatCacheHint()\` (\`"cached · 3m ago · use -f to refresh"\`) and \`appendCacheHint(existingHint)\` (joins with \` | \`). Wired in \`buildCommand\` (\`src/lib/command.ts\`): \`appendCacheHint(returned?.hint)\` runs only when generator returns a \`CommandReturn\` — bare \`return;\` paths (e.g. \`--web\`) skip the hint. Same chokepoint can host future cross-cutting hint decorators. Test-only \`\_setLastCacheHitAgeForTesting(ms)\` exposes state. - -* **Host-scoped token model: auth.host column + three-layer enforcement**: Host-scoped token model (PR #844): every token bound to issuing host via \`auth.host\` column (schema v16), lazy-migrated from boot-env. Trust established ONLY via \`sentry auth login --url\` or shell-exported \`SENTRY\_HOST\`/\`SENTRY\_URL\` at boot — \`.sentryclirc\` URL never a trust source (mtime-based freshness doesn't work: git clone resets, \`touch -t\` backdates). Three enforcement layers: (1) \`applySentryUrlContext\` throws on URL-arg mismatch; (2) \`applySentryCliRcEnvShim\` throws on rc-url mismatch (auth login/logout bypass via \`skipUrlTrustCheck\`); (3) fetch-layer \`isRequestOriginTrusted\`. Region trust: in-process Set in \`db/regions.ts\`, auto-synced by \`setOrgRegion(s)\`. \`clearTrustedHostState\` must NOT clear login anchor (breaks IAP re-auth). Login refusal scoped to \`--token\`. \`HostScopeError\` (\`src/lib/errors.ts\`) is canonical formatter with overloads \`(message)\` and \`(source, destinationUrl, tokenHost)\`; used by rc-shim, URL-arg, fetch bearer, sntrys\_ claim, OAuth refresh. E2E: pass \`--url ${ctx.serverUrl}\` to \`auth login --token\`; child \`SENTRY\_URL\` alone doesn't anchor. + +* **Seer trial prompt uses middleware layering in bin.ts error handling chain**: Seer trial prompt via error middleware layering: \`bin.ts\` chain is \`main() → executeWithAutoAuth() → executeWithSeerTrialPrompt() → runCommand()\`. Seer trial prompts (\`no\_budget\`/\`not\_enabled\`) caught by inner wrapper; auth errors bubble to outer. Trial API: \`GET /api/0/customers/{org}/\` → \`productTrials\[]\` (prefer \`seerUsers\`, fallback \`seerAutofix\`). Start: \`PUT /api/0/customers/{org}/product-trial/\`. SaaS-only; self-hosted 404s gracefully. \`ai\_disabled\` excluded. \`startSeerTrial\` accepts \`category\` from trial object — don't hardcode. - -* **isSentrySaasUrl vs isSaaSTrustOrigin: two intentional SaaS checks**: \`src/lib/sentry-urls.ts\` exports two SaaS-detection helpers with intentional split: (1) \`isSentrySaasUrl(url)\` — hostname-only check (\`sentry.io\` or \`\*.sentry.io\`), accepts any protocol/port. Used for routing/UX: custom-headers warning, \`getSentryBaseUrl\`/\`isSelfHosted\`, region resolution skip, telemetry \`is\_self\_hosted\` tag. (2) \`isSaaSTrustOrigin(url)\` — stricter: additionally requires \`https:\` and default port. Used for security decisions: token-host trust comparison, sentryclirc URL trust check, URL-arg trust, login refusal. Rule: hostname-only for routing/UX (don't break users behind TLS-terminating proxies with \`http://sentry.io\`); strict for credential scoping. JSDoc on \`isSentrySaasUrl\` points callers to \`isSaaSTrustOrigin\` for security contexts. Keep both implementations in sync re: hostname matching. +### Decision - -* **Magic @ selectors resolve issues dynamically via sort-based list API queries**: Magic @ selectors resolve issues dynamically: \`@latest\`, \`@most\_frequent\` in \`parseIssueArg\` detected before \`validateResourceId\` (@ not in forbidden charset). \`SELECTOR\_MAP\` provides case-insensitive matching. \`resolveSelector\` maps to \`IssueSort\` values, calls \`listIssuesPaginated\` with \`perPage: 1\`, \`query: 'is:unresolved'\`. Supports org-prefixed: \`sentry/@latest\`. Unrecognized \`@\`-prefixed strings fall through. \`ParsedIssueArg\` union includes \`{ type: 'selector' }\`. + +* **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. - -* **safe-read.ts wraps isRegularFile + Bun.file().text() for FIFO-safe user-path reads**: \`src/lib/safe-read.ts\` \`safeReadFile(path, operation): Promise\\` combines \`isRegularFile()\` + \`Bun.file().text()\` + broad error swallow (FIFO/ENOENT/EACCES/EPERM/EISDIR/ENOTDIR). Sole caller: \`apply-patchset.ts\`. \*\*Do NOT use for committed config loads\*\* — swallows EPERM/EISDIR, making \`chmod 000 .sentryclirc\` manifest as confusing 'no auth token'. For loud permission surfacing (\`tryReadSentryCliRc\`), call \`fs.promises.stat\` directly, gate on \`isFile()\`, catch only ENOENT/EACCES. \`read-files.ts\`/\`workflow-inputs.ts\` use direct stat to reuse one stat for size-gating. Test with real \`mkfifo\` + short timeout as hang detector. +### Gotcha - -* **Sentry SDK uses @sentry/node-core/light instead of @sentry/bun to avoid OTel overhead**: Sentry SDK uses \`@sentry/node-core/light\` instead of \`@sentry/bun\` to avoid OpenTelemetry overhead (~150ms, 24MB). \`@sentry/core\` barrel patched via \`bun patch\` to remove ~32 unused exports. Gotcha: \`LightNodeClient\` hardcodes \`runtime: { name: 'node' }\` AFTER spreading options — fix by patching \`client.getOptions().runtime\` post-init (mutable ref). Transport uses Node \`http\` instead of native \`fetch\`. Upstream: getsentry/sentry-javascript#19885, #19886. + +* **--json schema stability: collapse=organization drops nested org fields**: --json schema + response cache gotchas: (1) \`?collapse=organization\` shrinks \`organization\` to \`{id, slug}\` — silent --json regression. \`jsonTransform\` re-hydrates \`organization.name\` via \`resolveOrgDisplayName\` against \`org\_regions\` cache. (2) \`buildCacheKey()\` normalizes URL with sorted query params, so \`invalidateCachedResponse(baseUrl)\` misses entries with query suffixes. Use \`invalidateCachedResponsesMatching(prefix)\` (raw \`startsWith()\`); \`buildApiUrl()\` always emits trailing slash → safe prefix. (3) When \`jsonTransform\` is set, \`jsonExclude\` and \`filterFields\` are NOT applied — transform must call \`filterFields(result, fields)\` and omit excluded keys itself. - -* **Sentry token formats: only sntrys\_ embeds host claim, and it's unsigned**: Sentry token formats (verified in getsentry/sentry \`orgauthtoken\_token.py\`): \`sntryu\_\\` (user auth) — no claims; \`sntrys\_\\_\\` (org auth) — \*\*unsigned\*\*, plaintext base64, anyone can forge; \`sntrya\_\`/\`sntryi\_\` — random hex; OAuth — random, no prefix. \`sntrys\_\` payload is a UX hint, NOT verifiable; \`auth.host\` column \[\[019dc168-adb2-7bed-900e-cab5d3716099]] is strictly stronger. \`parseSntrysClaim\` in \`src/lib/token-claims.ts\` requires exactly 2 underscores, base64-decodes, requires \`iat\`, 2 KB cap, fail-open. Two consumers: (1) \`captureEnvTokenHost\` claim-first for \`sntrys\_\`: claim url > \`SENTRY\_HOST\`/\`SENTRY\_URL\` > \`DEFAULT\_SENTRY\_URL\` (defends against layered-CI \`$GITHUB\_ENV\` poisoning); for \`sntryu\_\`/OAuth, env wins (no \`SENTRY\_BOUND\_TOKEN\` protocol — narrow protection, broad UX cost). (2) \`prepareHeaders\` defense-in-depth — refuses bearer attach if request origin doesn't match claim url. + +* **@sentry/api SDK passes Request object to custom fetch — headers lost on Node.js**: @sentry/api SDK calls \`\_fetch(request)\` with no init object. In \`authenticatedFetch\`, \`init\` is undefined → \`prepareHeaders\` creates empty headers, stripping Content-Type on Node.js (HTTP 415). Fix: fall back to \`input.headers\` when \`init\` is undefined. Use \`unwrapPaginatedResult\` (not \`unwrapResult\`) to access Link header for pagination. \`per\_page\` not in SDK types — cast query at runtime. SDK returns \`data={}\` (not \`\[]\`) for empty/204/missing Content-Type responses — always guard with \`Array.isArray(data)\` before \`.map()\`. Self-hosted instances behind reverse proxies commonly trigger this. - -* **Telemetry opt-out is env-var-only — no persistent preference or DO\_NOT\_TRACK**: Telemetry opt-out priority: (1) \`SENTRY\_CLI\_NO\_TELEMETRY=1\`, (2) \`DO\_NOT\_TRACK=1\`, (3) \`metadata.defaults.telemetry\`, (4) default on. DB read try/catch wrapped (runs before DB init). Schema v13 merged \`defaults\` table into \`metadata\` KV with keys \`defaults.{org,project,telemetry,url}\`; getters/setters in \`src/lib/db/defaults.ts\`. \`sentry cli defaults\` uses variadic \`\[key, value?]\`: no args → show all; 1 arg → show key; 2 args → set; \`--clear\` without args → clear all (guarded); \`--clear key\` → clear specific. \`computeTelemetryEffective()\` returns resolved source for display. + +* **API tests must use useTestConfigDir to isolate disk response cache**: API tests that mock \`globalThis.fetch\` MUST call \`useTestConfigDir()\` from \`test/helpers.ts\` + \`setAuthToken()\`. The \`authenticatedFetch\` singleton in \`src/lib/sentry-client.ts\` checks a filesystem-based response cache (\`~/.sentry/cache/responses/\`, see \`response-cache.ts\`) BEFORE calling fetch. Without per-test config dirs, test N's API response gets cached to disk and served to test N+1 — fetch mock never fires, assertion sees stale data. TTL tiers in \`classifyUrl()\`: stable=5min (default), volatile=60s (issues, logs), immutable=24h (events/traces by ID). Symptom: test expects fresh mock value, receives prior test's value. Reference: \`test/lib/api/issues.test.ts\` (correct pattern), \`test/lib/api/repositories.test.ts\` regression fixed by adding \`useTestConfigDir("repo-cache-")\` + \`setAuthToken("test-token", 3600, "test-refresh")\` in beforeEach. - -* **Zod schema on OutputConfig enables self-documenting JSON fields in help and SKILL.md**: Zod schema on OutputConfig enables self-documenting JSON fields: List commands register \`schema?: ZodType\` on \`OutputConfig\\`. \`extractSchemaFields()\` produces \`SchemaFieldInfo\[]\` from Zod shapes. \`buildFieldsFlag()\` enriches \`--fields\` brief; \`enrichDocsWithSchema()\` appends fields to \`fullDescription\`. Schema exposed as \`\_\_jsonSchema\` on built commands — \`introspect.ts\` reads it into \`CommandInfo.jsonFields\`, \`help.ts\` and \`generate-skill.ts\` render it. For \`buildOrgListCommand\`/\`dispatchOrgScopedList\`, pass \`schema\` via \`OrgListConfig\`. + +* **Biome noUselessUndefined also rejects () => {} empty arrow callbacks**: Biome lint traps: (1) \`noUselessUndefined\` rejects \`() => undefined\` AND \`noEmptyBlockStatements\` rejects \`() => {}\` — use top-level \`function noop(): void {}\`. (2) \`noExcessiveCognitiveComplexity\` caps at 15. (3) \`expect(() => fn()).toThrow(X)\` must be one line. (4) Plugin forbids raw \`metadata\` table queries — use \`getMetadata\`/\`setMetadata\`/\`clearMetadata\`. (5) Also enforced: \`useBlockStatements\`, \`noNestedTernary\`, \`useAtIndex\`, \`noStaticOnlyClass\`, \`useSimplifiedLogicExpression\`, \`noShadow\`. Namespace imports forbidden. (6) \`useYield\` fires on \`async \*func()\` with statements but not empty bodies — only add \`biome-ignore\` to generators with statements. \`lint:fix\` differs from CI \`lint\`: auto-fix hides \`noPrecisionLoss\` on >2^53 literals, \`noIncrementDecrement\`, import ordering. Always \`bun run lint\` before pushing. -### Decision + +* **Bun --isolate coverage inflates LF count for files with verbose comments/JSDoc**: Bun --isolate coverage inflates LF count: under \`bun test --isolate --parallel\` (CI's \`test:unit\`), Bun's coverage instrumentation counts comments, blank lines, type annotations, and closing braces as 'executable'. E.g. \`zstd-transport.ts\` LF=165 locally → 210 under --isolate, dropping coverage 99%→78%. Codecov sees inflated number. Workaround: trim verbose inline comments inside function bodies (move rationale to JSDoc above function or module-level doc). Statement coverage stays 100% — 'missing' lines are non-executable. - -* **All view subcommands should use \ \ positional pattern**: All \`\* view\` subcommands use \`\ \\` positional pattern (Intent-First Correction UX): target is optional \`org/project\`. Use opportunistic arg swapping with \`log.warn()\` when args are wrong order — when intent is unambiguous, do what they meant. Normalize at command level, keep parsers pure. Model after \`gh\` CLI. Exception: \`auth\` uses \`defaultCommand: "status"\` (no viewable entity). Routes without defaults: \`cli\`, \`sourcemap\`, \`repo\`, \`team\`, \`trial\`, \`release\`, \`dashboard/widget\`. + +* **Bun 1.3.11 tty.ReadStream leaks libuv handle — process.stdin.unref is undefined**: Bun 1.3.11 macOS TTY bug: \`process.stdin\` via kqueue \`EVFILT\_READ\` on reopened non-session-leader TTY fd fails to deliver keystrokes when fd 0 inherited via \`exec bin \ -* **Sentry-derived terminal color palette tuned for dual-background contrast**: Terminal color palette tuned for dual-background contrast: 10-color chart palette derived from Sentry's categorical hues (\`static/app/utils/theme/scraps/tokens/color.tsx\`), adjusted to mid-luminance for ≥3:1 contrast on both dark and light backgrounds. Adjustments: orange #FF9838→#C06F20, green #67C800→#3D8F09, yellow #FFD00E→#9E8B18, purple #5D3EB2→#8B6AC8, indigo #50219C→#7B50D0; blurple/pink/magenta unchanged; teal #228A83 added. Hex preferred over ANSI 16-color for guaranteed contrast. + +* **MastraClient has no dispose API — use AbortController for cleanup**: MastraClient has no \`close()\`/\`dispose()\` API — cleanup via \`ClientOptions.abortSignal\` (constructor) or per-prompt \`signal\`. Without explicit abort, Bun's fetch dispatcher keep-alive sockets hold the event loop alive past natural exit. Pattern in \`src/lib/init/wizard-runner.ts\`: create \`AbortController\` per \`runWizard\`, pass \`abortSignal: controller.signal\` to \`new MastraClient(...)\`, abort via \`using \_ = { \[Symbol.dispose]: () => controller.abort() }\`. Custom \`fetch\` wrapper must preserve \`init.signal\` via spread. Tests capture \`ClientOptions\` via \`spyOn(MastraClient.prototype, 'getWorkflow').mockImplementation(function() { capturedOpts.push(this.options); ... })\`. -### Gotcha + +* **Multi-region fan-out: distinguish all-403 from empty orgs with hasSuccessfulRegion flag**: In \`listOrganizationsUncached\` (\`src/lib/api/organizations.ts\`), \`Promise.allSettled\` collects multi-region results. Don't use \`flatResults.length === 0\` to detect all-regions-failed — a region returning 200 OK with zero orgs pushes nothing into \`flatResults\`. Track a \`hasSuccessfulRegion\` boolean on any \`"fulfilled"\` settlement. Only re-throw 403 \`ApiError\` when \`!hasSuccessfulRegion && lastScopeError\`. - -* **AuthError constructor takes reason first, message second**: \`AuthError(reason: AuthErrorReason, message?: string)\` where \`AuthErrorReason\` is \`"not\_authenticated" | "expired" | "invalid"\`. Easy to accidentally swap args as \`new AuthError("Token expired", "expired")\` — the string \`"Token expired"\` gets assigned as \`reason\` (invalid enum value). Tests aren't type-checked (tsconfig excludes them), so TypeScript won't catch this. Correct: \`new AuthError("expired", "Token expired")\`. Default messages exist for each reason, so the second arg is often unnecessary. +### Pattern - -* **Biome noMisplacedAssertion fires on test-helper functions; use inline biome-ignore**: Biome's \`lint/suspicious/noMisplacedAssertion\` rule flags \`expect()\` calls outside \`test()\`/\`it()\` bodies, including in named helper functions used by multiple tests (e.g. \`expectTokenStored(spy, token)\`). File-level \`biome-ignore-all\` doesn't suppress this rule — must use individual \`// biome-ignore lint/suspicious/noMisplacedAssertion: \\` directly above each \`expect()\` line in the helper. Tests aren't type-checked but they ARE lint-checked, so this catches code that passes \`bun test\` but fails \`bun run lint\`. + +* **CLI-1D3 Windows download visibility race: poll statSync with exponential backoff**: Windows upgrade download visibility race (CLI-1D3): \`waitForBinaryVisible\` in \`src/lib/upgrade.ts\` polls \`statSync\` with exponential backoff (6 attempts, 5 sleeps: 100+200+400+800+1600 = 3.1s). Loop breaks BEFORE final sleep — \`VERIFY\_MAX\_ATTEMPTS=N\` yields N-1 sleeps (off-by-one trap). Covers Windows + Bun 1.3.9 race where \`Bun.file().writer().end()\` returns before OS surfaces file by path → opaque \`Executable not found in $PATH\` from \`Bun.spawn\`. Safety net \`isEnoentSpawnError()\` in \`src/commands/cli/upgrade.ts\` detects both \`code==='ENOENT'\` and Bun's path-string error → \`UpgradeError('execution\_failed')\`. Race-free delayed-write tests: writer must POLL until bad state exists THEN overwrite. - -* **GET response cache bypasses fetch wrapper across tests**: \`sentry-client.ts::createAuthenticatedFetch\` checks the response cache BEFORE calling fetch for GET requests. Tests that mock \`globalThis.fetch\` and assert call counts will see 0 calls if a prior test cached the same URL — the cached response is served without invoking the wrapper. Fix in test \`beforeEach\`: \`import('./response-cache.js')\` then call \`resetCacheState()\` + \`disableResponseCache()\`. Pair with \`resetAuthenticatedFetch()\` if cached fetch instance is also stale. Symptom: \`expect(fetchCalls).toHaveLength(1)\` fails with \`Received length: 0\` only when run after another test hitting the same URL; passes in isolation. + +* **Cross-compile sentry-cli with patched Bun: drop compile.target to use selfExePath**: Cross-compile sentry-cli with patched Bun: \`Bun.build({compile})\` downloads stock Bun from npm when \`compile.target\` is set. Workaround in \`script/build.ts\`: omit \`target\` entirely so Bun hits \`isDefault()\` branch → uses \`selfExePath()\` = the running Bun as embed runtime. Only works when host OS/arch matches desired output. Escape hatch: place file at \`$CWD/bun-\-\-v\\` (e.g. \`bun-darwin-arm64-v1.3.13\`) picked up via \`bun.FD.cwd().existsAt(version\_str)\` in \`src/compile\_target.zig:exePath\`. Build also requires \`SENTRY\_CLIENT\_ID\` env var. - -* **Node polyfill in script/node-polyfills.ts lacks Bun.file().stat() — use node:fs/promises stat instead**: \`script/node-polyfills.ts\` shims Bun APIs for npm (Node) distribution but is INCOMPLETE — \`Bun.file(path)\` only has \`size\`, \`lastModified\`, \`exists()\`, \`text()\`, \`json()\`, \`stat()\`; NOT \`.arrayBuffer()\`, \`.stream()\`, etc. Also no \`Bun.$\` shim. Tests run under Bun natively and never exercise the polyfill, so missing shims ship undetected (CLI-1EA/1EB: \`Bun.file().stat()\` regression, 400+ events). Prefer \`node:fs/promises\` directly for file ops; \`execSync\` from \`node:child\_process\` for shell. When extending polyfill, alias Node functions via \`bind\` not wrapper closures. Mirror polyfill tests to \`test/lib/\` — \`test:unit\` globs are narrow (\`test/lib test/commands test/types\`); tests under \`test/fixtures/\`, \`test/scripts/\`, \`test/script/\` are NOT picked up by CI. + +* **Dedupe resolved entity IDs in batch operations before API call**: Batch issue merge (src/commands/issue/merge.ts): (1) Dedupe by resolved numeric ID after \`Promise.all(args.map(resolveIssue))\`, not raw input (users pass same entity as \`CLI-K9\`, \`my-org/CLI-K9\`, \`123\`). Throw ValidationError if \`new Set(ids).size < 2\`. (2) Reject undefined orgs in cross-org check — bare numeric IDs without DSN/config resolve with \`org: undefined\`; filtering them out lets mixed-org merges slip through. (3) Pass \`--into\` through \`resolveIssue()\` for alias/org-qualified parity; compare by numeric \`id\`, not \`shortId\`. (4) Sentry bulk merge API picks canonical parent by event count — \`--into\` is preference only; warn when API's \`parent\` differs. Empty results return 204. - -* **process.stdin.isTTY unreliable in Bun — use isatty(0) and backfill for clack**: \`process.stdin.isTTY\` unreliable in Bun — use \`isatty(0)\` from \`node:tty\`. Bun's single-file binary can leave \`process.stdin.isTTY === undefined\` on TTY fds inherited via redirects like \`exec … \ +* **findProjectsByPattern as fuzzy fallback for exact slug misses**: When \`findProjectsBySlug\` returns empty (no exact match), use \`findProjectsByPattern\` as a fallback to suggest similar projects. \`findProjectsByPattern\` does bidirectional word-boundary matching (\`matchesWordBoundary\`) against all projects in all orgs — the same logic used for directory name inference. In the \`project-search\` handler, call it after the exact miss, format matches as \`\/\\` suggestions in the \`ResolutionError\`. This avoids a dead-end error for typos like 'patagonai' when 'patagon-ai' exists. Note: \`findProjectsByPattern\` makes additional API calls (lists all projects per org), so only call it on the failure path. - -* **runInteractiveLogin swallows errors and sets process.exitCode = 1**: \`runInteractiveLogin\` in \`src/lib/interactive-login.ts\` catches OAuth flow errors internally (device-code fetch failures, timeout, etc.) and returns falsy on failure. The login command then sets \`process.exitCode = 1\` and returns normally — the wrapped command function resolves, NOT rejects. Tests that mock fetch to throw and expect \`rejects.toThrow()\` will fail with \`resolved: Promise { \ }\`. Assert behavior via fetch-call inspection (\`fetchCalls.length > 0\`, header content) instead. \`requestDeviceCode\` requires \`SENTRY\_CLIENT\_ID\` env var — unset in tests means it throws \`ConfigError\` before any fetch fires. + +* **Grouped widget --limit auto-default via applyGroupLimitAutoDefault helper**: Dashboard widget flag normalization: (1) Dataset aliases (errors→error-events) normalize ONCE at top of \`func()\` via \`normalizeDataset()\` in \`src/commands/dashboard/resolve.ts\`. In \`edit.ts\`, pass \`normalizedFlags\` to \`buildReplacement\` — \`validateAggregateNames\` reads \`flags.dataset\` and rejects valid aggregates like \`failure\_rate\` if it sees raw alias. (2) Grouped widgets need \`limit\` (API rejects). \`applyGroupLimitAutoDefault\` defaults to \`DEFAULT\_GROUP\_BY\_LIMIT=5\` only when user passed \`--group-by\` without \`--limit\`; skip for auto-defaulted columns like \`\["issue"]\`. (3) Tests asserting \`--limit\` >10 survives into PUT body must use \`display: "line"\` — \`prepareWidgetQueries\` clamps bar/table to max=10. - -* **Stricli rejects unknown flags — pre-parsed global flags must be consumed from argv**: Stricli flag parsing traps: (1) Unknown \`--flag\`s rejected — global flags parsed in \`bin.ts\` MUST be spliced from argv (check both \`--flag value\` and \`--flag=value\`). (2) \`FLAG\_NAME\_PATTERN\` requires 2+ chars after \`--\`; single-char flags like \`--x\` silently become positionals — use aliases (\`-x\` → longer name). Bit \`dashboard widget --x\`/\`--y\`. (3) \`FlagDef.hidden\` is propagated by \`extractFlags\` so \`generateCommandDoc\` filters hidden flags alongside \`help\`/\`helpAll\`; hidden \`--log-level\`/\`--verbose\` appear only in global options docs. + +* **Hidden --org/--project compat flags via mergeGlobalFlags**: Hidden global \`--org\`/\`--project\` flags accept old \`sentry-cli\` syntax. Defined in \`GLOBAL\_FLAGS\` (global-flags.ts) so argv-hoist relocates them. \`mergeGlobalFlags()\` in command.ts injects hidden flag shapes (skip if command owns the flag — e.g. \`release create --project -p\`) and returns \`stripKeys\` set used by \`cleanRawFlags\`. \`applyOrgProjectFlags()\` writes values to \`SENTRY\_ORG\`/\`SENTRY\_PROJECT\` via \`getEnv()\` before auth guard, overwriting existing env vars (explicit CLI > env var). Resolution chain in resolve-target.ts picks them up at priority #2. No short aliases (\`-p\` conflicts). The helper extraction was needed to keep \`buildCommand\` under Biome's cognitive complexity limit of 15. - -* **Whole-buffer matchAll slower than split+test when aggregated over many files**: Grep/scan traps in \`src/lib/scan/\`: (1) Whole-buffer \`regex.exec\` 12× faster per-file but ~1.6× SLOWER over 10k files — early-exit at \`maxResults\` via \`mapFilesConcurrent.onResult\` wins. (2) Literal prefilter is FILE-LEVEL gate (\`indexOf\`→skip); per-line verify breaks cross-newline patterns and Unicode length-changing \`toLowerCase\` (Turkish \`İ\`→\`i̇\`). (3) Extractor \`hasTopLevelAlternation\`+\`skipGroup\` must call \`skipCharacterClass\` (PCRE \`\[]abc]\` ≠ JS empty class). (4) Wake-latch race: naive \`let notify=null; await new Promise(r=>notify=r)\` loses signals — use latched \`pendingWake\` flag. (5) \`mapFilesConcurrent\` filters \`null\` but NOT \`\[]\` — return \`null\` for no-op files. (6) \`collectGlob\`/\`collectGrep\` must NOT forward \`maxResults\` to iterator; drain uncapped, set \`truncated=true\`. + +* **Preserve ApiError type so classifySilenced can silence 4xx errors**: Preserve ApiError type for classifySilenced: \`classifySilenced\` (src/lib/error-reporting.ts) only silences \`ApiError\` with status 401-499 — wrapping in generic \`CliError\` loses \`status\` and causes 403s to be captured. Re-throw via \`new ApiError(msg, error.status, error.detail, error.endpoint)\` with terse message (\`ApiError.format()\` appends detail/endpoint). \`ValidationError\` without \`field\` collapses unfielded errors into one fingerprint; always pass \`field\`. Fingerprint rule changes don't retroactively re-fingerprint — manually merge new groups into canonical old parents. \`ApiError\` rule keys by \`api\_status + command\`. -### Pattern + +* **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. + + +* **Shared pagination infrastructure: buildPaginationContextKey and parseCursorFlag**: Bidirectional pagination via cursor stack in \`src/lib/db/pagination.ts\`. \`resolveCursor(flag, key, contextKey)\` maps keywords (next/prev/first/last) to \`{cursor, direction}\`. \`advancePaginationState\` manages stack — back-then-forward truncates stale entries. \`hasPreviousPage\` checks \`page\_index > 0\`. \`paginationHint()\` builds nav strings. All list commands use this. Critical: \`resolveCursor()\` must be called inside \`org-all\` override closures, not before \`dispatchOrgScopedList\`. + + +* **Telemetry instrumentation pattern: withTracingSpan + captureException for handled errors**: For graceful-fallback operations, use \`withTracingSpan\` from \`src/lib/telemetry.ts\` for child spans and \`captureException\` from \`@sentry/bun\` (named import — Biome forbids namespace imports) with \`level: 'warning'\` for non-fatal errors. \`withTracingSpan\` uses \`onlyIfParent: true\` — no-op without active transaction. User-visible fallbacks use \`log.warn()\` not \`log.debug()\`. Several commands bypass telemetry by importing \`buildCommand\` from \`@stricli/core\` directly instead of \`../../lib/command.js\` (trace/list, trace/view, log/view, api.ts, help.ts). + + +* **Testing Stricli command func() bodies via spyOn mocking**: Testing Stricli command func() bodies: (1) \`const func = await cmd.loader(); func.call(mockContext, flags, ...args)\` with mock \`stdout\`, \`stderr\`, \`cwd\`, \`setContext\`. \`loader()\` return type union causes \`.call()\` LSP false-positives that pass \`tsc --noEmit\`. (2) When API functions are renamed, update both spy target AND mock return shape. (3) \`normalizeSlug\` replaces \`\_\`→\`-\` but does NOT lowercase. (4) Bun \`mockFetch()\` replaces \`globalThis.fetch\` — use one unified mock dispatching by URL. (5) \`mock.module()\` pollutes module registry for ALL subsequent files — put in \`test/isolated/\` and run via \`test:isolated\`. (6) For \`Bun.spawn\`, use direct property assignment in \`beforeEach\`/\`afterEach\`. - -* **Test helpers for host-scoping security tests**: Test helpers for host-scoping security tests: \`test/helpers.ts\` provides shared utilities. \`useEnvSandbox(keys)\` registers beforeEach/afterEach to save+clear+restore env keys (do NOT use in tests that depend on preload's \`SENTRY\_AUTH\_TOKEN\`, e.g. \`sentryclirc-url-poison.test.ts\` calls \`getActiveTokenHost()\` which needs a token). \`resetHostScopingState()\` bundles \`resetEnvTokenHostForTesting\` + \`resetLoginTrustAnchorForTesting\` + \`resetTrustedRegionUrlsForTesting\` (always reset together). \`mintSntrysToken(payload)\` produces \`sntrys\_\\_\\` test tokens matching server format (rstrip \`=\`). \`extractFetchUrl(input)\` for fetch-mock assertions. \`useTestConfigDir\` \[\[019dc573-d853-735a-aeb5-68ff49afe037]] handles config-dir isolation separately. +### Preference - -* **Tests calling setAuthToken must pass {host} matching the mock URL**: Host-scoping test gotchas \[\[019dc168-adb2-7bed-900e-cab5d3716099]]: (1) Tests mocking \`fetch\` with non-SaaS URLs and calling \`setAuthToken(token, ttl)\` without \`{host}\` fail with \`HostScopeError\` — token defaults to SaaS via \`captureEnvTokenHost\`. Fix: \`setAuthToken("fake", 3600, { host: "https://sentry.example.com" })\`. SaaS URLs work via equivalence. (2) For \`assertRcUrlTrusted\` tests, env-token-host snapshot must lock BEFORE rc shim mutates env: sequence is \`resetEnvTokenHostForTesting()\` → delete \`SENTRY\_HOST\`/\`SENTRY\_URL\` → \`captureEnvTokenHost()\` → \`applySentryCliRcEnvShim(testDir)\` → \`assertRcUrlTrusted(testDir)\`. Without explicit capture, lazy auto-capture reads poisoned \`SENTRY\_URL\`. (3) E2E fixture \`createE2EContext\` parent must \`setAuthToken(token, ttl, {host: serverUrl})\` matching child's \`SENTRY\_URL\`; multi-region tests need \`registerTrustedRegionUrls\` during \`listOrganizationsUncached\` before fan-out (regional mocks on different localhost ports, no SaaS equivalence). Symptom: \`HostScopeError: Refusing to send credentials\`. + +* **Bot review triage: distinguish real bugs from SDK-mirroring false positives**: When Sentry Seer or Cursor Bugbot flags 'unusual' code that intentionally mirrors upstream SDK behavior (e.g., \`http\_proxy\` as last-resort fallback for HTTPS URLs — deliberate in \`@sentry/node-core\` \`applyNoProxyOption\`), decline with a written rationale referencing the SDK source rather than silently changing behavior. Removing the mirror creates a divergence where users get different proxy semantics from our transport vs. the SDK default. BYK's pattern: verify against \`node\_modules/@sentry/node-core/build/esm/transports/http.js\`, post a reply explaining the precedent, and resolve the thread. Real bugs (uppercase env var support, whitespace trimming, wildcard handling) get fixed; SDK-mirroring 'bugs' get explained and dismissed. diff --git a/biome.jsonc b/biome.jsonc index f80b99417..1cb2f9146 100644 --- a/biome.jsonc +++ b/biome.jsonc @@ -6,7 +6,8 @@ "./lint-rules/no-process-stdout-in-commands.grit", "./lint-rules/no-raw-metadata-queries.grit", "./lint-rules/no-manual-transactions.grit", - "./lint-rules/no-inline-touch-cache.grit" + "./lint-rules/no-inline-touch-cache.grit", + "./lint-rules/no-stderr-write-in-commands.grit" ], "files": { "includes": ["!docs", "!test/init-eval/templates"] diff --git a/lint-rules/no-stderr-write-in-commands.grit b/lint-rules/no-stderr-write-in-commands.grit new file mode 100644 index 000000000..86e194806 --- /dev/null +++ b/lint-rules/no-stderr-write-in-commands.grit @@ -0,0 +1,5 @@ +file($name, $body) where { + $name <: r".*src/commands/.*", + $body <: contains `stderr.write($args)` as $call, + register_diagnostic(span=$call, message="Don't call stderr.write() in commands. Use logger.withTag('command-name').warn() for warnings or log.debug() for diagnostics.") +} diff --git a/src/commands/issue/merge.ts b/src/commands/issue/merge.ts index b56ea9b13..d280dacd2 100644 --- a/src/commands/issue/merge.ts +++ b/src/commands/issue/merge.ts @@ -29,7 +29,7 @@ import { ResolutionError, ValidationError, } from "../../lib/errors.js"; -import { muted, warning } from "../../lib/formatters/index.js"; +import { muted } from "../../lib/formatters/index.js"; import { CommandOutput } from "../../lib/formatters/output.js"; import { logger } from "../../lib/logger.js"; import { buildIssueUrl } from "../../lib/sentry-urls.js"; @@ -313,9 +313,9 @@ export const mergeCommand = buildCommand({ if (requestedParentId && requestedParentId !== raw.parent) { const requestedShortId = idToShort.get(requestedParentId) ?? flags.into; - this.stderr.write( - `${warning("Warning:")} --into '${requestedShortId}' was a preference, not a guarantee. ` + - `Sentry selected ${parentShortId} as the canonical parent based on event count.\n` + log.warn( + `--into '${requestedShortId}' was a preference, not a guarantee. ` + + `Sentry selected ${parentShortId} as the canonical parent based on event count.` ); } diff --git a/test/commands/issue/merge.func.test.ts b/test/commands/issue/merge.func.test.ts index 8d117e94a..319a89d10 100644 --- a/test/commands/issue/merge.func.test.ts +++ b/test/commands/issue/merge.func.test.ts @@ -46,15 +46,13 @@ function makeMockIssue(overrides?: Partial): SentryIssue { function createMockContext() { const stdoutWrite = mock(() => true); - const stderrWrite = mock(() => true); return { context: { stdout: { write: stdoutWrite }, - stderr: { write: stderrWrite }, + stderr: { write: mock(() => true) }, cwd: "/tmp", }, stdoutWrite, - stderrWrite, }; } @@ -312,13 +310,25 @@ describe("mergeCommand.func()", () => { // User asked for CLI-B, but Sentry picked CLI-A (e.g. larger by count) mergeSpy.mockResolvedValue({ parent: "10A", children: ["10B"] }); - const { context, stderrWrite } = createMockContext(); - const func = await mergeCommand.loader(); - await func.call(context, { json: false, into: "CLI-B" }, "CLI-A", "CLI-B"); + // Warning now goes through log.warn() (consola → process.stderr), not + // this.stderr.write(). Spy on process.stderr.write to capture it. + const stderrSpy = spyOn(process.stderr, "write"); + try { + const { context } = createMockContext(); + const func = await mergeCommand.loader(); + await func.call( + context, + { json: false, into: "CLI-B" }, + "CLI-A", + "CLI-B" + ); - const stderr = stderrWrite.mock.calls.map((c) => String(c[0])).join(""); - expect(stderr).toContain("--into 'CLI-B' was a preference"); - expect(stderr).toContain("CLI-A as the canonical parent"); + const stderr = stderrSpy.mock.calls.map((c) => String(c[0])).join(""); + expect(stderr).toContain("--into 'CLI-B' was a preference"); + expect(stderr).toContain("CLI-A as the canonical parent"); + } finally { + stderrSpy.mockRestore(); + } }); test("does not warn when --into preference was honored", async () => { @@ -334,12 +344,22 @@ describe("mergeCommand.func()", () => { // User asked for CLI-B, Sentry agreed. mergeSpy.mockResolvedValue({ parent: "10B", children: ["10A"] }); - const { context, stderrWrite } = createMockContext(); - const func = await mergeCommand.loader(); - await func.call(context, { json: false, into: "CLI-B" }, "CLI-A", "CLI-B"); + const stderrSpy = spyOn(process.stderr, "write"); + try { + const { context } = createMockContext(); + const func = await mergeCommand.loader(); + await func.call( + context, + { json: false, into: "CLI-B" }, + "CLI-A", + "CLI-B" + ); - const stderr = stderrWrite.mock.calls.map((c) => String(c[0])).join(""); - expect(stderr).not.toContain("--into"); + const stderr = stderrSpy.mock.calls.map((c) => String(c[0])).join(""); + expect(stderr).not.toContain("--into"); + } finally { + stderrSpy.mockRestore(); + } }); test("rejects duplicate issue IDs after resolution (same issue in multiple forms)", async () => {