Skip to content

Commit 0d84f14

Browse files
committed
fix(dashboard): address review — placeholder, remove separator comments
- Add descriptive placeholder 'org/title-filter' for array positional (SKILL.md now shows '<org/title-filter...>' instead of '<args...>') - Remove all ASCII art separator dividers (// --------) per style rules - Regenerate SKILL.md
1 parent 9c63af9 commit 0d84f14

File tree

4 files changed

+88
-14
lines changed

4 files changed

+88
-14
lines changed

AGENTS.md

Lines changed: 85 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -840,8 +840,91 @@ mock.module("./some-module", () => ({
840840
<!-- This section is maintained by the coding agent via lore (https://github.com/BYK/opencode-lore) -->
841841
## Long-term Knowledge
842842
843+
### Architecture
844+
845+
<!-- lore:365e4299-37cf-48e0-8f2e-8503d4a249dd -->
846+
* **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).
847+
848+
<!-- lore:019d0804-a0cc-7e78-b3bc-d3d790b2d0f2 -->
849+
* **Completion fast-path skips Sentry SDK via SENTRY\_CLI\_NO\_TELEMETRY and SQLite telemetry queue**: Shell completions (\`\_\_complete\`) set \`SENTRY\_CLI\_NO\_TELEMETRY=1\` in \`bin.ts\` before any imports, which causes \`db/index.ts\` to skip the \`createTracedDatabase\` wrapper (lazy \`require\` of telemetry.ts). This avoids loading \`@sentry/node-core/light\` (~85ms). Completion timing is recorded to \`completion\_telemetry\_queue\` SQLite table via \`queueCompletionTelemetry()\` (~1ms overhead). During normal CLI runs, \`withTelemetry()\` calls \`drainCompletionTelemetry()\` which uses \`DELETE FROM ... RETURNING\` for atomic read+delete, then emits each entry as \`Sentry.metrics.distribution("completion.duration\_ms", ...)\`. Schema version 11 added this table. The fast-path achieves ~60ms dev / ~140ms CI, with a 200ms e2e test budget.
850+
851+
<!-- lore:019c8b60-d221-718a-823b-7c2c6e4ca1d5 -->
852+
* **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}\`.
853+
854+
<!-- lore:019cb6ab-ab98-7a9c-a25f-e154a5adbbe1 -->
855+
* **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.
856+
857+
<!-- lore:019c8c72-b871-7d5e-a1a4-5214359a5a77 -->
858+
* **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). As of PR #474, SDK is \`@sentry/node-core/light\` (not \`@sentry/bun\`), reducing import cost from ~218ms to ~85ms.
859+
860+
<!-- lore:019c8b60-d21a-7d44-8a88-729f74ec7e02 -->
861+
* **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.
862+
863+
### Decision
864+
865+
<!-- lore:019c9f9c-40ee-76b5-b98d-acf1e5867ebc -->
866+
* **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.
867+
868+
<!-- lore:019c8f05-c86f-7b46-babc-5e4faebff2e9 -->
869+
* **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.
870+
871+
### Gotcha
872+
873+
<!-- lore:019d2548-3e95-7673-8e9b-47811077388a -->
874+
* **Biome noExcessiveCognitiveComplexity max 15 requires extracting helpers from command handlers**: Biome enforces \`noExcessiveCognitiveComplexity\` with max 15. Stricli \`func()\` handlers that contain fetch loops, pagination, conditional logic, and output formatting easily exceed this. Fix: extract inner loops and multi-branch logic into standalone helper functions (e.g., \`fetchDashboards()\` separate from the command's \`func()\`). This also improves testability per the existing pattern of extracting logic from \`func()\` handlers. When a function still exceeds 15 after one extraction, split further — e.g., extract page-processing logic from the fetch loop into its own function.
875+
876+
<!-- lore:019d2548-3ea0-7802-8390-aac4749d6ca6 -->
877+
* **Biome prefers .at(-1) over arr\[arr.length - 1] indexing**: Biome's \`useAtIndex\` rule flags \`arr\[arr.length - 1]\` patterns. Use \`arr.at(-1)\` instead. This applies throughout the codebase — the lint check will fail CI otherwise.
878+
879+
<!-- lore:019d2548-3ea3-7a9b-b5bb-13bf595fd509 -->
880+
* **Biome requires block statements — no single-line if/break/return**: Biome's \`useBlockStatements\` rule rejects braceless control flow like \`if (match) return match.id;\` or \`if (!nextCursor) break;\`. Always wrap in braces: \`if (match) { return match.id; }\`. Similarly, nested ternaries are banned (\`noNestedTernary\`) — use if/else chains or extract into a variable with sequential assignments.
881+
882+
<!-- lore:019c8ee1-affd-7198-8d01-54aa164cde35 -->
883+
* **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.
884+
885+
<!-- lore:70319dc2-556d-4e30-9562-e51d1b68cf45 -->
886+
* **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).
887+
888+
<!-- lore:019cb8cc-bfa8-7dd8-8ec7-77c974fd7985 -->
889+
* **Making clearAuth() async breaks model-based tests — use non-async Promise\<void> 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.
890+
891+
<!-- lore:a28c4f2a-e2b6-4f24-9663-a85461bc6412 -->
892+
* **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.
893+
894+
<!-- lore:ce43057f-2eff-461f-b49b-fb9ebaadff9d -->
895+
* **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\`.
896+
897+
<!-- lore:019d1d28-b6d7-7d6d-afd4-02d2354b27fd -->
898+
* **Sentry chunk upload API returns camelCase not snake\_case**: The Sentry chunk upload options endpoint (\`/api/0/organizations/{org}/chunk-upload/\`) returns camelCase keys (\`chunkSize\`, \`chunksPerRequest\`, \`maxRequestSize\`, \`hashAlgorithm\`), NOT snake\_case. Zod schemas for these responses should use camelCase field names. This is an exception to the typical Sentry API convention of snake\_case. Verified by direct API query. The \`AssembleResponse\` also uses camelCase (\`missingChunks\`).
899+
900+
<!-- lore:019ce2c5-c9b0-7151-9579-5273c0397203 -->
901+
* **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.
902+
903+
<!-- lore:019c8bbe-bc63-7b5e-a4e0-de7e968dcacb -->
904+
* **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.
905+
906+
<!-- lore:019d2548-3ea7-7822-be37-528e7710490f -->
907+
* **Upgrade command tests are flaky when run in full suite due to test ordering**: The \`upgradeCommand.func\` tests in \`test/commands/cli/upgrade.test.ts\` sometimes fail when run as part of the full \`bun run test:unit\` suite but pass consistently in isolation (\`bun test test/commands/cli/upgrade.test.ts\`). This is a test-ordering/state-leak issue, not a code bug. Don't chase these failures when your changes are unrelated to the upgrade command.
908+
843909
### Pattern
844910
845-
<!-- lore:019d2495-54da-7506-b6ba-fee422164bca -->
846-
* **Target argument 4-mode parsing convention (project-search-first)**: \`parseOrgProjectArg()\` in \`src/lib/arg-parsing.ts\` returns a 4-mode discriminated union: \`auto-detect\` (empty), \`explicit\` (\`org/project\`), \`org-all\` (\`org/\` trailing slash), \`project-search\` (bare slug). Bare slugs are ALWAYS \`project-search\` first. The "is this an org?" check is secondary: list commands with \`orgSlugMatchBehavior\` pre-check cached orgs (\`redirect\` or \`error\` mode), and \`handleProjectSearch()\` has a safety net checking orgs after project search fails. Non-list commands (init, view) treat bare slugs purely as project search with no org pre-check. For \`init\`, unmatched bare slugs become new project names. Key files: \`src/lib/arg-parsing.ts\` (parsing), \`src/lib/org-list.ts\` (dispatch + org pre-check), \`src/lib/resolve-target.ts\` (resolution cascade).
911+
<!-- lore:019cb100-4630-79ac-8a13-185ea3d7bbb7 -->
912+
* **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).
913+
914+
<!-- lore:d441d9e5-3638-4b5a-8148-f88c349b8979 -->
915+
* **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).
916+
917+
<!-- lore:019ce2c5-c9a8-7219-bdb8-154ead871d27 -->
918+
* **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.
919+
920+
<!-- lore:019d1d28-b6ce-7257-be10-df456e2f9470 -->
921+
* **ZipWriter and file handle cleanup: always use try/finally with close()**: ZipWriter in \`src/lib/sourcemap/zip.ts\` has a \`close()\` method for error cleanup and \`finalize()\` uses try/finally to ensure \`fh.close()\` runs even if central directory writes fail. Callers (like \`buildArtifactBundle\`) must wrap ZipWriter usage in try/catch and call \`zip.close()\` in the catch path. The \`finalize()\` method handles its own cleanup internally. Pattern: create zip → try { add entries + finalize } catch { zip.close(); throw }. This prevents file handle leaks when entry writes or finalization fail partway through.
922+
923+
### Preference
924+
925+
<!-- lore:019d0804-a0eb-7ed5-aec9-d4f35af2fded -->
926+
* **Code style: Array.from() over spread for iterators, allowlist not whitelist**: User prefers \`Array.from(map.keys())\` over \`\[...map.keys()]\` for converting iterators to arrays (avoids intermediate spread). Use "allowlist" terminology instead of "whitelist" in comments and variable names. When a reviewer asks "Why not .filter() here?" — it may be a question, not a change request; the \`for..of\` loop may be intentionally more efficient. Confirm intent before changing.
927+
928+
<!-- lore:019cb3e6-da61-7dfe-83c2-17fe3257bece -->
929+
* **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\`.
847930
<!-- End lore-managed section -->

plugins/sentry-cli/skills/sentry-cli/SKILL.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -269,7 +269,7 @@ CLI-related commands
269269

270270
Manage Sentry dashboards
271271

272-
- `sentry dashboard list <args...>` — List dashboards
272+
- `sentry dashboard list <org/title-filter...>` — List dashboards
273273
- `sentry dashboard view <args...>` — View a dashboard
274274
- `sentry dashboard create <args...>` — Create a dashboard
275275
- `sentry dashboard widget add <args...>` — Add a widget to a dashboard

plugins/sentry-cli/skills/sentry-cli/references/dashboards.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@ requires:
1111

1212
Manage Sentry dashboards
1313

14-
### `sentry dashboard list <args...>`
14+
### `sentry dashboard list <org/title-filter...>`
1515

1616
List dashboards
1717

src/commands/dashboard/list.ts

Lines changed: 1 addition & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -60,9 +60,7 @@ type DashboardListResult = {
6060
allTitles?: string[];
6161
};
6262

63-
// ---------------------------------------------------------------------------
6463
// Extended cursor encoding
65-
// ---------------------------------------------------------------------------
6664

6765
/**
6866
* Encode a cursor with an optional dashboard-ID bookmark for mid-page resume.
@@ -110,9 +108,7 @@ export function decodeCursor(cursor: string): {
110108
};
111109
}
112110

113-
// ---------------------------------------------------------------------------
114111
// Human output
115-
// ---------------------------------------------------------------------------
116112

117113
/**
118114
* Format dashboard list for human-readable terminal output.
@@ -162,9 +158,7 @@ function formatDashboardListHuman(result: DashboardListResult): string {
162158
return parts.join("").trimEnd();
163159
}
164160

165-
// ---------------------------------------------------------------------------
166161
// JSON transform
167-
// ---------------------------------------------------------------------------
168162

169163
/**
170164
* Transform dashboard list result for JSON output.
@@ -191,9 +185,7 @@ function jsonTransformDashboardList(
191185
return envelope;
192186
}
193187

194-
// ---------------------------------------------------------------------------
195188
// Fetch with pagination + optional client-side glob filter
196-
// ---------------------------------------------------------------------------
197189

198190
/** Result of the paginated fetch loop */
199191
type FetchResult = {
@@ -326,9 +318,7 @@ async function fetchDashboards(
326318
return { dashboards: results, cursorToStore, allTitles };
327319
}
328320

329-
// ---------------------------------------------------------------------------
330321
// Command
331-
// ---------------------------------------------------------------------------
332322

333323
export const listCommand = buildListCommand("dashboard", {
334324
docs: {
@@ -356,6 +346,7 @@ export const listCommand = buildListCommand("dashboard", {
356346
positional: {
357347
kind: "array",
358348
parameter: {
349+
placeholder: "org/title-filter",
359350
brief: "[<org/project>] [<name-glob>]",
360351
parse: String,
361352
},

0 commit comments

Comments
 (0)