Skip to content

Commit 53be35d

Browse files
committed
fix: clean up upgrade output and hide empty table headers
Clean up the ugly `cli upgrade` output and fix a systemic issue with empty header rows in box-drawing tables across the codebase. ## Upgrade output Before: ``` Binary: Installed to /home/byk/.local/bin/sentry ✓ Upgraded to 0.18.0-dev.1773836960 0.18.0-dev.1773794950 → 0.18.0-dev.1773836960 ╭─────────┬─────────╮ │ │ │ ├─────────┼─────────┤ │ Method │ curl │ │ Channel │ nightly │ ╰─────────┴─────────╯ ``` After: ``` ✓ Upgraded to 0.18.0-dev.1773836960 (from 0.18.0-dev.1773794950) Method: curl · Channel: nightly ``` Changes: - Replace metadata table with a compact muted inline line - Remove redundant version arrow; fold "from" version into success line - Add `--quiet` to `cli setup` subprocess to suppress leaked stdout ## Systemic table fix `mdKvTable()` without a heading produces `| | |` empty header cells, which rendered as an ugly empty bordered row in box-drawing tables. This affected 12 call sites: auth status, issue/org/project details, trace/span/log views, and more. Fix: add `hideHeaders` option to `renderTextTable` and auto-detect all-empty headers in `renderTableToken` to skip the header row while still using it for column width measurement.
1 parent b58dd1b commit 53be35d

File tree

9 files changed

+186
-107
lines changed

9 files changed

+186
-107
lines changed

AGENTS.md

Lines changed: 0 additions & 69 deletions
Original file line numberDiff line numberDiff line change
@@ -781,73 +781,4 @@ mock.module("./some-module", () => ({
781781
| Add documentation | `docs/src/content/docs/` |
782782
783783
<!-- This section is maintained by the coding agent via lore (https://github.com/BYK/opencode-lore) -->
784-
## Long-term Knowledge
785-
786-
### Architecture
787-
788-
<!-- lore:019ce2be-39f1-7ad9-a4c5-4506b62f689c -->
789-
* **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.
790-
791-
<!-- lore:019cb8ea-c6f0-75d8-bda7-e32b4e217f92 -->
792-
* **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\`.
793-
794-
<!-- lore:019c978a-18b5-7a0d-a55f-b72f7789bdac -->
795-
* **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.
796-
797-
<!-- lore:019cbe93-19b8-7776-9705-20bbde226599 -->
798-
* **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.
799-
800-
<!-- lore:2c3eb7ab-1341-4392-89fd-d81095cfe9c4 -->
801-
* **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.
802-
803-
<!-- lore:019c972c-9f0f-75cd-9e24-9bdbb1ac03d6 -->
804-
* **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.
805-
806-
<!-- lore:019ce0bb-f35d-7380-b661-8dc56f9938cf -->
807-
* **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.
808-
809-
### Decision
810-
811-
<!-- lore:019c99d5-69f2-74eb-8c86-411f8512801d -->
812-
* **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.
813-
814-
<!-- lore:00166785-609d-4ab5-911e-ee205d17b90c -->
815-
* **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.
816-
817-
### Gotcha
818-
819-
<!-- lore:019c8ab6-d119-7365-9359-98ecf464b704 -->
820-
* **@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.
821-
822-
<!-- lore:019c9e98-7af4-7e25-95f4-fc06f7abf564 -->
823-
* **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.\`
824-
825-
<!-- lore:019c9776-e3dd-7632-88b8-358a19506218 -->
826-
* **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\`.
827-
828-
<!-- lore:019cb8c2-d7b5-780c-8a9f-d20001bc198f -->
829-
* **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\`.
830-
831-
<!-- lore:019c969a-1c90-7041-88a8-4e4d9a51ebed -->
832-
* **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\`.
833-
834-
<!-- lore:019c9741-d78e-73b1-87c2-e360ef6c7475 -->
835-
* **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\`.
836-
837-
### Pattern
838-
839-
<!-- lore:019c972c-9f11-7c0d-96ce-3f8cc2641175 -->
840-
* **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.
841-
842-
<!-- lore:5ac4e219-ea1f-41cb-8e97-7e946f5848c0 -->
843-
* **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 \<PR> --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\`.
844-
845-
<!-- lore:019cb162-d3ad-7b05-ab4f-f87892d517a6 -->
846-
* **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.
847-
848-
<!-- lore:019cbd5f-ec35-7e2d-8386-6d3a67adf0cf -->
849-
* **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).
850-
851-
<!-- lore:019cc43d-e651-7154-a88e-1309c4a2a2b6 -->
852-
* **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'\`).
853784
<!-- End lore-managed section -->

src/commands/cli/upgrade.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -379,6 +379,7 @@ async function runSetupOnNewBinary(opts: SetupOptions): Promise<void> {
379379
const args = [
380380
"cli",
381381
"setup",
382+
"--quiet",
382383
"--method",
383384
method,
384385
"--channel",

src/lib/formatters/human.ts

Lines changed: 16 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -2071,8 +2071,8 @@ const ACTION_DESCRIPTIONS: Record<UpgradeResult["action"], string> = {
20712071
/**
20722072
* Format upgrade result as rendered markdown.
20732073
*
2074-
* Produces a concise summary line with the action taken, version info,
2075-
* and any warnings (e.g., PATH shadowing from old package manager install).
2074+
* Produces a concise summary: action line, compact metadata, and any
2075+
* warnings (e.g., PATH shadowing from old package manager install).
20762076
* Designed as the `human` formatter for the `cli upgrade` command's
20772077
* {@link OutputConfig}.
20782078
*
@@ -2086,13 +2086,17 @@ export function formatUpgradeResult(data: UpgradeResult): string {
20862086
case "upgraded":
20872087
case "downgraded": {
20882088
const verb = ACTION_DESCRIPTIONS[data.action];
2089-
const offlineNote = data.offline ? " (offline, from cache)" : "";
2090-
lines.push(
2091-
`${colorTag("green", "✓")} ${verb} to ${safeCodeSpan(data.targetVersion)}${escapeMarkdownInline(offlineNote)}`
2092-
);
2093-
if (data.currentVersion !== data.targetVersion) {
2089+
if (data.offline) {
2090+
lines.push(
2091+
`${colorTag("green", "✓")} ${verb} to ${safeCodeSpan(data.targetVersion)}${escapeMarkdownInline(" (offline, from cache)")}`
2092+
);
2093+
} else if (data.currentVersion !== data.targetVersion) {
2094+
lines.push(
2095+
`${colorTag("green", "✓")} ${verb} to ${safeCodeSpan(data.targetVersion)} ${escapeMarkdownInline(`(from ${data.currentVersion})`)}`
2096+
);
2097+
} else {
20942098
lines.push(
2095-
`${escapeMarkdownInline(data.currentVersion)} ${escapeMarkdownInline(data.targetVersion)}`
2099+
`${colorTag("green", "✓")} ${verb} to ${safeCodeSpan(data.targetVersion)}`
20962100
);
20972101
}
20982102
break;
@@ -2123,19 +2127,13 @@ export function formatUpgradeResult(data: UpgradeResult): string {
21232127
}
21242128
}
21252129

2126-
// Metadata table — renders method, channel, and version info below the
2127-
// action line so diagnostics go to stdout (via the markdown pipeline)
2128-
// instead of cluttering stderr with repeated log.info() lines.
2129-
const kvRows: [string, string][] = [
2130-
["Method", escapeMarkdownInline(data.method)],
2131-
["Channel", escapeMarkdownInline(data.channel)],
2132-
];
2133-
lines.push("");
2134-
lines.push(mdKvTable(kvRows));
2130+
// Compact metadata line instead of a full table — method and channel
2131+
// are lightweight diagnostics that don't warrant box-drawing borders.
2132+
const meta = `Method: ${data.method} · Channel: ${data.channel}`;
2133+
lines.push(` ${colorTag("muted", escapeMarkdownInline(meta))}`);
21352134

21362135
// Append warnings with ⚠ markers
21372136
if (data.warnings && data.warnings.length > 0) {
2138-
lines.push("");
21392137
for (const warning of data.warnings) {
21402138
lines.push(`${colorTag("yellow", "⚠")} ${escapeMarkdownInline(warning)}`);
21412139
}

src/lib/formatters/markdown.ts

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -554,6 +554,10 @@ function renderList(list: Tokens.List, depth = 0): string {
554554
*
555555
* Converts marked's `Tokens.Table` into headers + rows + alignments and
556556
* delegates to `renderTextTable()` for column fitting and box drawing.
557+
*
558+
* When all header cells are empty (e.g. from {@link mdKvTable} without a
559+
* heading), the header row is hidden so the rendered table starts directly
560+
* with data rows — no empty cells or separator line.
557561
*/
558562
function renderTableToken(table: Tokens.Table): string {
559563
const headers = table.header.map((cell) => renderInline(cell.tokens));
@@ -571,7 +575,11 @@ function renderTableToken(table: Tokens.Table): string {
571575
return "left";
572576
});
573577

574-
return renderTextTable(headers, rows, { alignments });
578+
// Hide the header row when all cells are empty — avoids rendering a
579+
// visually empty bordered row above the data (common in mdKvTable output).
580+
const hideHeaders = headers.every((h) => h.trim() === "");
581+
582+
return renderTextTable(headers, rows, { alignments, hideHeaders });
575583
}
576584

577585
// ──────────────────────── Public API ─────────────────────────────────

src/lib/formatters/text-table.ts

Lines changed: 21 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -56,6 +56,17 @@ export type TextTableOptions = {
5656
shrinkable?: boolean[];
5757
/** Truncate cells to one line with "\u2026" instead of wrapping. @default false */
5858
truncate?: boolean;
59+
/**
60+
* Hide the header row entirely (omit from rendered output).
61+
*
62+
* Headers are still used for column width measurement, but the rendered
63+
* table starts directly with data rows. Useful for key-value tables where
64+
* the markdown source requires empty header cells (`| | |`) to satisfy
65+
* table syntax, but the visual output shouldn't show them.
66+
*
67+
* @default false
68+
*/
69+
hideHeaders?: boolean;
5970
/**
6071
* Show horizontal separator lines between data rows.
6172
*
@@ -94,6 +105,7 @@ export function renderTextTable(
94105
minWidths = [],
95106
shrinkable = [],
96107
truncate = false,
108+
hideHeaders = false,
97109
rowSeparator = false,
98110
} = options;
99111

@@ -139,6 +151,7 @@ export function renderTextTable(
139151
border,
140152
cellPadding,
141153
headerSeparator,
154+
hideHeaders,
142155
rowSeparator,
143156
});
144157
}
@@ -512,6 +525,8 @@ type GridParams = {
512525
border: BorderCharacters;
513526
cellPadding: number;
514527
headerSeparator: boolean;
528+
/** Skip rendering the header row (allRows[0]) entirely. */
529+
hideHeaders: boolean;
515530
/** Draw separator between data rows. `true` for plain, or ANSI color prefix string. */
516531
rowSeparator: boolean | string;
517532
};
@@ -562,6 +577,7 @@ function renderGrid(params: GridParams): string {
562577
border,
563578
cellPadding,
564579
headerSeparator,
580+
hideHeaders,
565581
rowSeparator,
566582
} = params;
567583
const lines: string[] = [];
@@ -607,7 +623,11 @@ function renderGrid(params: GridParams): string {
607623
vert,
608624
};
609625

610-
for (let r = 0; r < allRows.length; r++) {
626+
// When hideHeaders is set, skip allRows[0] (the header) and its separator.
627+
// The header is still included in allRows for column width measurement.
628+
const startRow = hideHeaders ? 1 : 0;
629+
630+
for (let r = startRow; r < allRows.length; r++) {
611631
const wrappedCells = allRows[r] ?? [];
612632
lines.push(...renderRowLines(wrappedCells, rowCtx));
613633

0 commit comments

Comments
 (0)