diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 52949dc0..8d185c74 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -179,6 +179,10 @@ jobs: run: bun run test:isolated --coverage --coverage-reporter=lcov --coverage-dir=coverage-isolated - name: Merge Coverage Reports run: bun run script/merge-lcov.ts coverage/lcov.info coverage-isolated/lcov.info > coverage/merged.lcov + - name: Make coverage checks informational on release branches + if: github.event_name == 'push' + run: | + sed -i 's/informational: false/informational: true/' codecov.yml - name: Coverage Report uses: getsentry/codecov-action@main with: diff --git a/AGENTS.md b/AGENTS.md index 4459e5ed..171259b2 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -814,22 +814,84 @@ mock.module("./some-module", () => ({ ### Architecture - -* **Sentry SDK switched from @sentry/bun to @sentry/node-core/light**: The CLI switched from \`@sentry/bun\` to \`@sentry/node-core/light\` (PR #474) to eliminate the OpenTelemetry dependency tree (~170ms startup savings). All imports use \`@sentry/node-core/light\` not \`@sentry/bun\`. \`LightNodeClient\` replaces \`BunClient\` in telemetry.ts. \`Span\` type exports from \`@sentry/core\`. The \`@sentry/core\` barrel is patched to remove ~32 unused exports (AI tracing, MCP server, Supabase, feature flags). Light mode is labeled experimental but safe because SDK versions are pinned exactly (not caret). \`makeNodeTransport\` works fine in Bun. The \`cli.runtime\` tag still distinguishes Bun vs Node at runtime. + +* **api-client.ts split into domain modules under src/lib/api/**: The original monolithic \`src/lib/api-client.ts\` (1,977 lines) was split into 12 focused domain modules under \`src/lib/api/\`: infrastructure.ts (shared helpers, types, raw requests), organizations.ts, projects.ts, teams.ts, repositories.ts, issues.ts, events.ts, traces.ts, logs.ts, seer.ts, trials.ts, users.ts. The original \`api-client.ts\` was converted to a ~100-line barrel re-export file preserving all existing import paths. The \`biome.jsonc\` override for \`noBarrelFile\` already includes \`api-client.ts\`. When adding new API functions, place them in the appropriate domain module under \`src/lib/api/\`, not in the barrel file. + + +* **Bun compiled binary sourcemap options and size impact**: Binary build (\`script/build.ts\`) uses two steps: (1) \`Bun.build()\` produces \`dist-bin/bin.js\` + \`.map\` with \`sourcemap: "linked"\` and minification. (2) \`Bun.build()\` with \`compile: true\` produces native binary — no sourcemap embedded. Bun's compiled binaries use \`/$bunfs/root/bin.js\` as the virtual path in stack traces. Sourcemap upload must use \`--url-prefix '/$bunfs/root/'\` so Sentry can match frames. The upload runs \`sentry-cli sourcemaps inject dist-bin/\` first (adds debug IDs), then uploads both JS and map. Bun's compile step strips comments (including \`//# debugId=\`), but debug ID matching still works via the injected runtime snippet + URL prefix matching. Size: +0.04 MB gzipped vs +2.30 MB for inline sourcemaps. Without \`SENTRY\_AUTH\_TOKEN\`, upload is skipped gracefully. + + +* **CLI telemetry DSN is public write-only — safe to embed in install script**: The CLI's Sentry DSN (\`SENTRY\_CLI\_DSN\` in \`src/lib/constants.ts\`) is a public write-only ingest key already baked into every binary. Safe to hardcode in install scripts. Opt-out: \`SENTRY\_CLI\_NO\_TELEMETRY=1\`. + + +* **cli.sentry.dev is served from gh-pages branch via GitHub Pages**: \`cli.sentry.dev\` is served from gh-pages branch via GitHub Pages. Craft's gh-pages target runs \`git rm -r -f .\` before extracting docs — persist extra files via \`postReleaseCommand\` in \`.craft.yml\`. Install script supports \`--channel nightly\`, downloading from the \`nightly\` release tag directly. version.json is only used by upgrade/version-check flow. + + +* **Nightly delta upgrade buildNightlyPatchGraph fetches ALL patch tags — O(N) HTTP calls**: Delta upgrade in \`src/lib/delta-upgrade.ts\` supports stable (GitHub Releases) and nightly (GHCR) channels. \`filterAndSortChainTags\` filters \`patch-\*\` tags by version range using \`Bun.semver.order()\`. GHCR uses \`fetchWithRetry\` (10s timeout + 1 retry; blobs 30s) with optional \`signal?: AbortSignal\` combined via \`AbortSignal.any()\`. \`isExternalAbort(error, signal)\` skips retries for external aborts — critical for background prefetch. Patches cached to \`~/.sentry/patch-cache/\` (file-based, 7-day TTL). \`loadCachedChain\` stitches patches for multi-hop offline upgrades. + + +* **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. + + +* **Numeric issue ID resolution returns org:undefined despite API success**: Numeric issue ID resolution in \`resolveNumericIssue()\`: (1) try DSN/env/config for org, (2) if found use \`getIssueInOrg(org, id)\` with region routing, (3) else fall back to unscoped \`getIssue(id)\`, (4) extract org from \`issue.permalink\` via \`parseSentryUrl\` as final fallback. \`parseSentryUrl\` handles path-based (\`/organizations/{org}/...\`) and subdomain-style URLs. \`matchSubdomainOrg()\` filters region subdomains by requiring slug length > 2. Self-hosted uses path-based only. + + +* **Seer trial prompt uses middleware layering in bin.ts error handling chain**: Error recovery middlewares in \`bin.ts\` are layered: \`main() → executeWithAutoAuth() → executeWithSeerTrialPrompt() → runCommand()\`. Seer trial prompts (for \`no\_budget\`/\`not\_enabled\`) caught by inner wrapper; auth errors bubble to outer. Auth retry goes through full chain. 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. + +### Decision + + +* **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. + + +* **whoami should be separate from auth status command**: The \`sentry auth whoami\` command should be a dedicated command separate from \`sentry auth status\`. They serve different purposes: \`status\` shows everything about auth state (token, expiry, defaults, org verification), while \`whoami\` just shows user identity (name, email, username, ID) by fetching live from \`/auth/\` endpoint. \`sentry whoami\` should be a top-level alias (like \`sentry issues\` → \`sentry issue list\`). \`whoami\` should support \`--json\` for machine consumption and be lightweight — no credential verification, no defaults listing. ### Gotcha - -* **issue explain/plan commands never set sentry.org telemetry tag**: The \`issue explain\` and \`issue plan\` commands were missing \`setContext(\[org], \[])\` calls after \`resolveOrgAndIssueId()\`, so \`sentry.org\` and \`sentry.project\` tags were never set on SeerError events. Fixed in commit 816b44b5 on branch \`fix/seer-org-telemetry-tag\` — both commands now destructure \`setContext\` from \`this\` and call it immediately after resolving the org, before any Seer API call. The \`issue view\` and \`issue list\` commands already had this call. Pattern: always call \`setContext()\` right after org resolution, before any API call that might throw, so error events carry the org tag. + +* **@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 so \`prepareHeaders\` creates empty headers — on Node.js this strips Content-Type (HTTP 415). Fix: fall back to \`input.headers\` when \`init\` is undefined. Use \`unwrapPaginatedResult\` (not \`unwrapResult\`) to access the Response's Link header for pagination. \`per\_page\` is not in SDK types; cast query to pass it at runtime. + + +* **Bun binary build requires SENTRY\_CLIENT\_ID env var**: The build script (\`script/bundle.ts\`) requires \`SENTRY\_CLIENT\_ID\` environment variable and exits with code 1 if missing. When building locally, use \`bun run --env-file=.env.local build\` or set the env var explicitly. The binary build (\`bun run build\`) also needs it. Without it you get: \`Error: SENTRY\_CLIENT\_ID environment variable is required.\` - -* **project list bare-slug org fallback requires custom handleProjectSearch**: The \`project list\` command has a custom \`handleProjectSearch\` override (not the shared one from \`org-list.ts\`). When a user types \`sentry project list acme-corp\` where \`acme-corp\` is an org slug (not a project), the custom handler must check if the bare slug matches an organization and fall back to listing that org's projects. The shared \`handleProjectSearch\` in \`org-list.ts\` already does this, but custom overrides in individual commands can miss it, causing a confusing \`ResolutionError\` (PR #475 fix). + +* **GitHub immutable releases prevent rolling nightly tag pattern**: getsentry/cli has immutable GitHub releases — assets can't be modified and tags can NEVER be reused. Nightly builds publish to GHCR with versioned tags like \`nightly-0.14.0-dev.1772661724\`, not GitHub Releases or npm. \`fetchManifest()\` throws \`UpgradeError("network\_error")\` for both network failures and non-200 — callers must check message for HTTP 404/403. Craft with no \`preReleaseCommand\` silently skips \`bump-version.sh\` if only target is \`github\`. + + +* **Install script: BSD sed and awk JSON parsing breaks OCI digest extraction**: The install script parses OCI manifests with awk (no jq). Key trap: BSD sed \`\n\` is literal, not newline. Fix: single awk pass tracking last-seen \`"digest"\`, printing when \`"org.opencontainers.image.title"\` matches target. The config digest (\`sha256:44136fa...\`) is a 2-byte \`{}\` blob — downloading it instead of the real binary causes \`gunzip: unexpected end of file\`. + + +* **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\`. + + +* **Multiple mockFetch calls replace each other — use unified mocks for multi-endpoint tests**: Bun test mocking gotchas: (1) \`mockFetch()\` replaces \`globalThis.fetch\` — calling it twice replaces the first mock. Use a single unified fetch mock dispatching by URL pattern. (2) \`mock.module()\` pollutes the module registry for ALL subsequent test files. Tests using it must live in \`test/isolated/\` and run via \`test:isolated\`. This also causes \`delta-upgrade.test.ts\` to fail when run alongside \`test/isolated/delta-upgrade.test.ts\` — the isolated test's \`mock.module()\` replaces \`CLI\_VERSION\` for all subsequent files. (3) For \`Bun.spawn\`, use direct property assignment in \`beforeEach\`/\`afterEach\`. + + +* **Seer trial prompt requires interactive terminal — AI agents never see it**: The Seer trial prompt middleware (\`executeWithSeerTrialPrompt\`) checks \`isatty(0)\` — AI agents running non-interactively never see it, so \`SeerError\` propagates with a generic message. Fix: \`SeerError.format()\` should include an actionable command like \`sentry trial start seer \\`. Also, \`handleSeerApiError\` must return the original \`ApiError\`, not wrap in plain \`Error\` — wrapping loses the type and breaks downstream \`instanceof ApiError\` checks in middleware. + + +* **useTestConfigDir without isolateProjectRoot causes DSN scanning of repo tree**: \`useTestConfigDir()\` creates temp dirs under \`.test-tmp/\` in the repo tree. Without \`{ isolateProjectRoot: true }\`, \`findProjectRoot\` walks up and finds the repo's \`.git\`, causing DSN detection to scan real source code and trigger network calls against test mocks (timeouts). Always pass \`isolateProjectRoot: true\` when tests exercise \`resolveOrg\`, \`detectDsn\`, or \`findProjectRoot\`. ### Pattern - -* **Dashboard widget layout uses 6-column grid with x,y,w,h**: Sentry dashboards use a 6-column grid with \`{x, y, w, h, minH}\` layout. Default big\_number is \`w=2, h=1\`; to center it use \`w=4, x=1\`. Tables default to \`w=6, h=2\`. The \`dashboard widget edit\` command only handles query/display/title — use \`sentry api\` with PUT to \`/api/0/organizations/{org}/dashboards/{id}/\` for layout, default period, environment, and project changes. The PUT payload must include the full \`widgets\` array. Widget queries need both \`fields\` (all columns + aggregates) and \`columns\` (non-aggregate columns only) arrays. Use \`user.display\` instead of \`user.email\` for user columns — it auto-selects the best available identifier. + +* **Delta upgrade patch generation uses zig-bsdiff with zstd in CI**: CI generates delta patches between nightly builds using \`zig-bsdiff\` (v0.1.19) with \`--use-zstd\` flag. Workflow: download previous nightly from GHCR, run \`bsdiff old new patch --use-zstd\` for each platform binary, push patch manifest as \`patch-\\` GHCR tag with SHA-256 annotations per binary. To test locally: download zig-bsdiff from github.com/blackboardsh/zig-bsdiff/releases, build two versions with a source change between them, run \`bsdiff v1 v2 output.patch --use-zstd\`. ORAS CLI (v1.2.3) handles GHCR push/fetch. Patches are non-fatal (\`continue-on-error: true\` in CI). + + +* **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. + + +* **Org-scoped SDK calls follow getOrgSdkConfig + unwrapResult pattern**: All org-scoped API calls in src/lib/api-client.ts: (1) call \`getOrgSdkConfig(orgSlug)\` for regional URL + SDK config, (2) spread into SDK function: \`{ ...config, path: { organization\_id\_or\_slug: orgSlug, ... } }\`, (3) pass to \`unwrapResult(result, errorContext)\`. Shared helpers \`resolveAllTargets\`/\`resolveOrgAndProject\` must NOT call \`fetchProjectId\` — commands that need it enrich targets themselves. + + +* **PR workflow: wait for Seer and Cursor BugBot before resolving**: CI includes Seer Code Review and Cursor Bugbot as advisory checks (~2-3 min, only on ready-for-review PRs). Workflow: push → wait for all CI (including npm build) → check inline review comments from Seer/BugBot → fix valid findings → repeat. Bugbot sometimes catches real logic bugs, not just style — always review before merging. Use \`gh pr checks \ --watch\` to monitor. Fetch comments via \`gh api repos/OWNER/REPO/pulls/NUM/comments\`. + + +* **Shared pagination infrastructure: buildPaginationContextKey and parseCursorFlag**: List commands with cursor pagination use \`buildPaginationContextKey(type, identifier, flags)\` for composite context keys and \`parseCursorFlag(value)\` accepting \`"last"\` magic value. Critical: \`resolveCursor()\` must be called inside the \`org-all\` override closure, not before \`dispatchOrgScopedList\` — otherwise cursor validation errors fire before the correct mode-specific error. + + +* **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). - -* **Issue resolution fan-out uses unbounded Promise.all across orgs**: \`resolveProjectSearch()\` in \`src/commands/issue/utils.ts\` fans out \`tryGetIssueByShortId\` across ALL orgs via \`Promise.all\`. Pattern: expand short ID, list all orgs, fire lookups in parallel, collect successes. Exactly one success → use it; multiple → ambiguity error; all fail → fall back to \`resolveProjectSearchFallback()\`. All org fan-out concurrency is controlled by \`ORG\_FANOUT\_CONCURRENCY\` (exported from \`src/lib/api/infrastructure.ts\`, re-exported via \`api-client.ts\`). This single constant is used in \`events.ts\`, \`projects.ts\`, \`issue/utils.ts\`, and any future fan-out sites. Never define a local concurrency limit for org fan-out — always import the shared constant. + +* **Testing Stricli command func() bodies via spyOn mocking**: To unit-test a Stricli command's \`func()\` body: (1) \`const func = await cmd.loader()\`, (2) \`func.call(mockContext, flags, ...args)\` with mock \`stdout\`, \`stderr\`, \`cwd\`, \`setContext\`. (3) \`spyOn\` namespace imports to mock dependencies (e.g., \`spyOn(apiClient, 'getLogs')\`). The \`loader()\` return type union causes \`.call()\` LSP errors — these are false positives that pass \`tsc --noEmit\`. When API functions are renamed (e.g., \`getLog\` → \`getLogs\`), update both spy target name AND mock return shape (single → array). Slug normalization (\`normalizeSlug\`) replaces underscores with dashes but does NOT lowercase — test assertions must match original casing (e.g., \`'CAM-82X'\` not \`'cam-82x'\`). diff --git a/codecov.yml b/codecov.yml index 37b098c4..86691fbc 100644 --- a/codecov.yml +++ b/codecov.yml @@ -1,3 +1,11 @@ comment: true config: files: changed +coverage: + status: + project: + default: + informational: true + patch: + default: + informational: false