diff --git a/AGENTS.md b/AGENTS.md index 27eed8233..786d1a9b7 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -991,89 +991,86 @@ mock.module("./some-module", () => ({ ### Architecture - -* **api-client.ts split into domain modules under src/lib/api/**: Monolithic \`src/lib/api-client.ts\` was split into domain modules under \`src/lib/api/\`: infrastructure.ts, organizations.ts, projects.ts, teams.ts, repositories.ts, issues.ts, events.ts, traces.ts, logs.ts, seer.ts, trials.ts, users.ts. Original file is now a ~100-line barrel re-export (already in \`biome.jsonc\` \`noBarrelFile\` override). Add new API functions to the appropriate domain module, not the barrel file. + +* **@sentry/api SDK integration: type wrapping pattern and pagination helpers**: @sentry/api SDK integration: wrap SDK types at \`src/lib/api/\*.ts\` boundaries with \`as unknown as SentryX\` casts; never leak SDK types to commands. Wrappers in \`src/types/sentry.ts\` use \`Partial\ & RequiredCore\`. \`src/lib/region.ts\` imports \`retrieveAnOrganization\` directly to avoid circular dep with api-client. \`unwrapResult\`/\`unwrapPaginatedResult\` MUST stay CLI-owned — SDK versions throw plain \`Error\`, breaking the 'all errors are CliError subclasses' invariant (see also 365e4299). Body-shape casts use \`Parameters\\[0]\["body"]\`. - -* **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. + +* **apiRequestToRegion/rawApiRequest options shape — no timeout/signal/headers on the typed API**: \`ApiRequestOptions\\` in \`src/lib/api/infrastructure.ts\` has only \`{ method, body, params, schema }\`. \`rawApiRequest\` adds \`headers?\`; neither exposes \`timeout\`/\`signal\`. Call sites pass \`(url, init: RequestInit)\` to authenticated fetch — never a \`Request\` (only @sentry/api SDK does). \`apiRequestToRegion\` auto-sets JSON Content-Type and \`JSON.stringify\`s body; \`rawApiRequest\` preserves string bodies, only sets JSON Content-Type when body is object and caller didn't provide one. 204/205 throw \`ApiError\` rather than crashing on \`response.json()\` — bulk-mutate callers must catch. - -* **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. + +* **Completion fast-path skips Sentry SDK via SENTRY\_CLI\_NO\_TELEMETRY and SQLite telemetry queue**: Shell completions (\`\_\_complete\`) set \`SENTRY\_CLI\_NO\_TELEMETRY=1\` in \`bin.ts\` before any imports, skipping \`createTracedDatabase\` and avoiding \`@sentry/node-core/light\` load (~85ms). Completion timing queued to \`completion\_telemetry\_queue\` SQLite table (~1ms); normal runs drain via \`DELETE FROM ... RETURNING\` and emit as \`Sentry.metrics.distribution\`. Achieves ~60ms dev / ~140ms CI within 200ms e2e budget. - -* **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\`. + +* **Fuzzy recovery auto-resolves dash/underscore slug mismatches without original-slug fallback**: Display-name project input (contains spaces) skips slug lookup, goes to name-based fuzzy search across four resolution sites: \`resolveProjectBySlug\`, \`resolveOrgProjectTarget\` (project-search), \`org-list.ts#handleProjectSearch\`, \`project/list.ts#handleProjectNotFound\`. \`parseOrgProjectArg\` detects spaces via \`looksLikeDisplayName()\` and sets \`originalSlug\` on \`project-search\`; sites check \`isDisplayName = originalSlug !== undefined\` and skip \`findProjectsBySlug\` (404s on URL-encoded spaces), going directly to \`triageProjectNotFound\` → \`findSimilarProjectsAcrossOrgs\`. \*\*Critical\*\*: recursive fuzzy recovery calls must NOT pass \`originalSlug\` — otherwise the recovered slug also skips lookup, causing infinite skip→empty→not-found loop. - -* **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. + +* **Project cache is org-scoped with three key formats and three population paths**: \`project\_cache\` SQLite table uses three key shapes: \`{orgId}:{projectId}\` (DSN resolution), \`dsn:{publicKey}\` (DSN without orgId), \`list:{orgSlug}/{projectSlug}\` (batch from API). Helpers: \`getCachedProject\`, \`getCachedProjectByDsnKey\`, \`getCachedProjectsForOrg\` (completions), \`getCachedProjectBySlug\` (queries all three shapes for hot-path slug lookups; used by \`fetchProjectId\` to skip \`GET /projects/{org}/{project}/\`). Population paths: DSN resolution in resolve-target.ts, \`listProjects()\` batch via \`cacheProjectsForOrg\`, \`fetchProjectId\` seeds on API success. Resolution errors use live API via \`findSimilarProjectsAcrossOrgs\` — no cross-org cache search. -### 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. + +* **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. -### Gotcha + +* **Sentry API: events require org+project, issues have legacy global endpoint**: Sentry API scoping/auth quirks: (1) Events require org+project (\`/projects/{org}/{project}/events/{id}/\`); issues use legacy global \`/api/0/issues/{id}/\`; traces need only org. Cross-project search via Discover \`/organizations/{org}/events/?query=id:{eventId}\`. (2) \`/users/me/\` returns 403 for OAuth tokens — use \`/auth/\` instead (all token types, control silo). \`getControlSiloUrl()\` routes; \`SentryUserSchema\` uses \`.passthrough()\` since \`/auth/\` only requires \`id\`. (3) Chunk upload endpoint returns camelCase (\`chunkSize\`, \`chunksPerRequest\`, \`maxRequestSize\`, \`hashAlgorithm\`); \`AssembleResponse\` also camelCase — exception to snake\_case convention. - -* **--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 CLI authenticated fetch architecture with response caching**: Authenticated fetch (\`createAuthenticatedFetch\` in \`src/lib/sentry-client.ts\`): auth headers, 30s \`REQUEST\_TIMEOUT\_MS\`, retry max 2, 401 refresh, span tracing. Dual input: SDK \`Request\` vs \`(url, init)\`. Body-reuse: \`buildAttemptFactory\` yields fresh \`(input, init)\` per attempt; \`Request\` clones; \`FormData\`/\`Blob\`/\`URLSearchParams\` pass through. Only bare \`ReadableStream\` needs materialization. Do NOT materialize FormData — strips auto-negotiated multipart boundary. Timeouts: internal aborts tagged \`INTERNAL\_TIMEOUT\_MARKER\` Symbol; last attempt throws \`TimeoutError\`. Per-endpoint \`ENDPOINT\_TIMEOUT\_OVERRIDES\` (e.g. \`/autofix/\` 120s). Response cache: \`http-cache-semantics\` RFC 7234 at \`~/.sentry/cache/responses/\`; GET 2xx only. On 4xx/5xx, \`apiRequestToRegion\` attaches allow-listed response headers to Sentry scope as \`api\_response\_headers\` context. - -* **@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 Link header for pagination. \`per\_page\` not in SDK types; cast query to pass at runtime. Also: SDK returns \`data = {}\` (not \`\[]\`) when response body is empty/204/missing Content-Type — \`unwrapResult\` returns as-is, so always guard array results with \`Array.isArray(data)\` before \`.map()\`. Self-hosted instances behind reverse proxies commonly trigger this. + +* **Sentry CLI resolve-target cascade has 5 priority levels with env var support**: resolve-target.ts cascade has 5 priority levels: (1) Explicit CLI flags, (2) SENTRY\_ORG/SENTRY\_PROJECT env vars, (3) SQLite config defaults, (4) DSN auto-detection, (5) Directory name inference. SENTRY\_PROJECT supports combo notation \`org/project\` — when used, SENTRY\_ORG is ignored. If combo parse fails (e.g. \`org/\`), the entire value is discarded. \`resolveFromEnvVars()\` helper is injected into all four resolution functions. - -* **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 (empty-body suppressions trigger \`suppressions/unused\`). \`lint:fix\` differs from CI \`lint\`: auto-fix hides \`noPrecisionLoss\` on >2^53 literals, \`noIncrementDecrement\`, and import ordering. biome-ignore suppressions only valid when rule actually fires. Always \`bun run lint\` before pushing. +### Decision - -* **Bun --isolate coverage inflates LF count for files with verbose comments/JSDoc**: Under \`bun test --isolate --parallel\` (used in CI's \`test:unit\`), Bun's coverage instrumentation reports a higher \`LF\` (lines found) count for files than non-isolate runs — comments, blank lines, type annotations, and closing braces inside function bodies get counted as "executable". Locally \`LF=165\` for \`zstd-transport.ts\` becomes \`LF=210\` under \`--isolate\`, dropping coverage from 99% to 78%. Codecov sees the inflated number. Workaround: trim verbose inline comments inside function bodies (move rationale to JSDoc above the function or extract to module-level doc). Doesn't affect correctness, just codecov's patch coverage gate. Statement coverage is still 100% — the "missing" lines are non-executable. + +* **Issue list global limit with fair per-project distribution and representation guarantees**: \`issue list --limit\` is a global total across all detected projects. \`fetchWithBudget\` Phase 1 divides evenly, Phase 2 redistributes surplus via cursor resume. \`trimWithProjectGuarantee\` ensures ≥1 issue per project before filling remaining slots. JSON output wraps in \`{ data, hasMore }\` with optional \`errors\` array. Compound cursor (pipe-separated) enables \`-c last\` for multi-target pagination, keyed by sorted target fingerprint. - -* **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 \ +* **Prefer dedicated SQLite tables + migrations over metadata KV for non-trivial caches**: Prefer dedicated SQLite tables + migrations over \`metadata\` KV for non-trivial caches. Schema migrations are cheap — don't shoehorn structured caches into \`metadata\` with dotted-prefix keys. Dedicated tables give clearer schema, proper indexes, simpler bulk-clear, no prefix collisions. \`metadata\` KV is fine for small scalars (defaults.\*, install.\*). Example: \`issue\_org\_cache\` (schema v15) replaced \`metadata\` keys \`issue\_org.{numericId}\`. Migration pattern: bump \`CURRENT\_SCHEMA\_VERSION\`, add \`EXPECTED\_TABLES.foo\`, add \`if (currentVersion < N) db.exec(EXPECTED\_TABLES.foo)\`. HTTP response cache (URL+headers, short TTLs) can't answer structural questions like 'which org owns issue 123?' — use dedicated tables for structural/mapping questions, HTTP cache for content. - -* **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\`. + +* **@sentry/api SDK can return non-array data for empty/edge responses**: \`@sentry/api\` SDK (in \`node\_modules/@sentry/api/dist/index.js\`) returns \`data = {}\` (not \`\[]\`) when response body is empty, has \`Content-Length: 0\`, or status 204; and returns a \`ReadableStream\` when \`Content-Type\` is missing. \`unwrapResult\` from \`src/lib/api/infrastructure.ts\` returns \`data\` as-is, and \`as unknown as SentryX\[]\` casts silently lie. Always guard array-typed SDK results with \`Array.isArray(data)\` before \`.map()\` — applied in \`listOrganizationsInRegion\` (CLI-1CQ). Self-hosted instances behind reverse proxies (nginx, Cloudflare, WAFs) commonly trigger this by stripping bodies or wrapping responses. Throw a descriptive \`ApiError\` on mismatch rather than letting \`TypeError: x.map is not a function\` bubble up minified. -### Pattern + +* **Bun bytecode: true crashes esbuild→compile ESM bundles (Bun 1.3.11)**: Bun build flags for compiled CLI (\`script/build.ts\`): (1) Do NOT enable \`bytecode: true\` with esbuild→\`Bun.build({ compile })\` pipeline. Still broken on Bun 1.3.13 — crashes \`TypeError: Expected CommonJS module to have a function wrapper\` at entry.instantiate (esbuild emits ESM; bytecode loader mis-caches as CJS). Exit 0, no output. Upstream: oven-sh/bun#21097, #23490. (2) Pass \`autoloadDotenv: false\` and \`autoloadBunfig: false\` — otherwise user's \`.env\`/\`bunfig.toml\` silently injects into \`process.env\` (e.g. Next.js \`.env.local\` could override stored OAuth token). Shell env vars still work; suggest direnv for dir-scoped vars. - -* **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 via \`statSync\` with exponential backoff. Budget: 6 attempts, 5 sleeps (100+200+400+800+1600 = 3.1s). Loop breaks BEFORE final sleep, so \`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, causing 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, translates to \`UpgradeError('execution\_failed')\`. + +* **dist/bin.cjs runtime Node version check must match engines.node**: \`engines.node >=22.12\` matches Astro 6 floor. CI builds matrix \`\["22","24"]\`; docs jobs pin \`actions/setup-node@v6\` with \`node-version: "24"\` after \`setup-bun\`. The npm package's \`dist/bin.cjs\` (from \`script/bundle.ts\`) contains an inline Node guard that MUST match \`engines.node\`. Simple \`parseInt(process.versions.node) < 22\` misses 22.0.0–22.11.x — use explicit major+minor: \`let v=process.versions.node.split('.').map(Number);if(v\[0]<22||(v\[0]===22&\&v\[1]<12)){...}\`. When bumping, update BIN\_WRAPPER string AND error message in lockstep. Without \`engine-strict=true\`, npm only warns — the runtime guard is real enforcement. - -* **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. + +* **Making clearAuth() async breaks model-based tests — use non-async Promise\ return instead**: Making \`clearAuth()\` \`async\` breaks fast-check model-based tests — real async yields during \`asyncModelRun\` cause \`createIsolatedDbContext\` cleanup to interleave. Keep non-async; return \`clearResponseCache().catch(...)\` directly. Model-based tests also need explicit timeouts (e.g., \`30\_000\`) — Bun's default 5s causes false failures during shrinking. - -* **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. + +* **script/generate-api-schema.ts regex is brittle against SDK bundler output changes**: \`script/generate-api-schema.ts\` parses \`node\_modules/@sentry/api/dist/index.js\` with a regex (\`/var (\w+) = \\(options\S\*\\) => \\(options\S\*client \\?\\? client\\)\\.(\w+)\\(/g\`) to map SDK function names to URL+method pairs, producing \`src/generated/api-schema.json\`. If the SDK changes its generator's bundle format (e.g., switches to \`const\`, arrow vs function, different client-selection pattern), schema generation silently produces empty \`fn\` fields. When bumping \`@sentry/api\`, verify \`sentry schema\` output still populates function names. \`src/generated/api-schema.json\` is gitignored — regenerates on every dev/build/typecheck via \`bun run generate:schema\`. - -* **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. + +* **Source Map v3 spec allows null entries in sources array**: The Source Map v3 spec allows \`null\` entries in the \`sources\` array, and bundlers like esbuild actually produce them. Any code iterating over \`sources\` and calling string methods (e.g., \`.replaceAll()\`) must guard against null: \`map.sources.map((s) => typeof s === "string" ? s.replaceAll("\\\\", "/") : s)\`. Without the guard, \`null.replaceAll()\` throws \`TypeError\`. This applies to \`src/lib/sourcemap/debug-id.ts\` and any future sourcemap manipulation code. - -* **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. + +* **Starlight 0.33+ route data moved from Astro.props to Astro.locals.starlightRoute**: Starlight 0.33+ / Astro 6 docs migration: (1) Route data moved from \`Astro.props\` to \`Astro.locals.starlightRoute\` — old \`Astro.props.sidebar\` is \`undefined\`. Field rename: \`slug\` → \`id\`. Import types via \`@astrojs/starlight/route-data\`. Built-in children (SiteTitle, Search, SocialIcons) take no props. \`starlight.social\` is array-form. (2) Content collections must migrate to Content Layer API: rename \`src/content/config.ts\` → \`src/content.config.ts\`, use \`docsLoader()\` + \`docsSchema()\`. Landing-page detection: \`id === ""\` (\`normalizeIndexSlug\` maps \`"index"\` → \`""\`). - -* **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. +### Pattern - -* **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\`. + +* **Bun global installs use .bun path segment for detection**: Bun global installs place scripts under \`~/.bun/install/global/node\_modules/\`. In \`detectPackageManagerFromPath()\`, check \`segments.includes('.bun')\` before npm fallback. Order: \`.pnpm\` → pnpm, \`.bun\` → bun, other \`node\_modules\` → npm. Yarn classic shares npm layout so falls through — acceptable because path detection is \*\*fallback\*\* after subprocess calls (which identify yarn correctly). Path detection must NOT override stored DB info, only serve as fallback when subprocess fails (e.g., Windows \`.cmd\` ENOENT). - -* **Race-free delayed-write test: poll for precondition before mutating**: PTY/stdin test gotchas: (1) Race-free delayed-write test — when testing retry/recovery that mutates a file mid-loop, don't use \`Bun.sleep(N)\` + \`Bun.write(good)\`; slow CI can let good write land BEFORE production's bad write. Pattern: delayed writer polls until bad state exists, THEN overwrites. Add \`expect(onDisk).toEqual(good)\` after. Example: \`test/lib/upgrade.test.ts\`. (2) PTY reproduction of \`curl | bash\` requires Python \`pty.fork()\` + \`os.dup2\` to set fd 0 to a pipe before \`execvp('bash', '-c', 'exec bin \ +* **Evict-then-read pattern: return cacheEvicted flag from helpers that clear cache on 404**: When a helper function transparently evicts a stale cache entry on 404 and falls back to an unscoped call, callers holding the now-stale cached value will let it win \`??\` chains. Fix: helper must return \`{ result, cacheEvicted }\` so callers compute \`effectiveCachedValue = cacheEvicted ? null : cachedValue\` before the \`??\` fallback, and re-cache the freshly-derived value. Applied in \`fetchIssueByNumericId\` in \`src/commands/issue/utils.ts\` — both \`resolveNumericIssue\` and \`resolveShareIssue\` consume the flag. A local cached variable outliving its DB entry is the common shape of this bug; always audit post-eviction read paths. - -* **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. + +* **Non-essential DB cache writes should be guarded with try-catch**: Non-essential DB cache writes (e.g., \`setUserInfo()\`, \`setInstallInfo()\`) must be wrapped in try-catch so a broken/read-only DB doesn't crash a command whose primary operation succeeded. Pattern: \`try { setInstallInfo(...) } catch { log.debug(...) }\`. In login.ts, \`getCurrentUser()\` failure after token save must not block auth — log warning, continue. In upgrade.ts, \`setInstallInfo\` after legacy detection is guarded same way. Exception: \`getUserRegions()\` failure should \`clearAuth()\` and fail hard (indicates invalid token). This is enforced by BugBot reviews — any \`setInstallInfo\`/\`setUserInfo\` call outside setup.ts's \`bestEffort()\` wrapper needs its own try-catch. - -* **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\`. + +* **Sentry CLI command docs are auto-generated from Stricli route tree with CI freshness check**: Sentry CLI command docs are auto-generated from Stricli route tree: Docs in \`docs/src/content/docs/commands/\*.md\` and skill files in \`plugins/sentry-cli/skills/sentry-cli/references/\*.md\` are generated via \`bun run generate:docs\`. Content between \`\\` markers is regenerated; hand-written examples go in \`docs/src/fragments/commands/\`. CI checks \`check:command-docs\` and \`check:skill\` fail if stale. Run generators after changing command parameters/flags/docs. - -* **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). + +* **Stricli buildCommand output config injects json flag into func params**: Stricli command gotchas: (1) In \`func()\` handlers use \`this.stdout\`/\`this.stderr\` directly — NOT \`this.process.stdout\`. \`SentryContext\` has \`process\` and \`stdout\`/\`stderr\` as separate top-level properties; test mocks omit full \`process\` so \`this.process.stdout\` throws \`TypeError\` at runtime (TS doesn't catch). (2) \`output: { json: true, human: formatFn }\` auto-injects \`--json\`/\`--fields\` flags — type flags explicitly (\`flags: { json?: boolean }\`). Commands with interactive side effects (prompts, QR codes) should check \`flags.json\` and skip. (3) Route maps with \`defaultCommand\` blend the default command's flags into subcommand completions — completion tests must track \`hasDefaultCommand\` and skip strict subcommand-matching. - -* **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\`. + +* **Token-type classification via literal prefix match (classifySentryToken)**: Token-type classification via literal prefix match: \`src/lib/token-type.ts\` \`classifySentryToken(token)\` returns \`'org-auth-token'\` (\`sntrys\_\` prefix), \`'user-auth-token'\` (\`sntryu\_\` prefix), or \`'oauth-or-legacy'\`. Case-sensitive \`startsWith\` matches Sentry backend's \`SENTRY\_ORG\_AUTH\_TOKEN\_PREFIX\`. Use to short-circuit commands where a token type is semantically invalid (e.g. \`whoami\` with org token — \`/auth/\` rejects \`sntrys\_\`) before a confusing API failure. \`getAuthToken()\` from \`db/auth\` returns the effective token (env + DB fallback). ### Preference - -* **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. + +* **PR workflow: address review comments, resolve threads, wait for CI**: PR workflow: (1) wait for CI; (2) check unresolved comments via \`gh api repos/.../pulls/N/comments\`; (3) fix in follow-up commits (NEVER amend a pushed commit without explicit user request + force push); (4) reply explaining fix; (5) resolve thread via \`gh api graphql resolveReviewThread\`; (6) push + re-check CI. BugBot/Seer/Warden/Cursor post new comments per-commit and often catch bugs in fix commits — re-check after each push. Dispatch a subagent review before declaring merge-ready; has caught real backwards-compat, error-path, and patch-hygiene bugs. Branches: \`fix/\*\` or \`feat/\*\`. Style: \`Array.from(set)\` over spreads; 'allowlist' not 'whitelist'; \`arr.at(-1)\` over index math. Reviewer questions may be inquiries — confirm intent before changing. diff --git a/src/lib/cache-hint.ts b/src/lib/cache-hint.ts new file mode 100644 index 000000000..4254bff3e --- /dev/null +++ b/src/lib/cache-hint.ts @@ -0,0 +1,81 @@ +/** + * Cache-age hint for command output. + * + * Reads the process-global cache-hit state from `response-cache.ts` and + * formats a human-readable hint like "cached · 3m ago · use -f to refresh". + * Applied automatically by `buildCommand` in `command.ts` — individual + * commands don't need to call this themselves. + * + * getsentry/cli#785 item #1 — `-f/--fresh` flag discoverability. + * + * @module + */ + +import { getLastCacheHitAge } from "./response-cache.js"; + +/** + * Format a millisecond duration as a compact human-readable string. + * + * - `< 5s` → `"just now"` + * - `5s–59s` → `"Ns ago"` + * - `1m–59m` → `"Nm ago"` + * - `1h–23h` → `"Nh ago"` + * - `≥ 24h` → `"Nd ago"` + * + * @internal Exported for testing + */ +export function formatAge(ms: number): string { + const seconds = Math.floor(ms / 1000); + if (seconds < 5) { + return "just now"; + } + if (seconds < 60) { + return `${seconds}s ago`; + } + const minutes = Math.floor(seconds / 60); + if (minutes < 60) { + return `${minutes}m ago`; + } + const hours = Math.floor(minutes / 60); + if (hours < 24) { + return `${hours}h ago`; + } + const days = Math.floor(hours / 24); + return `${days}d ago`; +} + +/** + * Build a cache-age hint string, or `undefined` when the last request was + * not served from cache. + * + * When multiple API calls run in parallel (e.g. `Promise.all`), the + * displayed age corresponds to whichever resolves last — acceptable + * since all hits share similar ages in practice. + * + * Example output: `"cached · 3m ago · use -f to refresh"` + */ +export function formatCacheHint(): string | undefined { + const ageMs = getLastCacheHitAge(); + if (ageMs === undefined) { + return; + } + return `cached · ${formatAge(ageMs)} · use -f to refresh`; +} + +/** + * Append a cache-age hint to an existing hint string. + * + * - Both present → `"existingHint | cached · 3m ago · ..."` + * - Only cache hint → `"cached · 3m ago · ..."` + * - Only existing → `"existingHint"` (unchanged) + * - Neither → `undefined` + */ +export function appendCacheHint( + existingHint: string | undefined +): string | undefined { + const cacheHint = formatCacheHint(); + if (existingHint && cacheHint) { + return `${existingHint} | ${cacheHint}`; + } + return cacheHint ?? existingHint; +} diff --git a/src/lib/command.ts b/src/lib/command.ts index a23ed346f..97bfbc784 100644 --- a/src/lib/command.ts +++ b/src/lib/command.ts @@ -37,6 +37,7 @@ import { numberParser as stricliNumberParser, } from "@stricli/core"; import type { Writer } from "../types/index.js"; +import { appendCacheHint } from "./cache-hint.js"; import { getAuthConfig } from "./db/auth.js"; import { getEnv } from "./env.js"; import { AuthError, CliError, OutputError } from "./errors.js"; @@ -768,9 +769,14 @@ export function buildCommand< } ); - // Render phase: output finalization + // Render phase: output finalization. + // Append cache-age hint automatically so every command gets + // "cached · 3m ago · use -f to refresh" when data was cached. + // Skip bare `return;` paths (e.g. `--web` which opens a browser + // without yielding) — no rendered output should mean no footer. + const finalHint = returned ? appendCacheHint(returned.hint) : undefined; await withTracing("render", "cli.command.render", () => { - writeFinalization(stdout, returned?.hint, cleanFlags.json, renderer); + writeFinalization(stdout, finalHint, cleanFlags.json, renderer); }); } catch (err) { // Finalize before error handling to close streaming state diff --git a/src/lib/help.ts b/src/lib/help.ts index e7f74eb13..487e67505 100644 --- a/src/lib/help.ts +++ b/src/lib/help.ts @@ -30,6 +30,56 @@ type HelpCommand = { description: string; }; +// --------------------------------------------------------------------------- +// Common flags — surfaced in the branded `sentry --help` output +// --------------------------------------------------------------------------- + +/** + * Metadata for a flag shown in the top-level help. + * + * Only the highest-signal flags belong here — per-command flags are + * documented in each command's own `--help`. + */ +type CommonFlagEntry = { + /** Long flag name with `--` prefix (e.g., `"--json"`). */ + long: string; + /** Short alias with `-` prefix, or `undefined` if none. */ + short?: string; + /** One-line description for the branded help. */ + description: string; +}; + +/** + * Flags surfaced in the branded `sentry` help output. + * + * Includes both truly global flags (injected into every command) and + * widely-used flags present on most list/view commands. + */ +const COMMON_FLAGS: readonly CommonFlagEntry[] = [ + { + long: "--json", + description: "Output as JSON (with --fields to select)", + }, + { + long: "--fresh", + short: "-f", + description: "Bypass cache and fetch fresh data", + }, + { + long: "--verbose", + short: "-v", + description: "Enable debug logging", + }, + { + long: "--help", + description: "Show help for a command", + }, + { + long: "--version", + description: "Show version", + }, +]; + /** * Generate the commands list dynamically from Stricli's route structure. * This ensures help text stays in sync with actual registered commands. @@ -100,6 +150,27 @@ function formatCommands(commands: HelpCommand[]): string { .join("\n"); } +/** + * Format the common flags list with aligned descriptions. + * + * Renders flag names (with optional short alias) left-aligned and + * descriptions right-aligned, styled identically to the env-var section. + */ +function formatCommonFlags(): string { + if (COMMON_FLAGS.length === 0) { + return ""; + } + const padding = 4; + const labels = COMMON_FLAGS.map((f) => + f.short ? `${f.short}, ${f.long}` : ` ${f.long}` + ); + const maxLabelLength = Math.max(...labels.map((l) => l.length)); + return COMMON_FLAGS.map((f, i) => { + const labelPadded = (labels[i] ?? "").padEnd(maxLabelLength + padding); + return ` ${cyan(labelPadded)}${muted(f.description)}`; + }).join("\n"); +} + /** * Format the top-level environment variable list with aligned descriptions. * @@ -146,6 +217,14 @@ export function printCustomHelp(): string { lines.push(formatCommands(generateCommands())); lines.push(""); + // Common flags + const flags = formatCommonFlags(); + if (flags) { + lines.push(` ${muted("Flags:")}`); + lines.push(flags); + lines.push(""); + } + // Environment variables (auto-generated from env-registry) const envVars = formatEnvVars(); if (envVars) { @@ -187,6 +266,18 @@ export type HelpEnvVarInfo = { defaultValue?: string; }; +/** + * Metadata for a common flag as exposed to JSON callers of `sentry help --json`. + */ +export type HelpFlagInfo = { + /** Long flag name including `--` prefix (e.g. `"--json"`). */ + long: string; + /** Short alias including `-` prefix (e.g. `"-f"`), or undefined if none. */ + short?: string; + /** One-line description. */ + description: string; +}; + /** * Result of introspecting the CLI. * Yielded as CommandOutput — JSON mode serializes directly, human mode @@ -196,7 +287,13 @@ export type HelpEnvVarInfo = { * it's stripped from JSON output via `jsonExclude`. */ export type HelpJsonResult = - | ({ routes: RouteInfo[]; envVars: HelpEnvVarInfo[] } & { _banner?: string }) + | ({ + routes: RouteInfo[]; + envVars: HelpEnvVarInfo[]; + flags: HelpFlagInfo[]; + } & { + _banner?: string; + }) | CommandInfo | RouteInfo | { error: string; suggestions?: string[] }; @@ -218,19 +315,35 @@ function buildTopLevelEnvVars(): HelpEnvVarInfo[] { })); } +/** + * Build the common flags list for JSON output. + * + * Exposes the same entries shown in the branded help so that consumers + * (AI agents, docs tooling) can discover them programmatically. + */ +function buildCommonFlags(): HelpFlagInfo[] { + return COMMON_FLAGS.map((f) => ({ + long: f.long, + short: f.short, + description: f.description, + })); +} + /** * Introspect the full command tree. * Returns all visible routes with all flags included, plus the top-level - * environment variables recognized by the CLI. + * environment variables and common flags recognized by the CLI. */ export function introspectAllCommands(): { routes: RouteInfo[]; envVars: HelpEnvVarInfo[]; + flags: HelpFlagInfo[]; } { const routeMap = routes as unknown as RouteMap; return { routes: extractAllRoutes(routeMap), envVars: buildTopLevelEnvVars(), + flags: buildCommonFlags(), }; } @@ -378,6 +491,32 @@ function formatGroupHuman(group: RouteInfo): string { return lines.join("\n"); } +/** + * Format the full command tree (without banner) as human-readable text. + * Includes flags and env-var sections when present. + */ +function formatFullTreeHuman(data: { + routes: RouteInfo[]; + envVars?: HelpEnvVarInfo[]; + flags?: HelpFlagInfo[]; +}): string { + const lines: string[] = [data.routes.map(formatGroupHuman).join("\n\n")]; + if (data.flags && data.flags.length > 0) { + lines.push("", "Flags:"); + for (const f of data.flags) { + const label = f.short ? `${f.short}, ${f.long}` : f.long; + lines.push(` ${label} — ${f.description}`); + } + } + if (data.envVars && data.envVars.length > 0) { + lines.push("", "Environment Variables:"); + for (const v of data.envVars) { + lines.push(` ${v.name} — ${v.brief}`); + } + } + return lines.join("\n"); +} + /** * Human renderer for help introspection data. * @@ -413,19 +552,9 @@ export function formatHelpHuman(data: HelpJsonResult): string { } // Full tree without banner (shouldn't happen in practice — the help - // command always attaches one). Keep the envVars in sync anyway. + // command always attaches one). Keep the envVars/flags in sync anyway. if ("routes" in data) { - const routesText = data.routes.map(formatGroupHuman).join("\n\n"); - if ("envVars" in data && data.envVars.length > 0) { - const lines = [ - routesText, - "", - "Environment Variables:", - ...data.envVars.map((v) => ` ${v.name} — ${v.brief}`), - ]; - return lines.join("\n"); - } - return routesText; + return formatFullTreeHuman(data); } return ""; diff --git a/src/lib/response-cache.ts b/src/lib/response-cache.ts index 154b9c755..c1b57a3a2 100644 --- a/src/lib/response-cache.ts +++ b/src/lib/response-cache.ts @@ -314,6 +314,47 @@ function buildResponseHeaders( return result; } +// --------------------------------------------------------------------------- +// Last cache-hit age — process-global signal for cache-age hints +// --------------------------------------------------------------------------- + +/** + * Age of the most recent cache hit, in milliseconds. + * + * Set inside {@link getCachedResponse} on a hit, cleared at the start of each + * `authenticatedFetch` call in `sentry-client.ts`. Commands read it via + * {@link getLastCacheHitAge} to show "cached · 3m ago · use -f to refresh". + * + * Safe because the CLI is single-process, single-command — no races. + */ +let lastCacheHitAgeMs: number | undefined; + +/** + * Get the age (in ms) of the most recent cache hit, or `undefined` if the + * last request was not served from cache. + */ +export function getLastCacheHitAge(): number | undefined { + return lastCacheHitAgeMs; +} + +/** + * Clear the last cache-hit age. Called at the top of each `authenticatedFetch` + * call so the signal reflects only the current request. + */ +export function clearLastCacheHitAge(): void { + lastCacheHitAgeMs = undefined; +} + +/** + * Set the last cache-hit age directly. Test-only — production code paths + * set this implicitly via {@link getCachedResponse} on a real cache hit. + * + * @internal Exported for testing + */ +export function setLastCacheHitAgeForTesting(ageMs: number): void { + lastCacheHitAgeMs = ageMs; +} + // --------------------------------------------------------------------------- // Cache bypass control // --------------------------------------------------------------------------- @@ -426,6 +467,9 @@ export async function getCachedResponse( recordCacheHit("http", true); span.setAttribute("cache.item_size", body.length); + // Surface cache age for command-level hints (getsentry/cli#785 #1) + lastCacheHitAgeMs = Date.now() - entry.createdAt; + const responseHeaders = buildResponseHeaders(policy, entry); return new Response(body, { status: entry.status, diff --git a/src/lib/sentry-client.ts b/src/lib/sentry-client.ts index e7dbb125c..84e240ea3 100644 --- a/src/lib/sentry-client.ts +++ b/src/lib/sentry-client.ts @@ -21,6 +21,7 @@ import { getAuthToken, refreshToken } from "./db/auth.js"; import { HostScopeError, TimeoutError } from "./errors.js"; import { logger } from "./logger.js"; import { + clearLastCacheHitAge, getCachedResponse, invalidateCachedResponsesMatching, storeCachedResponse, @@ -548,6 +549,10 @@ function createAuthenticatedFetch(): ( input: Request | string | URL, init?: RequestInit ): Promise { + // Reset cache-hit age so it reflects only this request's outcome. + // Commands read it after their primary API call to show cache-age hints. + clearLastCacheHitAge(); + // Once-per-process hint when env-var auth token is shadowed by a // stored OAuth login. Runs here (rather than at command entry) so // the hint only fires for commands that actually exercise auth — diff --git a/test/lib/cache-hint.test.ts b/test/lib/cache-hint.test.ts new file mode 100644 index 000000000..1959b4386 --- /dev/null +++ b/test/lib/cache-hint.test.ts @@ -0,0 +1,117 @@ +/** + * Cache-hint formatting tests. + * + * Tests for the human-readable cache-age hint shown in command footers. + * Core invariants (round-trips, validation) are tested here since the + * module is small and the formatting has specific boundary values. + */ + +import { describe, expect, test } from "bun:test"; +import { + appendCacheHint, + formatAge, + formatCacheHint, +} from "../../src/lib/cache-hint.js"; +import { + clearLastCacheHitAge, + getLastCacheHitAge, + setLastCacheHitAgeForTesting, +} from "../../src/lib/response-cache.js"; + +describe("formatAge", () => { + test("returns 'just now' for ages under 5 seconds", () => { + expect(formatAge(0)).toBe("just now"); + expect(formatAge(1000)).toBe("just now"); + expect(formatAge(4999)).toBe("just now"); + }); + + test("returns seconds for ages 5s–59s", () => { + expect(formatAge(5000)).toBe("5s ago"); + expect(formatAge(30_000)).toBe("30s ago"); + expect(formatAge(59_999)).toBe("59s ago"); + }); + + test("returns minutes for ages 1m–59m", () => { + expect(formatAge(60_000)).toBe("1m ago"); + expect(formatAge(180_000)).toBe("3m ago"); + expect(formatAge(59 * 60_000 + 59_999)).toBe("59m ago"); + }); + + test("returns hours for ages 1h–23h", () => { + expect(formatAge(60 * 60_000)).toBe("1h ago"); + expect(formatAge(3 * 60 * 60_000)).toBe("3h ago"); + expect(formatAge(23 * 60 * 60_000 + 59 * 60_000)).toBe("23h ago"); + }); + + test("returns days for ages >= 24h", () => { + expect(formatAge(24 * 60 * 60_000)).toBe("1d ago"); + expect(formatAge(48 * 60 * 60_000)).toBe("2d ago"); + expect(formatAge(7 * 24 * 60 * 60_000)).toBe("7d ago"); + }); +}); + +describe("formatCacheHint", () => { + test("returns undefined when no cache hit recorded", () => { + clearLastCacheHitAge(); + expect(formatCacheHint()).toBeUndefined(); + }); + + test("returns undefined when getLastCacheHitAge is undefined", () => { + clearLastCacheHitAge(); + expect(getLastCacheHitAge()).toBeUndefined(); + expect(formatCacheHint()).toBeUndefined(); + }); + + test("returns formatted hint when cache hit recorded (3m)", () => { + setLastCacheHitAgeForTesting(180_000); + expect(formatCacheHint()).toBe("cached · 3m ago · use -f to refresh"); + clearLastCacheHitAge(); + }); + + test("returns formatted hint when cache hit recorded (just now)", () => { + setLastCacheHitAgeForTesting(0); + expect(formatCacheHint()).toBe("cached · just now · use -f to refresh"); + clearLastCacheHitAge(); + }); + + test("returns formatted hint when cache hit recorded (2h)", () => { + setLastCacheHitAgeForTesting(2 * 60 * 60_000); + expect(formatCacheHint()).toBe("cached · 2h ago · use -f to refresh"); + clearLastCacheHitAge(); + }); +}); + +describe("appendCacheHint", () => { + test("returns undefined when no existing hint and no cache hit", () => { + clearLastCacheHitAge(); + expect(appendCacheHint(undefined)).toBeUndefined(); + }); + + test("returns existing hint unchanged when no cache hit", () => { + clearLastCacheHitAge(); + expect(appendCacheHint("Tip: use -v")).toBe("Tip: use -v"); + }); + + test("returns just the cache hint when no existing hint", () => { + setLastCacheHitAgeForTesting(180_000); + expect(appendCacheHint(undefined)).toBe( + "cached · 3m ago · use -f to refresh" + ); + clearLastCacheHitAge(); + }); + + test("joins existing and cache hints with ' | ' separator", () => { + setLastCacheHitAgeForTesting(180_000); + expect(appendCacheHint("Showing 5 issues")).toBe( + "Showing 5 issues | cached · 3m ago · use -f to refresh" + ); + clearLastCacheHitAge(); + }); + + test("treats empty existing hint as falsy (no separator)", () => { + setLastCacheHitAgeForTesting(180_000); + // Empty string is falsy — appendCacheHint returns just the cache hint. + expect(appendCacheHint("")).toBe("cached · 3m ago · use -f to refresh"); + clearLastCacheHitAge(); + }); +}); diff --git a/test/lib/help.test.ts b/test/lib/help.test.ts index 6d1c61332..6a53a57a6 100644 --- a/test/lib/help.test.ts +++ b/test/lib/help.test.ts @@ -83,6 +83,27 @@ describe("printCustomHelp", () => { expect(output).toContain("SENTRY_LOG_LEVEL"); expect(output).toContain("NO_COLOR"); }); + + test("includes a Flags section with common flags", () => { + const output = stripAnsi(printCustomHelp()); + expect(output).toContain("Flags:"); + expect(output).toContain("--json"); + expect(output).toContain("--fresh"); + expect(output).toContain("-f"); + expect(output).toContain("--verbose"); + expect(output).toContain("-v"); + expect(output).toContain("--help"); + expect(output).toContain("--version"); + }); + + test("Flags section appears before Environment Variables section", () => { + const output = stripAnsi(printCustomHelp()); + const flagsIndex = output.indexOf("Flags:"); + const envVarsIndex = output.indexOf("Environment Variables:"); + expect(flagsIndex).toBeGreaterThan(-1); + expect(envVarsIndex).toBeGreaterThan(-1); + expect(flagsIndex).toBeLessThan(envVarsIndex); + }); }); describe("introspectAllCommands", () => { @@ -113,4 +134,35 @@ describe("introspectAllCommands", () => { expect(v.description.length).toBeGreaterThan(0); } }); + + test("includes a flags array with common flags", () => { + const result = introspectAllCommands(); + expect(Array.isArray(result.flags)).toBe(true); + const longs = result.flags.map((f) => f.long); + expect(longs).toContain("--json"); + expect(longs).toContain("--fresh"); + expect(longs).toContain("--verbose"); + expect(longs).toContain("--help"); + expect(longs).toContain("--version"); + }); + + test("each flags entry has long and description", () => { + const { flags } = introspectAllCommands(); + for (const f of flags) { + expect(typeof f.long).toBe("string"); + expect(f.long.startsWith("--")).toBe(true); + expect(typeof f.description).toBe("string"); + expect(f.description.length).toBeGreaterThan(0); + } + }); + + test("flags with short aliases have the correct format", () => { + const { flags } = introspectAllCommands(); + const fresh = flags.find((f) => f.long === "--fresh"); + expect(fresh).toBeDefined(); + expect(fresh?.short).toBe("-f"); + const verbose = flags.find((f) => f.long === "--verbose"); + expect(verbose).toBeDefined(); + expect(verbose?.short).toBe("-v"); + }); });