From 6d0934d4d9545abcfb18467e99968132fdd1f39f Mon Sep 17 00:00:00 2001 From: betegon Date: Tue, 17 Mar 2026 12:59:40 +0100 Subject: [PATCH 01/17] perf(issue): skip org listing when DSN or cache resolves project issue view with project-search format (e.g., `buybridge-5BS`) was making 5 sequential HTTP calls totaling ~2.4s. The biggest overhead was listOrganizations() (getUserRegions + listOrganizationsInRegion = ~800ms) inside findProjectsBySlug, even when DSN detection already identified the project. Two optimizations: 1. DSN-aware shortcut in resolveProjectSearch: after alias cache miss, check resolveFromDsn() for a matching project before falling back to findProjectsBySlug. Saves ~1200ms on cached runs, ~800ms first run. 2. Cached-orgs fast path in findProjectsBySlug: check org_regions cache before calling listOrganizations(). Saves ~800ms when org data is cached from any previous command. Co-Authored-By: Claude Opus 4.6 (1M context) --- src/commands/issue/utils.ts | 26 ++++++++++++-- src/lib/api/projects.ts | 69 ++++++++++++++++++++++++------------- 2 files changed, 70 insertions(+), 25 deletions(-) diff --git a/src/commands/issue/utils.ts b/src/commands/issue/utils.ts index 2a8094566..0a0c18ff6 100644 --- a/src/commands/issue/utils.ts +++ b/src/commands/issue/utils.ts @@ -23,7 +23,11 @@ import { expandToFullShortId, isShortSuffix } from "../../lib/issue-id.js"; import { logger } from "../../lib/logger.js"; import { poll } from "../../lib/polling.js"; import { resolveEffectiveOrg } from "../../lib/region.js"; -import { resolveOrg, resolveOrgAndProject } from "../../lib/resolve-target.js"; +import { + resolveFromDsn, + resolveOrg, + resolveOrgAndProject, +} from "../../lib/resolve-target.js"; import { parseSentryUrl } from "../../lib/sentry-url-parser.js"; import { isAllDigits } from "../../lib/utils.js"; import type { SentryIssue } from "../../types/index.js"; @@ -148,7 +152,25 @@ async function resolveProjectSearch( return aliasResult; } - // 2. Search for project across all accessible orgs + // 2. Check if DSN detection already resolved this project. + // resolveFromDsn() reads from the DSN cache (populated by detectAllDsns + // in tryResolveFromAlias above) + project cache. This avoids the expensive + // listOrganizations() fan-out when the DSN matches the target project. + try { + const dsnTarget = await resolveFromDsn(cwd); + if ( + dsnTarget && + dsnTarget.project.toLowerCase() === projectSlug.toLowerCase() + ) { + const fullShortId = expandToFullShortId(suffix, dsnTarget.project); + const issue = await getIssueByShortId(dsnTarget.org, fullShortId); + return { org: dsnTarget.org, issue }; + } + } catch { + // DSN resolution failed — fall through to full search + } + + // 3. Search for project across all accessible orgs const { projects } = await findProjectsBySlug(projectSlug.toLowerCase()); if (projects.length === 0) { diff --git a/src/lib/api/projects.ts b/src/lib/api/projects.ts index 48adc5223..94a33d452 100644 --- a/src/lib/api/projects.ts +++ b/src/lib/api/projects.ts @@ -18,6 +18,7 @@ import type { SentryProject, } from "../../types/index.js"; +import { getAllOrgRegions } from "../db/regions.js"; import { type AuthGuardSuccess, withAuthGuard } from "../errors.js"; import { logger } from "../logger.js"; import { getApiBaseUrl } from "../sentry-client.js"; @@ -176,34 +177,56 @@ export type ProjectSearchResult = { export async function findProjectsBySlug( projectSlug: string ): Promise { - const orgs = await listOrganizations(); const isNumericId = isAllDigits(projectSlug); - // Direct lookup in parallel — one API call per org instead of paginating all projects - const searchResults = await Promise.all( - orgs.map((org) => - withAuthGuard(async () => { - const project = await getProject(org.slug, projectSlug); - // The API accepts project_id_or_slug, so a numeric input could - // resolve by ID instead of slug. When the input is all digits, - // accept the match (the user passed a numeric project ID). - // For non-numeric inputs, verify the slug actually matches to - // avoid false positives from coincidental ID collisions. - // Note: Sentry enforces that project slugs must start with a letter, - // so an all-digits input can only ever be a numeric ID, never a slug. - if (!isNumericId && project.slug !== projectSlug) { - return null; - } - return { ...project, orgSlug: org.slug }; - }) - ) - ); + /** Search for the project in a list of org slugs (parallel getProject per org). */ + const searchOrgs = (orgSlugs: string[]) => + Promise.all( + orgSlugs.map((orgSlug) => + withAuthGuard(async () => { + const project = await getProject(orgSlug, projectSlug); + // The API accepts project_id_or_slug, so a numeric input could + // resolve by ID instead of slug. When the input is all digits, + // accept the match (the user passed a numeric project ID). + // For non-numeric inputs, verify the slug actually matches to + // avoid false positives from coincidental ID collisions. + // Note: Sentry enforces that project slugs must start with a letter, + // so an all-digits input can only ever be a numeric ID, never a slug. + if (!isNumericId && project.slug !== projectSlug) { + return null; + } + return { ...project, orgSlug }; + }) + ) + ); - return { - projects: searchResults + const extractProjects = ( + results: Awaited> + ): ProjectWithOrg[] => + results .filter((r): r is AuthGuardSuccess => r.ok) .map((r) => r.value) - .filter((v): v is ProjectWithOrg => v !== null), + .filter((v): v is ProjectWithOrg => v !== null); + + // Fast path: use cached org slugs to skip the expensive listOrganizations() + // round-trip (getUserRegions + listOrganizationsInRegion). + const cachedOrgRegions = await getAllOrgRegions(); + if (cachedOrgRegions.size > 0) { + const cachedSlugs = [...cachedOrgRegions.keys()]; + const cachedResults = await searchOrgs(cachedSlugs); + const cachedProjects = extractProjects(cachedResults); + if (cachedProjects.length > 0) { + return { projects: cachedProjects, orgs: [] }; + } + // Fall through: project might be in a new org not yet cached + } + + // Full listing: fetch all orgs from API, then search each + const orgs = await listOrganizations(); + const searchResults = await searchOrgs(orgs.map((o) => o.slug)); + + return { + projects: extractProjects(searchResults), orgs, }; } From c08cb9180cd05b219bfe6576184bec345ae402f4 Mon Sep 17 00:00:00 2001 From: betegon Date: Tue, 17 Mar 2026 13:18:58 +0100 Subject: [PATCH 02/17] test: verify DSN shortcut and cached-orgs optimizations Add two targeted tests proving the performance optimizations work: 1. resolveIssue with project-search format uses DSN shortcut when the DSN-detected project matches, skipping the expensive listOrganizations() fan-out entirely. 2. findProjectsBySlug uses cached org_regions to skip getUserRegions() + listOrganizationsInRegion() when org data is already cached from a previous command. Both tests verify the optimization by tracking HTTP requests and asserting that /users/me/regions/ and /organizations/ are never called. Co-Authored-By: Claude Opus 4.6 (1M context) --- test/commands/issue/utils.test.ts | 93 +++++++++++++++++++++++++++++++ test/lib/api-client.test.ts | 46 +++++++++++++++ 2 files changed, 139 insertions(+) diff --git a/test/commands/issue/utils.test.ts b/test/commands/issue/utils.test.ts index 5278ca535..ea52fa07b 100644 --- a/test/commands/issue/utils.test.ts +++ b/test/commands/issue/utils.test.ts @@ -14,6 +14,7 @@ import { } from "../../../src/commands/issue/utils.js"; import { DEFAULT_SENTRY_URL } from "../../../src/lib/constants.js"; import { setAuthToken } from "../../../src/lib/db/auth.js"; +import { setCachedProject } from "../../../src/lib/db/project-cache.js"; import { setOrgRegion } from "../../../src/lib/db/regions.js"; import { ResolutionError } from "../../../src/lib/errors.js"; import { useTestConfigDir } from "../../helpers.js"; @@ -1657,3 +1658,95 @@ describe("resolveIssue: numeric 404 error handling", () => { ).rejects.not.toBeInstanceOf(ResolutionError); }); }); + +describe("resolveIssue: project-search DSN shortcut", () => { + const getDsnTestConfigDir = useTestConfigDir("test-dsn-shortcut-", { + isolateProjectRoot: true, + }); + + let dsnOriginalFetch: typeof globalThis.fetch; + + beforeEach(async () => { + dsnOriginalFetch = globalThis.fetch; + await setAuthToken("test-token"); + await setOrgRegion("my-org", DEFAULT_SENTRY_URL); + // Seed project cache so resolveFromDsn resolves without any API call. + // orgId is "123" (DSN parser strips the "o" prefix from o123.ingest.*) + await setCachedProject("123", "456", { + orgSlug: "my-org", + orgName: "My Org", + projectSlug: "my-project", + projectName: "My Project", + projectId: "456", + }); + }); + + afterEach(() => { + globalThis.fetch = dsnOriginalFetch; + }); + + test("uses DSN shortcut when project matches, skips listOrganizations", async () => { + const { writeFileSync } = await import("node:fs"); + const { join } = await import("node:path"); + const cwd = getDsnTestConfigDir(); + + // Write a DSN so detectDsn finds it + writeFileSync( + join(cwd, ".env"), + "SENTRY_DSN=https://abc@o123.ingest.us.sentry.io/456" + ); + + const requests: string[] = []; + + // @ts-expect-error - partial mock + globalThis.fetch = async (input: RequestInfo | URL, init?: RequestInit) => { + const req = new Request(input, init); + const url = req.url; + requests.push(url); + + // Short ID resolution — the only HTTP call the shortcut should make + if (url.includes("/shortids/MY-PROJECT-5BS/")) { + return new Response( + JSON.stringify({ + organizationSlug: "my-org", + projectSlug: "my-project", + group: { + id: "999", + shortId: "MY-PROJECT-5BS", + title: "Test Issue", + status: "unresolved", + platform: "javascript", + type: "error", + count: "1", + userCount: 1, + }, + }), + { status: 200, headers: { "Content-Type": "application/json" } } + ); + } + + return new Response(JSON.stringify({ detail: "Not found" }), { + status: 404, + headers: { "Content-Type": "application/json" }, + }); + }; + + const result = await resolveIssue({ + issueArg: "my-project-5BS", + cwd, + command: "view", + }); + + // Shortcut resolved correctly + expect(result.org).toBe("my-org"); + expect(result.issue.id).toBe("999"); + + // The expensive listOrganizations calls were skipped + expect(requests.some((r) => r.includes("/users/me/regions/"))).toBe(false); + expect( + requests.some( + (r) => r.includes("/organizations/") && !r.includes("/shortids/") + ) + ).toBe(false); + }); +}); diff --git a/test/lib/api-client.test.ts b/test/lib/api-client.test.ts index 54646b69c..2bda30673 100644 --- a/test/lib/api-client.test.ts +++ b/test/lib/api-client.test.ts @@ -827,6 +827,52 @@ describe("findProjectsBySlug", () => { const { projects } = await findProjectsBySlug("wrong-slug"); expect(projects).toHaveLength(0); }); + + test("uses cached org regions to skip listOrganizations", async () => { + const { findProjectsBySlug } = await import("../../src/lib/api-client.js"); + + // Seed org_regions cache — this is the optimization under test + await setOrgRegion("acme", DEFAULT_SENTRY_URL); + + const requests: string[] = []; + + globalThis.fetch = async (input: RequestInfo | URL, init?: RequestInit) => { + const req = new Request(input, init); + const url = req.url; + requests.push(url); + + // getProject for acme/frontend — success + if (url.includes("/projects/acme/frontend/")) { + return new Response( + JSON.stringify({ id: "101", slug: "frontend", name: "Frontend" }), + { status: 200, headers: { "Content-Type": "application/json" } } + ); + } + + return new Response(JSON.stringify({ detail: "Not found" }), { + status: 404, + headers: { "Content-Type": "application/json" }, + }); + }; + + const { projects, orgs } = await findProjectsBySlug("frontend"); + + // Found via cached orgs + expect(projects).toHaveLength(1); + expect(projects[0].slug).toBe("frontend"); + expect(projects[0].orgSlug).toBe("acme"); + + // orgs is empty because we used cached path (no full listing) + expect(orgs).toHaveLength(0); + + // The expensive listOrganizations calls were skipped + expect(requests.some((r) => r.includes("/users/me/regions/"))).toBe(false); + expect( + requests.some( + (r) => r.includes("/organizations/") && !r.includes("/projects/") + ) + ).toBe(false); + }); }); describe("resolveEventInOrg", () => { From 70d1730793c6001b1e2e68075653f541fad9a6c0 Mon Sep 17 00:00:00 2001 From: betegon Date: Tue, 17 Mar 2026 13:38:27 +0100 Subject: [PATCH 03/17] =?UTF-8?q?fix:=20address=20PR=20review=20=E2=80=94?= =?UTF-8?q?=20narrow=20catch=20scope,=20document=20cache=20assumption?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 1. Narrow try/catch in resolveProjectSearch to only wrap resolveFromDsn(). Previously getIssueByShortId errors (e.g. 404) were silently caught, causing a fallback to findProjectsBySlug which would duplicate the expensive short ID resolution call before failing with the same error. 2. Add comment explaining why the org_regions cache is expected to be complete when findProjectsBySlug runs: the project-search format's short suffix only comes from `sentry issue list` output, which already called listOrganizations() and populated all orgs. Co-Authored-By: Claude Opus 4.6 (1M context) --- src/commands/issue/utils.ts | 21 ++++++++++++--------- src/lib/api/projects.ts | 4 ++++ 2 files changed, 16 insertions(+), 9 deletions(-) diff --git a/src/commands/issue/utils.ts b/src/commands/issue/utils.ts index 0a0c18ff6..3c2a62fa4 100644 --- a/src/commands/issue/utils.ts +++ b/src/commands/issue/utils.ts @@ -156,19 +156,22 @@ async function resolveProjectSearch( // resolveFromDsn() reads from the DSN cache (populated by detectAllDsns // in tryResolveFromAlias above) + project cache. This avoids the expensive // listOrganizations() fan-out when the DSN matches the target project. + // Only catch resolveFromDsn errors — getIssueByShortId errors (e.g. 404) + // must propagate so we don't duplicate the expensive call via fallback. + let dsnTarget: Awaited> | undefined; try { - const dsnTarget = await resolveFromDsn(cwd); - if ( - dsnTarget && - dsnTarget.project.toLowerCase() === projectSlug.toLowerCase() - ) { - const fullShortId = expandToFullShortId(suffix, dsnTarget.project); - const issue = await getIssueByShortId(dsnTarget.org, fullShortId); - return { org: dsnTarget.org, issue }; - } + dsnTarget = await resolveFromDsn(cwd); } catch { // DSN resolution failed — fall through to full search } + if ( + dsnTarget && + dsnTarget.project.toLowerCase() === projectSlug.toLowerCase() + ) { + const fullShortId = expandToFullShortId(suffix, dsnTarget.project); + const issue = await getIssueByShortId(dsnTarget.org, fullShortId); + return { org: dsnTarget.org, issue }; + } // 3. Search for project across all accessible orgs const { projects } = await findProjectsBySlug(projectSlug.toLowerCase()); diff --git a/src/lib/api/projects.ts b/src/lib/api/projects.ts index 94a33d452..d7addb0ed 100644 --- a/src/lib/api/projects.ts +++ b/src/lib/api/projects.ts @@ -210,6 +210,10 @@ export async function findProjectsBySlug( // Fast path: use cached org slugs to skip the expensive listOrganizations() // round-trip (getUserRegions + listOrganizationsInRegion). + // The cache is expected to be complete: callers reach this function via the + // project-search format (e.g. "buybridge-5BS") whose short suffix only comes + // from `sentry issue list` output — which already called listOrganizations() + // and populated org_regions with all accessible orgs. const cachedOrgRegions = await getAllOrgRegions(); if (cachedOrgRegions.size > 0) { const cachedSlugs = [...cachedOrgRegions.keys()]; From 580e733a85d885bb5f94d5d81ea9eaad69af01bd Mon Sep 17 00:00:00 2001 From: betegon Date: Tue, 17 Mar 2026 14:19:29 +0100 Subject: [PATCH 04/17] fix: skip already-searched orgs on findProjectsBySlug fallthrough When the cached-orgs fast path finds no projects and falls through to the full listOrganizations() path, avoid re-searching orgs that already returned 404 by filtering them out with a Set lookup. Co-Authored-By: Claude Opus 4.6 (1M context) --- src/lib/api/projects.ts | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/src/lib/api/projects.ts b/src/lib/api/projects.ts index d7addb0ed..f96299e46 100644 --- a/src/lib/api/projects.ts +++ b/src/lib/api/projects.ts @@ -215,8 +215,8 @@ export async function findProjectsBySlug( // from `sentry issue list` output — which already called listOrganizations() // and populated org_regions with all accessible orgs. const cachedOrgRegions = await getAllOrgRegions(); - if (cachedOrgRegions.size > 0) { - const cachedSlugs = [...cachedOrgRegions.keys()]; + const cachedSlugs = [...cachedOrgRegions.keys()]; + if (cachedSlugs.length > 0) { const cachedResults = await searchOrgs(cachedSlugs); const cachedProjects = extractProjects(cachedResults); if (cachedProjects.length > 0) { @@ -225,9 +225,13 @@ export async function findProjectsBySlug( // Fall through: project might be in a new org not yet cached } - // Full listing: fetch all orgs from API, then search each + // Full listing: fetch all orgs from API, then skip already-searched cached orgs const orgs = await listOrganizations(); - const searchResults = await searchOrgs(orgs.map((o) => o.slug)); + const cachedSet = new Set(cachedSlugs); + const newOrgSlugs = orgs + .filter((o) => !cachedSet.has(o.slug)) + .map((o) => o.slug); + const searchResults = await searchOrgs(newOrgSlugs); return { projects: extractProjects(searchResults), From 0bef71ca70308729ee581178fc7570ce18a460da Mon Sep 17 00:00:00 2001 From: Burak Yigit Kaya Date: Tue, 17 Mar 2026 13:27:17 +0000 Subject: [PATCH 05/17] refactor: generalize org listing cache into listOrganizations itself MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Instead of patching findProjectsBySlug with a per-callsite cache check, move the caching into listOrganizations() so all 12 callers benefit: - Add org_name column to org_regions table (schema v9) - Split listOrganizations into cached + uncached variants: - listOrganizations() returns from SQLite cache when populated - listOrganizationsUncached() always hits the API (for org list, auth status, resolveEffectiveOrg) - Add getCachedOrganizations() to db/regions.ts - Simplify findProjectsBySlug back to single-path (no getAllOrgRegions fast-path needed — listOrganizations handles it) - Update org list and auth status to use uncached variant - Update resolveEffectiveOrg to use uncached (it explicitly refreshes) --- AGENTS.md | 81 +++++++++++++------------------ src/commands/auth/status.ts | 4 +- src/commands/org/list.ts | 4 +- src/lib/api-client.ts | 1 + src/lib/api/organizations.ts | 40 +++++++++++++-- src/lib/api/projects.ts | 80 ++++++++++-------------------- src/lib/db/regions.ts | 39 ++++++++++++++- src/lib/db/schema.ts | 8 ++- src/lib/region.ts | 8 +-- test/commands/auth/status.test.ts | 2 +- test/lib/api-client.test.ts | 24 +++++---- 11 files changed, 168 insertions(+), 123 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index 0644a2871..6570cd4af 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -767,69 +767,58 @@ mock.module("./some-module", () => ({ ### Architecture - -* **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. + +* **DSN org prefix normalization in arg-parsing.ts**: Sentry DSN hosts encode org IDs as \`oNNNNN\` (e.g., \`o1081365.ingest.us.sentry.io\`). The Sentry API rejects the \`o\`-prefixed form. \`stripDsnOrgPrefix()\` in \`src/lib/arg-parsing.ts\` uses \`/^o(\d+)$/\` to strip the prefix — safe for slugs like \`organic\`. Applied in \`parseOrgProjectArg()\` and \`parseWithSlash()\`, covering all API call paths consuming \`parsed.org\`. - -* **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\`. + +* **GHCR versioned nightly tags for delta upgrade support**: GHCR nightly distribution uses three tag types: \`:nightly\` (rolling), \`:nightly-\\` (immutable), \`:patch-\\` (delta manifest). Delta patches use zig-bsdiff TRDIFF10 (zstd-compressed), ~50KB vs ~29MB full. Client bspatch via \`Bun.zstdDecompressSync()\`. N-1 patches only, full download fallback, SHA-256 verify, 60% size threshold. npm/Node excluded. Test mocks: use \`mockGhcrNightlyVersion()\` helper. - -* **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. + +* **Issue list auto-pagination beyond API's 100-item cap**: Sentry API silently caps \`limit\` at 100 per request. \`listIssuesAllPages()\` auto-paginates using Link headers, bounded by MAX\_PAGINATION\_PAGES (50). \`API\_MAX\_PER\_PAGE\` constant is shared across all paginated consumers. \`--limit\` means total results everywhere (max 1000, default 25). Org-all mode uses \`fetchOrgAllIssues()\`; explicit \`--cursor\` does single-page fetch to preserve cursor chain. - -* **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. + +* **Self-hosted OAuth device flow requires Sentry 26.1.0+ and SENTRY\_CLIENT\_ID**: Self-hosted OAuth device flow requires Sentry 26.1.0+ and both \`SENTRY\_URL\` and \`SENTRY\_CLIENT\_ID\` env vars. Users must create a public OAuth app in Settings → Developer Settings. The client ID is NOT optional for self-hosted. Fallback for older instances: \`sentry auth login --token\`. \`getSentryUrl()\` and \`getClientId()\` in \`src/lib/oauth.ts\` read lazily (not at module load) so URL parsing from arguments can set \`SENTRY\_URL\` after import. - -* **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. + +* **Sentry CLI markdown-first formatting pipeline replaces ad-hoc ANSI**: Formatters build CommonMark strings; \`renderMarkdown()\` renders to ANSI for TTY or raw markdown for non-TTY. Key helpers: \`colorTag()\`, \`mdKvTable()\`, \`mdRow()\`, \`mdTableHeader()\` (\`:\` suffix = right-aligned), \`renderTextTable()\`. \`isPlainOutput()\` checks \`SENTRY\_PLAIN\_OUTPUT\` > \`NO\_COLOR\` > \`!isTTY\`. Batch path: \`formatXxxTable()\`. Streaming path: \`StreamingTable\` (TTY) or raw markdown rows (plain). Both share \`buildXxxRowCells()\`. - -* **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. + +* **Sentry issue stats field: time-series controlled by groupStatsPeriod**: The \`stats\` field on issues is \`{ '24h': \[\[ts, count], ...] }\`. Key depends on \`groupStatsPeriod\` param (\`""\`, \`"14d"\`, \`"24h"\`, \`"auto"\`). \`statsPeriod\` controls time window; \`groupStatsPeriod\` controls stats key. \*\*Critical\*\*: \`count\` is period-scoped — \`lifetime.count\` is the true lifetime total. Issue list table uses \`groupStatsPeriod: 'auto'\` for sparkline data. Column order: SHORT ID, ISSUE, SEEN, AGE, TREND, EVENTS, USERS, TRIAGE. TREND auto-hidden when terminal < 100 cols. \`--compact\` tri-state: explicit overrides; \`undefined\` triggers \`shouldAutoCompact(rowCount)\` — compact if \`3N + 3 > termHeight\`, false for non-TTY. Height is \`3N + 3\` (not \`3N + 4\`) because last data row has no trailing separator. - -* **Seer trial prompt uses middleware layering in bin.ts error handling chain**: The CLI's error recovery middlewares in \`bin.ts\` are layered: \`main() → executeWithAutoAuth() → executeWithSeerTrialPrompt() → runCommand()\`. Seer trial prompts (for \`no\_budget\`/\`not\_enabled\` errors) are caught by the inner wrapper; auth errors bubble up to the outer wrapper. After successful auth login retry, the retry also goes through \`executeWithSeerTrialPrompt\` (not \`runCommand\` directly) so the full middleware chain applies. Trial check API: \`GET /api/0/customers/{org}/\` → \`productTrials\[]\` (prefer \`seerUsers\`, fallback \`seerAutofix\`). Start trial: \`PUT /api/0/customers/{org}/product-trial/\`. The \`/customers/\` endpoint is getsentry SaaS-only; self-hosted 404s gracefully. \`ai\_disabled\` errors are excluded (admin's explicit choice). \`startSeerTrial\` accepts \`category\` from the trial object — don't hardcode it. + +* **Sentry trace-logs API is org-scoped, not project-scoped**: The Sentry trace-logs endpoint (\`/organizations/{org}/trace-logs/\`) is org-scoped, so \`trace logs\` uses \`resolveOrg()\` not \`resolveOrgAndProject()\`. The endpoint is PRIVATE in Sentry source, excluded from the public OpenAPI schema — \`@sentry/api\` has no generated types. The hand-written \`TraceLogSchema\` in \`src/types/sentry.ts\` is required until Sentry makes it public. -### 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. + +* **withAuthGuard returns discriminated Result type, not fallback+onError**: \`withAuthGuard\(fn)\` in \`src/lib/errors.ts\` returns a discriminated Result: \`{ ok: true, value: T } | { ok: false, error: unknown }\`. AuthErrors always re-throw (triggers bin.ts auto-login). All other errors are captured. Callers inspect \`result.ok\` to degrade gracefully. Used across 12+ files. ### Gotcha - -* **@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.\` + +* **Biome lint: Response.redirect() required, nested ternaries forbidden**: Biome lint rules that frequently trip up this codebase: (1) \`useResponseRedirect\`: use \`Response.redirect(url, status)\` not \`new Response\`. (2) \`noNestedTernary\`: use \`if/else\`. (3) \`noComputedPropertyAccess\`: use \`obj.property\` not \`obj\["property"]\`. (4) Max cognitive complexity 15 per function — extract helpers to stay under. - -* **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\`. + +* **Bugbot flags defensive null-checks as dead code — keep them with JSDoc justification**: Cursor Bugbot and Sentry Seer repeatedly flag two false positives: (1) defensive null-checks as "dead code" — keep them with JSDoc explaining why the guard exists for future safety, especially when removing would require \`!\` assertions banned by \`noNonNullAssertion\`. (2) stderr spinner output during \`--json\` mode — always a false positive since progress goes to stderr, JSON to stdout. Reply explaining the rationale and resolve. - -* **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\`. - - -* **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\`. - - -* **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\`. + +* **Bun mock.module for node:tty requires default export and class stubs**: Bun testing gotchas: (1) \`mock.module()\` for CJS built-ins requires a \`default\` re-export plus all named exports. Missing any causes \`SyntaxError: Export named 'X' not found\`. Always check the real module's full export list. (2) \`Bun.mmap()\` always opens with PROT\_WRITE — macOS SIGKILL on signed Mach-O, Linux ETXTBSY. Fix: use \`new Uint8Array(await Bun.file(path).arrayBuffer())\` in bspatch.ts. (3) Wrap \`Bun.which()\` with optional \`pathEnv\` param for deterministic testing without mocks. ### Pattern - -* **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. + +* **Branch naming and commit message conventions for Sentry CLI**: Branch naming: \`feat/\\` or \`fix/\-\\` (e.g., \`feat/ghcr-nightly-distribution\`, \`fix/268-limit-auto-pagination\`). Commit message format: \`type(scope): description (#issue)\` (e.g., \`fix(issue-list): auto-paginate --limit beyond 100 (#268)\`, \`feat(nightly): distribute via GHCR instead of GitHub Releases\`). Types seen: fix, refactor, meta, release, feat. PRs are created as drafts via \`gh pr create --draft\`. Implementation plans are attached to commits via \`git notes add\` rather than in PR body or commit message. + + +* **Codecov patch coverage only counts test:unit and test:isolated, not E2E**: CI coverage merges \`test:unit\` (\`test/lib test/commands test/types --coverage\`) and \`test:isolated\` (\`test/isolated --coverage\`) into \`coverage/merged.lcov\`. E2E tests (\`test/e2e\`) are NOT included in coverage reports. So func tests that spy on exports (e.g., \`spyOn(apiClient, 'getLogs')\`) give zero coverage to the mocked function's body. To cover \`api-client.ts\` function bodies in unit tests, mock \`globalThis.fetch\` + \`setOrgRegion()\` + \`setAuthToken()\` and call the real function. - -* **PR workflow: wait for Seer and Cursor BugBot before resolving**: After pushing a PR in the getsentry/cli repo, the CI pipeline includes Seer Code Review and Cursor Bugbot as advisory checks. Both typically take 2-3 minutes but may not trigger on draft PRs — only ready-for-review PRs reliably get bot reviews. The workflow is: push → wait for all CI (including npm build jobs which test the actual bundle) → check for inline review comments from Seer/BugBot → fix if needed → repeat. Use \`gh pr checks \ --watch\` to monitor. Review comments are fetched via \`gh api repos/OWNER/REPO/pulls/NUM/comments\` and \`gh api repos/OWNER/REPO/pulls/NUM/reviews\`. + +* **Pagination contextKey must include all query-varying parameters with escaping**: Pagination \`contextKey\` must encode every query-varying parameter (sort, query, period) with \`escapeContextKeyValue()\` (replaces \`|\` with \`%7C\`). Always provide a fallback before escaping since \`flags.period\` may be \`undefined\` in tests despite having a default: \`flags.period ? escapeContextKeyValue(flags.period) : "90d"\`. - -* **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. + +* **PR review workflow: reply, resolve, amend, force-push**: PR review workflow: (1) Read unresolved threads via GraphQL, (2) make code changes, (3) run lint+typecheck+tests, (4) create a SEPARATE commit per review round (not amend) for incremental review, (5) push normally, (6) reply to comments via REST API, (7) resolve threads via GraphQL \`resolveReviewThread\`. Only amend+force-push when user explicitly asks or pre-commit hook modified files. - -* **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 optional boolean flags produce tri-state (true/false/undefined)**: Stricli boolean flags with \`optional: true\` (no \`default\`) produce \`boolean | undefined\` in the flags type. \`--flag\` → \`true\`, \`--no-flag\` → \`false\`, omitted → \`undefined\`. This enables auto-detect patterns: explicit user choice overrides, \`undefined\` triggers heuristic. Used by \`--compact\` on issue list. The flag type must be \`readonly field?: boolean\` (not \`readonly field: boolean\`). This differs from \`default: false\` which always produces a defined boolean. - -* **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'\`). + +* **Testing Stricli command func() bodies via spyOn mocking**: Stricli/Bun test patterns: (1) Command func tests: \`const func = await cmd.loader()\`, then \`func.call(mockContext, flags, ...args)\`. \`loader()\` return type union causes LSP errors — false positives that pass \`tsc\`. File naming: \`\*.func.test.ts\`. (2) ESM prevents \`vi.spyOn\` on Node built-in exports. Workaround: test subclass that overrides the method calling the built-in. (3) Follow-mode uses \`setTimeout\`-based scheduling; test with \`interceptSigint()\` helper. \`Bun.sleep()\` has no AbortSignal so \`setTimeout\`/\`clearTimeout\` required. diff --git a/src/commands/auth/status.ts b/src/commands/auth/status.ts index 20222fc4a..c54019e4d 100644 --- a/src/commands/auth/status.ts +++ b/src/commands/auth/status.ts @@ -5,7 +5,7 @@ */ import type { SentryContext } from "../../context.js"; -import { listOrganizations } from "../../lib/api-client.js"; +import { listOrganizationsUncached } from "../../lib/api-client.js"; import { buildCommand } from "../../lib/command.js"; import { type AuthConfig, @@ -124,7 +124,7 @@ async function collectDefaults(): Promise { */ async function verifyCredentials(): Promise { try { - const orgs = await listOrganizations(); + const orgs = await listOrganizationsUncached(); return { success: true, organizations: orgs.map((o) => ({ name: o.name, slug: o.slug })), diff --git a/src/commands/org/list.ts b/src/commands/org/list.ts index 8df2ea5bd..1352c2024 100644 --- a/src/commands/org/list.ts +++ b/src/commands/org/list.ts @@ -5,7 +5,7 @@ */ import type { SentryContext } from "../../context.js"; -import { listOrganizations } from "../../lib/api-client.js"; +import { listOrganizationsUncached } from "../../lib/api-client.js"; import { buildCommand } from "../../lib/command.js"; import { DEFAULT_SENTRY_HOST } from "../../lib/constants.js"; import { getAllOrgRegions } from "../../lib/db/regions.js"; @@ -130,7 +130,7 @@ export const listCommand = buildCommand({ async *func(this: SentryContext, flags: ListFlags) { applyFreshFlag(flags); - const orgs = await listOrganizations(); + const orgs = await listOrganizationsUncached(); const limitedOrgs = orgs.slice(0, flags.limit); // Check if user has orgs in multiple regions diff --git a/src/lib/api-client.ts b/src/lib/api-client.ts index e71e39fbc..6fde3b793 100644 --- a/src/lib/api-client.ts +++ b/src/lib/api-client.ts @@ -56,6 +56,7 @@ export { getUserRegions, listOrganizations, listOrganizationsInRegion, + listOrganizationsUncached, } from "./api/organizations.js"; export { createProject, diff --git a/src/lib/api/organizations.ts b/src/lib/api/organizations.ts index b73dd6d1b..7e5225ad5 100644 --- a/src/lib/api/organizations.ts +++ b/src/lib/api/organizations.ts @@ -65,11 +65,44 @@ export async function listOrganizationsInRegion( } /** - * List all organizations the user has access to across all regions. - * Performs a fan-out to each region and combines results. - * Also caches the region URL for each organization. + * List all organizations, returning cached data when available. + * + * On first call (cold cache), fetches from the API and populates the cache. + * On subsequent calls, returns organizations from the SQLite cache without + * any HTTP requests. This avoids the expensive getUserRegions() + + * listOrganizationsInRegion() fan-out (~800ms) on every command. + * + * Callers that need guaranteed-fresh data (e.g., `org list`, `auth status`) + * should use {@link listOrganizationsUncached} instead. */ export async function listOrganizations(): Promise { + const { getCachedOrganizations } = await import("../db/regions.js"); + + const cached = await getCachedOrganizations(); + if (cached.length > 0) { + return cached.map((org) => ({ + id: org.id, + slug: org.slug, + name: org.name, + })); + } + + // Cache miss — fetch from API (also populates cache for next time) + return listOrganizationsUncached(); +} + +/** + * List all organizations by fetching from the API, bypassing the cache. + * + * Performs a fan-out to each region and combines results. + * Populates the org_regions cache with slug, region URL, org ID, and org name. + * + * Use this when you need guaranteed-fresh data (e.g., `org list`, `auth status`). + * Most callers should use {@link listOrganizations} instead. + */ +export async function listOrganizationsUncached(): Promise< + SentryOrganization[] +> { const { setOrgRegions } = await import("../db/regions.js"); // Self-hosted instances may not have the regions endpoint (404) @@ -102,6 +135,7 @@ export async function listOrganizations(): Promise { slug: r.org.slug, regionUrl: r.regionUrl, orgId: r.org.id, + orgName: r.org.name, })); await setOrgRegions(regionEntries); diff --git a/src/lib/api/projects.ts b/src/lib/api/projects.ts index f96299e46..e97715da0 100644 --- a/src/lib/api/projects.ts +++ b/src/lib/api/projects.ts @@ -18,7 +18,6 @@ import type { SentryProject, } from "../../types/index.js"; -import { getAllOrgRegions } from "../db/regions.js"; import { type AuthGuardSuccess, withAuthGuard } from "../errors.js"; import { logger } from "../logger.js"; import { getApiBaseUrl } from "../sentry-client.js"; @@ -179,62 +178,35 @@ export async function findProjectsBySlug( ): Promise { const isNumericId = isAllDigits(projectSlug); - /** Search for the project in a list of org slugs (parallel getProject per org). */ - const searchOrgs = (orgSlugs: string[]) => - Promise.all( - orgSlugs.map((orgSlug) => - withAuthGuard(async () => { - const project = await getProject(orgSlug, projectSlug); - // The API accepts project_id_or_slug, so a numeric input could - // resolve by ID instead of slug. When the input is all digits, - // accept the match (the user passed a numeric project ID). - // For non-numeric inputs, verify the slug actually matches to - // avoid false positives from coincidental ID collisions. - // Note: Sentry enforces that project slugs must start with a letter, - // so an all-digits input can only ever be a numeric ID, never a slug. - if (!isNumericId && project.slug !== projectSlug) { - return null; - } - return { ...project, orgSlug }; - }) - ) - ); - - const extractProjects = ( - results: Awaited> - ): ProjectWithOrg[] => - results - .filter((r): r is AuthGuardSuccess => r.ok) - .map((r) => r.value) - .filter((v): v is ProjectWithOrg => v !== null); - - // Fast path: use cached org slugs to skip the expensive listOrganizations() - // round-trip (getUserRegions + listOrganizationsInRegion). - // The cache is expected to be complete: callers reach this function via the - // project-search format (e.g. "buybridge-5BS") whose short suffix only comes - // from `sentry issue list` output — which already called listOrganizations() - // and populated org_regions with all accessible orgs. - const cachedOrgRegions = await getAllOrgRegions(); - const cachedSlugs = [...cachedOrgRegions.keys()]; - if (cachedSlugs.length > 0) { - const cachedResults = await searchOrgs(cachedSlugs); - const cachedProjects = extractProjects(cachedResults); - if (cachedProjects.length > 0) { - return { projects: cachedProjects, orgs: [] }; - } - // Fall through: project might be in a new org not yet cached - } - - // Full listing: fetch all orgs from API, then skip already-searched cached orgs + // listOrganizations() returns from cache when populated, avoiding + // the expensive getUserRegions() + listOrganizationsInRegion() fan-out. const orgs = await listOrganizations(); - const cachedSet = new Set(cachedSlugs); - const newOrgSlugs = orgs - .filter((o) => !cachedSet.has(o.slug)) - .map((o) => o.slug); - const searchResults = await searchOrgs(newOrgSlugs); + + // Direct lookup in parallel — one API call per org instead of paginating all projects + const searchResults = await Promise.all( + orgs.map((org) => + withAuthGuard(async () => { + const project = await getProject(org.slug, projectSlug); + // The API accepts project_id_or_slug, so a numeric input could + // resolve by ID instead of slug. When the input is all digits, + // accept the match (the user passed a numeric project ID). + // For non-numeric inputs, verify the slug actually matches to + // avoid false positives from coincidental ID collisions. + // Note: Sentry enforces that project slugs must start with a letter, + // so an all-digits input can only ever be a numeric ID, never a slug. + if (!isNumericId && project.slug !== projectSlug) { + return null; + } + return { ...project, orgSlug: org.slug }; + }) + ) + ); return { - projects: extractProjects(searchResults), + projects: searchResults + .filter((r): r is AuthGuardSuccess => r.ok) + .map((r) => r.value) + .filter((v): v is ProjectWithOrg => v !== null), orgs, }; } diff --git a/src/lib/db/regions.ts b/src/lib/db/regions.ts index d1a61faa6..98ab1f7b6 100644 --- a/src/lib/db/regions.ts +++ b/src/lib/db/regions.ts @@ -18,15 +18,17 @@ const TABLE = "org_regions"; type OrgRegionRow = { org_slug: string; org_id: string | null; + org_name: string | null; region_url: string; updated_at: number; }; -/** Entry for batch-caching org regions with optional numeric ID. */ +/** Entry for batch-caching org regions with optional metadata. */ export type OrgRegionEntry = { slug: string; regionUrl: string; orgId?: string; + orgName?: string; }; /** @@ -119,6 +121,9 @@ export async function setOrgRegions(entries: OrgRegionEntry[]): Promise { if (entry.orgId) { row.org_id = entry.orgId; } + if (entry.orgName) { + row.org_name = entry.orgName; + } runUpsert(db, TABLE, row, ["org_slug"]); } })(); @@ -147,3 +152,35 @@ export async function getAllOrgRegions(): Promise> { return new Map(rows.map((row) => [row.org_slug, row.region_url])); } + +/** Cached org entry with the fields needed to reconstruct a SentryOrganization. */ +export type CachedOrg = { + slug: string; + id: string; + name: string; +}; + +/** + * Get all cached organizations with id, slug, and name. + * + * Returns organizations that have all three fields populated in the cache. + * Rows with missing `org_id` or `org_name` (from before schema v9) are + * excluded — callers should fall back to the API when the result is empty. + * + * @returns Array of cached org entries, or empty if cache is cold/incomplete + */ +export async function getCachedOrganizations(): Promise { + const db = getDatabase(); + const rows = db + .query( + `SELECT org_slug, org_id, org_name FROM ${TABLE} WHERE org_id IS NOT NULL AND org_name IS NOT NULL` + ) + .all() as Pick[]; + + return rows.map((row) => ({ + slug: row.org_slug, + // org_id and org_name are guaranteed non-null by the WHERE clause + id: row.org_id as string, + name: row.org_name as string, + })); +} diff --git a/src/lib/db/schema.ts b/src/lib/db/schema.ts index 52ac81c93..d33a63296 100644 --- a/src/lib/db/schema.ts +++ b/src/lib/db/schema.ts @@ -15,7 +15,7 @@ import type { Database } from "bun:sqlite"; import { stringifyUnknown } from "../errors.js"; import { logger } from "../logger.js"; -export const CURRENT_SCHEMA_VERSION = 8; +export const CURRENT_SCHEMA_VERSION = 9; /** Environment variable to disable auto-repair */ const NO_AUTO_REPAIR_ENV = "SENTRY_CLI_NO_AUTO_REPAIR"; @@ -166,6 +166,7 @@ export const TABLE_SCHEMAS: Record = { columns: { org_slug: { type: "TEXT", primaryKey: true }, org_id: { type: "TEXT", addedInVersion: 8 }, + org_name: { type: "TEXT", addedInVersion: 9 }, region_url: { type: "TEXT", notNull: true }, updated_at: { type: "INTEGER", @@ -721,6 +722,11 @@ export function runMigrations(db: Database): void { addColumnIfMissing(db, "org_regions", "org_id", "TEXT"); } + // Migration 8 -> 9: Add org_name column to org_regions for cached org listing + if (currentVersion < 9) { + addColumnIfMissing(db, "org_regions", "org_name", "TEXT"); + } + if (currentVersion < CURRENT_SCHEMA_VERSION) { db.query("UPDATE schema_version SET version = ?").run( CURRENT_SCHEMA_VERSION diff --git a/src/lib/region.ts b/src/lib/region.ts index 9187d8235..8ee336616 100644 --- a/src/lib/region.ts +++ b/src/lib/region.ts @@ -162,13 +162,13 @@ export async function resolveEffectiveOrg(orgSlug: string): Promise { return fromCache; } - // Cache is cold or identifier is unknown — refresh the org list. - // listOrganizations() populates org_regions with slug, region, and org_id. + // Cache is cold or identifier is unknown — refresh the org list from API. + // listOrganizationsUncached() populates org_regions with slug, region, org_id, and name. // Any error (auth failure, network error, etc.) falls back to the original // slug; the downstream API call will produce a relevant error if needed. try { - const { listOrganizations } = await import("./api-client.js"); - await listOrganizations(); + const { listOrganizationsUncached } = await import("./api-client.js"); + await listOrganizationsUncached(); } catch { return orgSlug; } diff --git a/test/commands/auth/status.test.ts b/test/commands/auth/status.test.ts index 890145901..5e9f15085 100644 --- a/test/commands/auth/status.test.ts +++ b/test/commands/auth/status.test.ts @@ -81,7 +81,7 @@ describe("statusCommand.func", () => { getDefaultOrgSpy = spyOn(dbDefaults, "getDefaultOrganization"); getDefaultProjectSpy = spyOn(dbDefaults, "getDefaultProject"); getDbPathSpy = spyOn(dbIndex, "getDbPath"); - listOrgsSpy = spyOn(apiClient, "listOrganizations"); + listOrgsSpy = spyOn(apiClient, "listOrganizationsUncached"); // Defaults that most tests override getUserInfoSpy.mockReturnValue(null); diff --git a/test/lib/api-client.test.ts b/test/lib/api-client.test.ts index 2bda30673..afa03474a 100644 --- a/test/lib/api-client.test.ts +++ b/test/lib/api-client.test.ts @@ -19,7 +19,7 @@ import { } from "../../src/lib/api-client.js"; import { DEFAULT_SENTRY_URL } from "../../src/lib/constants.js"; import { setAuthToken } from "../../src/lib/db/auth.js"; -import { setOrgRegion } from "../../src/lib/db/regions.js"; +import { setOrgRegion, setOrgRegions } from "../../src/lib/db/regions.js"; import { useTestConfigDir } from "../helpers.js"; useTestConfigDir("test-api-"); @@ -828,14 +828,23 @@ describe("findProjectsBySlug", () => { expect(projects).toHaveLength(0); }); - test("uses cached org regions to skip listOrganizations", async () => { + test("uses cached orgs to skip listOrganizations API calls", async () => { const { findProjectsBySlug } = await import("../../src/lib/api-client.js"); - // Seed org_regions cache — this is the optimization under test - await setOrgRegion("acme", DEFAULT_SENTRY_URL); + // Seed org_regions cache with full org data (slug, id, name, region) + // so listOrganizations() returns from cache without HTTP calls. + await setOrgRegions([ + { + slug: "acme", + regionUrl: DEFAULT_SENTRY_URL, + orgId: "42", + orgName: "Acme Corp", + }, + ]); const requests: string[] = []; + // @ts-expect-error - partial mock globalThis.fetch = async (input: RequestInfo | URL, init?: RequestInit) => { const req = new Request(input, init); const url = req.url; @@ -855,17 +864,14 @@ describe("findProjectsBySlug", () => { }); }; - const { projects, orgs } = await findProjectsBySlug("frontend"); + const { projects } = await findProjectsBySlug("frontend"); // Found via cached orgs expect(projects).toHaveLength(1); expect(projects[0].slug).toBe("frontend"); expect(projects[0].orgSlug).toBe("acme"); - // orgs is empty because we used cached path (no full listing) - expect(orgs).toHaveLength(0); - - // The expensive listOrganizations calls were skipped + // The expensive listOrganizations API calls were skipped expect(requests.some((r) => r.includes("/users/me/regions/"))).toBe(false); expect( requests.some( From 3c623f580f0983695a8aa70c2f573bb143dcb950 Mon Sep 17 00:00:00 2001 From: Burak Yigit Kaya Date: Tue, 17 Mar 2026 14:05:40 +0000 Subject: [PATCH 06/17] fix: add 7-day TTL to org listing cache getCachedOrganizations() now checks updated_at against a 7-day TTL. Stale entries are ignored, forcing a fresh API fetch. This ensures users who join new orgs see them within a bounded window without needing to run --fresh or auth status. --- src/lib/db/regions.ts | 22 ++++++++++++++++------ 1 file changed, 16 insertions(+), 6 deletions(-) diff --git a/src/lib/db/regions.ts b/src/lib/db/regions.ts index 98ab1f7b6..c46c0d082 100644 --- a/src/lib/db/regions.ts +++ b/src/lib/db/regions.ts @@ -160,22 +160,32 @@ export type CachedOrg = { name: string; }; +/** + * Maximum age (ms) for cached organization entries. + * Entries older than this are considered stale and ignored, forcing a + * fresh API fetch. 7 days balances offline usability with picking up + * new org memberships. + */ +const ORG_CACHE_TTL_MS = 7 * 24 * 60 * 60 * 1000; + /** * Get all cached organizations with id, slug, and name. * - * Returns organizations that have all three fields populated in the cache. - * Rows with missing `org_id` or `org_name` (from before schema v9) are - * excluded — callers should fall back to the API when the result is empty. + * Returns organizations that have all three fields populated and were + * updated within the TTL window. Rows with missing `org_id` or `org_name` + * (from before schema v9) or stale `updated_at` are excluded — callers + * should fall back to the API when the result is empty. * - * @returns Array of cached org entries, or empty if cache is cold/incomplete + * @returns Array of cached org entries, or empty if cache is cold/stale/incomplete */ export async function getCachedOrganizations(): Promise { const db = getDatabase(); + const cutoff = Date.now() - ORG_CACHE_TTL_MS; const rows = db .query( - `SELECT org_slug, org_id, org_name FROM ${TABLE} WHERE org_id IS NOT NULL AND org_name IS NOT NULL` + `SELECT org_slug, org_id, org_name FROM ${TABLE} WHERE org_id IS NOT NULL AND org_name IS NOT NULL AND updated_at > ?` ) - .all() as Pick[]; + .all(cutoff) as Pick[]; return rows.map((row) => ({ slug: row.org_slug, From d0238c06f51e9229e6f2803d60637d485ec24701 Mon Sep 17 00:00:00 2001 From: Burak Yigit Kaya Date: Tue, 17 Mar 2026 14:57:28 +0000 Subject: [PATCH 07/17] chore: sync AGENTS.md from main --- AGENTS.md | 77 ++++++++++++++++++++++++++++++------------------------- 1 file changed, 42 insertions(+), 35 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index 6570cd4af..8201696b8 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -767,58 +767,65 @@ mock.module("./some-module", () => ({ ### Architecture - -* **DSN org prefix normalization in arg-parsing.ts**: Sentry DSN hosts encode org IDs as \`oNNNNN\` (e.g., \`o1081365.ingest.us.sentry.io\`). The Sentry API rejects the \`o\`-prefixed form. \`stripDsnOrgPrefix()\` in \`src/lib/arg-parsing.ts\` uses \`/^o(\d+)$/\` to strip the prefix — safe for slugs like \`organic\`. Applied in \`parseOrgProjectArg()\` and \`parseWithSlash()\`, covering all API call paths consuming \`parsed.org\`. + +* **API client wraps all errors as CliError subclasses — no raw exceptions escape**: The API client (src/lib/api-client.ts) wraps ALL errors as CliError subclasses (ApiError or AuthError) — no raw exceptions escape. Commands don't need try-catch for error display; the central handler in app.ts formats CliError cleanly. Only add try-catch when a command needs to handle errors specially (e.g., login continuing despite user-info fetch failure). - -* **GHCR versioned nightly tags for delta upgrade support**: GHCR nightly distribution uses three tag types: \`:nightly\` (rolling), \`:nightly-\\` (immutable), \`:patch-\\` (delta manifest). Delta patches use zig-bsdiff TRDIFF10 (zstd-compressed), ~50KB vs ~29MB full. Client bspatch via \`Bun.zstdDecompressSync()\`. N-1 patches only, full download fallback, SHA-256 verify, 60% size threshold. npm/Node excluded. Test mocks: use \`mockGhcrNightlyVersion()\` helper. + +* **Sentry API: events require org+project, issues have legacy global endpoint**: Sentry API scoping: Events require org+project in URL path (\`/projects/{org}/{project}/events/{id}/\`). Issues use legacy global endpoint (\`/api/0/issues/{id}/\`) without org context. Traces need only org (\`/organizations/{org}/trace/{traceId}/\`). Two-step lookup for events: fetch issue → extract org/project from response → fetch event. Cross-project event search possible via Discover endpoint \`/organizations/{org}/events/\` with \`query=id:{eventId}\`. - -* **Issue list auto-pagination beyond API's 100-item cap**: Sentry API silently caps \`limit\` at 100 per request. \`listIssuesAllPages()\` auto-paginates using Link headers, bounded by MAX\_PAGINATION\_PAGES (50). \`API\_MAX\_PER\_PAGE\` constant is shared across all paginated consumers. \`--limit\` means total results everywhere (max 1000, default 25). Org-all mode uses \`fetchOrgAllIssues()\`; explicit \`--cursor\` does single-page fetch to preserve cursor chain. + +* **Sentry CLI authenticated fetch architecture with response caching**: \`createAuthenticatedFetch()\` wraps fetch with auth, 30s timeout, retry (max 2), 401 refresh, and span tracing. Response caching integrates BEFORE auth/retry via \`http-cache-semantics\` (RFC 7234) with filesystem storage at \`~/.sentry/cache/responses/\`. URL-based fallback TTL tiers: immutable (24hr), stable (5min), volatile (60s), no-cache (0). Only GET 2xx cached. \`--fresh\` and \`SENTRY\_NO\_CACHE=1\` bypass cache. Cache cleared on login/logout. \`hasServerCacheDirectives(policy)\` distinguishes \`max-age=0\` from missing headers. - -* **Self-hosted OAuth device flow requires Sentry 26.1.0+ and SENTRY\_CLIENT\_ID**: Self-hosted OAuth device flow requires Sentry 26.1.0+ and both \`SENTRY\_URL\` and \`SENTRY\_CLIENT\_ID\` env vars. Users must create a public OAuth app in Settings → Developer Settings. The client ID is NOT optional for self-hosted. Fallback for older instances: \`sentry auth login --token\`. \`getSentryUrl()\` and \`getClientId()\` in \`src/lib/oauth.ts\` read lazily (not at module load) so URL parsing from arguments can set \`SENTRY\_URL\` after import. + +* **Sentry CLI has two distribution channels with different runtimes**: Sentry CLI ships two ways: (1) Standalone binary via \`Bun.build()\` with \`compile: true\`. (2) npm package via esbuild producing CJS \`dist/bin.cjs\` for Node 22+, with Bun API polyfills from \`script/node-polyfills.ts\`. \`Bun.$\` has NO polyfill — use \`execSync\` instead. \`require()\` in ESM is safe (Bun native, esbuild resolves at bundle time). - -* **Sentry CLI markdown-first formatting pipeline replaces ad-hoc ANSI**: Formatters build CommonMark strings; \`renderMarkdown()\` renders to ANSI for TTY or raw markdown for non-TTY. Key helpers: \`colorTag()\`, \`mdKvTable()\`, \`mdRow()\`, \`mdTableHeader()\` (\`:\` suffix = right-aligned), \`renderTextTable()\`. \`isPlainOutput()\` checks \`SENTRY\_PLAIN\_OUTPUT\` > \`NO\_COLOR\` > \`!isTTY\`. Batch path: \`formatXxxTable()\`. Streaming path: \`StreamingTable\` (TTY) or raw markdown rows (plain). Both share \`buildXxxRowCells()\`. + +* **Sentry CLI resolve-target cascade has 5 priority levels with env var support**: Resolve-target cascade (src/lib/resolve-target.ts) 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. The \`resolveFromEnvVars()\` helper is injected into all four resolution functions. - -* **Sentry issue stats field: time-series controlled by groupStatsPeriod**: The \`stats\` field on issues is \`{ '24h': \[\[ts, count], ...] }\`. Key depends on \`groupStatsPeriod\` param (\`""\`, \`"14d"\`, \`"24h"\`, \`"auto"\`). \`statsPeriod\` controls time window; \`groupStatsPeriod\` controls stats key. \*\*Critical\*\*: \`count\` is period-scoped — \`lifetime.count\` is the true lifetime total. Issue list table uses \`groupStatsPeriod: 'auto'\` for sparkline data. Column order: SHORT ID, ISSUE, SEEN, AGE, TREND, EVENTS, USERS, TRIAGE. TREND auto-hidden when terminal < 100 cols. \`--compact\` tri-state: explicit overrides; \`undefined\` triggers \`shouldAutoCompact(rowCount)\` — compact if \`3N + 3 > termHeight\`, false for non-TTY. Height is \`3N + 3\` (not \`3N + 4\`) because last data row has no trailing separator. +### Decision - -* **Sentry trace-logs API is org-scoped, not project-scoped**: The Sentry trace-logs endpoint (\`/organizations/{org}/trace-logs/\`) is org-scoped, so \`trace logs\` uses \`resolveOrg()\` not \`resolveOrgAndProject()\`. The endpoint is PRIVATE in Sentry source, excluded from the public OpenAPI schema — \`@sentry/api\` has no generated types. The hand-written \`TraceLogSchema\` in \`src/types/sentry.ts\` is required until Sentry makes it public. + +* **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 at least 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. - -* **withAuthGuard returns discriminated Result type, not fallback+onError**: \`withAuthGuard\(fn)\` in \`src/lib/errors.ts\` returns a discriminated Result: \`{ ok: true, value: T } | { ok: false, error: unknown }\`. AuthErrors always re-throw (triggers bin.ts auto-login). All other errors are captured. Callers inspect \`result.ok\` to degrade gracefully. Used across 12+ files. + +* **Sentry CLI config dir should stay at ~/.sentry/, not move to XDG**: Config dir stays at \`~/.sentry/\` (not XDG). The readonly DB errors on macOS are from \`sudo brew install\` creating root-owned files. Fixes: (1) bestEffort() makes setup steps non-fatal, (2) tryRepairReadonly() detects root-owned files and prints \`sudo chown\` instructions, (3) \`sentry cli fix\` handles ownership repair. Ownership must be checked BEFORE permissions — root-owned files cause chmod to EPERM. ### Gotcha - -* **Biome lint: Response.redirect() required, nested ternaries forbidden**: Biome lint rules that frequently trip up this codebase: (1) \`useResponseRedirect\`: use \`Response.redirect(url, status)\` not \`new Response\`. (2) \`noNestedTernary\`: use \`if/else\`. (3) \`noComputedPropertyAccess\`: use \`obj.property\` not \`obj\["property"]\`. (4) Max cognitive complexity 15 per function — extract helpers to stay under. + +* **brew is not in VALID\_METHODS but Homebrew formula passes --method brew**: Homebrew install: \`isHomebrewInstall()\` detects via Cellar realpath (checked before stored install info). Upgrade command tells users \`brew upgrade getsentry/tools/sentry\`. Formula runs \`sentry cli setup --method brew --no-modify-path\` as post\_install. Version pinning throws 'unsupported\_operation'. Uses .gz artifacts. Tap at getsentry/tools. - -* **Bugbot flags defensive null-checks as dead code — keep them with JSDoc justification**: Cursor Bugbot and Sentry Seer repeatedly flag two false positives: (1) defensive null-checks as "dead code" — keep them with JSDoc explaining why the guard exists for future safety, especially when removing would require \`!\` assertions banned by \`noNonNullAssertion\`. (2) stderr spinner output during \`--json\` mode — always a false positive since progress goes to stderr, JSON to stdout. Reply explaining the rationale and resolve. + +* **Bun mock.module() leaks globally across test files in same process**: Bun's mock.module() replaces modules globally and leaks across test files in the same process. Solution: tests using mock.module() must run in a separate \`bun test\` invocation. In package.json, use \`bun run test:unit && bun run test:isolated\` instead of \`bun test\`. The \`test/isolated/\` directory exists for these tests. This was the root cause of ~100 test failures (getsentry/cli#258). - -* **Bun mock.module for node:tty requires default export and class stubs**: Bun testing gotchas: (1) \`mock.module()\` for CJS built-ins requires a \`default\` re-export plus all named exports. Missing any causes \`SyntaxError: Export named 'X' not found\`. Always check the real module's full export list. (2) \`Bun.mmap()\` always opens with PROT\_WRITE — macOS SIGKILL on signed Mach-O, Linux ETXTBSY. Fix: use \`new Uint8Array(await Bun.file(path).arrayBuffer())\` in bspatch.ts. (3) Wrap \`Bun.which()\` with optional \`pathEnv\` param for deterministic testing without mocks. + +* **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 (macrotasks) during \`asyncModelRun\` cause \`createIsolatedDbContext\` cleanup to interleave. Fix: keep non-async, return \`clearResponseCache().catch(...)\` directly. Model-based tests should NOT await it. Also: model-based tests need explicit timeouts (e.g., \`30\_000\`) — Bun's default 5s causes false failures during shrinking. -### Pattern + +* **Multiregion mock must include all control silo API routes**: When changing which Sentry API endpoint a function uses, mock routes must be updated in BOTH \`test/mocks/routes.ts\` (single-region) AND \`test/mocks/multiregion.ts\` \`createControlSiloRoutes()\`. Missing the multiregion mock causes 404s in multi-region test scenarios. + + +* **Sentry /users/me/ endpoint returns 403 for OAuth tokens — use /auth/ instead**: The Sentry \`/users/me/\` endpoint returns 403 for OAuth tokens. Use \`/auth/\` instead — it works with ALL token types and lives on the control silo. In the CLI, \`getControlSiloUrl()\` handles routing correctly. \`SentryUserSchema\` (with \`.passthrough()\`) handles the \`/auth/\` response since it only requires \`id\`. + + +* **Stricli command context uses this.stdout not this.process.stdout**: In Stricli command \`func()\` handlers, use \`this.stdout\` and \`this.stderr\` directly — NOT \`this.process.stdout\`. The \`SentryContext\` interface has both \`process\` and \`stdout\`/\`stderr\` as separate top-level properties. Test mock contexts typically provide \`stdout\` but not a full \`process\` object, so \`this.process.stdout\` causes \`TypeError: undefined is not an object\` at runtime in tests even though TypeScript doesn't flag it. - -* **Branch naming and commit message conventions for Sentry CLI**: Branch naming: \`feat/\\` or \`fix/\-\\` (e.g., \`feat/ghcr-nightly-distribution\`, \`fix/268-limit-auto-pagination\`). Commit message format: \`type(scope): description (#issue)\` (e.g., \`fix(issue-list): auto-paginate --limit beyond 100 (#268)\`, \`feat(nightly): distribute via GHCR instead of GitHub Releases\`). Types seen: fix, refactor, meta, release, feat. PRs are created as drafts via \`gh pr create --draft\`. Implementation plans are attached to commits via \`git notes add\` rather than in PR body or commit message. + +* **Stricli defaultCommand blends default command flags into route completions**: When a Stricli route map has \`defaultCommand\` set, requesting completions for that route (e.g. \`\["issues", ""]\`) returns both the subcommand names AND the default command's flags/positional completions. This means completion tests that compare against \`extractCommandTree()\` subcommand lists will fail for groups with defaultCommand, since the actual completions include extra entries like \`--limit\`, \`--query\`, etc. Solution: track \`hasDefaultCommand\` in the command tree and skip strict subcommand-matching assertions for those groups. + +### Pattern - -* **Codecov patch coverage only counts test:unit and test:isolated, not E2E**: CI coverage merges \`test:unit\` (\`test/lib test/commands test/types --coverage\`) and \`test:isolated\` (\`test/isolated --coverage\`) into \`coverage/merged.lcov\`. E2E tests (\`test/e2e\`) are NOT included in coverage reports. So func tests that spy on exports (e.g., \`spyOn(apiClient, 'getLogs')\`) give zero coverage to the mocked function's body. To cover \`api-client.ts\` function bodies in unit tests, mock \`globalThis.fetch\` + \`setOrgRegion()\` + \`setAuthToken()\` and call the real function. + +* **Extract logic from Stricli func() handlers into standalone functions for testability**: Stricli command \`func()\` handlers are hard to unit test because they require full command context setup. To boost coverage, extract flag validation and body-building logic into standalone exported functions (e.g., \`resolveBody()\` extracted from the \`api\` command's \`func()\`). This moved ~20 lines of mutual-exclusivity checks and flag routing from an untestable handler into a directly testable pure function. Property-based tests on the extracted function drove patch coverage from 78% to 97%. The general pattern: keep \`func()\` as a thin orchestrator that calls exported helpers. This also keeps biome complexity under the limit (max 15). - -* **Pagination contextKey must include all query-varying parameters with escaping**: Pagination \`contextKey\` must encode every query-varying parameter (sort, query, period) with \`escapeContextKeyValue()\` (replaces \`|\` with \`%7C\`). Always provide a fallback before escaping since \`flags.period\` may be \`undefined\` in tests despite having a default: \`flags.period ? escapeContextKeyValue(flags.period) : "90d"\`. + +* **Non-essential DB cache writes should be guarded with try-catch**: Non-essential DB cache writes (e.g., \`setUserInfo()\` in whoami.ts and login.ts) must be wrapped in try-catch. If the DB is broken, the cache write shouldn't crash the command when its primary operation already succeeded. In login.ts specifically, \`getCurrentUser()\` failure after token save must not block authentication — wrap in try-catch, log warning to stderr, let login succeed. This differs from \`getUserRegions()\` failure which should \`clearAuth()\` and fail hard (indicates invalid token). - -* **PR review workflow: reply, resolve, amend, force-push**: PR review workflow: (1) Read unresolved threads via GraphQL, (2) make code changes, (3) run lint+typecheck+tests, (4) create a SEPARATE commit per review round (not amend) for incremental review, (5) push normally, (6) reply to comments via REST API, (7) resolve threads via GraphQL \`resolveReviewThread\`. Only amend+force-push when user explicitly asks or pre-commit hook modified files. + +* **Stricli buildCommand output config injects json flag into func params**: When a Stricli command uses \`output: { json: true, human: formatFn }\`, the framework injects \`--json\` and \`--fields\` flags automatically. The \`func\` handler receives these as its first parameter. Type it explicitly (e.g., \`flags: { json?: boolean }\`) rather than \`\_flags: unknown\` to access the json flag for conditional behavior (e.g., skipping interactive output in JSON mode). The \`human\` formatter runs on the returned \`data\` for non-JSON output. Commands that produce interactive side effects (browser prompts, QR codes) should check \`flags.json\` and skip them when true. - -* **Stricli optional boolean flags produce tri-state (true/false/undefined)**: Stricli boolean flags with \`optional: true\` (no \`default\`) produce \`boolean | undefined\` in the flags type. \`--flag\` → \`true\`, \`--no-flag\` → \`false\`, omitted → \`undefined\`. This enables auto-detect patterns: explicit user choice overrides, \`undefined\` triggers heuristic. Used by \`--compact\` on issue list. The flag type must be \`readonly field?: boolean\` (not \`readonly field: boolean\`). This differs from \`default: false\` which always produces a defined boolean. +### Preference - -* **Testing Stricli command func() bodies via spyOn mocking**: Stricli/Bun test patterns: (1) Command func tests: \`const func = await cmd.loader()\`, then \`func.call(mockContext, flags, ...args)\`. \`loader()\` return type union causes LSP errors — false positives that pass \`tsc\`. File naming: \`\*.func.test.ts\`. (2) ESM prevents \`vi.spyOn\` on Node built-in exports. Workaround: test subclass that overrides the method calling the built-in. (3) Follow-mode uses \`setTimeout\`-based scheduling; test with \`interceptSigint()\` helper. \`Bun.sleep()\` has no AbortSignal so \`setTimeout\`/\`clearTimeout\` required. + +* **PR workflow: address review comments, resolve threads, wait for CI**: User's PR workflow after creation: (1) Wait for CI checks to pass, (2) Check for unresolved review comments via \`gh api\` for PR review comments, (3) Fix issues in follow-up commits (not amends), (4) Reply to the comment thread explaining the fix, (5) Resolve the thread programmatically via \`gh api graphql\` with \`resolveReviewThread\` mutation, (6) Push and wait for CI again, (7) Final sweep for any remaining unresolved comments. Use \`git notes add\` to attach implementation plans to commits. Branch naming: \`fix/descriptive-slug\` or \`feat/descriptive-slug\`. From 23820648de5973ab47172ccbc6ec0962d94cedec Mon Sep 17 00:00:00 2001 From: Burak Yigit Kaya Date: Tue, 17 Mar 2026 15:23:38 +0000 Subject: [PATCH 08/17] fix: cache self-hosted orgs + wire --fresh to org cache - Self-hosted early return in listOrganizationsUncached now calls setOrgRegions before returning (Seer: cache was skipped) - Add disableOrgCache() to db/regions.ts, called by applyFreshFlag so --fresh bypasses the org listing cache (BugBot: stale data) - getCachedOrganizations returns empty when cache is disabled --- src/lib/api/organizations.ts | 12 +++++++++++- src/lib/db/regions.ts | 22 +++++++++++++++++++++- src/lib/list-command.ts | 2 ++ 3 files changed, 34 insertions(+), 2 deletions(-) diff --git a/src/lib/api/organizations.ts b/src/lib/api/organizations.ts index 7e5225ad5..5fa6ecb2d 100644 --- a/src/lib/api/organizations.ts +++ b/src/lib/api/organizations.ts @@ -111,7 +111,17 @@ export async function listOrganizationsUncached(): Promise< if (regions.length === 0) { // Fall back to default API for self-hosted instances - return listOrganizationsInRegion(getApiBaseUrl()); + const orgs = await listOrganizationsInRegion(getApiBaseUrl()); + const baseUrl = getApiBaseUrl(); + await setOrgRegions( + orgs.map((org) => ({ + slug: org.slug, + regionUrl: baseUrl, + orgId: org.id, + orgName: org.name, + })) + ); + return orgs; } const results = await Promise.all( diff --git a/src/lib/db/regions.ts b/src/lib/db/regions.ts index c46c0d082..6e28bd323 100644 --- a/src/lib/db/regions.ts +++ b/src/lib/db/regions.ts @@ -15,6 +15,19 @@ import { runUpsert } from "./utils.js"; const TABLE = "org_regions"; +/** When true, getCachedOrganizations() returns empty (forces API fetch). */ +let orgCacheDisabled = false; + +/** Disable the org listing cache for this invocation (e.g., `--fresh` flag). */ +export function disableOrgCache(): void { + orgCacheDisabled = true; +} + +/** Re-enable the org listing cache. Exported for testing. */ +export function enableOrgCache(): void { + orgCacheDisabled = false; +} + type OrgRegionRow = { org_slug: string; org_id: string | null; @@ -176,9 +189,16 @@ const ORG_CACHE_TTL_MS = 7 * 24 * 60 * 60 * 1000; * (from before schema v9) or stale `updated_at` are excluded — callers * should fall back to the API when the result is empty. * - * @returns Array of cached org entries, or empty if cache is cold/stale/incomplete + * Returns empty when the cache is disabled via {@link disableOrgCache} + * (e.g., `--fresh` flag). + * + * @returns Array of cached org entries, or empty if cache is cold/stale/disabled/incomplete */ export async function getCachedOrganizations(): Promise { + if (orgCacheDisabled) { + return []; + } + const db = getDatabase(); const cutoff = Date.now() - ORG_CACHE_TTL_MS; const rows = db diff --git a/src/lib/list-command.ts b/src/lib/list-command.ts index 6d649f46f..637be6fe5 100644 --- a/src/lib/list-command.ts +++ b/src/lib/list-command.ts @@ -17,6 +17,7 @@ import type { Aliases, Command, CommandContext } from "@stricli/core"; import type { SentryContext } from "../context.js"; import { parseOrgProjectArg } from "./arg-parsing.js"; import { buildCommand, numberParser } from "./command.js"; +import { disableOrgCache } from "./db/regions.js"; import { disableDsnCache } from "./dsn/index.js"; import { warning } from "./formatters/colors.js"; import { @@ -146,6 +147,7 @@ export function applyFreshFlag(flags: { readonly fresh: boolean }): void { if (flags.fresh) { disableResponseCache(); disableDsnCache(); + disableOrgCache(); } } From 5cd67b94d17a80502e16248667c64163a1e1399f Mon Sep 17 00:00:00 2001 From: Burak Yigit Kaya Date: Tue, 17 Mar 2026 23:30:50 +0000 Subject: [PATCH 09/17] refactor(list): align all list commands to issue list standards - Add isTraceId() non-throwing boolean helper to trace-id.ts - Rewrite log list: remove --trace flag, accept trace-id as positional arg (sentry log list , sentry log list /), add --period/-t flag, JSON envelope { data, hasMore } - Add withProgress spinner to all list commands: trace list, span list, trace logs, org list, trial list, project list, and org-list.ts framework (team/repo list get spinners automatically) - Align trace logs JSON output to { data, hasMore } envelope - Fix pre-existing 5s timeout failures in dispatchOrgScopedList tests by mocking resolveEffectiveOrg and withProgress --- AGENTS.md | 77 ++- plugins/sentry-cli/skills/sentry-cli/SKILL.md | 4 +- src/commands/log/list.ts | 196 +++++--- src/commands/org/list.ts | 6 +- src/commands/project/list.ts | 30 +- src/commands/span/list.ts | 17 +- src/commands/trace/list.ts | 17 +- src/commands/trace/logs.ts | 25 +- src/commands/trial/list.ts | 5 +- src/lib/org-list.ts | 49 +- src/lib/trace-id.ts | 23 +- test/commands/log/list.test.ts | 473 ++++++++++++++---- test/commands/trace/logs.test.ts | 27 +- test/lib/org-list.test.ts | 29 ++ test/lib/trace-id.test.ts | 73 ++- 15 files changed, 781 insertions(+), 270 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index 8201696b8..6570cd4af 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -767,65 +767,58 @@ mock.module("./some-module", () => ({ ### Architecture - -* **API client wraps all errors as CliError subclasses — no raw exceptions escape**: The API client (src/lib/api-client.ts) wraps ALL errors as CliError subclasses (ApiError or AuthError) — no raw exceptions escape. Commands don't need try-catch for error display; the central handler in app.ts formats CliError cleanly. Only add try-catch when a command needs to handle errors specially (e.g., login continuing despite user-info fetch failure). + +* **DSN org prefix normalization in arg-parsing.ts**: Sentry DSN hosts encode org IDs as \`oNNNNN\` (e.g., \`o1081365.ingest.us.sentry.io\`). The Sentry API rejects the \`o\`-prefixed form. \`stripDsnOrgPrefix()\` in \`src/lib/arg-parsing.ts\` uses \`/^o(\d+)$/\` to strip the prefix — safe for slugs like \`organic\`. Applied in \`parseOrgProjectArg()\` and \`parseWithSlash()\`, covering all API call paths consuming \`parsed.org\`. - -* **Sentry API: events require org+project, issues have legacy global endpoint**: Sentry API scoping: Events require org+project in URL path (\`/projects/{org}/{project}/events/{id}/\`). Issues use legacy global endpoint (\`/api/0/issues/{id}/\`) without org context. Traces need only org (\`/organizations/{org}/trace/{traceId}/\`). Two-step lookup for events: fetch issue → extract org/project from response → fetch event. Cross-project event search possible via Discover endpoint \`/organizations/{org}/events/\` with \`query=id:{eventId}\`. + +* **GHCR versioned nightly tags for delta upgrade support**: GHCR nightly distribution uses three tag types: \`:nightly\` (rolling), \`:nightly-\\` (immutable), \`:patch-\\` (delta manifest). Delta patches use zig-bsdiff TRDIFF10 (zstd-compressed), ~50KB vs ~29MB full. Client bspatch via \`Bun.zstdDecompressSync()\`. N-1 patches only, full download fallback, SHA-256 verify, 60% size threshold. npm/Node excluded. Test mocks: use \`mockGhcrNightlyVersion()\` helper. - -* **Sentry CLI authenticated fetch architecture with response caching**: \`createAuthenticatedFetch()\` wraps fetch with auth, 30s timeout, retry (max 2), 401 refresh, and span tracing. Response caching integrates BEFORE auth/retry via \`http-cache-semantics\` (RFC 7234) with filesystem storage at \`~/.sentry/cache/responses/\`. URL-based fallback TTL tiers: immutable (24hr), stable (5min), volatile (60s), no-cache (0). Only GET 2xx cached. \`--fresh\` and \`SENTRY\_NO\_CACHE=1\` bypass cache. Cache cleared on login/logout. \`hasServerCacheDirectives(policy)\` distinguishes \`max-age=0\` from missing headers. + +* **Issue list auto-pagination beyond API's 100-item cap**: Sentry API silently caps \`limit\` at 100 per request. \`listIssuesAllPages()\` auto-paginates using Link headers, bounded by MAX\_PAGINATION\_PAGES (50). \`API\_MAX\_PER\_PAGE\` constant is shared across all paginated consumers. \`--limit\` means total results everywhere (max 1000, default 25). Org-all mode uses \`fetchOrgAllIssues()\`; explicit \`--cursor\` does single-page fetch to preserve cursor chain. - -* **Sentry CLI has two distribution channels with different runtimes**: Sentry CLI ships two ways: (1) Standalone binary via \`Bun.build()\` with \`compile: true\`. (2) npm package via esbuild producing CJS \`dist/bin.cjs\` for Node 22+, with Bun API polyfills from \`script/node-polyfills.ts\`. \`Bun.$\` has NO polyfill — use \`execSync\` instead. \`require()\` in ESM is safe (Bun native, esbuild resolves at bundle time). + +* **Self-hosted OAuth device flow requires Sentry 26.1.0+ and SENTRY\_CLIENT\_ID**: Self-hosted OAuth device flow requires Sentry 26.1.0+ and both \`SENTRY\_URL\` and \`SENTRY\_CLIENT\_ID\` env vars. Users must create a public OAuth app in Settings → Developer Settings. The client ID is NOT optional for self-hosted. Fallback for older instances: \`sentry auth login --token\`. \`getSentryUrl()\` and \`getClientId()\` in \`src/lib/oauth.ts\` read lazily (not at module load) so URL parsing from arguments can set \`SENTRY\_URL\` after import. - -* **Sentry CLI resolve-target cascade has 5 priority levels with env var support**: Resolve-target cascade (src/lib/resolve-target.ts) 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. The \`resolveFromEnvVars()\` helper is injected into all four resolution functions. + +* **Sentry CLI markdown-first formatting pipeline replaces ad-hoc ANSI**: Formatters build CommonMark strings; \`renderMarkdown()\` renders to ANSI for TTY or raw markdown for non-TTY. Key helpers: \`colorTag()\`, \`mdKvTable()\`, \`mdRow()\`, \`mdTableHeader()\` (\`:\` suffix = right-aligned), \`renderTextTable()\`. \`isPlainOutput()\` checks \`SENTRY\_PLAIN\_OUTPUT\` > \`NO\_COLOR\` > \`!isTTY\`. Batch path: \`formatXxxTable()\`. Streaming path: \`StreamingTable\` (TTY) or raw markdown rows (plain). Both share \`buildXxxRowCells()\`. -### Decision + +* **Sentry issue stats field: time-series controlled by groupStatsPeriod**: The \`stats\` field on issues is \`{ '24h': \[\[ts, count], ...] }\`. Key depends on \`groupStatsPeriod\` param (\`""\`, \`"14d"\`, \`"24h"\`, \`"auto"\`). \`statsPeriod\` controls time window; \`groupStatsPeriod\` controls stats key. \*\*Critical\*\*: \`count\` is period-scoped — \`lifetime.count\` is the true lifetime total. Issue list table uses \`groupStatsPeriod: 'auto'\` for sparkline data. Column order: SHORT ID, ISSUE, SEEN, AGE, TREND, EVENTS, USERS, TRIAGE. TREND auto-hidden when terminal < 100 cols. \`--compact\` tri-state: explicit overrides; \`undefined\` triggers \`shouldAutoCompact(rowCount)\` — compact if \`3N + 3 > termHeight\`, false for non-TTY. Height is \`3N + 3\` (not \`3N + 4\`) because last data row has no trailing separator. - -* **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 at least 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. + +* **Sentry trace-logs API is org-scoped, not project-scoped**: The Sentry trace-logs endpoint (\`/organizations/{org}/trace-logs/\`) is org-scoped, so \`trace logs\` uses \`resolveOrg()\` not \`resolveOrgAndProject()\`. The endpoint is PRIVATE in Sentry source, excluded from the public OpenAPI schema — \`@sentry/api\` has no generated types. The hand-written \`TraceLogSchema\` in \`src/types/sentry.ts\` is required until Sentry makes it public. - -* **Sentry CLI config dir should stay at ~/.sentry/, not move to XDG**: Config dir stays at \`~/.sentry/\` (not XDG). The readonly DB errors on macOS are from \`sudo brew install\` creating root-owned files. Fixes: (1) bestEffort() makes setup steps non-fatal, (2) tryRepairReadonly() detects root-owned files and prints \`sudo chown\` instructions, (3) \`sentry cli fix\` handles ownership repair. Ownership must be checked BEFORE permissions — root-owned files cause chmod to EPERM. + +* **withAuthGuard returns discriminated Result type, not fallback+onError**: \`withAuthGuard\(fn)\` in \`src/lib/errors.ts\` returns a discriminated Result: \`{ ok: true, value: T } | { ok: false, error: unknown }\`. AuthErrors always re-throw (triggers bin.ts auto-login). All other errors are captured. Callers inspect \`result.ok\` to degrade gracefully. Used across 12+ files. ### Gotcha - -* **brew is not in VALID\_METHODS but Homebrew formula passes --method brew**: Homebrew install: \`isHomebrewInstall()\` detects via Cellar realpath (checked before stored install info). Upgrade command tells users \`brew upgrade getsentry/tools/sentry\`. Formula runs \`sentry cli setup --method brew --no-modify-path\` as post\_install. Version pinning throws 'unsupported\_operation'. Uses .gz artifacts. Tap at getsentry/tools. + +* **Biome lint: Response.redirect() required, nested ternaries forbidden**: Biome lint rules that frequently trip up this codebase: (1) \`useResponseRedirect\`: use \`Response.redirect(url, status)\` not \`new Response\`. (2) \`noNestedTernary\`: use \`if/else\`. (3) \`noComputedPropertyAccess\`: use \`obj.property\` not \`obj\["property"]\`. (4) Max cognitive complexity 15 per function — extract helpers to stay under. - -* **Bun mock.module() leaks globally across test files in same process**: Bun's mock.module() replaces modules globally and leaks across test files in the same process. Solution: tests using mock.module() must run in a separate \`bun test\` invocation. In package.json, use \`bun run test:unit && bun run test:isolated\` instead of \`bun test\`. The \`test/isolated/\` directory exists for these tests. This was the root cause of ~100 test failures (getsentry/cli#258). + +* **Bugbot flags defensive null-checks as dead code — keep them with JSDoc justification**: Cursor Bugbot and Sentry Seer repeatedly flag two false positives: (1) defensive null-checks as "dead code" — keep them with JSDoc explaining why the guard exists for future safety, especially when removing would require \`!\` assertions banned by \`noNonNullAssertion\`. (2) stderr spinner output during \`--json\` mode — always a false positive since progress goes to stderr, JSON to stdout. Reply explaining the rationale and resolve. - -* **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 (macrotasks) during \`asyncModelRun\` cause \`createIsolatedDbContext\` cleanup to interleave. Fix: keep non-async, return \`clearResponseCache().catch(...)\` directly. Model-based tests should NOT await it. Also: model-based tests need explicit timeouts (e.g., \`30\_000\`) — Bun's default 5s causes false failures during shrinking. - - -* **Multiregion mock must include all control silo API routes**: When changing which Sentry API endpoint a function uses, mock routes must be updated in BOTH \`test/mocks/routes.ts\` (single-region) AND \`test/mocks/multiregion.ts\` \`createControlSiloRoutes()\`. Missing the multiregion mock causes 404s in multi-region test scenarios. - - -* **Sentry /users/me/ endpoint returns 403 for OAuth tokens — use /auth/ instead**: The Sentry \`/users/me/\` endpoint returns 403 for OAuth tokens. Use \`/auth/\` instead — it works with ALL token types and lives on the control silo. In the CLI, \`getControlSiloUrl()\` handles routing correctly. \`SentryUserSchema\` (with \`.passthrough()\`) handles the \`/auth/\` response since it only requires \`id\`. - - -* **Stricli command context uses this.stdout not this.process.stdout**: In Stricli command \`func()\` handlers, use \`this.stdout\` and \`this.stderr\` directly — NOT \`this.process.stdout\`. The \`SentryContext\` interface has both \`process\` and \`stdout\`/\`stderr\` as separate top-level properties. Test mock contexts typically provide \`stdout\` but not a full \`process\` object, so \`this.process.stdout\` causes \`TypeError: undefined is not an object\` at runtime in tests even though TypeScript doesn't flag it. - - -* **Stricli defaultCommand blends default command flags into route completions**: When a Stricli route map has \`defaultCommand\` set, requesting completions for that route (e.g. \`\["issues", ""]\`) returns both the subcommand names AND the default command's flags/positional completions. This means completion tests that compare against \`extractCommandTree()\` subcommand lists will fail for groups with defaultCommand, since the actual completions include extra entries like \`--limit\`, \`--query\`, etc. Solution: track \`hasDefaultCommand\` in the command tree and skip strict subcommand-matching assertions for those groups. + +* **Bun mock.module for node:tty requires default export and class stubs**: Bun testing gotchas: (1) \`mock.module()\` for CJS built-ins requires a \`default\` re-export plus all named exports. Missing any causes \`SyntaxError: Export named 'X' not found\`. Always check the real module's full export list. (2) \`Bun.mmap()\` always opens with PROT\_WRITE — macOS SIGKILL on signed Mach-O, Linux ETXTBSY. Fix: use \`new Uint8Array(await Bun.file(path).arrayBuffer())\` in bspatch.ts. (3) Wrap \`Bun.which()\` with optional \`pathEnv\` param for deterministic testing without mocks. ### Pattern - -* **Extract logic from Stricli func() handlers into standalone functions for testability**: Stricli command \`func()\` handlers are hard to unit test because they require full command context setup. To boost coverage, extract flag validation and body-building logic into standalone exported functions (e.g., \`resolveBody()\` extracted from the \`api\` command's \`func()\`). This moved ~20 lines of mutual-exclusivity checks and flag routing from an untestable handler into a directly testable pure function. Property-based tests on the extracted function drove patch coverage from 78% to 97%. The general pattern: keep \`func()\` as a thin orchestrator that calls exported helpers. This also keeps biome complexity under the limit (max 15). + +* **Branch naming and commit message conventions for Sentry CLI**: Branch naming: \`feat/\\` or \`fix/\-\\` (e.g., \`feat/ghcr-nightly-distribution\`, \`fix/268-limit-auto-pagination\`). Commit message format: \`type(scope): description (#issue)\` (e.g., \`fix(issue-list): auto-paginate --limit beyond 100 (#268)\`, \`feat(nightly): distribute via GHCR instead of GitHub Releases\`). Types seen: fix, refactor, meta, release, feat. PRs are created as drafts via \`gh pr create --draft\`. Implementation plans are attached to commits via \`git notes add\` rather than in PR body or commit message. + + +* **Codecov patch coverage only counts test:unit and test:isolated, not E2E**: CI coverage merges \`test:unit\` (\`test/lib test/commands test/types --coverage\`) and \`test:isolated\` (\`test/isolated --coverage\`) into \`coverage/merged.lcov\`. E2E tests (\`test/e2e\`) are NOT included in coverage reports. So func tests that spy on exports (e.g., \`spyOn(apiClient, 'getLogs')\`) give zero coverage to the mocked function's body. To cover \`api-client.ts\` function bodies in unit tests, mock \`globalThis.fetch\` + \`setOrgRegion()\` + \`setAuthToken()\` and call the real function. - -* **Non-essential DB cache writes should be guarded with try-catch**: Non-essential DB cache writes (e.g., \`setUserInfo()\` in whoami.ts and login.ts) must be wrapped in try-catch. If the DB is broken, the cache write shouldn't crash the command when its primary operation already succeeded. In login.ts specifically, \`getCurrentUser()\` failure after token save must not block authentication — wrap in try-catch, log warning to stderr, let login succeed. This differs from \`getUserRegions()\` failure which should \`clearAuth()\` and fail hard (indicates invalid token). + +* **Pagination contextKey must include all query-varying parameters with escaping**: Pagination \`contextKey\` must encode every query-varying parameter (sort, query, period) with \`escapeContextKeyValue()\` (replaces \`|\` with \`%7C\`). Always provide a fallback before escaping since \`flags.period\` may be \`undefined\` in tests despite having a default: \`flags.period ? escapeContextKeyValue(flags.period) : "90d"\`. - -* **Stricli buildCommand output config injects json flag into func params**: When a Stricli command uses \`output: { json: true, human: formatFn }\`, the framework injects \`--json\` and \`--fields\` flags automatically. The \`func\` handler receives these as its first parameter. Type it explicitly (e.g., \`flags: { json?: boolean }\`) rather than \`\_flags: unknown\` to access the json flag for conditional behavior (e.g., skipping interactive output in JSON mode). The \`human\` formatter runs on the returned \`data\` for non-JSON output. Commands that produce interactive side effects (browser prompts, QR codes) should check \`flags.json\` and skip them when true. + +* **PR review workflow: reply, resolve, amend, force-push**: PR review workflow: (1) Read unresolved threads via GraphQL, (2) make code changes, (3) run lint+typecheck+tests, (4) create a SEPARATE commit per review round (not amend) for incremental review, (5) push normally, (6) reply to comments via REST API, (7) resolve threads via GraphQL \`resolveReviewThread\`. Only amend+force-push when user explicitly asks or pre-commit hook modified files. -### Preference + +* **Stricli optional boolean flags produce tri-state (true/false/undefined)**: Stricli boolean flags with \`optional: true\` (no \`default\`) produce \`boolean | undefined\` in the flags type. \`--flag\` → \`true\`, \`--no-flag\` → \`false\`, omitted → \`undefined\`. This enables auto-detect patterns: explicit user choice overrides, \`undefined\` triggers heuristic. Used by \`--compact\` on issue list. The flag type must be \`readonly field?: boolean\` (not \`readonly field: boolean\`). This differs from \`default: false\` which always produces a defined boolean. - -* **PR workflow: address review comments, resolve threads, wait for CI**: User's PR workflow after creation: (1) Wait for CI checks to pass, (2) Check for unresolved review comments via \`gh api\` for PR review comments, (3) Fix issues in follow-up commits (not amends), (4) Reply to the comment thread explaining the fix, (5) Resolve the thread programmatically via \`gh api graphql\` with \`resolveReviewThread\` mutation, (6) Push and wait for CI again, (7) Final sweep for any remaining unresolved comments. Use \`git notes add\` to attach implementation plans to commits. Branch naming: \`fix/descriptive-slug\` or \`feat/descriptive-slug\`. + +* **Testing Stricli command func() bodies via spyOn mocking**: Stricli/Bun test patterns: (1) Command func tests: \`const func = await cmd.loader()\`, then \`func.call(mockContext, flags, ...args)\`. \`loader()\` return type union causes LSP errors — false positives that pass \`tsc\`. File naming: \`\*.func.test.ts\`. (2) ESM prevents \`vi.spyOn\` on Node built-in exports. Workaround: test subclass that overrides the method calling the built-in. (3) Follow-mode uses \`setTimeout\`-based scheduling; test with \`interceptSigint()\` helper. \`Bun.sleep()\` has no AbortSignal so \`setTimeout\`/\`clearTimeout\` required. diff --git a/plugins/sentry-cli/skills/sentry-cli/SKILL.md b/plugins/sentry-cli/skills/sentry-cli/SKILL.md index 885643cc7..3729a73a6 100644 --- a/plugins/sentry-cli/skills/sentry-cli/SKILL.md +++ b/plugins/sentry-cli/skills/sentry-cli/SKILL.md @@ -554,7 +554,7 @@ sentry team list --json View Sentry logs -#### `sentry log list ` +#### `sentry log list ` List logs from a project @@ -562,7 +562,7 @@ List logs from a project - `-n, --limit - Number of log entries (1-1000) - (default: "100")` - `-q, --query - Filter query (Sentry search syntax)` - `-f, --follow - Stream logs (optionally specify poll interval in seconds)` -- `--trace - Filter logs by trace ID (32-character hex string)` +- `-t, --period - Time period (e.g., "90d", "14d", "24h") - (default: "90d")` - `--fresh - Bypass cache, re-detect projects, and fetch fresh data` - `--json - Output as JSON` - `--fields - Comma-separated fields to include in JSON output (dot.notation supported)` diff --git a/src/commands/log/list.ts b/src/commands/log/list.ts index cf3388559..a92d5b9cb 100644 --- a/src/commands/log/list.ts +++ b/src/commands/log/list.ts @@ -3,7 +3,7 @@ * * List and stream logs from Sentry projects. * Supports real-time streaming with --follow flag. - * Supports --trace flag to filter logs by trace ID. + * Supports trace ID as a positional argument to filter logs by trace. */ // biome-ignore lint/performance/noNamespaceImport: Sentry SDK recommends namespace import @@ -11,7 +11,7 @@ import * as Sentry from "@sentry/bun"; import type { SentryContext } from "../../context.js"; import { listLogs, listTraceLogs } from "../../lib/api-client.js"; import { validateLimit } from "../../lib/arg-parsing.js"; -import { AuthError, ContextError, stringifyUnknown } from "../../lib/errors.js"; +import { AuthError, stringifyUnknown } from "../../lib/errors.js"; import { buildLogRowCells, createLogStreamingTable, @@ -34,19 +34,23 @@ import { TARGET_PATTERN_NOTE, } from "../../lib/list-command.js"; import { logger } from "../../lib/logger.js"; +import { withProgress } from "../../lib/polling.js"; +import { resolveOrgProjectFromArg } from "../../lib/resolve-target.js"; +import { isTraceId } from "../../lib/trace-id.js"; import { - resolveOrg, - resolveOrgProjectFromArg, -} from "../../lib/resolve-target.js"; -import { validateTraceId } from "../../lib/trace-id.js"; + type ParsedTraceTarget, + parseTraceTarget, + resolveTraceOrg, + warnIfNormalized, +} from "../../lib/trace-target.js"; import { getUpdateNotification } from "../../lib/version-check.js"; type ListFlags = { readonly limit: number; readonly query?: string; readonly follow?: number; + readonly period: string; readonly json: boolean; - readonly trace?: string; readonly fresh: boolean; readonly fields?: string[]; }; @@ -62,6 +66,8 @@ type LogListResult = { logs: LogLike[]; /** Trace ID, present for trace-filtered queries */ traceId?: string; + /** Whether more results are available beyond the limit */ + hasMore: boolean; }; /** Output yielded by log list: either a batch (single-fetch) or an individual item (follow). */ @@ -82,6 +88,12 @@ const DEFAULT_POLL_INTERVAL = 2; /** Command name used in resolver error messages */ const COMMAND_NAME = "log list"; +/** Usage hint for trace mode error messages */ +const TRACE_USAGE_HINT = "sentry log list [/]"; + +/** Default time period for trace-logs queries */ +const DEFAULT_TRACE_PERIOD = "14d"; + /** * Parse --limit flag, delegating range validation to shared utility. */ @@ -130,6 +142,56 @@ type FetchResult = { hint: string; }; +// --------------------------------------------------------------------------- +// Positional argument disambiguation +// --------------------------------------------------------------------------- + +/** + * Parsed result from log list positional arguments. + * + * Discriminated on `mode`: + * - `"project"` — standard project-scoped log listing (existing path) + * - `"trace"` — trace-filtered log listing via trace-logs endpoint + */ +type ParsedLogArgs = + | { mode: "project"; target?: string } + | { mode: "trace"; parsed: ParsedTraceTarget }; + +/** + * Disambiguate log list positional arguments. + * + * Checks if the "tail" segment (last part after `/`, or the entire arg + * if no `/`) looks like a 32-char hex trace ID. If so, delegates to + * {@link parseTraceTarget} for full trace target parsing. Otherwise, + * treats the argument as a project target. + * + * @param args - Positional arguments from CLI + * @returns Parsed args with mode discrimination + */ +function parseLogListArgs(args: string[]): ParsedLogArgs { + if (args.length === 0) { + return { mode: "project" }; + } + + const first = args[0]; + if (first === undefined) { + return { mode: "project" }; + } + + // Check the tail segment: last part after `/`, or the entire arg + const lastSlash = first.lastIndexOf("/"); + const tail = lastSlash === -1 ? first : first.slice(lastSlash + 1); + + if (isTraceId(tail)) { + return { + mode: "trace", + parsed: parseTraceTarget(args, TRACE_USAGE_HINT), + }; + } + + return { mode: "project", target: first }; +} + /** * Execute a single fetch of logs (non-streaming mode). * @@ -144,11 +206,11 @@ async function executeSingleFetch( const logs = await listLogs(org, project, { query: flags.query, limit: flags.limit, - statsPeriod: "90d", + statsPeriod: flags.period, }); if (logs.length === 0) { - return { result: { logs: [] }, hint: "No logs found." }; + return { result: { logs: [], hasMore: false }, hint: "No logs found." }; } // Reverse for chronological order (API returns newest first, tail shows oldest first) @@ -158,7 +220,10 @@ async function executeSingleFetch( const countText = `Showing ${logs.length} log${logs.length === 1 ? "" : "s"}.`; const tip = hasMore ? " Use --limit to show more, or -f to follow." : ""; - return { result: { logs: chronological }, hint: `${countText}${tip}` }; + return { + result: { logs: chronological, hasMore }, + hint: `${countText}${tip}`, + }; } // --------------------------------------------------------------------------- @@ -368,7 +433,11 @@ async function* yieldTraceFollowItems( for await (const batch of generator) { if (!contextSent && batch.length > 0) { // First non-empty batch: yield as LogListResult to set trace context - yield new CommandOutput({ logs: batch, traceId }); + yield new CommandOutput({ + logs: batch, + traceId, + hasMore: false, + }); contextSent = true; } else { for (const item of batch) { @@ -378,11 +447,8 @@ async function* yieldTraceFollowItems( } } -/** Default time period for trace-logs queries */ -const DEFAULT_TRACE_PERIOD = "14d"; - /** - * Execute a single fetch of trace-filtered logs (non-streaming, --trace mode). + * Execute a single fetch of trace-filtered logs (non-streaming, trace mode). * Uses the dedicated trace-logs endpoint which is org-scoped. * * Returns the fetched logs, trace ID, and a human-readable hint. @@ -393,17 +459,21 @@ async function executeTraceSingleFetch( traceId: string, flags: ListFlags ): Promise { + // In trace mode, use a shorter default period if the user hasn't + // explicitly changed it from the command-level default of "90d". + const period = flags.period === "90d" ? DEFAULT_TRACE_PERIOD : flags.period; + const logs = await listTraceLogs(org, traceId, { query: flags.query, limit: flags.limit, - statsPeriod: DEFAULT_TRACE_PERIOD, + statsPeriod: period, }); if (logs.length === 0) { return { - result: { logs: [], traceId }, + result: { logs: [], traceId, hasMore: false }, hint: - `No logs found for trace ${traceId} in the last ${DEFAULT_TRACE_PERIOD}.\n\n` + + `No logs found for trace ${traceId} in the last ${period}.\n\n` + "Try 'sentry trace logs' for more options (e.g., --period 30d).", }; } @@ -415,7 +485,7 @@ async function executeTraceSingleFetch( const tip = hasMore ? " Use --limit to show more." : ""; return { - result: { logs: chronological, traceId }, + result: { logs: chronological, traceId, hasMore }, hint: `${countText}${tip}`, }; } @@ -518,16 +588,18 @@ function createLogRenderer(): HumanRenderer { * Transform log output into the JSON shape. * * Discriminates between {@link LogListResult} (single-fetch) and bare - * {@link LogLike} items (follow mode). Single-fetch yields a JSON array; - * follow mode yields one JSON object per line (JSONL). + * {@link LogLike} items (follow mode). Single-fetch yields a JSON envelope + * with `data` and `hasMore`; follow mode yields one JSON object per line (JSONL). */ function jsonTransformLogOutput(data: LogOutput, fields?: string[]): unknown { if ("logs" in data && Array.isArray((data as LogListResult).logs)) { - // Batch (single-fetch): return array - const logs = (data as LogListResult).logs; - return fields && fields.length > 0 - ? logs.map((log) => filterFields(log, fields)) - : logs; + // Batch (single-fetch): return envelope with data + hasMore + const logList = data as LogListResult; + const items = + fields && fields.length > 0 + ? logList.logs.map((log) => filterFields(log, fields)) + : logList.logs; + return { data: items, hasMore: logList.hasMore }; } // Single item (follow mode): return bare object for JSONL return fields && fields.length > 0 ? filterFields(data, fields) : data; @@ -544,16 +616,15 @@ export const listCommand = buildListCommand("log", { " sentry log list # find project across all orgs\n\n" + `${TARGET_PATTERN_NOTE}\n\n` + "Trace filtering:\n" + - " When --trace is given, only org resolution is needed (the trace-logs\n" + - " endpoint is org-scoped). The positional target is treated as an org\n" + - " slug, not an org/project pair.\n\n" + + " sentry log list # Filter by trace (auto-detect org)\n" + + " sentry log list / # Filter by trace (explicit org)\n\n" + "Examples:\n" + " sentry log list # List last 100 logs\n" + " sentry log list -f # Stream logs (2s poll interval)\n" + " sentry log list -f 5 # Stream logs (5s poll interval)\n" + " sentry log list --limit 50 # Show last 50 logs\n" + " sentry log list -q 'level:error' # Filter to errors only\n" + - " sentry log list --trace abc123def456abc123def456abc123de # Filter by trace\n\n" + + " sentry log list abc123def456abc123def456abc123de # Filter by trace\n\n" + "Alias: `sentry logs` → `sentry log list`", }, output: { @@ -562,15 +633,12 @@ export const listCommand = buildListCommand("log", { }, parameters: { positional: { - kind: "tuple", - parameters: [ - { - placeholder: "org/project", - brief: "/ or (search)", - parse: String, - optional: true, - }, - ], + kind: "array", + parameter: { + placeholder: "org/project-or-trace-id", + brief: "[/[/]], /, or ", + parse: String, + }, }, flags: { limit: { @@ -592,11 +660,11 @@ export const listCommand = buildListCommand("log", { optional: true, inferEmpty: true, }, - trace: { + period: { kind: "parsed", - parse: validateTraceId, - brief: "Filter logs by trace ID (32-character hex string)", - optional: true, + parse: String, + brief: 'Time period (e.g., "90d", "14d", "24h")', + default: "90d", }, fresh: FRESH_FLAG, }, @@ -604,31 +672,26 @@ export const listCommand = buildListCommand("log", { n: "limit", q: "query", f: "follow", + t: "period", }, }, - async *func(this: SentryContext, flags: ListFlags, target?: string) { + async *func(this: SentryContext, flags: ListFlags, ...args: string[]) { applyFreshFlag(flags); const { cwd, setContext } = this; - if (flags.trace) { + const parsed = parseLogListArgs(args); + + if (parsed.mode === "trace") { // Trace mode: use the org-scoped trace-logs endpoint. - // The positional target is treated as an org slug (not org/project). - const resolved = await resolveOrg({ - org: target, + warnIfNormalized(parsed.parsed, "log.list"); + const { traceId, org } = await resolveTraceOrg( + parsed.parsed, cwd, - }); - if (!resolved) { - throw new ContextError("Organization", "sentry log list --trace ", [ - "Set a default org with 'sentry org list', or specify one explicitly", - `Example: sentry log list myorg --trace ${flags.trace}`, - ]); - } - const { org } = resolved; + TRACE_USAGE_HINT + ); setContext([org], []); if (flags.follow) { - const traceId = flags.trace; - // Banner (suppressed in JSON mode) writeFollowBanner( flags.follow ?? DEFAULT_POLL_INTERVAL, @@ -676,20 +739,18 @@ export const listCommand = buildListCommand("log", { return; } - const { result, hint } = await executeTraceSingleFetch( - org, - flags.trace, - flags + const { result, hint } = await withProgress( + { message: "Fetching logs..." }, + () => executeTraceSingleFetch(org, traceId, flags) ); yield new CommandOutput(result); return { hint }; } - // Standard project-scoped mode — kept in else-like block to avoid - // `org` shadowing the trace-mode `org` declaration above. + // Standard project-scoped mode { const { org, project } = await resolveOrgProjectFromArg( - target, + parsed.target, cwd, COMMAND_NAME ); @@ -719,7 +780,10 @@ export const listCommand = buildListCommand("log", { return; } - const { result, hint } = await executeSingleFetch(org, project, flags); + const { result, hint } = await withProgress( + { message: "Fetching logs..." }, + () => executeSingleFetch(org, project, flags) + ); yield new CommandOutput(result); return { hint }; } diff --git a/src/commands/org/list.ts b/src/commands/org/list.ts index 1352c2024..a1f2253f0 100644 --- a/src/commands/org/list.ts +++ b/src/commands/org/list.ts @@ -18,6 +18,7 @@ import { FRESH_ALIASES, FRESH_FLAG, } from "../../lib/list-command.js"; +import { withProgress } from "../../lib/polling.js"; import type { SentryOrganization, Writer } from "../../types/index.js"; type ListFlags = { @@ -130,7 +131,10 @@ export const listCommand = buildCommand({ async *func(this: SentryContext, flags: ListFlags) { applyFreshFlag(flags); - const orgs = await listOrganizationsUncached(); + const orgs = await withProgress( + { message: "Fetching organizations..." }, + () => listOrganizationsUncached() + ); const limitedOrgs = orgs.slice(0, flags.limit); // Check if user has orgs in multiple regions diff --git a/src/commands/project/list.ts b/src/commands/project/list.ts index 071acd74e..e4b7d2d72 100644 --- a/src/commands/project/list.ts +++ b/src/commands/project/list.ts @@ -54,6 +54,7 @@ import { type ListCommandMeta, type ListResult, } from "../../lib/org-list.js"; +import { withProgress } from "../../lib/polling.js"; import { type ResolvedTarget, resolveAllTargets, @@ -333,9 +334,9 @@ export async function handleAutoDetect( skippedSelfHosted, } = await resolveOrgsForAutoDetect(cwd); - const { projects: allProjects, nextCursor } = await fetchAutoDetectProjects( - orgsToFetch, - flags + const { projects: allProjects, nextCursor } = await withProgress( + { message: "Fetching projects..." }, + () => fetchAutoDetectProjects(orgsToFetch, flags) ); const filtered = filterByPlatform(allProjects, flags.platform); @@ -390,7 +391,10 @@ export async function handleExplicit( projectSlug: string, flags: ListFlags ): Promise> { - const projectResult = await withAuthGuard(() => getProject(org, projectSlug)); + const projectResult = await withProgress( + { message: "Fetching projects..." }, + () => withAuthGuard(() => getProject(org, projectSlug)) + ); if (!projectResult.ok) { return { items: [], @@ -437,11 +441,14 @@ export async function handleOrgAll( options: OrgAllOptions ): Promise> { const { org, flags, contextKey, cursor } = options; - const response: PaginatedResponse = - await listProjectsPaginated(org, { - cursor, - perPage: flags.limit, - }); + const response: PaginatedResponse = await withProgress( + { message: "Fetching projects..." }, + () => + listProjectsPaginated(org, { + cursor, + perPage: flags.limit, + }) + ); const projects: ProjectWithOrg[] = response.data.map((p) => ({ ...p, @@ -498,7 +505,10 @@ export async function handleProjectSearch( projectSlug: string, flags: ListFlags ): Promise> { - const { projects } = await findProjectsBySlug(projectSlug); + const { projects } = await withProgress( + { message: "Fetching projects..." }, + () => findProjectsBySlug(projectSlug) + ); const filtered = filterByPlatform(projects, flags.platform); if (filtered.length === 0) { diff --git a/src/commands/span/list.ts b/src/commands/span/list.ts index 32b8c37b9..524263b46 100644 --- a/src/commands/span/list.ts +++ b/src/commands/span/list.ts @@ -30,6 +30,7 @@ import { FRESH_FLAG, LIST_CURSOR_FLAG, } from "../../lib/list-command.js"; +import { withProgress } from "../../lib/polling.js"; import { parseTraceTarget, resolveTraceOrgProject, @@ -260,12 +261,16 @@ export const listCommand = buildCommand({ const cursor = resolveOrgCursor(flags.cursor, PAGINATION_KEY, contextKey); // Fetch spans from EAP endpoint - const { data: spanItems, nextCursor } = await listSpans(org, project, { - query: apiQuery, - sort: flags.sort, - limit: flags.limit, - cursor, - }); + const { data: spanItems, nextCursor } = await withProgress( + { message: "Fetching spans..." }, + () => + listSpans(org, project, { + query: apiQuery, + sort: flags.sort, + limit: flags.limit, + cursor, + }) + ); // Store or clear pagination cursor if (nextCursor) { diff --git a/src/commands/trace/list.ts b/src/commands/trace/list.ts index 45345caf9..3d52da93f 100644 --- a/src/commands/trace/list.ts +++ b/src/commands/trace/list.ts @@ -24,6 +24,7 @@ import { LIST_CURSOR_FLAG, TARGET_PATTERN_NOTE, } from "../../lib/list-command.js"; +import { withProgress } from "../../lib/polling.js"; import { resolveOrgProjectFromArg } from "../../lib/resolve-target.js"; import type { TransactionListItem } from "../../types/index.js"; @@ -245,12 +246,16 @@ export const listCommand = buildListCommand("trace", { }); const cursor = resolveOrgCursor(flags.cursor, PAGINATION_KEY, contextKey); - const { data: traces, nextCursor } = await listTransactions(org, project, { - query: flags.query, - limit: flags.limit, - sort: flags.sort, - cursor, - }); + const { data: traces, nextCursor } = await withProgress( + { message: "Fetching traces..." }, + () => + listTransactions(org, project, { + query: flags.query, + limit: flags.limit, + sort: flags.sort, + cursor, + }) + ); // Store or clear pagination cursor if (nextCursor) { diff --git a/src/commands/trace/logs.ts b/src/commands/trace/logs.ts index c78a1737f..6761d7d6f 100644 --- a/src/commands/trace/logs.ts +++ b/src/commands/trace/logs.ts @@ -17,6 +17,7 @@ import { FRESH_ALIASES, FRESH_FLAG, } from "../../lib/list-command.js"; +import { withProgress } from "../../lib/polling.js"; import { buildTraceUrl } from "../../lib/sentry-urls.js"; import { parseTraceTarget, @@ -47,6 +48,7 @@ type TraceLogsData = { logs: LogLike[]; traceId: string; limit: number; + hasMore: boolean; /** Message shown when no logs found */ emptyMessage?: string; }; @@ -107,10 +109,11 @@ export const logsCommand = buildCommand({ output: { human: formatTraceLogsHuman, jsonTransform: (data: TraceLogsData, fields?: string[]) => { - if (fields && fields.length > 0) { - return data.logs.map((entry) => filterFields(entry, fields)); - } - return data.logs; + const items = + fields && fields.length > 0 + ? data.logs.map((entry) => filterFields(entry, fields)) + : data.logs; + return { data: items, hasMore: data.hasMore }; }, }, parameters: { @@ -171,14 +174,17 @@ export const logsCommand = buildCommand({ return; } - const logs = await listTraceLogs(org, traceId, { - statsPeriod: flags.period, - limit: flags.limit, - query: flags.query, - }); + const logs = await withProgress({ message: "Fetching trace logs..." }, () => + listTraceLogs(org, traceId, { + statsPeriod: flags.period, + limit: flags.limit, + query: flags.query, + }) + ); // Reverse to chronological order (API returns newest-first) const chronological = [...logs].reverse(); + const hasMore = chronological.length >= flags.limit; const emptyMessage = `No logs found for trace ${traceId} in the last ${flags.period}.\n\n` + @@ -188,6 +194,7 @@ export const logsCommand = buildCommand({ logs: chronological, traceId, limit: flags.limit, + hasMore, emptyMessage, }); }, diff --git a/src/commands/trial/list.ts b/src/commands/trial/list.ts index c49b6e79e..a837a9604 100644 --- a/src/commands/trial/list.ts +++ b/src/commands/trial/list.ts @@ -13,6 +13,7 @@ import { ContextError } from "../../lib/errors.js"; import { colorTag } from "../../lib/formatters/markdown.js"; import { CommandOutput } from "../../lib/formatters/output.js"; import { type Column, writeTable } from "../../lib/formatters/table.js"; +import { withProgress } from "../../lib/polling.js"; import { resolveOrg } from "../../lib/resolve-target.js"; import { daysRemainingFromDate, @@ -230,7 +231,9 @@ export const listCommand = buildCommand({ throw new ContextError("Organization", "sentry trial list "); } - const info = await getCustomerTrialInfo(resolved.org); + const info = await withProgress({ message: "Fetching trials..." }, () => + getCustomerTrialInfo(resolved.org) + ); const productTrials = info.productTrials ?? []; const entries: TrialListEntry[] = deduplicateTrials( diff --git a/src/lib/org-list.ts b/src/lib/org-list.ts index 6f4bb0b75..3595e5aa7 100644 --- a/src/lib/org-list.ts +++ b/src/lib/org-list.ts @@ -50,6 +50,7 @@ import { } from "./errors.js"; import { filterFields } from "./formatters/json.js"; import { logger } from "./logger.js"; +import { withProgress } from "./polling.js"; import { resolveEffectiveOrg } from "./region.js"; import { resolveOrgsForListing } from "./resolve-target.js"; @@ -345,10 +346,14 @@ export async function handleOrgAll( ): Promise> { const { config, org, flags, contextKey, cursor } = options; - const response = await config.listPaginated(org, { - cursor, - perPage: flags.limit, - }); + const response = await withProgress( + { message: `Fetching ${config.entityPlural}...` }, + () => + config.listPaginated(org, { + cursor, + perPage: flags.limit, + }) + ); const { data: rawItems, nextCursor } = response; const items = rawItems.map((entity) => config.withOrg(entity, org)); @@ -402,15 +407,18 @@ export async function handleAutoDetect( skippedSelfHosted, } = await resolveOrgsForListing(undefined, cwd); - let allItems: TWithOrg[]; - if (orgsToFetch.length > 0) { - const results = await Promise.all( - orgsToFetch.map((org) => fetchOrgSafe(config, org)) - ); - allItems = results.flat(); - } else { - allItems = await fetchAllOrgs(config); - } + const allItems = await withProgress( + { message: `Fetching ${config.entityPlural}...` }, + async () => { + if (orgsToFetch.length > 0) { + const results = await Promise.all( + orgsToFetch.map((org) => fetchOrgSafe(config, org)) + ); + return results.flat(); + } + return fetchAllOrgs(config); + } + ); const limitCount = orgsToFetch.length > 1 ? flags.limit * orgsToFetch.length : flags.limit; @@ -527,7 +535,10 @@ export async function handleExplicitOrg( options: ExplicitOrgOptions ): Promise> { const { config, org, flags, noteOrgScoped = false } = options; - const items = await fetchOrgSafe(config, org); + const items = await withProgress( + { message: `Fetching ${config.entityPlural}...` }, + () => fetchOrgSafe(config, org) + ); const result = buildFetchedItemsResult({ config, @@ -576,7 +587,10 @@ export async function handleExplicitProject( "handleExplicitProject called but config.listForProject is not defined" ); } - const raw = await listForProject(org, project); + const raw = await withProgress( + { message: `Fetching ${config.entityPlural}...` }, + () => listForProject(org, project) + ); const items = raw.map((entity) => config.withOrg(entity, org)); const result = buildFetchedItemsResult({ @@ -622,7 +636,10 @@ export async function handleProjectSearch( } ): Promise> { const { flags, orgAllFallback } = options; - const { projects: matches, orgs } = await findProjectsBySlug(projectSlug); + const { projects: matches, orgs } = await withProgress( + { message: `Fetching ${config.entityPlural}...` }, + () => findProjectsBySlug(projectSlug) + ); if (matches.length === 0) { const matchingOrg = orgs.find((o) => o.slug === projectSlug); diff --git a/src/lib/trace-id.ts b/src/lib/trace-id.ts index 71435f58b..d7edcaf26 100644 --- a/src/lib/trace-id.ts +++ b/src/lib/trace-id.ts @@ -2,10 +2,10 @@ * Trace ID Validation * * Re-exports shared hex ID validation specialized for trace IDs. - * Used by `trace logs` and `log list --trace` commands. + * Used by `trace logs`, `log list`, and `span list` commands. */ -import { HEX_ID_RE, validateHexId } from "./hex-id.js"; +import { HEX_ID_RE, UUID_DASH_RE, validateHexId } from "./hex-id.js"; /** * Regex for a valid 32-character hexadecimal trace ID. @@ -13,6 +13,25 @@ import { HEX_ID_RE, validateHexId } from "./hex-id.js"; */ export const TRACE_ID_RE = HEX_ID_RE; +/** + * Non-throwing check: does the string look like a valid 32-char hex trace ID? + * + * Handles UUID-dash format (8-4-4-4-12) and whitespace trimming, matching + * the same normalization as {@link validateTraceId}. Use this when you need + * to disambiguate between a trace ID and another kind of identifier (e.g., + * a project slug) without throwing. + * + * @param value - The string to test + * @returns `true` if the value would pass {@link validateTraceId} + */ +export function isTraceId(value: string): boolean { + let trimmed = value.trim().toLowerCase(); + if (UUID_DASH_RE.test(trimmed)) { + trimmed = trimmed.replace(/-/g, ""); + } + return HEX_ID_RE.test(trimmed); +} + /** * Validate that a string looks like a 32-character hex trace ID. * diff --git a/test/commands/log/list.test.ts b/test/commands/log/list.test.ts index 7038bf5a9..ce2603eeb 100644 --- a/test/commands/log/list.test.ts +++ b/test/commands/log/list.test.ts @@ -2,15 +2,20 @@ * Log List Command Tests * * Tests for the `sentry log list` command func() body, covering: - * - Standard project-scoped mode (no --trace) - * - Trace-filtered mode (--trace ) - * - Org resolution failure in trace mode + * - Standard project-scoped mode (positional org/project) + * - Trace-filtered mode (positional 32-char hex trace-id) + * - Positional argument disambiguation (trace vs project) + * - Period flag behavior * - Follow/streaming mode for both standard and trace modes * * Uses spyOn mocking to avoid real HTTP calls or database access. * Follow-mode tests use SIGINT to cleanly stop the setTimeout-based * poll loop — the promise resolves on SIGINT (normal termination). * AuthError tests verify that fetch failures reject the promise. + * + * Non-follow (single-fetch) tests mock `withProgress` from `polling.ts` + * to bypass the spinner. Follow-mode tests do NOT mock `withProgress` + * because follow mode uses its own streaming banner, not the spinner. */ import { @@ -29,8 +34,12 @@ import { AuthError, ContextError } from "../../../src/lib/errors.js"; // biome-ignore lint/performance/noNamespaceImport: needed for spyOn mocking import * as formatters from "../../../src/lib/formatters/index.js"; // biome-ignore lint/performance/noNamespaceImport: needed for spyOn mocking +import * as polling from "../../../src/lib/polling.js"; +// biome-ignore lint/performance/noNamespaceImport: needed for spyOn mocking import * as resolveTarget from "../../../src/lib/resolve-target.js"; // biome-ignore lint/performance/noNamespaceImport: needed for spyOn mocking +import * as traceTarget from "../../../src/lib/trace-target.js"; +// biome-ignore lint/performance/noNamespaceImport: needed for spyOn mocking import * as versionCheck from "../../../src/lib/version-check.js"; import type { SentryLog, TraceLog } from "../../../src/types/sentry.js"; @@ -112,6 +121,33 @@ function createMockContext() { }; } +/** No-op setMessage callback for withProgress mock */ +function noop() { + // no-op for test +} + +/** Passthrough mock for `withProgress` — bypasses spinner, calls fn directly */ +function mockWithProgress( + _opts: unknown, + fn: (setMessage: () => void) => unknown +) { + return fn(noop); +} + +/** Standard flags for non-follow batch mode */ +const BATCH_FLAGS = { + json: true, + limit: 100, + period: "90d", +} as const; + +/** Human-mode flags for non-follow batch mode */ +const HUMAN_FLAGS = { + json: false, + limit: 100, + period: "90d", +} as const; + /** Sample project-scoped logs (SentryLog) */ const sampleLogs: SentryLog[] = [ { @@ -201,35 +237,42 @@ const newerLogs: SentryLog[] = [ ]; // ============================================================================ -// Standard mode (no --trace) +// Standard mode (project-scoped, no trace-id positional) // ============================================================================ describe("listCommand.func — standard mode", () => { let listLogsSpy: ReturnType; let resolveOrgProjectSpy: ReturnType; + let withProgressSpy: ReturnType; beforeEach(() => { listLogsSpy = spyOn(apiClient, "listLogs"); resolveOrgProjectSpy = spyOn(resolveTarget, "resolveOrgProjectFromArg"); + withProgressSpy = spyOn(polling, "withProgress").mockImplementation( + mockWithProgress + ); }); afterEach(() => { listLogsSpy.mockRestore(); resolveOrgProjectSpy.mockRestore(); + withProgressSpy.mockRestore(); }); - test("outputs JSON array for --json", async () => { + test("outputs JSON envelope with data and hasMore for --json", async () => { listLogsSpy.mockResolvedValue(sampleLogs); resolveOrgProjectSpy.mockResolvedValue({ org: ORG, project: PROJECT }); const { context, stdoutWrite } = createMockContext(); const func = await listCommand.loader(); - await func.call(context, { json: true, limit: 100 }, `${ORG}/${PROJECT}`); + await func.call(context, BATCH_FLAGS, `${ORG}/${PROJECT}`); const output = stdoutWrite.mock.calls.map((c) => c[0]).join(""); const parsed = JSON.parse(output); - expect(Array.isArray(parsed)).toBe(true); - expect(parsed).toHaveLength(3); + expect(parsed).toHaveProperty("data"); + expect(parsed).toHaveProperty("hasMore"); + expect(Array.isArray(parsed.data)).toBe(true); + expect(parsed.data).toHaveLength(3); }); test("outputs JSON in chronological order (oldest first)", async () => { @@ -240,13 +283,13 @@ describe("listCommand.func — standard mode", () => { const { context, stdoutWrite } = createMockContext(); const func = await listCommand.loader(); - await func.call(context, { json: true, limit: 100 }, `${ORG}/${PROJECT}`); + await func.call(context, BATCH_FLAGS, `${ORG}/${PROJECT}`); const output = stdoutWrite.mock.calls.map((c) => c[0]).join(""); const parsed = JSON.parse(output); // After reversal, oldest should be first - expect(parsed[0]["sentry.item_id"]).toBe("item001"); - expect(parsed[2]["sentry.item_id"]).toBe("item003"); + expect(parsed.data[0]["sentry.item_id"]).toBe("item001"); + expect(parsed.data[2]["sentry.item_id"]).toBe("item003"); }); test("shows 'No logs found' for empty result (human mode)", async () => { @@ -255,7 +298,7 @@ describe("listCommand.func — standard mode", () => { const { context, stdoutWrite } = createMockContext(); const func = await listCommand.loader(); - await func.call(context, { json: false, limit: 100 }, `${ORG}/${PROJECT}`); + await func.call(context, HUMAN_FLAGS, `${ORG}/${PROJECT}`); const output = stdoutWrite.mock.calls.map((c) => c[0]).join(""); expect(output).toContain("No logs found"); @@ -267,7 +310,7 @@ describe("listCommand.func — standard mode", () => { const { context, stdoutWrite } = createMockContext(); const func = await listCommand.loader(); - await func.call(context, { json: false, limit: 100 }, `${ORG}/${PROJECT}`); + await func.call(context, HUMAN_FLAGS, `${ORG}/${PROJECT}`); const output = stdoutWrite.mock.calls.map((c) => c[0]).join(""); expect(output).toContain("Request received"); @@ -281,7 +324,7 @@ describe("listCommand.func — standard mode", () => { const { context, stdoutWrite } = createMockContext(); const func = await listCommand.loader(); - await func.call(context, { json: false, limit: 100 }, `${ORG}/${PROJECT}`); + await func.call(context, HUMAN_FLAGS, `${ORG}/${PROJECT}`); const output = stdoutWrite.mock.calls.map((c) => c[0]).join(""); expect(output).toContain("Showing 3 log"); @@ -294,7 +337,11 @@ describe("listCommand.func — standard mode", () => { const { context, stdoutWrite } = createMockContext(); const func = await listCommand.loader(); // limit equals number of returned logs → hasMore = true - await func.call(context, { json: false, limit: 3 }, `${ORG}/${PROJECT}`); + await func.call( + context, + { json: false, limit: 3, period: "90d" }, + `${ORG}/${PROJECT}` + ); const output = stdoutWrite.mock.calls.map((c) => c[0]).join(""); expect(output).toContain("Use --limit to show more"); @@ -306,13 +353,13 @@ describe("listCommand.func — standard mode", () => { const { context, stdoutWrite } = createMockContext(); const func = await listCommand.loader(); - await func.call(context, { json: false, limit: 100 }, `${ORG}/${PROJECT}`); + await func.call(context, HUMAN_FLAGS, `${ORG}/${PROJECT}`); const output = stdoutWrite.mock.calls.map((c) => c[0]).join(""); expect(output).not.toContain("Use --limit to show more"); }); - test("passes query and limit to listLogs", async () => { + test("passes query, limit, and period to listLogs", async () => { listLogsSpy.mockResolvedValue([]); resolveOrgProjectSpy.mockResolvedValue({ org: ORG, project: PROJECT }); @@ -320,7 +367,7 @@ describe("listCommand.func — standard mode", () => { const func = await listCommand.loader(); await func.call( context, - { json: false, limit: 50, query: "level:error" }, + { json: false, limit: 50, query: "level:error", period: "90d" }, `${ORG}/${PROJECT}` ); @@ -337,66 +384,109 @@ describe("listCommand.func — standard mode", () => { const { context } = createMockContext(); const func = await listCommand.loader(); - await func.call(context, { json: true, limit: 100 }, `${ORG}/${PROJECT}`); + await func.call(context, BATCH_FLAGS, `${ORG}/${PROJECT}`); expect(context.setContext).toHaveBeenCalledWith([ORG], [PROJECT]); }); + + test("hasMore is true when results match limit", async () => { + listLogsSpy.mockResolvedValue(sampleLogs); + resolveOrgProjectSpy.mockResolvedValue({ org: ORG, project: PROJECT }); + + const { context, stdoutWrite } = createMockContext(); + const func = await listCommand.loader(); + await func.call( + context, + { json: true, limit: 3, period: "90d" }, + `${ORG}/${PROJECT}` + ); + + const output = stdoutWrite.mock.calls.map((c) => c[0]).join(""); + const parsed = JSON.parse(output); + expect(parsed.hasMore).toBe(true); + }); + + test("hasMore is false when fewer results than limit", async () => { + listLogsSpy.mockResolvedValue(sampleLogs); + resolveOrgProjectSpy.mockResolvedValue({ org: ORG, project: PROJECT }); + + const { context, stdoutWrite } = createMockContext(); + const func = await listCommand.loader(); + await func.call(context, BATCH_FLAGS, `${ORG}/${PROJECT}`); + + const output = stdoutWrite.mock.calls.map((c) => c[0]).join(""); + const parsed = JSON.parse(output); + expect(parsed.hasMore).toBe(false); + }); }); // ============================================================================ -// Trace mode (--trace) +// Trace mode (positional trace-id) // ============================================================================ describe("listCommand.func — trace mode", () => { let listTraceLogsSpy: ReturnType; - let resolveOrgSpy: ReturnType; + let resolveTraceOrgSpy: ReturnType; + let warnIfNormalizedSpy: ReturnType; + let withProgressSpy: ReturnType; beforeEach(() => { listTraceLogsSpy = spyOn(apiClient, "listTraceLogs"); - resolveOrgSpy = spyOn(resolveTarget, "resolveOrg"); + resolveTraceOrgSpy = spyOn(traceTarget, "resolveTraceOrg"); + warnIfNormalizedSpy = spyOn( + traceTarget, + "warnIfNormalized" + ).mockReturnValue(undefined); + withProgressSpy = spyOn(polling, "withProgress").mockImplementation( + mockWithProgress + ); }); afterEach(() => { listTraceLogsSpy.mockRestore(); - resolveOrgSpy.mockRestore(); + resolveTraceOrgSpy.mockRestore(); + warnIfNormalizedSpy.mockRestore(); + withProgressSpy.mockRestore(); }); - test("outputs JSON array for --json --trace", async () => { + test("outputs JSON envelope with data and hasMore for --json", async () => { listTraceLogsSpy.mockResolvedValue(sampleTraceLogs); - resolveOrgSpy.mockResolvedValue({ org: ORG }); + resolveTraceOrgSpy.mockResolvedValue({ traceId: TRACE_ID, org: ORG }); const { context, stdoutWrite } = createMockContext(); const func = await listCommand.loader(); - await func.call(context, { json: true, limit: 100, trace: TRACE_ID }); + await func.call(context, BATCH_FLAGS, TRACE_ID); const output = stdoutWrite.mock.calls.map((c) => c[0]).join(""); const parsed = JSON.parse(output); - expect(Array.isArray(parsed)).toBe(true); - expect(parsed).toHaveLength(3); + expect(parsed).toHaveProperty("data"); + expect(parsed).toHaveProperty("hasMore"); + expect(Array.isArray(parsed.data)).toBe(true); + expect(parsed.data).toHaveLength(3); }); test("outputs JSON in chronological order (oldest first)", async () => { const newestFirst = [...sampleTraceLogs].reverse(); listTraceLogsSpy.mockResolvedValue(newestFirst); - resolveOrgSpy.mockResolvedValue({ org: ORG }); + resolveTraceOrgSpy.mockResolvedValue({ traceId: TRACE_ID, org: ORG }); const { context, stdoutWrite } = createMockContext(); const func = await listCommand.loader(); - await func.call(context, { json: true, limit: 100, trace: TRACE_ID }); + await func.call(context, BATCH_FLAGS, TRACE_ID); const output = stdoutWrite.mock.calls.map((c) => c[0]).join(""); const parsed = JSON.parse(output); - expect(parsed[0].id).toBe("log001"); - expect(parsed[2].id).toBe("log003"); + expect(parsed.data[0].id).toBe("log001"); + expect(parsed.data[2].id).toBe("log003"); }); test("shows empty-trace message in human mode", async () => { listTraceLogsSpy.mockResolvedValue([]); - resolveOrgSpy.mockResolvedValue({ org: ORG }); + resolveTraceOrgSpy.mockResolvedValue({ traceId: TRACE_ID, org: ORG }); const { context, stdoutWrite } = createMockContext(); const func = await listCommand.loader(); - await func.call(context, { json: false, limit: 100, trace: TRACE_ID }); + await func.call(context, HUMAN_FLAGS, TRACE_ID); const output = stdoutWrite.mock.calls.map((c) => c[0]).join(""); expect(output).toContain("No logs found"); @@ -405,11 +495,11 @@ describe("listCommand.func — trace mode", () => { test("renders trace log messages in human output", async () => { listTraceLogsSpy.mockResolvedValue(sampleTraceLogs); - resolveOrgSpy.mockResolvedValue({ org: ORG }); + resolveTraceOrgSpy.mockResolvedValue({ traceId: TRACE_ID, org: ORG }); const { context, stdoutWrite } = createMockContext(); const func = await listCommand.loader(); - await func.call(context, { json: false, limit: 100, trace: TRACE_ID }); + await func.call(context, HUMAN_FLAGS, TRACE_ID); const output = stdoutWrite.mock.calls.map((c) => c[0]).join(""); expect(output).toContain("Request received"); @@ -419,11 +509,11 @@ describe("listCommand.func — trace mode", () => { test("shows count footer with trace ID", async () => { listTraceLogsSpy.mockResolvedValue(sampleTraceLogs); - resolveOrgSpy.mockResolvedValue({ org: ORG }); + resolveTraceOrgSpy.mockResolvedValue({ traceId: TRACE_ID, org: ORG }); const { context, stdoutWrite } = createMockContext(); const func = await listCommand.loader(); - await func.call(context, { json: false, limit: 100, trace: TRACE_ID }); + await func.call(context, HUMAN_FLAGS, TRACE_ID); const output = stdoutWrite.mock.calls.map((c) => c[0]).join(""); expect(output).toContain("Showing 3 log"); @@ -432,28 +522,31 @@ describe("listCommand.func — trace mode", () => { test("shows --limit tip when trace results match limit", async () => { listTraceLogsSpy.mockResolvedValue(sampleTraceLogs); - resolveOrgSpy.mockResolvedValue({ org: ORG }); + resolveTraceOrgSpy.mockResolvedValue({ traceId: TRACE_ID, org: ORG }); const { context, stdoutWrite } = createMockContext(); const func = await listCommand.loader(); - await func.call(context, { json: false, limit: 3, trace: TRACE_ID }); + await func.call( + context, + { json: false, limit: 3, period: "90d" }, + TRACE_ID + ); const output = stdoutWrite.mock.calls.map((c) => c[0]).join(""); expect(output).toContain("Use --limit to show more."); }); - test("passes traceId, limit, and query to listTraceLogs", async () => { + test("passes traceId, limit, and query to listTraceLogs with 14d default", async () => { listTraceLogsSpy.mockResolvedValue([]); - resolveOrgSpy.mockResolvedValue({ org: ORG }); + resolveTraceOrgSpy.mockResolvedValue({ traceId: TRACE_ID, org: ORG }); const { context } = createMockContext(); const func = await listCommand.loader(); - await func.call(context, { - json: false, - limit: 50, - trace: TRACE_ID, - query: "level:error", - }); + await func.call( + context, + { json: false, limit: 50, query: "level:error", period: "90d" }, + TRACE_ID + ); expect(listTraceLogsSpy).toHaveBeenCalledWith(ORG, TRACE_ID, { query: "level:error", @@ -464,72 +557,234 @@ describe("listCommand.func — trace mode", () => { test("calls setContext with org and empty project array", async () => { listTraceLogsSpy.mockResolvedValue([]); - resolveOrgSpy.mockResolvedValue({ org: ORG }); + resolveTraceOrgSpy.mockResolvedValue({ traceId: TRACE_ID, org: ORG }); const { context } = createMockContext(); const func = await listCommand.loader(); - await func.call(context, { json: true, limit: 100, trace: TRACE_ID }); + await func.call(context, BATCH_FLAGS, TRACE_ID); expect(context.setContext).toHaveBeenCalledWith([ORG], []); }); - test("uses positional target as org slug in trace mode", async () => { + test("uses positional org/trace-id to resolve trace org", async () => { listTraceLogsSpy.mockResolvedValue([]); - resolveOrgSpy.mockResolvedValue({ org: "my-org" }); + resolveTraceOrgSpy.mockResolvedValue({ traceId: TRACE_ID, org: "my-org" }); + + const { context } = createMockContext(); + const func = await listCommand.loader(); + await func.call(context, BATCH_FLAGS, `my-org/${TRACE_ID}`); + + // resolveTraceOrg receives the parsed ParsedTraceTarget + expect(resolveTraceOrgSpy).toHaveBeenCalledWith( + expect.objectContaining({ + type: "org-scoped", + org: "my-org", + traceId: TRACE_ID, + }), + "/tmp", + expect.any(String) + ); + }); +}); + +// ============================================================================ +// Positional argument disambiguation +// ============================================================================ + +describe("listCommand.func — positional disambiguation", () => { + let listLogsSpy: ReturnType; + let listTraceLogsSpy: ReturnType; + let resolveOrgProjectSpy: ReturnType; + let resolveTraceOrgSpy: ReturnType; + let warnIfNormalizedSpy: ReturnType; + let withProgressSpy: ReturnType; + + beforeEach(() => { + listLogsSpy = spyOn(apiClient, "listLogs").mockResolvedValue([]); + listTraceLogsSpy = spyOn(apiClient, "listTraceLogs").mockResolvedValue([]); + resolveOrgProjectSpy = spyOn( + resolveTarget, + "resolveOrgProjectFromArg" + ).mockResolvedValue({ org: ORG, project: PROJECT }); + resolveTraceOrgSpy = spyOn( + traceTarget, + "resolveTraceOrg" + ).mockResolvedValue({ + traceId: TRACE_ID, + org: ORG, + }); + warnIfNormalizedSpy = spyOn( + traceTarget, + "warnIfNormalized" + ).mockReturnValue(undefined); + withProgressSpy = spyOn(polling, "withProgress").mockImplementation( + mockWithProgress + ); + }); + afterEach(() => { + listLogsSpy.mockRestore(); + listTraceLogsSpy.mockRestore(); + resolveOrgProjectSpy.mockRestore(); + resolveTraceOrgSpy.mockRestore(); + warnIfNormalizedSpy.mockRestore(); + withProgressSpy.mockRestore(); + }); + + test("32-char hex string triggers trace mode", async () => { + const { context } = createMockContext(); + const func = await listCommand.loader(); + await func.call(context, BATCH_FLAGS, TRACE_ID); + + expect(listTraceLogsSpy).toHaveBeenCalled(); + expect(listLogsSpy).not.toHaveBeenCalled(); + }); + + test("non-hex string triggers project mode", async () => { + const { context } = createMockContext(); + const func = await listCommand.loader(); + await func.call(context, BATCH_FLAGS, `${ORG}/${PROJECT}`); + + expect(listLogsSpy).toHaveBeenCalled(); + expect(listTraceLogsSpy).not.toHaveBeenCalled(); + }); + + test("org/trace-id triggers trace mode", async () => { + const { context } = createMockContext(); + const func = await listCommand.loader(); + await func.call(context, BATCH_FLAGS, `${ORG}/${TRACE_ID}`); + + expect(listTraceLogsSpy).toHaveBeenCalled(); + expect(listLogsSpy).not.toHaveBeenCalled(); + }); + + test("org/project (non-hex) triggers project mode", async () => { + const { context } = createMockContext(); + const func = await listCommand.loader(); + await func.call(context, BATCH_FLAGS, "my-org/my-project"); + + expect(listLogsSpy).toHaveBeenCalled(); + expect(listTraceLogsSpy).not.toHaveBeenCalled(); + }); +}); + +// ============================================================================ +// Period flag behavior +// ============================================================================ + +describe("listCommand.func — period flag", () => { + let listTraceLogsSpy: ReturnType; + let resolveTraceOrgSpy: ReturnType; + let warnIfNormalizedSpy: ReturnType; + let withProgressSpy: ReturnType; + + beforeEach(() => { + listTraceLogsSpy = spyOn(apiClient, "listTraceLogs").mockResolvedValue([]); + resolveTraceOrgSpy = spyOn( + traceTarget, + "resolveTraceOrg" + ).mockResolvedValue({ + traceId: TRACE_ID, + org: ORG, + }); + warnIfNormalizedSpy = spyOn( + traceTarget, + "warnIfNormalized" + ).mockReturnValue(undefined); + withProgressSpy = spyOn(polling, "withProgress").mockImplementation( + mockWithProgress + ); + }); + + afterEach(() => { + listTraceLogsSpy.mockRestore(); + resolveTraceOrgSpy.mockRestore(); + warnIfNormalizedSpy.mockRestore(); + withProgressSpy.mockRestore(); + }); + + test("trace mode uses 14d default when period is the command default 90d", async () => { const { context } = createMockContext(); const func = await listCommand.loader(); await func.call( context, - { json: true, limit: 100, trace: TRACE_ID }, - "my-org" + { json: true, limit: 100, period: "90d" }, + TRACE_ID ); - expect(resolveOrgSpy).toHaveBeenCalledWith({ - org: "my-org", - cwd: "/tmp", + expect(listTraceLogsSpy).toHaveBeenCalledWith(ORG, TRACE_ID, { + query: undefined, + limit: 100, + statsPeriod: "14d", + }); + }); + + test("trace mode uses explicit period when set to non-default", async () => { + const { context } = createMockContext(); + const func = await listCommand.loader(); + await func.call( + context, + { json: true, limit: 100, period: "30d" }, + TRACE_ID + ); + + expect(listTraceLogsSpy).toHaveBeenCalledWith(ORG, TRACE_ID, { + query: undefined, + limit: 100, + statsPeriod: "30d", }); }); }); // ============================================================================ -// Org resolution failure in trace mode +// Trace mode org resolution failure // ============================================================================ describe("listCommand.func — trace mode org resolution failure", () => { - let resolveOrgSpy: ReturnType; + let resolveTraceOrgSpy: ReturnType; + let warnIfNormalizedSpy: ReturnType; + let withProgressSpy: ReturnType; beforeEach(() => { - resolveOrgSpy = spyOn(resolveTarget, "resolveOrg"); + resolveTraceOrgSpy = spyOn(traceTarget, "resolveTraceOrg"); + warnIfNormalizedSpy = spyOn( + traceTarget, + "warnIfNormalized" + ).mockReturnValue(undefined); + withProgressSpy = spyOn(polling, "withProgress").mockImplementation( + mockWithProgress + ); }); afterEach(() => { - resolveOrgSpy.mockRestore(); + resolveTraceOrgSpy.mockRestore(); + warnIfNormalizedSpy.mockRestore(); + withProgressSpy.mockRestore(); }); test("throws ContextError when org cannot be resolved", async () => { - resolveOrgSpy.mockResolvedValue(null); + resolveTraceOrgSpy.mockRejectedValue( + new ContextError("Organization", "sentry log list [/]") + ); const { context } = createMockContext(); const func = await listCommand.loader(); - await expect( - func.call(context, { json: false, limit: 100, trace: TRACE_ID }) - ).rejects.toThrow(ContextError); + await expect(func.call(context, HUMAN_FLAGS, TRACE_ID)).rejects.toThrow( + ContextError + ); }); test("ContextError mentions Organization", async () => { - resolveOrgSpy.mockResolvedValue(null); + resolveTraceOrgSpy.mockRejectedValue( + new ContextError("Organization", "sentry log list [/]") + ); const { context } = createMockContext(); const func = await listCommand.loader(); try { - await func.call(context, { - json: false, - limit: 100, - trace: TRACE_ID, - }); + await func.call(context, HUMAN_FLAGS, TRACE_ID); expect.unreachable("Should have thrown"); } catch (error) { expect(error).toBeInstanceOf(ContextError); @@ -539,12 +794,15 @@ describe("listCommand.func — trace mode org resolution failure", () => { }); // ============================================================================ -// Follow mode — standard (--follow, no --trace) +// Follow mode — standard (--follow, project-scoped) // // Strategy: SIGINT resolves the promise (normal termination). AuthError // from fetch rejects the promise. Tests use interceptSigint() to capture // the SIGINT handler and invoke it directly (process.emit("SIGINT") // kills the Bun test runner). +// +// Follow mode does NOT use withProgress (it has its own streaming banner), +// so withProgress is NOT mocked here. // ============================================================================ /** @@ -602,6 +860,7 @@ describe("listCommand.func — follow mode (standard)", () => { json: false, limit: 100, follow: 1, + period: "90d", } as const; test("writes initial logs then resolves on SIGINT", async () => { @@ -851,12 +1110,13 @@ describe("listCommand.func — follow mode (standard)", () => { }); // ============================================================================ -// Follow mode — trace (--follow + --trace) +// Follow mode — trace (--follow + positional trace-id) // ============================================================================ describe("listCommand.func — follow mode (trace)", () => { let listTraceLogsSpy: ReturnType; - let resolveOrgSpy: ReturnType; + let resolveTraceOrgSpy: ReturnType; + let warnIfNormalizedSpy: ReturnType; let isPlainSpy: ReturnType; let updateNotifSpy: ReturnType; let sigint: ReturnType; @@ -865,7 +1125,11 @@ describe("listCommand.func — follow mode (trace)", () => { beforeEach(() => { sigint = interceptSigint(); listTraceLogsSpy = spyOn(apiClient, "listTraceLogs"); - resolveOrgSpy = spyOn(resolveTarget, "resolveOrg"); + resolveTraceOrgSpy = spyOn(traceTarget, "resolveTraceOrg"); + warnIfNormalizedSpy = spyOn( + traceTarget, + "warnIfNormalized" + ).mockReturnValue(undefined); isPlainSpy = spyOn(formatters, "isPlainOutput").mockReturnValue(true); updateNotifSpy = spyOn( versionCheck, @@ -876,7 +1140,8 @@ describe("listCommand.func — follow mode (trace)", () => { afterEach(() => { listTraceLogsSpy.mockRestore(); - resolveOrgSpy.mockRestore(); + resolveTraceOrgSpy.mockRestore(); + warnIfNormalizedSpy.mockRestore(); isPlainSpy.mockRestore(); updateNotifSpy.mockRestore(); stderrSpy.mockRestore(); @@ -887,17 +1152,17 @@ describe("listCommand.func — follow mode (trace)", () => { json: false, limit: 100, follow: 1, - trace: TRACE_ID, + period: "90d", } as const; test("writes initial trace logs then resolves on SIGINT", async () => { listTraceLogsSpy.mockResolvedValueOnce(sampleTraceLogs); - resolveOrgSpy.mockResolvedValue({ org: ORG }); + resolveTraceOrgSpy.mockResolvedValue({ traceId: TRACE_ID, org: ORG }); const { context, stdoutWrite } = createMockContext(); const func = await listCommand.loader(); - const promise = func.call(context, traceFollowFlags); + const promise = func.call(context, traceFollowFlags, TRACE_ID); await Bun.sleep(50); sigint.trigger(); await promise; @@ -908,12 +1173,12 @@ describe("listCommand.func — follow mode (trace)", () => { test("writes stderr banner with trace ID in follow mode", async () => { listTraceLogsSpy.mockResolvedValueOnce([]); - resolveOrgSpy.mockResolvedValue({ org: ORG }); + resolveTraceOrgSpy.mockResolvedValue({ traceId: TRACE_ID, org: ORG }); const { context } = createMockContext(); const func = await listCommand.loader(); - const promise = func.call(context, traceFollowFlags); + const promise = func.call(context, traceFollowFlags, TRACE_ID); await Bun.sleep(50); sigint.trigger(); await promise; @@ -934,12 +1199,12 @@ describe("listCommand.func — follow mode (trace)", () => { listTraceLogsSpy .mockResolvedValueOnce(sampleTraceLogs) .mockResolvedValueOnce(mixedLogs); - resolveOrgSpy.mockResolvedValue({ org: ORG }); + resolveTraceOrgSpy.mockResolvedValue({ traceId: TRACE_ID, org: ORG }); const { context, stdoutWrite } = createMockContext(); const func = await listCommand.loader(); - const promise = func.call(context, traceFollowFlags); + const promise = func.call(context, traceFollowFlags, TRACE_ID); // Wait for initial fetch + poll timer (1s) + poll execution await Bun.sleep(1200); sigint.trigger(); @@ -956,12 +1221,16 @@ describe("listCommand.func — follow mode (trace)", () => { listTraceLogsSpy .mockResolvedValueOnce(sampleTraceLogs) .mockResolvedValueOnce(newerTraceLogs); - resolveOrgSpy.mockResolvedValue({ org: ORG }); + resolveTraceOrgSpy.mockResolvedValue({ traceId: TRACE_ID, org: ORG }); const { context, stdoutWrite } = createMockContext(); const func = await listCommand.loader(); - const promise = func.call(context, { ...traceFollowFlags, json: true }); + const promise = func.call( + context, + { ...traceFollowFlags, json: true }, + TRACE_ID + ); // Wait for initial fetch + poll timer (1s) + poll execution await Bun.sleep(1200); sigint.trigger(); @@ -976,16 +1245,16 @@ describe("listCommand.func — follow mode (trace)", () => { return false; } }); - // First batch: 1 JSON line (array of 3 items from LogListResult) + // First batch: 1 JSON line (envelope with data array from LogListResult) // Poll batch: 1 JSON line per item (bare JSONL) expect(jsonLines.length).toBe(2); - // First line is an array (the initial trace batch) + // First line is an envelope with data array (the initial trace batch) const firstBatch = JSON.parse(jsonLines[0]); - expect(Array.isArray(firstBatch)).toBe(true); - expect(firstBatch).toHaveLength(3); + expect(firstBatch).toHaveProperty("data"); + expect(Array.isArray(firstBatch.data)).toBe(true); + expect(firstBatch.data).toHaveLength(3); // Second line is a bare object (polled item) const pollItem = JSON.parse(jsonLines[1]); - expect(Array.isArray(pollItem)).toBe(false); expect(pollItem.message).toBe("New poll result"); }); @@ -993,26 +1262,26 @@ describe("listCommand.func — follow mode (trace)", () => { listTraceLogsSpy .mockResolvedValueOnce([]) .mockRejectedValueOnce(new AuthError("expired")); - resolveOrgSpy.mockResolvedValue({ org: ORG }); + resolveTraceOrgSpy.mockResolvedValue({ traceId: TRACE_ID, org: ORG }); const { context } = createMockContext(); const func = await listCommand.loader(); - await expect(func.call(context, traceFollowFlags)).rejects.toThrow( - AuthError - ); + await expect( + func.call(context, traceFollowFlags, TRACE_ID) + ).rejects.toThrow(AuthError); }); test("continues polling after transient error (trace mode)", async () => { listTraceLogsSpy .mockResolvedValueOnce([]) .mockRejectedValueOnce(new Error("server error")); - resolveOrgSpy.mockResolvedValue({ org: ORG }); + resolveTraceOrgSpy.mockResolvedValue({ traceId: TRACE_ID, org: ORG }); const { context } = createMockContext(); const func = await listCommand.loader(); - const promise = func.call(context, traceFollowFlags); + const promise = func.call(context, traceFollowFlags, TRACE_ID); // Wait for initial fetch + poll timer (1s) + poll execution await Bun.sleep(1200); sigint.trigger(); @@ -1026,12 +1295,12 @@ describe("listCommand.func — follow mode (trace)", () => { test("uses 1m statsPeriod for initial trace follow fetch", async () => { listTraceLogsSpy.mockResolvedValueOnce([]); - resolveOrgSpy.mockResolvedValue({ org: ORG }); + resolveTraceOrgSpy.mockResolvedValue({ traceId: TRACE_ID, org: ORG }); const { context } = createMockContext(); const func = await listCommand.loader(); - const promise = func.call(context, traceFollowFlags); + const promise = func.call(context, traceFollowFlags, TRACE_ID); await Bun.sleep(50); sigint.trigger(); await promise; @@ -1047,12 +1316,12 @@ describe("listCommand.func — follow mode (trace)", () => { listTraceLogsSpy .mockResolvedValueOnce(sampleTraceLogs) .mockResolvedValueOnce([]); - resolveOrgSpy.mockResolvedValue({ org: ORG }); + resolveTraceOrgSpy.mockResolvedValue({ traceId: TRACE_ID, org: ORG }); const { context } = createMockContext(); const func = await listCommand.loader(); - const promise = func.call(context, traceFollowFlags); + const promise = func.call(context, traceFollowFlags, TRACE_ID); // Wait for initial fetch + poll timer (1s) + poll execution await Bun.sleep(1200); sigint.trigger(); @@ -1071,12 +1340,12 @@ describe("listCommand.func — follow mode (trace)", () => { listTraceLogsSpy .mockResolvedValueOnce([]) .mockResolvedValueOnce(newerTraceLogs); - resolveOrgSpy.mockResolvedValue({ org: ORG }); + resolveTraceOrgSpy.mockResolvedValue({ traceId: TRACE_ID, org: ORG }); const { context, stdoutWrite } = createMockContext(); const func = await listCommand.loader(); - const promise = func.call(context, traceFollowFlags); + const promise = func.call(context, traceFollowFlags, TRACE_ID); // Wait for initial fetch + poll timer (1s) + poll execution await Bun.sleep(1200); sigint.trigger(); diff --git a/test/commands/trace/logs.test.ts b/test/commands/trace/logs.test.ts index 31a54520b..d39146996 100644 --- a/test/commands/trace/logs.test.ts +++ b/test/commands/trace/logs.test.ts @@ -25,6 +25,8 @@ import { logsCommand } from "../../../src/commands/trace/logs.js"; import * as apiClient from "../../../src/lib/api-client.js"; import { ContextError } from "../../../src/lib/errors.js"; // biome-ignore lint/performance/noNamespaceImport: needed for spyOn mocking +import * as polling from "../../../src/lib/polling.js"; +// biome-ignore lint/performance/noNamespaceImport: needed for spyOn mocking import * as resolveTarget from "../../../src/lib/resolve-target.js"; import type { TraceLog } from "../../../src/types/sentry.js"; @@ -103,19 +105,28 @@ function collectMockOutput( describe("logsCommand.func", () => { let listTraceLogsSpy: ReturnType; let resolveOrgSpy: ReturnType; + let withProgressSpy: ReturnType; beforeEach(() => { listTraceLogsSpy = spyOn(apiClient, "listTraceLogs"); resolveOrgSpy = spyOn(resolveTarget, "resolveOrg"); + // Bypass the withProgress spinner to prevent real stderr timers + withProgressSpy = spyOn(polling, "withProgress").mockImplementation( + (_opts, fn) => + fn(() => { + /* no-op setMessage */ + }) + ); }); afterEach(() => { listTraceLogsSpy.mockRestore(); resolveOrgSpy.mockRestore(); + withProgressSpy.mockRestore(); }); describe("JSON output mode", () => { - test("outputs JSON array when --json flag is set", async () => { + test("outputs JSON envelope when --json flag is set", async () => { listTraceLogsSpy.mockResolvedValue(sampleLogs); resolveOrgSpy.mockResolvedValue({ org: ORG }); @@ -129,13 +140,16 @@ describe("logsCommand.func", () => { const output = collectMockOutput(stdoutWrite); const parsed = JSON.parse(output); - expect(Array.isArray(parsed)).toBe(true); - expect(parsed).toHaveLength(3); + expect(parsed).toHaveProperty("data"); + expect(parsed).toHaveProperty("hasMore"); + expect(Array.isArray(parsed.data)).toBe(true); + expect(parsed.data).toHaveLength(3); // formatTraceLogs reverses to chronological order for JSON output - expect(parsed[0].id).toBe("log003"); + expect(parsed.data[0].id).toBe("log003"); + expect(parsed.hasMore).toBe(false); }); - test("outputs empty JSON array when no logs found with --json", async () => { + test("outputs empty JSON envelope when no logs found with --json", async () => { listTraceLogsSpy.mockResolvedValue([]); resolveOrgSpy.mockResolvedValue({ org: ORG }); @@ -148,7 +162,8 @@ describe("logsCommand.func", () => { ); const output = collectMockOutput(stdoutWrite); - expect(JSON.parse(output)).toEqual([]); + const parsed = JSON.parse(output); + expect(parsed).toEqual({ data: [], hasMore: false }); }); }); diff --git a/test/lib/org-list.test.ts b/test/lib/org-list.test.ts index abd43a48e..08f24e518 100644 --- a/test/lib/org-list.test.ts +++ b/test/lib/org-list.test.ts @@ -40,8 +40,29 @@ import { type OrgListConfig, } from "../../src/lib/org-list.js"; // biome-ignore lint/performance/noNamespaceImport: needed for spyOn mocking +import * as polling from "../../src/lib/polling.js"; +// biome-ignore lint/performance/noNamespaceImport: needed for spyOn mocking +import * as region from "../../src/lib/region.js"; +// biome-ignore lint/performance/noNamespaceImport: needed for spyOn mocking import * as resolveTarget from "../../src/lib/resolve-target.js"; +/** + * Bypass the withProgress spinner in all tests — prevents real stderr + * timers from piling up during full-suite runs and causing 5s timeouts. + */ +let withProgressSpy: ReturnType; +beforeEach(() => { + withProgressSpy = spyOn(polling, "withProgress").mockImplementation( + (_opts, fn) => + fn(() => { + /* no-op setMessage */ + }) + ); +}); +afterEach(() => { + withProgressSpy.mockRestore(); +}); + type FakeEntity = { id: string; name: string }; type FakeWithOrg = FakeEntity & { orgSlug: string }; @@ -625,12 +646,19 @@ describe("dispatchOrgScopedList", () => { let resolveAllTargetsSpy: ReturnType; let setPaginationCursorSpy: ReturnType; let clearPaginationCursorSpy: ReturnType; + let resolveEffectiveOrgSpy: ReturnType; beforeEach(() => { getDefaultOrganizationSpy = spyOn(defaults, "getDefaultOrganization"); resolveAllTargetsSpy = spyOn(resolveTarget, "resolveAllTargets"); setPaginationCursorSpy = spyOn(paginationDb, "setPaginationCursor"); clearPaginationCursorSpy = spyOn(paginationDb, "clearPaginationCursor"); + // Prevent resolveEffectiveOrg from making real HTTP calls during + // full-suite runs where earlier tests may leave auth state behind. + resolveEffectiveOrgSpy = spyOn( + region, + "resolveEffectiveOrg" + ).mockImplementation((org: string) => Promise.resolve(org)); getDefaultOrganizationSpy.mockResolvedValue(null); resolveAllTargetsSpy.mockResolvedValue({ targets: [] }); @@ -643,6 +671,7 @@ describe("dispatchOrgScopedList", () => { resolveAllTargetsSpy.mockRestore(); setPaginationCursorSpy.mockRestore(); clearPaginationCursorSpy.mockRestore(); + resolveEffectiveOrgSpy.mockRestore(); }); test("throws ValidationError when --cursor used outside org-all mode", async () => { diff --git a/test/lib/trace-id.test.ts b/test/lib/trace-id.test.ts index db69bc3fe..14d3870d3 100644 --- a/test/lib/trace-id.test.ts +++ b/test/lib/trace-id.test.ts @@ -8,7 +8,11 @@ import { describe, expect, test } from "bun:test"; import { array, constantFrom, assert as fcAssert, property } from "fast-check"; import { ValidationError } from "../../src/lib/errors.js"; -import { TRACE_ID_RE, validateTraceId } from "../../src/lib/trace-id.js"; +import { + isTraceId, + TRACE_ID_RE, + validateTraceId, +} from "../../src/lib/trace-id.js"; const HEX_CHARS = "0123456789abcdefABCDEF".split(""); const VALID_TRACE_ID = "aaaa1111bbbb2222cccc3333dddd4444"; @@ -127,6 +131,73 @@ describe("validateTraceId", () => { }); }); +describe("isTraceId", () => { + test("returns true for valid 32-char hex string", () => { + expect(isTraceId(VALID_TRACE_ID)).toBe(true); + }); + + test("returns true for uppercase hex", () => { + expect(isTraceId("AAAA1111BBBB2222CCCC3333DDDD4444")).toBe(true); + }); + + test("returns true for UUID-format trace ID", () => { + expect(isTraceId("ed29abc8-71c4-475b-9675-4655ef1a02d0")).toBe(true); + }); + + test("returns false for project slug", () => { + expect(isTraceId("my-project")).toBe(false); + }); + + test("returns false for org slug", () => { + expect(isTraceId("my-org")).toBe(false); + }); + + test("returns false for short hex", () => { + expect(isTraceId("abc123")).toBe(false); + }); + + test("returns false for empty string", () => { + expect(isTraceId("")).toBe(false); + }); + + test("returns false for 33-char hex", () => { + expect(isTraceId(`${VALID_TRACE_ID}a`)).toBe(false); + }); + + test("handles whitespace", () => { + expect(isTraceId(` ${VALID_TRACE_ID} `)).toBe(true); + }); +}); + +describe("property: isTraceId ↔ validateTraceId consistency", () => { + test("isTraceId(x) === true iff validateTraceId(x) does not throw", () => { + fcAssert( + property(validTraceIdArb, (id) => { + const isValid = isTraceId(id); + let validates = true; + try { + validateTraceId(id); + } catch { + validates = false; + } + expect(isValid).toBe(validates); + }), + { numRuns: 100 } + ); + }); + + test("isTraceId returns false for invalid inputs", () => { + for (const invalid of [ + "", + "abc", + "my-project", + "not-a-hex-string-at-all", + ]) { + expect(isTraceId(invalid)).toBe(false); + } + }); +}); + describe("property: validateTraceId", () => { test("accepts any 32-char hex string and normalizes to lowercase", () => { fcAssert( From 2f3728b5f71702f9536709e3865121d2d023b3fc Mon Sep 17 00:00:00 2001 From: Burak Yigit Kaya Date: Wed, 18 Mar 2026 00:03:27 +0000 Subject: [PATCH 10/17] fix(e2e): update log list e2e tests for trace-id positional and JSON envelope - Remove --trace flag usage from e2e tests, use positional trace-id - Update JSON assertions to expect { data, hasMore } envelope - Rename describe block from '--trace' to '(trace mode)' --- test/e2e/log.test.ts | 58 +++++++++++++++----------------------------- 1 file changed, 19 insertions(+), 39 deletions(-) diff --git a/test/e2e/log.test.ts b/test/e2e/log.test.ts index fb3d8745e..05081d9a5 100644 --- a/test/e2e/log.test.ts +++ b/test/e2e/log.test.ts @@ -82,9 +82,11 @@ describe("sentry log list", () => { ]); expect(result.exitCode).toBe(0); - // Should be valid JSON array - const data = JSON.parse(result.stdout); - expect(Array.isArray(data)).toBe(true); + // Should be valid JSON envelope with data array and hasMore boolean + const parsed = JSON.parse(result.stdout); + expect(parsed).toHaveProperty("data"); + expect(parsed).toHaveProperty("hasMore"); + expect(Array.isArray(parsed.data)).toBe(true); }); test("supports --limit flag", async () => { @@ -144,16 +146,14 @@ describe("sentry log list", () => { }); }); -describe("sentry log list --trace", () => { - test("filters logs by trace ID", async () => { +describe("sentry log list (trace mode)", () => { + test("filters logs by trace ID (positional)", async () => { await ctx.setAuthToken(TEST_TOKEN); const result = await ctx.run([ "log", "list", - TEST_ORG, - "--trace", - TEST_TRACE_ID, + `${TEST_ORG}/${TEST_TRACE_ID}`, ]); expect(result.exitCode).toBe(0); @@ -161,40 +161,22 @@ describe("sentry log list --trace", () => { expect(result.stdout).toContain("Trace log message"); }); - test("supports --json with --trace", async () => { + test("supports --json with trace ID positional", async () => { await ctx.setAuthToken(TEST_TOKEN); const result = await ctx.run([ "log", "list", - TEST_ORG, - "--trace", - TEST_TRACE_ID, + `${TEST_ORG}/${TEST_TRACE_ID}`, "--json", ]); expect(result.exitCode).toBe(0); - const data = JSON.parse(result.stdout); - expect(Array.isArray(data)).toBe(true); - expect(data.length).toBe(2); - }); - - test("validates trace ID format", async () => { - await ctx.setAuthToken(TEST_TOKEN); - - const result = await ctx.run([ - "log", - "list", - TEST_ORG, - "--trace", - "not-a-valid-trace-id", - ]); - - // Stricli uses exit code 252 for parse errors - expect(result.exitCode).not.toBe(0); - expect(result.stderr + result.stdout).toMatch( - /invalid trace id|32-character hex/i - ); + const parsed = JSON.parse(result.stdout); + expect(parsed).toHaveProperty("data"); + expect(parsed).toHaveProperty("hasMore"); + expect(Array.isArray(parsed.data)).toBe(true); + expect(parsed.data.length).toBe(2); }); test("shows empty state for unknown trace", async () => { @@ -203,21 +185,19 @@ describe("sentry log list --trace", () => { const result = await ctx.run([ "log", "list", - TEST_ORG, - "--trace", - "00000000000000000000000000000000", + `${TEST_ORG}/00000000000000000000000000000000`, ]); expect(result.exitCode).toBe(0); expect(result.stdout).toMatch(/no logs found/i); }); - test("shows --trace in help output", async () => { + test("help shows trace-id as positional argument", async () => { const result = await ctx.run(["log", "list", "--help"]); expect(result.exitCode).toBe(0); - expect(result.stdout).toMatch(/--trace/); - expect(result.stdout).toMatch(/trace id/i); + expect(result.stdout).toMatch(/trace-id/i); + expect(result.stdout).toMatch(/trace filtering/i); }); }); From 785c3f01b07c82d087fad1a02d184318d94470b9 Mon Sep 17 00:00:00 2001 From: Burak Yigit Kaya Date: Wed, 18 Mar 2026 00:22:48 +0000 Subject: [PATCH 11/17] fix: address BugBot and Seer review feedback - Make --period flag optional (no default) to fix sentinel-value bug where explicit --period 90d was silently overridden to 14d in trace mode. Now undefined = not set, project mode defaults to 90d, trace mode defaults to 14d. - Add json option to withProgress to suppress spinner in JSON mode, matching poll() behavior. Pass json: flags.json at all 19 call sites. --- AGENTS.md | 61 ++++++++++++++++++++++------------ src/commands/dashboard/list.ts | 5 ++- src/commands/issue/list.ts | 4 +-- src/commands/log/list.ts | 23 ++++++++----- src/commands/org/list.ts | 2 +- src/commands/project/list.ts | 8 ++--- src/commands/span/list.ts | 2 +- src/commands/trace/list.ts | 2 +- src/commands/trace/logs.ts | 14 ++++---- src/commands/trial/list.ts | 7 ++-- src/lib/org-list.ts | 10 +++--- src/lib/polling.ts | 14 ++++++++ test/commands/log/list.test.ts | 38 +++++---------------- 13 files changed, 107 insertions(+), 83 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index 022325e49..6570cd4af 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -767,39 +767,58 @@ 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. + +* **DSN org prefix normalization in arg-parsing.ts**: Sentry DSN hosts encode org IDs as \`oNNNNN\` (e.g., \`o1081365.ingest.us.sentry.io\`). The Sentry API rejects the \`o\`-prefixed form. \`stripDsnOrgPrefix()\` in \`src/lib/arg-parsing.ts\` uses \`/^o(\d+)$/\` to strip the prefix — safe for slugs like \`organic\`. Applied in \`parseOrgProjectArg()\` and \`parseWithSlash()\`, covering all API call paths consuming \`parsed.org\`. - -* **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. + +* **GHCR versioned nightly tags for delta upgrade support**: GHCR nightly distribution uses three tag types: \`:nightly\` (rolling), \`:nightly-\\` (immutable), \`:patch-\\` (delta manifest). Delta patches use zig-bsdiff TRDIFF10 (zstd-compressed), ~50KB vs ~29MB full. Client bspatch via \`Bun.zstdDecompressSync()\`. N-1 patches only, full download fallback, SHA-256 verify, 60% size threshold. npm/Node excluded. Test mocks: use \`mockGhcrNightlyVersion()\` helper. - -* **Input validation layer: src/lib/input-validation.ts guards CLI arg parsing**: Four validators in \`src/lib/input-validation.ts\` guard against agent-hallucinated inputs: \`rejectControlChars\` (ASCII < 0x20), \`rejectPreEncoded\` (%XX), \`validateResourceId\` (rejects ?, #, %, whitespace), \`validateEndpoint\` (rejects \`..\` traversal). Applied in \`parseSlashOrgProject\`, bare-slug path in \`parseOrgProjectArg\`, \`parseIssueArg\`, and \`normalizeEndpoint\` (api.ts). NOT applied in \`parseSlashSeparatedArg\` for no-slash plain IDs — those may contain structural separators (newlines for log view batch IDs) that callers split downstream. Validation targets user-facing parse boundaries only; env vars and DB cache values are trusted. + +* **Issue list auto-pagination beyond API's 100-item cap**: Sentry API silently caps \`limit\` at 100 per request. \`listIssuesAllPages()\` auto-paginates using Link headers, bounded by MAX\_PAGINATION\_PAGES (50). \`API\_MAX\_PER\_PAGE\` constant is shared across all paginated consumers. \`--limit\` means total results everywhere (max 1000, default 25). Org-all mode uses \`fetchOrgAllIssues()\`; explicit \`--cursor\` does single-page fetch to preserve cursor chain. - -* **Magic @ selectors resolve issues dynamically via sort-based list API queries**: Magic \`@\` selectors (\`@latest\`, \`@most\_frequent\`) in \`parseIssueArg\` are detected early (before \`validateResourceId\`) because \`@\` is not in the forbidden charset. \`SELECTOR\_MAP\` provides case-insensitive matching with common variations (\`@mostfrequent\`, \`@most-frequent\`). Resolution in \`resolveSelector\` (issue/utils.ts) maps selectors to \`IssueSort\` values (\`date\`, \`freq\`), calls \`listIssuesPaginated\` with \`perPage: 1\` and \`query: 'is:unresolved'\`. Supports org-prefixed form: \`sentry/@latest\`. Unrecognized \`@\`-prefixed strings fall through to suffix-only parsing (not an error). The \`ParsedIssueArg\` union includes \`{ type: 'selector'; selector: IssueSelector; org?: string }\`. + +* **Self-hosted OAuth device flow requires Sentry 26.1.0+ and SENTRY\_CLIENT\_ID**: Self-hosted OAuth device flow requires Sentry 26.1.0+ and both \`SENTRY\_URL\` and \`SENTRY\_CLIENT\_ID\` env vars. Users must create a public OAuth app in Settings → Developer Settings. The client ID is NOT optional for self-hosted. Fallback for older instances: \`sentry auth login --token\`. \`getSentryUrl()\` and \`getClientId()\` in \`src/lib/oauth.ts\` read lazily (not at module load) so URL parsing from arguments can set \`SENTRY\_URL\` after import. -### Decision + +* **Sentry CLI markdown-first formatting pipeline replaces ad-hoc ANSI**: Formatters build CommonMark strings; \`renderMarkdown()\` renders to ANSI for TTY or raw markdown for non-TTY. Key helpers: \`colorTag()\`, \`mdKvTable()\`, \`mdRow()\`, \`mdTableHeader()\` (\`:\` suffix = right-aligned), \`renderTextTable()\`. \`isPlainOutput()\` checks \`SENTRY\_PLAIN\_OUTPUT\` > \`NO\_COLOR\` > \`!isTTY\`. Batch path: \`formatXxxTable()\`. Streaming path: \`StreamingTable\` (TTY) or raw markdown rows (plain). Both share \`buildXxxRowCells()\`. - -* **All view subcommands should use \ \ positional pattern**: All \`\* view\` subcommands should follow a consistent \`\ \\` positional argument pattern where target is the optional \`org/project\` specifier. During migration, use opportunistic argument swapping with a stderr warning when args are in wrong order. This is an instance of the broader CLI UX auto-correction pattern: safe when input is already invalid, correction is unambiguous, warning goes to stderr. Normalize at command level, keep parsers pure. Model after \`gh\` CLI conventions. + +* **Sentry issue stats field: time-series controlled by groupStatsPeriod**: The \`stats\` field on issues is \`{ '24h': \[\[ts, count], ...] }\`. Key depends on \`groupStatsPeriod\` param (\`""\`, \`"14d"\`, \`"24h"\`, \`"auto"\`). \`statsPeriod\` controls time window; \`groupStatsPeriod\` controls stats key. \*\*Critical\*\*: \`count\` is period-scoped — \`lifetime.count\` is the true lifetime total. Issue list table uses \`groupStatsPeriod: 'auto'\` for sparkline data. Column order: SHORT ID, ISSUE, SEEN, AGE, TREND, EVENTS, USERS, TRIAGE. TREND auto-hidden when terminal < 100 cols. \`--compact\` tri-state: explicit overrides; \`undefined\` triggers \`shouldAutoCompact(rowCount)\` — compact if \`3N + 3 > termHeight\`, false for non-TTY. Height is \`3N + 3\` (not \`3N + 4\`) because last data row has no trailing separator. + + +* **Sentry trace-logs API is org-scoped, not project-scoped**: The Sentry trace-logs endpoint (\`/organizations/{org}/trace-logs/\`) is org-scoped, so \`trace logs\` uses \`resolveOrg()\` not \`resolveOrgAndProject()\`. The endpoint is PRIVATE in Sentry source, excluded from the public OpenAPI schema — \`@sentry/api\` has no generated types. The hand-written \`TraceLogSchema\` in \`src/types/sentry.ts\` is required until Sentry makes it public. + + +* **withAuthGuard returns discriminated Result type, not fallback+onError**: \`withAuthGuard\(fn)\` in \`src/lib/errors.ts\` returns a discriminated Result: \`{ ok: true, value: T } | { ok: false, error: unknown }\`. AuthErrors always re-throw (triggers bin.ts auto-login). All other errors are captured. Callers inspect \`result.ok\` to degrade gracefully. Used across 12+ files. ### Gotcha - -* **Dot-notation field filtering is ambiguous for keys containing dots**: The \`filterFields\` function in \`src/lib/formatters/json.ts\` uses dot-notation to address nested fields (e.g., \`metadata.value\`). This means object keys that literally contain dots are ambiguous and cannot be addressed. Property-based tests for this function must generate field name arbitraries that exclude dots — use a restricted charset like \`\[a-zA-Z0-9\_]\` in fast-check arbitraries. Counterexample found by fast-check: \`{"a":{".":false}}\` with path \`"a."\` splits into \`\["a", ""]\` and fails to resolve. + +* **Biome lint: Response.redirect() required, nested ternaries forbidden**: Biome lint rules that frequently trip up this codebase: (1) \`useResponseRedirect\`: use \`Response.redirect(url, status)\` not \`new Response\`. (2) \`noNestedTernary\`: use \`if/else\`. (3) \`noComputedPropertyAccess\`: use \`obj.property\` not \`obj\["property"]\`. (4) Max cognitive complexity 15 per function — extract helpers to stay under. + + +* **Bugbot flags defensive null-checks as dead code — keep them with JSDoc justification**: Cursor Bugbot and Sentry Seer repeatedly flag two false positives: (1) defensive null-checks as "dead code" — keep them with JSDoc explaining why the guard exists for future safety, especially when removing would require \`!\` assertions banned by \`noNonNullAssertion\`. (2) stderr spinner output during \`--json\` mode — always a false positive since progress goes to stderr, JSON to stdout. Reply explaining the rationale and resolve. - -* **Stricli rejects unknown flags — pre-parsed global flags must be consumed from argv**: Stricli's arg parser is strict: any \`--flag\` not registered on a command throws \`No flag registered for --flag\`. Global flags (parsed before Stricli in bin.ts) MUST be spliced out of argv. \`--log-level\` was correctly consumed but \`--verbose\` was intentionally left in (for the \`api\` command's own \`--verbose\`). This breaks every other command. Also, \`argv.indexOf('--flag')\` doesn't match \`--flag=value\` form — must check both space-separated and equals-sign forms when pre-parsing. A Biome \`noRestrictedImports\` lint rule in \`biome.jsonc\` now blocks \`import { buildCommand } from "@stricli/core"\` at error level — only \`src/lib/command.ts\` is exempted. Other \`@stricli/core\` exports (\`buildRouteMap\`, \`run\`, etc.) are allowed. + +* **Bun mock.module for node:tty requires default export and class stubs**: Bun testing gotchas: (1) \`mock.module()\` for CJS built-ins requires a \`default\` re-export plus all named exports. Missing any causes \`SyntaxError: Export named 'X' not found\`. Always check the real module's full export list. (2) \`Bun.mmap()\` always opens with PROT\_WRITE — macOS SIGKILL on signed Mach-O, Linux ETXTBSY. Fix: use \`new Uint8Array(await Bun.file(path).arrayBuffer())\` in bspatch.ts. (3) Wrap \`Bun.which()\` with optional \`pathEnv\` param for deterministic testing without mocks. ### Pattern - -* **Property-based tests for input validators use stringMatching for forbidden char coverage**: In \`test/lib/input-validation.property.test.ts\`, forbidden-character arbitraries are built with \`stringMatching\` targeting specific regex patterns (e.g., \`/^\[^\x00-\x1f]\*\[\x00-\x1f]\[^\x00-\x1f]\*$/\` for control chars). This ensures fast-check generates strings that always contain the forbidden character while varying surrounding content. The \`biome-ignore lint/suspicious/noControlCharactersInRegex\` suppression is needed on the control char regex constant in \`input-validation.ts\`. + +* **Branch naming and commit message conventions for Sentry CLI**: Branch naming: \`feat/\\` or \`fix/\-\\` (e.g., \`feat/ghcr-nightly-distribution\`, \`fix/268-limit-auto-pagination\`). Commit message format: \`type(scope): description (#issue)\` (e.g., \`fix(issue-list): auto-paginate --limit beyond 100 (#268)\`, \`feat(nightly): distribute via GHCR instead of GitHub Releases\`). Types seen: fix, refactor, meta, release, feat. PRs are created as drafts via \`gh pr create --draft\`. Implementation plans are attached to commits via \`git notes add\` rather than in PR body or commit message. + + +* **Codecov patch coverage only counts test:unit and test:isolated, not E2E**: CI coverage merges \`test:unit\` (\`test/lib test/commands test/types --coverage\`) and \`test:isolated\` (\`test/isolated --coverage\`) into \`coverage/merged.lcov\`. E2E tests (\`test/e2e\`) are NOT included in coverage reports. So func tests that spy on exports (e.g., \`spyOn(apiClient, 'getLogs')\`) give zero coverage to the mocked function's body. To cover \`api-client.ts\` function bodies in unit tests, mock \`globalThis.fetch\` + \`setOrgRegion()\` + \`setAuthToken()\` and call the real function. + + +* **Pagination contextKey must include all query-varying parameters with escaping**: Pagination \`contextKey\` must encode every query-varying parameter (sort, query, period) with \`escapeContextKeyValue()\` (replaces \`|\` with \`%7C\`). Always provide a fallback before escaping since \`flags.period\` may be \`undefined\` in tests despite having a default: \`flags.period ? escapeContextKeyValue(flags.period) : "90d"\`. + + +* **PR review workflow: reply, resolve, amend, force-push**: PR review workflow: (1) Read unresolved threads via GraphQL, (2) make code changes, (3) run lint+typecheck+tests, (4) create a SEPARATE commit per review round (not amend) for incremental review, (5) push normally, (6) reply to comments via REST API, (7) resolve threads via GraphQL \`resolveReviewThread\`. Only amend+force-push when user explicitly asks or pre-commit hook modified files. - -* **Shared flag constants in list-command.ts for cross-command consistency**: \`src/lib/list-command.ts\` exports shared Stricli flag definitions (\`FIELDS\_FLAG\`, \`FRESH\_FLAG\`, \`FRESH\_ALIASES\`) reused across all commands. When adding a new global-ish flag to multiple commands, define it once here as a const satisfying Stricli's flag shape, then spread into each command's \`flags\` object. The \`--fields\` flag is \`{ kind: 'parsed', parse: String, brief: '...', optional: true }\`. \`parseFieldsList()\` in \`formatters/json.ts\` handles comma-separated parsing with trim/dedup. \`writeJson()\` accepts an optional \`fields\` array and calls \`filterFields()\` before serialization. + +* **Stricli optional boolean flags produce tri-state (true/false/undefined)**: Stricli boolean flags with \`optional: true\` (no \`default\`) produce \`boolean | undefined\` in the flags type. \`--flag\` → \`true\`, \`--no-flag\` → \`false\`, omitted → \`undefined\`. This enables auto-detect patterns: explicit user choice overrides, \`undefined\` triggers heuristic. Used by \`--compact\` on issue list. The flag type must be \`readonly field?: boolean\` (not \`readonly field: boolean\`). This differs from \`default: false\` which always produces a defined boolean. - -* **SKILL.md generator must filter hidden Stricli flags**: \`script/generate-skill.ts\` introspects Stricli's route tree to auto-generate \`plugins/sentry-cli/skills/sentry-cli/SKILL.md\`. The \`FlagDef\` type must include \`hidden?: boolean\` and \`extractFlags\` must propagate it to \`FlagInfo\`. The filter in \`generateCommandDoc\` must exclude \`f.hidden\` alongside \`help\`/\`helpAll\`. Without this, hidden flags injected by \`buildCommand\` (like \`--log-level\`, \`--verbose\`) appear on every command in the AI agent skill file. Global flags should instead be documented once in \`docs/src/content/docs/commands/index.md\` Global Options section, which the generator pulls into SKILL.md via \`loadCommandsOverview\`. + +* **Testing Stricli command func() bodies via spyOn mocking**: Stricli/Bun test patterns: (1) Command func tests: \`const func = await cmd.loader()\`, then \`func.call(mockContext, flags, ...args)\`. \`loader()\` return type union causes LSP errors — false positives that pass \`tsc\`. File naming: \`\*.func.test.ts\`. (2) ESM prevents \`vi.spyOn\` on Node built-in exports. Workaround: test subclass that overrides the method calling the built-in. (3) Follow-mode uses \`setTimeout\`-based scheduling; test with \`interceptSigint()\` helper. \`Bun.sleep()\` has no AbortSignal so \`setTimeout\`/\`clearTimeout\` required. diff --git a/src/commands/dashboard/list.ts b/src/commands/dashboard/list.ts index c1d46bac2..c30e96a1e 100644 --- a/src/commands/dashboard/list.ts +++ b/src/commands/dashboard/list.ts @@ -135,7 +135,10 @@ export const listCommand = buildCommand({ } const dashboards = await withProgress( - { message: `Fetching dashboards (up to ${flags.limit})...` }, + { + message: `Fetching dashboards (up to ${flags.limit})...`, + json: flags.json, + }, () => listDashboards(orgSlug, { perPage: flags.limit }) ); const url = buildDashboardsListUrl(orgSlug); diff --git a/src/commands/issue/list.ts b/src/commands/issue/list.ts index 63e48ecd6..eae3388ff 100644 --- a/src/commands/issue/list.ts +++ b/src/commands/issue/list.ts @@ -812,7 +812,7 @@ async function handleOrgAllIssues( setContext([org], []); const { issues, nextCursor } = await withProgress( - { message: `Fetching issues (up to ${flags.limit})...` }, + { message: `Fetching issues (up to ${flags.limit})...`, json: flags.json }, (setMessage) => fetchOrgAllIssues(org, flags, cursor, (fetched, limit) => setMessage( @@ -954,7 +954,7 @@ async function handleResolvedTargets( : "Fetching issues"; const { results, hasMore } = await withProgress( - { message: `${baseMessage} (up to ${flags.limit})...` }, + { message: `${baseMessage} (up to ${flags.limit})...`, json: flags.json }, (setMessage) => fetchWithBudget( activeTargets, diff --git a/src/commands/log/list.ts b/src/commands/log/list.ts index a92d5b9cb..d930ccc21 100644 --- a/src/commands/log/list.ts +++ b/src/commands/log/list.ts @@ -49,7 +49,7 @@ type ListFlags = { readonly limit: number; readonly query?: string; readonly follow?: number; - readonly period: string; + readonly period?: string; readonly json: boolean; readonly fresh: boolean; readonly fields?: string[]; @@ -198,15 +198,19 @@ function parseLogListArgs(args: string[]): ParsedLogArgs { * Returns the logs and a hint. The caller yields the result and * returns the hint as a footer via `CommandReturn`. */ +/** Default time period for project-scoped log queries */ +const DEFAULT_PROJECT_PERIOD = "90d"; + async function executeSingleFetch( org: string, project: string, flags: ListFlags ): Promise { + const period = flags.period ?? DEFAULT_PROJECT_PERIOD; const logs = await listLogs(org, project, { query: flags.query, limit: flags.limit, - statsPeriod: flags.period, + statsPeriod: period, }); if (logs.length === 0) { @@ -459,9 +463,9 @@ async function executeTraceSingleFetch( traceId: string, flags: ListFlags ): Promise { - // In trace mode, use a shorter default period if the user hasn't - // explicitly changed it from the command-level default of "90d". - const period = flags.period === "90d" ? DEFAULT_TRACE_PERIOD : flags.period; + // Use the explicit period if set, otherwise default to 14d for trace mode. + // The flag is optional (no default) so undefined means "not explicitly set". + const period = flags.period ?? DEFAULT_TRACE_PERIOD; const logs = await listTraceLogs(org, traceId, { query: flags.query, @@ -663,8 +667,9 @@ export const listCommand = buildListCommand("log", { period: { kind: "parsed", parse: String, - brief: 'Time period (e.g., "90d", "14d", "24h")', - default: "90d", + brief: + 'Time period (e.g., "90d", "14d", "24h"). Default: 90d (project mode), 14d (trace mode)', + optional: true, }, fresh: FRESH_FLAG, }, @@ -740,7 +745,7 @@ export const listCommand = buildListCommand("log", { } const { result, hint } = await withProgress( - { message: "Fetching logs..." }, + { message: "Fetching logs...", json: flags.json }, () => executeTraceSingleFetch(org, traceId, flags) ); yield new CommandOutput(result); @@ -781,7 +786,7 @@ export const listCommand = buildListCommand("log", { } const { result, hint } = await withProgress( - { message: "Fetching logs..." }, + { message: "Fetching logs...", json: flags.json }, () => executeSingleFetch(org, project, flags) ); yield new CommandOutput(result); diff --git a/src/commands/org/list.ts b/src/commands/org/list.ts index a1f2253f0..d870a85c1 100644 --- a/src/commands/org/list.ts +++ b/src/commands/org/list.ts @@ -132,7 +132,7 @@ export const listCommand = buildCommand({ applyFreshFlag(flags); const orgs = await withProgress( - { message: "Fetching organizations..." }, + { message: "Fetching organizations...", json: flags.json }, () => listOrganizationsUncached() ); const limitedOrgs = orgs.slice(0, flags.limit); diff --git a/src/commands/project/list.ts b/src/commands/project/list.ts index e4b7d2d72..02081d149 100644 --- a/src/commands/project/list.ts +++ b/src/commands/project/list.ts @@ -335,7 +335,7 @@ export async function handleAutoDetect( } = await resolveOrgsForAutoDetect(cwd); const { projects: allProjects, nextCursor } = await withProgress( - { message: "Fetching projects..." }, + { message: "Fetching projects...", json: flags.json }, () => fetchAutoDetectProjects(orgsToFetch, flags) ); @@ -392,7 +392,7 @@ export async function handleExplicit( flags: ListFlags ): Promise> { const projectResult = await withProgress( - { message: "Fetching projects..." }, + { message: "Fetching projects...", json: flags.json }, () => withAuthGuard(() => getProject(org, projectSlug)) ); if (!projectResult.ok) { @@ -442,7 +442,7 @@ export async function handleOrgAll( ): Promise> { const { org, flags, contextKey, cursor } = options; const response: PaginatedResponse = await withProgress( - { message: "Fetching projects..." }, + { message: "Fetching projects...", json: flags.json }, () => listProjectsPaginated(org, { cursor, @@ -506,7 +506,7 @@ export async function handleProjectSearch( flags: ListFlags ): Promise> { const { projects } = await withProgress( - { message: "Fetching projects..." }, + { message: "Fetching projects...", json: flags.json }, () => findProjectsBySlug(projectSlug) ); const filtered = filterByPlatform(projects, flags.platform); diff --git a/src/commands/span/list.ts b/src/commands/span/list.ts index 524263b46..1a8e439ab 100644 --- a/src/commands/span/list.ts +++ b/src/commands/span/list.ts @@ -262,7 +262,7 @@ export const listCommand = buildCommand({ // Fetch spans from EAP endpoint const { data: spanItems, nextCursor } = await withProgress( - { message: "Fetching spans..." }, + { message: "Fetching spans...", json: flags.json }, () => listSpans(org, project, { query: apiQuery, diff --git a/src/commands/trace/list.ts b/src/commands/trace/list.ts index 3d52da93f..c90994702 100644 --- a/src/commands/trace/list.ts +++ b/src/commands/trace/list.ts @@ -247,7 +247,7 @@ export const listCommand = buildListCommand("trace", { const cursor = resolveOrgCursor(flags.cursor, PAGINATION_KEY, contextKey); const { data: traces, nextCursor } = await withProgress( - { message: "Fetching traces..." }, + { message: "Fetching traces...", json: flags.json }, () => listTransactions(org, project, { query: flags.query, diff --git a/src/commands/trace/logs.ts b/src/commands/trace/logs.ts index 6761d7d6f..cccab9a5f 100644 --- a/src/commands/trace/logs.ts +++ b/src/commands/trace/logs.ts @@ -174,12 +174,14 @@ export const logsCommand = buildCommand({ return; } - const logs = await withProgress({ message: "Fetching trace logs..." }, () => - listTraceLogs(org, traceId, { - statsPeriod: flags.period, - limit: flags.limit, - query: flags.query, - }) + const logs = await withProgress( + { message: "Fetching trace logs...", json: flags.json }, + () => + listTraceLogs(org, traceId, { + statsPeriod: flags.period, + limit: flags.limit, + query: flags.query, + }) ); // Reverse to chronological order (API returns newest-first) diff --git a/src/commands/trial/list.ts b/src/commands/trial/list.ts index a837a9604..c6944f5e0 100644 --- a/src/commands/trial/list.ts +++ b/src/commands/trial/list.ts @@ -221,7 +221,7 @@ export const listCommand = buildCommand({ ], }, }, - async *func(this: SentryContext, _flags: ListFlags, org?: string) { + async *func(this: SentryContext, flags: ListFlags, org?: string) { const resolved = await resolveOrg({ org, cwd: this.cwd, @@ -231,8 +231,9 @@ export const listCommand = buildCommand({ throw new ContextError("Organization", "sentry trial list "); } - const info = await withProgress({ message: "Fetching trials..." }, () => - getCustomerTrialInfo(resolved.org) + const info = await withProgress( + { message: "Fetching trials...", json: flags.json }, + () => getCustomerTrialInfo(resolved.org) ); const productTrials = info.productTrials ?? []; diff --git a/src/lib/org-list.ts b/src/lib/org-list.ts index 3595e5aa7..14554114d 100644 --- a/src/lib/org-list.ts +++ b/src/lib/org-list.ts @@ -347,7 +347,7 @@ export async function handleOrgAll( const { config, org, flags, contextKey, cursor } = options; const response = await withProgress( - { message: `Fetching ${config.entityPlural}...` }, + { message: `Fetching ${config.entityPlural}...`, json: flags.json }, () => config.listPaginated(org, { cursor, @@ -408,7 +408,7 @@ export async function handleAutoDetect( } = await resolveOrgsForListing(undefined, cwd); const allItems = await withProgress( - { message: `Fetching ${config.entityPlural}...` }, + { message: `Fetching ${config.entityPlural}...`, json: flags.json }, async () => { if (orgsToFetch.length > 0) { const results = await Promise.all( @@ -536,7 +536,7 @@ export async function handleExplicitOrg( ): Promise> { const { config, org, flags, noteOrgScoped = false } = options; const items = await withProgress( - { message: `Fetching ${config.entityPlural}...` }, + { message: `Fetching ${config.entityPlural}...`, json: flags.json }, () => fetchOrgSafe(config, org) ); @@ -588,7 +588,7 @@ export async function handleExplicitProject( ); } const raw = await withProgress( - { message: `Fetching ${config.entityPlural}...` }, + { message: `Fetching ${config.entityPlural}...`, json: flags.json }, () => listForProject(org, project) ); const items = raw.map((entity) => config.withOrg(entity, org)); @@ -637,7 +637,7 @@ export async function handleProjectSearch( ): Promise> { const { flags, orgAllFallback } = options; const { projects: matches, orgs } = await withProgress( - { message: `Fetching ${config.entityPlural}...` }, + { message: `Fetching ${config.entityPlural}...`, json: flags.json }, () => findProjectsBySlug(projectSlug) ); diff --git a/src/lib/polling.ts b/src/lib/polling.ts index f7348d5d8..4792d75a0 100644 --- a/src/lib/polling.ts +++ b/src/lib/polling.ts @@ -148,6 +148,9 @@ function startSpinner(initialMessage: string): { export type WithProgressOptions = { /** Initial spinner message */ message: string; + /** Suppress progress output (JSON mode). When true, the operation runs + * without a spinner — matching the behaviour of {@link poll}. */ + json?: boolean; }; /** @@ -157,6 +160,10 @@ export type WithProgressOptions = { * giving a consistent look across all CLI commands. Progress output goes * to stderr, so it never contaminates stdout (safe to use alongside JSON output). * + * When `options.json` is true the spinner is suppressed entirely, matching + * the behaviour of {@link poll}. This avoids noisy ANSI escape sequences on + * stderr when agents or CI pipelines consume `--json` output. + * * The callback receives a `setMessage` function to update the displayed * message as work progresses (e.g. to show page counts during pagination). * Progress is automatically cleared when the operation completes. @@ -182,6 +189,13 @@ export async function withProgress( options: WithProgressOptions, fn: (setMessage: (msg: string) => void) => Promise ): Promise { + if (options.json) { + // JSON mode: skip the spinner entirely, pass a no-op setMessage + return fn(() => { + /* spinner suppressed in JSON mode */ + }); + } + const spinner = startSpinner(options.message); try { diff --git a/test/commands/log/list.test.ts b/test/commands/log/list.test.ts index ce2603eeb..822f3e0cc 100644 --- a/test/commands/log/list.test.ts +++ b/test/commands/log/list.test.ts @@ -134,18 +134,16 @@ function mockWithProgress( return fn(noop); } -/** Standard flags for non-follow batch mode */ +/** Standard flags for non-follow batch mode (period omitted = use mode default) */ const BATCH_FLAGS = { json: true, limit: 100, - period: "90d", } as const; -/** Human-mode flags for non-follow batch mode */ +/** Human-mode flags for non-follow batch mode (period omitted = use mode default) */ const HUMAN_FLAGS = { json: false, limit: 100, - period: "90d", } as const; /** Sample project-scoped logs (SentryLog) */ @@ -337,11 +335,7 @@ describe("listCommand.func — standard mode", () => { const { context, stdoutWrite } = createMockContext(); const func = await listCommand.loader(); // limit equals number of returned logs → hasMore = true - await func.call( - context, - { json: false, limit: 3, period: "90d" }, - `${ORG}/${PROJECT}` - ); + await func.call(context, { json: false, limit: 3 }, `${ORG}/${PROJECT}`); const output = stdoutWrite.mock.calls.map((c) => c[0]).join(""); expect(output).toContain("Use --limit to show more"); @@ -367,7 +361,7 @@ describe("listCommand.func — standard mode", () => { const func = await listCommand.loader(); await func.call( context, - { json: false, limit: 50, query: "level:error", period: "90d" }, + { json: false, limit: 50, query: "level:error" }, `${ORG}/${PROJECT}` ); @@ -395,11 +389,7 @@ describe("listCommand.func — standard mode", () => { const { context, stdoutWrite } = createMockContext(); const func = await listCommand.loader(); - await func.call( - context, - { json: true, limit: 3, period: "90d" }, - `${ORG}/${PROJECT}` - ); + await func.call(context, { json: true, limit: 3 }, `${ORG}/${PROJECT}`); const output = stdoutWrite.mock.calls.map((c) => c[0]).join(""); const parsed = JSON.parse(output); @@ -526,11 +516,7 @@ describe("listCommand.func — trace mode", () => { const { context, stdoutWrite } = createMockContext(); const func = await listCommand.loader(); - await func.call( - context, - { json: false, limit: 3, period: "90d" }, - TRACE_ID - ); + await func.call(context, { json: false, limit: 3 }, TRACE_ID); const output = stdoutWrite.mock.calls.map((c) => c[0]).join(""); expect(output).toContain("Use --limit to show more."); @@ -544,7 +530,7 @@ describe("listCommand.func — trace mode", () => { const func = await listCommand.loader(); await func.call( context, - { json: false, limit: 50, query: "level:error", period: "90d" }, + { json: false, limit: 50, query: "level:error" }, TRACE_ID ); @@ -703,14 +689,10 @@ describe("listCommand.func — period flag", () => { withProgressSpy.mockRestore(); }); - test("trace mode uses 14d default when period is the command default 90d", async () => { + test("trace mode uses 14d default when period is omitted", async () => { const { context } = createMockContext(); const func = await listCommand.loader(); - await func.call( - context, - { json: true, limit: 100, period: "90d" }, - TRACE_ID - ); + await func.call(context, { json: true, limit: 100 }, TRACE_ID); expect(listTraceLogsSpy).toHaveBeenCalledWith(ORG, TRACE_ID, { query: undefined, @@ -860,7 +842,6 @@ describe("listCommand.func — follow mode (standard)", () => { json: false, limit: 100, follow: 1, - period: "90d", } as const; test("writes initial logs then resolves on SIGINT", async () => { @@ -1152,7 +1133,6 @@ describe("listCommand.func — follow mode (trace)", () => { json: false, limit: 100, follow: 1, - period: "90d", } as const; test("writes initial trace logs then resolves on SIGINT", async () => { From bd6b5f414a85beba9f193be09524731a4c0f0beb Mon Sep 17 00:00:00 2001 From: "github-actions[bot]" Date: Wed, 18 Mar 2026 00:23:16 +0000 Subject: [PATCH 12/17] chore: regenerate SKILL.md --- plugins/sentry-cli/skills/sentry-cli/SKILL.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/plugins/sentry-cli/skills/sentry-cli/SKILL.md b/plugins/sentry-cli/skills/sentry-cli/SKILL.md index 128219807..1a0df5b89 100644 --- a/plugins/sentry-cli/skills/sentry-cli/SKILL.md +++ b/plugins/sentry-cli/skills/sentry-cli/SKILL.md @@ -607,7 +607,7 @@ List logs from a project - `-n, --limit - Number of log entries (1-1000) - (default: "100")` - `-q, --query - Filter query (Sentry search syntax)` - `-f, --follow - Stream logs (optionally specify poll interval in seconds)` -- `-t, --period - Time period (e.g., "90d", "14d", "24h") - (default: "90d")` +- `-t, --period - Time period (e.g., "90d", "14d", "24h"). Default: 90d (project mode), 14d (trace mode)` - `--fresh - Bypass cache, re-detect projects, and fetch fresh data` - `--json - Output as JSON` - `--fields - Comma-separated fields to include in JSON output (dot.notation supported)` From fb2bc4a7047b6209b9fcda7b658629267a179f38 Mon Sep 17 00:00:00 2001 From: Burak Yigit Kaya Date: Wed, 18 Mar 2026 00:33:39 +0000 Subject: [PATCH 13/17] fix: handle multi-arg trace mode and fix orphaned JSDoc - parseLogListArgs now checks last arg for trace-id in 2+ arg case (e.g., 'sentry log list my-org ') - Move DEFAULT_PROJECT_PERIOD above JSDoc so it attaches correctly to executeSingleFetch --- src/commands/log/list.ts | 35 +++++++++++++++++++++++++++-------- 1 file changed, 27 insertions(+), 8 deletions(-) diff --git a/src/commands/log/list.ts b/src/commands/log/list.ts index d930ccc21..cc99519e2 100644 --- a/src/commands/log/list.ts +++ b/src/commands/log/list.ts @@ -160,10 +160,17 @@ type ParsedLogArgs = /** * Disambiguate log list positional arguments. * - * Checks if the "tail" segment (last part after `/`, or the entire arg - * if no `/`) looks like a 32-char hex trace ID. If so, delegates to - * {@link parseTraceTarget} for full trace target parsing. Otherwise, - * treats the argument as a project target. + * Detects trace mode by checking whether any argument segment looks like + * a 32-char hex trace ID: + * + * - **Single arg**: checks the tail segment (last part after `/`, or the + * entire arg). ``, `/`, `//`. + * - **Two+ args**: checks the last positional (` ` or + * `/ ` space-separated forms). + * - **No match**: treats the argument as a project target. + * + * When trace mode is detected, delegates to {@link parseTraceTarget} for + * full parsing and validation. * * @param args - Positional arguments from CLI * @returns Parsed args with mode discrimination @@ -178,7 +185,19 @@ function parseLogListArgs(args: string[]): ParsedLogArgs { return { mode: "project" }; } - // Check the tail segment: last part after `/`, or the entire arg + // Two+ args: check if the last arg is a trace ID (space-separated form) + // e.g., `sentry log list my-org abc123...` or `sentry log list my-org/proj abc123...` + if (args.length >= 2) { + const last = args.at(-1); + if (last && isTraceId(last)) { + return { + mode: "trace", + parsed: parseTraceTarget(args, TRACE_USAGE_HINT), + }; + } + } + + // Single arg: check the tail segment (last part after `/`, or the entire arg) const lastSlash = first.lastIndexOf("/"); const tail = lastSlash === -1 ? first : first.slice(lastSlash + 1); @@ -192,15 +211,15 @@ function parseLogListArgs(args: string[]): ParsedLogArgs { return { mode: "project", target: first }; } +/** Default time period for project-scoped log queries */ +const DEFAULT_PROJECT_PERIOD = "90d"; + /** * Execute a single fetch of logs (non-streaming mode). * * Returns the logs and a hint. The caller yields the result and * returns the hint as a footer via `CommandReturn`. */ -/** Default time period for project-scoped log queries */ -const DEFAULT_PROJECT_PERIOD = "90d"; - async function executeSingleFetch( org: string, project: string, From 3006b228aed04369156ea3ab751afd45da0603ef Mon Sep 17 00:00:00 2001 From: Burak Yigit Kaya Date: Wed, 18 Mar 2026 00:49:27 +0000 Subject: [PATCH 14/17] fix: add 'up to N' to all spinner messages, consolidate hex-id normalization - All withProgress messages now show '(up to N)...' matching issue list - Extract normalizeHexId() in hex-id.ts, use in both validateHexId and isTraceId to eliminate duplicated normalization logic - validateTraceId now delegates through isTraceId's normalization path --- src/commands/log/list.ts | 10 ++++++++-- src/commands/org/list.ts | 5 ++++- src/commands/project/list.ts | 20 ++++++++++++++++---- src/commands/span/list.ts | 2 +- src/commands/trace/list.ts | 5 ++++- src/commands/trace/logs.ts | 5 ++++- src/lib/hex-id.ts | 35 ++++++++++++++++++++++++----------- src/lib/org-list.ts | 25 ++++++++++++++++++++----- src/lib/trace-id.ts | 8 ++------ 9 files changed, 83 insertions(+), 32 deletions(-) diff --git a/src/commands/log/list.ts b/src/commands/log/list.ts index cc99519e2..274a70035 100644 --- a/src/commands/log/list.ts +++ b/src/commands/log/list.ts @@ -764,7 +764,10 @@ export const listCommand = buildListCommand("log", { } const { result, hint } = await withProgress( - { message: "Fetching logs...", json: flags.json }, + { + message: `Fetching logs (up to ${flags.limit})...`, + json: flags.json, + }, () => executeTraceSingleFetch(org, traceId, flags) ); yield new CommandOutput(result); @@ -805,7 +808,10 @@ export const listCommand = buildListCommand("log", { } const { result, hint } = await withProgress( - { message: "Fetching logs...", json: flags.json }, + { + message: `Fetching logs (up to ${flags.limit})...`, + json: flags.json, + }, () => executeSingleFetch(org, project, flags) ); yield new CommandOutput(result); diff --git a/src/commands/org/list.ts b/src/commands/org/list.ts index d870a85c1..429b0c148 100644 --- a/src/commands/org/list.ts +++ b/src/commands/org/list.ts @@ -132,7 +132,10 @@ export const listCommand = buildCommand({ applyFreshFlag(flags); const orgs = await withProgress( - { message: "Fetching organizations...", json: flags.json }, + { + message: `Fetching organizations (up to ${flags.limit})...`, + json: flags.json, + }, () => listOrganizationsUncached() ); const limitedOrgs = orgs.slice(0, flags.limit); diff --git a/src/commands/project/list.ts b/src/commands/project/list.ts index 02081d149..03c70e9cb 100644 --- a/src/commands/project/list.ts +++ b/src/commands/project/list.ts @@ -335,7 +335,10 @@ export async function handleAutoDetect( } = await resolveOrgsForAutoDetect(cwd); const { projects: allProjects, nextCursor } = await withProgress( - { message: "Fetching projects...", json: flags.json }, + { + message: `Fetching projects (up to ${flags.limit})...`, + json: flags.json, + }, () => fetchAutoDetectProjects(orgsToFetch, flags) ); @@ -392,7 +395,10 @@ export async function handleExplicit( flags: ListFlags ): Promise> { const projectResult = await withProgress( - { message: "Fetching projects...", json: flags.json }, + { + message: `Fetching projects (up to ${flags.limit})...`, + json: flags.json, + }, () => withAuthGuard(() => getProject(org, projectSlug)) ); if (!projectResult.ok) { @@ -442,7 +448,10 @@ export async function handleOrgAll( ): Promise> { const { org, flags, contextKey, cursor } = options; const response: PaginatedResponse = await withProgress( - { message: "Fetching projects...", json: flags.json }, + { + message: `Fetching projects (up to ${flags.limit})...`, + json: flags.json, + }, () => listProjectsPaginated(org, { cursor, @@ -506,7 +515,10 @@ export async function handleProjectSearch( flags: ListFlags ): Promise> { const { projects } = await withProgress( - { message: "Fetching projects...", json: flags.json }, + { + message: `Fetching projects (up to ${flags.limit})...`, + json: flags.json, + }, () => findProjectsBySlug(projectSlug) ); const filtered = filterByPlatform(projects, flags.platform); diff --git a/src/commands/span/list.ts b/src/commands/span/list.ts index 1a8e439ab..55e68b448 100644 --- a/src/commands/span/list.ts +++ b/src/commands/span/list.ts @@ -262,7 +262,7 @@ export const listCommand = buildCommand({ // Fetch spans from EAP endpoint const { data: spanItems, nextCursor } = await withProgress( - { message: "Fetching spans...", json: flags.json }, + { message: `Fetching spans (up to ${flags.limit})...`, json: flags.json }, () => listSpans(org, project, { query: apiQuery, diff --git a/src/commands/trace/list.ts b/src/commands/trace/list.ts index c90994702..7ff63811d 100644 --- a/src/commands/trace/list.ts +++ b/src/commands/trace/list.ts @@ -247,7 +247,10 @@ export const listCommand = buildListCommand("trace", { const cursor = resolveOrgCursor(flags.cursor, PAGINATION_KEY, contextKey); const { data: traces, nextCursor } = await withProgress( - { message: "Fetching traces...", json: flags.json }, + { + message: `Fetching traces (up to ${flags.limit})...`, + json: flags.json, + }, () => listTransactions(org, project, { query: flags.query, diff --git a/src/commands/trace/logs.ts b/src/commands/trace/logs.ts index cccab9a5f..ffb9fc6c1 100644 --- a/src/commands/trace/logs.ts +++ b/src/commands/trace/logs.ts @@ -175,7 +175,10 @@ export const logsCommand = buildCommand({ } const logs = await withProgress( - { message: "Fetching trace logs...", json: flags.json }, + { + message: `Fetching trace logs (up to ${flags.limit})...`, + json: flags.json, + }, () => listTraceLogs(org, traceId, { statsPeriod: flags.period, diff --git a/src/lib/hex-id.ts b/src/lib/hex-id.ts index 4d6d5fede..a0134a868 100644 --- a/src/lib/hex-id.ts +++ b/src/lib/hex-id.ts @@ -24,6 +24,24 @@ export const UUID_DASH_RE = /** Max display length for invalid IDs in error messages before truncation */ const MAX_DISPLAY_LENGTH = 40; +/** + * Normalize a potential hex ID: trim, lowercase, strip UUID dashes. + * Does NOT validate — call this before checking {@link HEX_ID_RE}. + * + * Extracted so that both {@link validateHexId} and non-throwing predicates + * (like `isTraceId`) share identical normalization logic. + * + * @param value - The raw string to normalize + * @returns The trimmed, lowercased string with UUID dashes stripped if applicable + */ +export function normalizeHexId(value: string): string { + let trimmed = value.trim().toLowerCase(); + if (UUID_DASH_RE.test(trimmed)) { + trimmed = trimmed.replace(/-/g, ""); + } + return trimmed; +} + /** * Validate that a string is a 32-character hexadecimal ID. * Trims whitespace and normalizes to lowercase before validation. @@ -44,25 +62,20 @@ const MAX_DISPLAY_LENGTH = 40; * @throws {ValidationError} If the format is invalid */ export function validateHexId(value: string, label: string): string { - let trimmed = value.trim().toLowerCase(); - - // Auto-correct UUID format: strip dashes (8-4-4-4-12 → 32 hex chars) - if (UUID_DASH_RE.test(trimmed)) { - trimmed = trimmed.replace(/-/g, ""); - } + const normalized = normalizeHexId(value); - if (!HEX_ID_RE.test(trimmed)) { + if (!HEX_ID_RE.test(normalized)) { const display = - trimmed.length > MAX_DISPLAY_LENGTH - ? `${trimmed.slice(0, MAX_DISPLAY_LENGTH - 3)}...` - : trimmed; + normalized.length > MAX_DISPLAY_LENGTH + ? `${normalized.slice(0, MAX_DISPLAY_LENGTH - 3)}...` + : normalized; throw new ValidationError( `Invalid ${label} "${display}". Expected a 32-character hexadecimal string.\n\n` + "Example: abc123def456abc123def456abc123de" ); } - return trimmed; + return normalized; } /** diff --git a/src/lib/org-list.ts b/src/lib/org-list.ts index 14554114d..6df4d99a6 100644 --- a/src/lib/org-list.ts +++ b/src/lib/org-list.ts @@ -347,7 +347,10 @@ export async function handleOrgAll( const { config, org, flags, contextKey, cursor } = options; const response = await withProgress( - { message: `Fetching ${config.entityPlural}...`, json: flags.json }, + { + message: `Fetching ${config.entityPlural} (up to ${flags.limit})...`, + json: flags.json, + }, () => config.listPaginated(org, { cursor, @@ -408,7 +411,10 @@ export async function handleAutoDetect( } = await resolveOrgsForListing(undefined, cwd); const allItems = await withProgress( - { message: `Fetching ${config.entityPlural}...`, json: flags.json }, + { + message: `Fetching ${config.entityPlural} (up to ${flags.limit})...`, + json: flags.json, + }, async () => { if (orgsToFetch.length > 0) { const results = await Promise.all( @@ -536,7 +542,10 @@ export async function handleExplicitOrg( ): Promise> { const { config, org, flags, noteOrgScoped = false } = options; const items = await withProgress( - { message: `Fetching ${config.entityPlural}...`, json: flags.json }, + { + message: `Fetching ${config.entityPlural} (up to ${flags.limit})...`, + json: flags.json, + }, () => fetchOrgSafe(config, org) ); @@ -588,7 +597,10 @@ export async function handleExplicitProject( ); } const raw = await withProgress( - { message: `Fetching ${config.entityPlural}...`, json: flags.json }, + { + message: `Fetching ${config.entityPlural} (up to ${flags.limit})...`, + json: flags.json, + }, () => listForProject(org, project) ); const items = raw.map((entity) => config.withOrg(entity, org)); @@ -637,7 +649,10 @@ export async function handleProjectSearch( ): Promise> { const { flags, orgAllFallback } = options; const { projects: matches, orgs } = await withProgress( - { message: `Fetching ${config.entityPlural}...`, json: flags.json }, + { + message: `Fetching ${config.entityPlural} (up to ${flags.limit})...`, + json: flags.json, + }, () => findProjectsBySlug(projectSlug) ); diff --git a/src/lib/trace-id.ts b/src/lib/trace-id.ts index d7edcaf26..5309b8050 100644 --- a/src/lib/trace-id.ts +++ b/src/lib/trace-id.ts @@ -5,7 +5,7 @@ * Used by `trace logs`, `log list`, and `span list` commands. */ -import { HEX_ID_RE, UUID_DASH_RE, validateHexId } from "./hex-id.js"; +import { HEX_ID_RE, normalizeHexId, validateHexId } from "./hex-id.js"; /** * Regex for a valid 32-character hexadecimal trace ID. @@ -25,11 +25,7 @@ export const TRACE_ID_RE = HEX_ID_RE; * @returns `true` if the value would pass {@link validateTraceId} */ export function isTraceId(value: string): boolean { - let trimmed = value.trim().toLowerCase(); - if (UUID_DASH_RE.test(trimmed)) { - trimmed = trimmed.replace(/-/g, ""); - } - return HEX_ID_RE.test(trimmed); + return HEX_ID_RE.test(normalizeHexId(value)); } /** From 7b71f7e8610378034f603c07d4bcfb9a6a8873c6 Mon Sep 17 00:00:00 2001 From: Burak Yigit Kaya Date: Wed, 18 Mar 2026 00:57:51 +0000 Subject: [PATCH 15/17] fix: use data.hasMore instead of re-deriving in trace logs human formatter --- src/commands/trace/logs.ts | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/commands/trace/logs.ts b/src/commands/trace/logs.ts index ffb9fc6c1..1ab1dde91 100644 --- a/src/commands/trace/logs.ts +++ b/src/commands/trace/logs.ts @@ -59,9 +59,8 @@ function formatTraceLogsHuman(data: TraceLogsData): string { return data.emptyMessage ?? "No logs found."; } const parts = [formatLogTable(data.logs, false)]; - const hasMore = data.logs.length >= data.limit; const countText = `Showing ${data.logs.length} log${data.logs.length === 1 ? "" : "s"} for trace ${data.traceId}.`; - const tip = hasMore ? " Use --limit to show more." : ""; + const tip = data.hasMore ? " Use --limit to show more." : ""; parts.push(formatFooter(`${countText}${tip}`)); return parts.join("").trimEnd(); } From f2a68470c7f62722b73c6c3c5a5e407fa98b1908 Mon Sep 17 00:00:00 2001 From: Burak Yigit Kaya Date: Wed, 18 Mar 2026 01:06:28 +0000 Subject: [PATCH 16/17] fix: remove dead limit field from TraceLogsData (replaced by hasMore) --- src/commands/trace/logs.ts | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/commands/trace/logs.ts b/src/commands/trace/logs.ts index 1ab1dde91..76556c194 100644 --- a/src/commands/trace/logs.ts +++ b/src/commands/trace/logs.ts @@ -47,7 +47,6 @@ type LogLike = { type TraceLogsData = { logs: LogLike[]; traceId: string; - limit: number; hasMore: boolean; /** Message shown when no logs found */ emptyMessage?: string; @@ -197,7 +196,6 @@ export const logsCommand = buildCommand({ return yield new CommandOutput({ logs: chronological, traceId, - limit: flags.limit, hasMore, emptyMessage, }); From fe82fcdcef87bbbef41c222a5b176000db0cf5ba Mon Sep 17 00:00:00 2001 From: Burak Yigit Kaya Date: Wed, 18 Mar 2026 01:14:18 +0000 Subject: [PATCH 17/17] fix: use accurate spinner message for single-project fetch --- src/commands/project/list.ts | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/src/commands/project/list.ts b/src/commands/project/list.ts index 03c70e9cb..20c6ee9c4 100644 --- a/src/commands/project/list.ts +++ b/src/commands/project/list.ts @@ -395,10 +395,7 @@ export async function handleExplicit( flags: ListFlags ): Promise> { const projectResult = await withProgress( - { - message: `Fetching projects (up to ${flags.limit})...`, - json: flags.json, - }, + { message: "Fetching project...", json: flags.json }, () => withAuthGuard(() => getProject(org, projectSlug)) ); if (!projectResult.ok) {