From 4651aae46e01d1e0bc81f3e90ee888ea36df4013 Mon Sep 17 00:00:00 2001 From: Burak Yigit Kaya Date: Tue, 3 Mar 2026 01:07:28 +0000 Subject: [PATCH] fix(trace-logs): timestamp_precise is a number, not a string The trace-logs API returns timestamp_precise as a nanosecond integer, same as the Explore/Events logs endpoint. The schema incorrectly declared it as z.string() which caused Zod validation failures at runtime. --- AGENTS.md | 102 +++++-------------------------- src/types/sentry.ts | 6 +- test/commands/trace/logs.test.ts | 6 +- 3 files changed, 20 insertions(+), 94 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index 0a0aff0f0..6fdf77c2d 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -628,82 +628,28 @@ 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). - -* **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 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}\`. - -* **Sentry CLI has two distribution channels with different runtimes**: Sentry CLI ships via two pipelines: (1) Standalone binary — \`Bun.build()\` with \`compile: true\`, produces native executables with embedded Bun runtime. (2) npm package — esbuild produces CJS \`dist/bin.cjs\` requiring Node.js 22+. esbuild aliases \`@sentry/bun\` → \`@sentry/node\` and injects Bun API polyfills from \`script/node-polyfills.ts\` covering \`Bun.file()\`, \`Bun.write()\`, \`Bun.which()\`, \`Bun.spawn()\`, \`Bun.sleep()\`, \`Bun.Glob\`, \`Bun.randomUUIDv7()\`, \`Bun.semver.order()\`, and \`bun:sqlite\`. \`Bun.$\` has NO polyfill — use \`execSync\` from \`node:child\_process\` instead. \`require()\` in ESM is safe: Bun supports it natively, and esbuild resolves it at bundle time. - -* **@sentry/node-core pulls in @apm-js-collab/code-transformer (3.3MB) but it's tree-shaken**: The dependency chain \`@sentry/node\` → \`@sentry/node-core\` → \`@apm-js-collab/code-transformer\` adds 3.3MB in esbuild metafile inputs but contributes 0 bytes to output (fully tree-shaken). When analyzing bundle size, use \`metafile.outputs\[outfile].inputs\` for actual per-file contribution, not \`metafile.inputs\` which shows pre-minification sizes. - -* **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. The \`explicit-org-numeric\` case uses \`getIssueInOrg\`. \`resolveOrgAndIssueId\` no longer throws for bare numeric IDs when permalink contains the org slug. - -* **parseSentryUrl does not handle subdomain-style SaaS URLs**: parseSentryUrl in src/lib/sentry-url-parser.ts handles both path-based (\`/organizations/{org}/...\`) and subdomain-style (\`https://{org}.sentry.io/issues/123/\`) URLs. \`matchSubdomainOrg()\` extracts org from hostname ending in \`.sentry.io\`. Region subdomains (\`us\`, \`de\`) filtered by requiring org slug length > 2. Supports \`/issues/{id}/\`, \`/issues/{id}/events/{eventId}/\`, and \`/traces/{traceId}/\` paths. Self-hosted uses path-based only. - -* **npm bundle requires Node.js >= 22 due to node:sqlite polyfill**: The Sentry CLI npm package (dist/bin.cjs) requires Node.js >= 22 because the bun:sqlite polyfill uses Node.js 22's built-in \`node:sqlite\` module. The \`require('node:sqlite')\` happens during module loading (via esbuild's inject option) — before any user code runs. package.json declares \`engines: { node: '>=22' }\` but pnpm/npm don't enforce this by default. A runtime version guard in the esbuild banner catches this early with a clear error message pointing users to either upgrade Node.js or use the standalone binary (\`curl -fsSL https://cli.sentry.dev/install | bash\`). - -* **Install script nightly channel support**: The curl install script (\`cli.sentry.dev/install\`) supports \`--channel nightly\`. For nightly: downloads the binary directly from the \`nightly\` release tag (no version.json fetch needed — just uses \`nightly\` as both download tag and display string). Passes \`--channel nightly\` to \`$binary cli setup --install --method curl --channel nightly\` so the channel is persisted in DB. Usage: \`curl -fsSL https://cli.sentry.dev/install | bash -s -- --channel nightly\`. The install script does NOT fetch version.json — that's only used by the upgrade/version-check flow to compare versions. - -* **cli.sentry.dev is served from gh-pages branch via GitHub Pages**: \`cli.sentry.dev\` is served from gh-pages branch via GitHub Pages. Contains Astro/Starlight docs plus \`/install\` script. Craft's gh-pages target runs \`git rm -r -f .\` before extracting docs, wiping ALL files. Non-docs files would be destroyed on each stable release. To persist extra files, use \`postReleaseCommand\` in \`.craft.yml\`. GitHub Packages has no generic file registry — only typed registries (Container/npm/Maven/etc). For binary distribution without Releases, GHCR+ORAS is the only viable option but requires 3-step download. + +* **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 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. + +* **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\`. + +* **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 pages). \`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()\` helper; explicit \`--cursor\` does single-page fetch to preserve cursor chain. ### Decision - -* **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. - -* **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. - -* **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. - -* **Raw markdown output for non-interactive terminals, rendered for TTY**: Output raw CommonMark when stdout is not a TTY; render through marked-terminal only for TTY. Detection: \`process.stdout.isTTY\`. Override precedence: \`SENTRY\_PLAIN\_OUTPUT\` > \`NO\_COLOR\` > auto-detect. \`--json\` always outputs JSON. Streaming formatters (log/trace) use ANSI-colored text for TTY, markdown table rows for non-TTY. * **Use -t (not -p) as shortcut alias for --period flag**: The --period flag on issue list uses -t (for 'time period') as its short alias, not -p. The rationale: -p could be confused with --platform from other CLI tools/contexts. -t maps naturally to 'time period' and avoids collision. This was a deliberate choice after initial implementation used -p. ### Gotcha - -* **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\`. - -* **pagination\_cursors table schema mismatch requires repair migration**: The pagination\_cursors SQLite table may have wrong PK (single-column instead of composite). Migration 5→6 detects via \`hasCompositePrimaryKey()\` and drops/recreates. \`repairWrongPrimaryKeys()\` in \`repairSchema()\` handles auto-repair. \`isSchemaError()\` catches 'on conflict clause does not match' to trigger repair. Data loss acceptable since cursors are ephemeral (5-min TTL). - -* **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. - -* **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). - -* **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. - -* **Multiregion mock must include all control silo API routes**: When changing which Sentry API endpoint a function uses (e.g., switching getCurrentUser() from /users/me/ to /auth/), the mock route must be updated in BOTH test/mocks/routes.ts (single-region) AND test/mocks/multiregion.ts createControlSiloRoutes() (multi-region). Missing the multiregion mock causes 404s in multi-region test scenarios. The multiregion control silo mock serves auth, user info, and region discovery routes. Cursor Bugbot caught this gap when /api/0/auth/ was added to routes.ts but not multiregion.ts. - -* **resolveCursor must be called inside org-all closure, not before dispatch**: In list commands using dispatchOrgScopedList with cursor pagination (e.g., project/list.ts), resolveCursor() must be called inside the 'org-all' override closure, not before dispatchOrgScopedList. If called before, it throws a ContextError before dispatch can throw the correct ValidationError for --cursor being used in non-org-all modes. - -* **ghcr.io blob download: curl -L breaks because auth header leaks to Azure redirect**: ghcr.io gotchas: (1) \`curl -L\` with Authorization header fails on blob downloads because curl forwards the auth header to the Azure redirect target (404). Fix: two-step curl — extract redirect URL without following, then fetch without auth. (2) New GHCR packages default to private. Anonymous pulls fail until manually made public via GitHub UI package settings. This is a one-time step per package. - -* **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\`. - -* **Multiple mockFetch calls replace each other — use unified mocks for multi-endpoint tests**: In getsentry/cli tests, \`mockFetch()\` replaces \`globalThis.fetch\`. Calling it twice replaces the first mock entirely. Fix: create a single unified fetch mock handling ALL endpoints the test needs, dispatching by URL pattern. - -* **@sentry/api SDK passes Request object to custom fetch — headers lost on Node.js**: The @sentry/api SDK calls \`\_fetch(request)\` with only a Request object (no init). In \`authenticatedFetch\`, \`init\` is undefined, so \`prepareHeaders(init, token)\` creates empty headers. On Node.js, init headers replace Request headers entirely — stripping Content-Type (HTTP 415 on POST). Fix: \`prepareHeaders\` must fall back to \`input.headers\` when \`init\` is undefined, and extract method from \`input\` not just \`init\`. File SDK issues at https://github.com/getsentry/sentry-api-schema/ (@MathurAditya724). Note: the Link header / pagination issues (sentry-api-schema#59, #60) were fixed in @sentry/api 0.21.0 — all list functions now use the SDK with \`unwrapPaginatedResult\` instead of raw HTTP workarounds. - -* **mock.module bleeds across Bun test files — never include test/isolated/ in test:unit**: Bun's \`mock.module()\` pollutes the shared module registry for ALL subsequently-loaded test files in the same \`bun test\` invocation. Tests using it must live in \`test/isolated/\` and run via \`test:isolated\` (never \`test:unit\`). Consequence: isolated test coverage doesn't appear in Codecov. For \`Bun.spawn\` (which is writable on the global \`Bun\` object), prefer direct property assignment (\`Bun.spawn = mockFn\`) in \`beforeEach\`/\`afterEach\` — this works in regular test files and counts toward coverage. - -* **GitHub Actions: use deterministic timestamps across jobs, not Date.now()**: GitHub Actions gotchas: (1) Use deterministic timestamps across jobs — never use \`Date.now()\` independently per job. Derive from \`github.event.head\_commit.timestamp\` converted to Unix seconds. (2) Skipped \`needs\` jobs don't block downstream — outputs are empty strings, use conditional \`if: needs.job.outputs.value != ''\` to skip steps. (3) upload-artifact strips directory prefixes from glob paths — \`dist-bin/sentry-\*\` stores as \`sentry-\*\` at root. download-artifact puts files at \`artifacts/sentry-\*\`, not \`artifacts/dist-bin/sentry-\*\`. - -* **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.\` - -* **Several commands bypass telemetry by importing buildCommand from @stricli/core directly**: src/lib/command.ts wraps Stricli's buildCommand to auto-capture flag/arg telemetry via Sentry. But trace/list, trace/view, log/view, api.ts, and help.ts import buildCommand directly from @stricli/core, silently skipping telemetry. Fix: change their imports to use ../../lib/command.js. Consider adding a Biome lint rule (noRestrictedImports equivalent) to prevent future regressions. - -* **Esbuild banner template literal double-escape for newlines**: When using esbuild's \`banner\` option with a TypeScript template literal containing string literals that need \`\n\` escape sequences: use \`\\\\\\\n\` in the TS source. The chain is: TS template literal \`\\\\\\\n\` → esbuild banner output \`\\\n\` → JS runtime interprets as newline. Using only \`\\\n\` in the TS source produces a literal newline character inside a JS double-quoted string, which is a SyntaxError. This applies to any esbuild banner/footer that injects JS strings containing escape sequences. Discovered in script/bundle.ts for the Node.js version guard error message. - -* **GitHub immutable releases prevent rolling nightly tag pattern**: getsentry/cli has immutable releases enabled. Once published, assets can't be modified and the tag can NEVER be reused. Nightly uses per-version tags (e.g., \`0.13.0-dev.1772062077\`) with API-based latest discovery. The publish-nightly job deletes all existing assets before uploading (not \`--clobber\`) to prevent stale assets. - -* **sudo scripts: $HOME resolves to /root, use SUDO\_USER for original user paths**: When a script runs under sudo, $HOME resolves to /root, not the invoking user's home. SSH keys, config files, etc. referenced via $HOME will fail. Fix: use \`REAL\_HOME=$(eval echo ~${SUDO\_USER:-$USER})\` at the top of the script and reference that instead of $HOME for user-owned paths like ~/.ssh/id\_ed25519. + +* **React useState async pitfall**: React useState setter is async — reading state immediately after setState returns stale value in dashboard components + +* **TypeScript strict mode caveat**: TypeScript strict null checks require explicit undefined handling * **getsentry/codecov-action enables JUnit XML test reporting by default**: The \`getsentry/codecov-action@main\` has \`enable-tests: true\` by default, which searches for JUnit XML files matching \`\*\*/\*.junit.xml\`. If the test framework doesn't produce JUnit XML, the action emits 3 warnings on every CI run: "No files found matching pattern", "No JUnit XML files found", and "Please ensure your test framework is generating JUnit XML output". Fix: either set \`enable-tests: false\` in the action inputs, or configure the test runner to output JUnit XML. For Bun, add \`\[test.reporter] junit = "test-results.junit.xml"\` to \`bunfig.toml\` and add \`\*.junit.xml\` to \`.gitignore\`. - -* **OpenCode worktrees accumulate massive .test-tmp dirs — clean periodically**: OpenCode worktrees at \`~/.local/share/opencode/worktree/\` accumulate massive \`.test-tmp/\` dirs (14+ GB in getsentry/cli). Worktrees are ephemeral — safe to delete entirely between sessions. Targeted: \`rm -rf ~/.local/share/opencode/worktree/\*/.test-tmp\`. Also: \`git checkout main\` fails because main is used by the worktree. Workaround: always use \`origin/main\` — \`git checkout -b \ origin/main\` or rebase onto \`origin/main\`, never the local \`main\` branch. * **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. @@ -713,28 +659,12 @@ mock.module("./some-module", () => ({ ### Pattern - -* **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). - -* **Sentry CLI issue resolution wraps getIssue 404 in ContextError with ID hint**: In resolveIssue() (src/commands/issue/utils.ts), bare getIssue() calls for numeric and explicit-org-numeric cases should catch ApiError with status 404 and re-throw as ContextError that includes: (1) the ID the user provided, (2) a hint about access/deletion, (3) a suggestion to use short-ID format (\-\) if the user confused numeric group IDs with short-ID suffixes. Without this, the user gets a generic 'Issue not found' without knowing which ID failed or what to try instead. - -* **Sentry CLI setFlagContext redacts sensitive flags before telemetry**: The setFlagContext() function in src/lib/telemetry.ts must redact sensitive flag values (like --token) before setting Sentry tags. A SENSITIVE\_FLAGS set contains flag names that should have their values replaced with '\[REDACTED]' instead of the actual value. This prevents secrets from leaking into telemetry. The scrub happens at the source (in setFlagContext itself) rather than in beforeSend, so the sensitive value never reaches the Sentry SDK. - -* **Sentry CLI api command: normalizeFields auto-corrects colon separators with stderr warning**: The \`sentry api\` command's field flags require \`key=value\` format. Key implementation details: (1) \`normalizeFields()\` auto-corrects \`:\` to \`=\` with stderr warning, but skips JSON-shaped strings (starting with \`{\` or \`\[\`). (2) \`--data\`/\`-d\` flag handles inline JSON bodies, mutually exclusive with \`--input\` and field flags — mixing throws \`ValidationError\`. (3) \`extractJsonBody()\` scans fields for bare JSON and extracts as body with hint about \`--data\`, but is skipped for GET requests (GET bodies are silently ignored by servers). (4) \`tryParseJsonField\` needs the \`startsWith('{')\` guard — not just optimization, prevents \`TypeError\` when \`in\` operator hits a primitive. (5) When JSON body is merged with remaining fields, top-level key conflicts throw \`ValidationError\` (shallow merge drops nested fields). (6) Array JSON bodies cannot be mixed with fields. - -* **Org-scoped SDK calls follow getOrgSdkConfig + unwrapResult pattern**: All org-scoped API calls in src/lib/api-client.ts follow this pattern: (1) call \`getOrgSdkConfig(orgSlug)\` which resolves the org's regional URL and returns an SDK client config, (2) spread config into the SDK function call: \`{ ...config, path: { organization\_id\_or\_slug: orgSlug, ... } }\`, (3) pass result to \`unwrapResult(result, errorContext)\`. There are 14+ usages of this pattern. The \`getOrgSdkConfig\` function (line ~167) calls \`resolveOrgRegion(orgSlug)\` then \`getSdkConfig(regionUrl)\`. Follow this exact pattern when adding new org-scoped endpoints like \`getIssueInOrg\`. - -* **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 required or advisory checks. Both typically take 2-3 minutes. 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\`. - -* **Markdown table structure for marked-terminal: blank header row + separator + data rows**: Markdown tables for marked-terminal: use blank header row (\`| | |\`), separator (\`|---|---|\`), then data rows (\`| \*\*Label\*\* | value |\`). Data rows before the separator produce malformed output. When escaping user content in table cells, escape backslashes first then pipes: \`str.replace(/\\\\/g, '\\\\\\\\').replace(/\\|/g, '\\\\|')\`. This is centralized as \`escapeMarkdownCell()\` in \`src/lib/formatters/markdown.ts\` — all formatters must use it (CodeQL flags incomplete escaping as high severity). Formatters return \`string\`; test with \`result.includes(...)\` since rendered markdown uses Unicode box-drawing characters. - -* **resolveAllTargets/resolveOrgAndProject should not call fetchProjectId — callers enrich**: The shared helpers \`resolveAllTargets\` and \`resolveOrgAndProject\` must NOT call \`fetchProjectId\`/\`getProject\` — many commands don't need projectId. Commands that need it (like \`issue list\`) should enrich targets themselves with silent fallback. \`toNumericId()\` validates with \`Number.isInteger(n) && n > 0\` (not truthiness) and accepts \`null\` since API responses can return null IDs. + +* **Kubernetes deployment pattern**: Use helm charts for Kubernetes deployments with resource limits * **Make Bun.which testable by accepting optional PATH parameter**: When wrapping \`Bun.which()\` in a helper function, accept an optional \`pathEnv?: string\` parameter and pass it as \`{ PATH: pathEnv }\` to \`Bun.which\`. This makes the function deterministically testable without mocking — tests can pass a controlled PATH (e.g., \`/nonexistent\` for false, \`dirname(Bun.which('bash'))\` for true). Pattern: \`const opts = pathEnv !== undefined ? { PATH: pathEnv } : undefined; return Bun.which(name, opts) !== null;\` * **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. - -* **Re-run prior art search when task scope pivots**: When a conversation pivots from one approach to another, always run a fresh prior-art search with keywords specific to the new scope. The failure mode: anchoring on the original framing's keywords and never re-searching. Simple searches like \`gh pr list --search "embed"\` would have surfaced a near-identical open PR. Treat each material scope change as a new research task. * **Multi-target concurrent progress needs per-target delta tracking**: When multiple targets fetch concurrently and each reports cumulative progress, maintain a \`prevFetched\` array and \`totalFetched\` running sum. Each callback computes \`delta = fetched - prevFetched\[i]\`, adds to total. This prevents display jumps and double-counting. Use \`totalFetched += delta\` (O(1)), not \`reduce()\` on every callback. @@ -744,10 +674,6 @@ mock.module("./some-module", () => ({ ### Preference - -* **Progress message format: 'N and counting (up to M)...' pattern**: User prefers progress messages that frame the limit as a ceiling rather than an expected total. Format: \`Fetching issues, 30 and counting (up to 50)...\` — not \`Fetching issues... 30/50\`. The 'up to' framing makes it clear the denominator is a max, not an expected count, avoiding confusion when fewer items exist than the limit. For multi-target fetches, include target count: \`Fetching issues from 10 projects, 30 and counting (up to 50)...\`. Initial message before any results: \`Fetching issues (up to 50)...\` or \`Fetching issues from 10 projects (up to 50)...\`. - -* **CI scripts: prefer jq/sed over node -e for JSON manipulation**: Reviewer (BYK) prefers using standard Unix tools (\`jq\`, \`sed\`, \`awk\`) over \`node -e\` for simple JSON manipulation in CI workflow scripts. For example, reading/modifying package.json version: \`jq -r .version package.json\` to read, \`jq --arg v "$NEW" '.version = $v' package.json > tmp && mv tmp package.json\` to write. This avoids requiring Node.js to be installed in CI steps that only need basic JSON operations, and is more readable for shell-centric workflows. * **Use captureException (not captureMessage) for unexpected states, or Sentry logs**: When reporting unexpected/defensive-guard situations to Sentry (e.g., non-numeric input where a number was expected), the reviewer prefers \`Sentry.captureException(new Error(...))\` over \`Sentry.captureMessage(...)\`. \`captureMessage\` with 'warning' level was rejected in PR review. Alternatively, use the Sentry structured logger (\`Sentry.logger.warn(...)\`) for less severe diagnostic cases — this was accepted in the abbreviateCount NaN handler. diff --git a/src/types/sentry.ts b/src/types/sentry.ts index c3133b02a..ee2294e59 100644 --- a/src/types/sentry.ts +++ b/src/types/sentry.ts @@ -578,7 +578,7 @@ export type DetailedLogsResponse = z.infer; * Key differences from {@link SentryLog} (Explore/Events): * - `id` instead of `sentry.item_id` * - Includes `project.id` (integer) and `severity_number` - * - `timestamp_precise` is an ISO string (not a nanosecond integer) + * - `timestamp_precise` is a nanosecond integer (same as Explore/Events logs) */ export const TraceLogSchema = z .object({ @@ -594,8 +594,8 @@ export const TraceLogSchema = z severity: z.string(), /** ISO 8601 timestamp */ timestamp: z.string(), - /** High-precision ISO 8601 timestamp */ - timestamp_precise: z.string(), + /** High-precision timestamp in nanoseconds */ + timestamp_precise: z.number(), /** Log message content */ message: z.string().nullable().optional(), }) diff --git a/test/commands/trace/logs.test.ts b/test/commands/trace/logs.test.ts index 47ef70fde..f54e91b7e 100644 --- a/test/commands/trace/logs.test.ts +++ b/test/commands/trace/logs.test.ts @@ -159,7 +159,7 @@ const sampleLogs: TraceLog[] = [ severity_number: 9, severity: "info", timestamp: "2025-01-30T14:32:15+00:00", - timestamp_precise: "2025-01-30T14:32:15.123456+00:00", + timestamp_precise: 1_738_247_535_123_456_000, message: "Request received", }, { @@ -169,7 +169,7 @@ const sampleLogs: TraceLog[] = [ severity_number: 13, severity: "warn", timestamp: "2025-01-30T14:32:16+00:00", - timestamp_precise: "2025-01-30T14:32:16.456789+00:00", + timestamp_precise: 1_738_247_536_456_789_000, message: "Slow query detected", }, { @@ -179,7 +179,7 @@ const sampleLogs: TraceLog[] = [ severity_number: 17, severity: "error", timestamp: "2025-01-30T14:32:17+00:00", - timestamp_precise: "2025-01-30T14:32:17.789012+00:00", + timestamp_precise: 1_738_247_537_789_012_000, message: "Database connection failed", }, ];