Skip to content

Commit 4bfa18b

Browse files
BYKBYK
committed
fix: improve CLI output for auth login and upgrade flows
Migrate diagnostic output from cluttered stderr log.info() calls to the markdown-rendering pipeline for cleaner, more consistent formatting. **Auth login (interactive-login.ts):** - Show verification_uri_complete (with user_code embedded) instead of bare verification_uri so the URL works in headless environments - Use log.log() for URL/code detail lines instead of log.info() to avoid repeated ℹ icon prefixes - Only show copy hint when browser didn't open - Render user code as inline code span via renderInlineMarkdown() **CLI upgrade (upgrade.ts + human.ts):** - Demote method/version/channel/latest log.info() calls to log.debug() - Add metadata key-value table (method, channel) to formatUpgradeResult via mdKvTable(), rendering through the standard markdown pipeline - Keep log.info() only for progress indicators that fire before the result is ready (Upgrading to..., nightly migration) Closes #452
1 parent 011a0dc commit 4bfa18b

File tree

6 files changed

+211
-29
lines changed

6 files changed

+211
-29
lines changed

‎src/commands/cli/upgrade.ts‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -271,10 +271,10 @@ async function resolveTargetVersion(
271271
const latest = await fetchLatestVersion(method, channel);
272272
const target = versionArg?.replace(VERSION_PREFIX_REGEX, "") ?? latest;
273273

274-
log.info(`Channel: ${channel}`);
275-
log.info(`Latest version: ${latest}`);
274+
log.debug(`Channel: ${channel}`);
275+
log.debug(`Latest version: ${latest}`);
276276
if (versionArg) {
277-
log.info(`Target version: ${target}`);
277+
log.debug(`Target version: ${target}`);
278278
}
279279

280280
if (flags.check) {
@@ -639,8 +639,8 @@ export const upgradeCommand = buildCommand({
639639
const { channel, versionArg, channelChanged, method } =
640640
await resolveContext(version, flags);
641641

642-
log.info(`Installation method: ${method}`);
643-
log.info(`Current version: ${CLI_VERSION}`);
642+
log.debug(`Installation method: ${method}`);
643+
log.debug(`Current version: ${CLI_VERSION}`);
644644

645645
const resolved = await resolveTargetWithFallback({
646646
resolveOpts: { method, channel, versionArg, channelChanged, flags },

‎src/lib/formatters/human.ts‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2123,6 +2123,16 @@ export function formatUpgradeResult(data: UpgradeResult): string {
21232123
}
21242124
}
21252125

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));
2135+
21262136
// Append warnings with ⚠ markers
21272137
if (data.warnings && data.warnings.length > 0) {
21282138
lines.push("");

‎src/lib/interactive-login.ts‎

Lines changed: 50 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@ import { setupCopyKeyListener } from "./clipboard.js";
1212
import { getDbPath } from "./db/index.js";
1313
import { setUserInfo } from "./db/user.js";
1414
import { formatError } from "./errors.js";
15-
import { muted } from "./formatters/colors.js";
15+
import { renderInlineMarkdown } from "./formatters/markdown.js";
1616
import { logger } from "./logger.js";
1717
import { completeOAuthFlow, performDeviceFlow } from "./oauth.js";
1818
import { generateQRCode } from "./qrcode.js";
@@ -37,6 +37,43 @@ export type InteractiveLoginOptions = {
3737
timeout?: number;
3838
};
3939

40+
/**
41+
* Build the user-facing display lines for the device flow verification step.
42+
*
43+
* Shows the complete verification URL (with embedded user code) so it works
44+
* in environments where the browser can't auto-open (SSH, CI, etc.).
45+
* Uses `renderInlineMarkdown()` for styled output that respects `NO_COLOR`.
46+
*
47+
* Extracted from the `onUserCode` callback for testability.
48+
*
49+
* @param userCode - The device flow user code (e.g., "ABCD-EFGH")
50+
* @param verificationUriComplete - Full URL with `?user_code=...` embedded
51+
* @param browserOpened - Whether the browser was successfully opened
52+
* @param isTTY - Whether stdin is a TTY (controls copy-to-clipboard hint)
53+
* @returns Array of strings to emit via `log.log()`, one per call
54+
*/
55+
export function buildDeviceFlowDisplay(
56+
userCode: string,
57+
verificationUriComplete: string,
58+
browserOpened: boolean,
59+
isTTY: boolean
60+
): string[] {
61+
const lines: string[] = [];
62+
63+
lines.push("");
64+
lines.push(renderInlineMarkdown(` URL: ${verificationUriComplete}`));
65+
lines.push(renderInlineMarkdown(` Code: \`${userCode}\``));
66+
lines.push("");
67+
68+
if (!browserOpened) {
69+
const copyHint = isTTY ? renderInlineMarkdown(" *(`c` to copy URL)*") : "";
70+
lines.push(` Copy the URL above to sign in.${copyHint}`);
71+
lines.push("");
72+
}
73+
74+
return lines;
75+
}
76+
4077
/**
4178
* Run the interactive OAuth device flow login.
4279
*
@@ -69,7 +106,7 @@ export async function runInteractiveLogin(
69106
{
70107
onUserCode: async (
71108
userCode,
72-
verificationUri,
109+
_verificationUri,
73110
verificationUriComplete
74111
) => {
75112
urlToCopy = verificationUriComplete;
@@ -86,21 +123,24 @@ export async function runInteractiveLogin(
86123
log.log(qr);
87124
}
88125

89-
log.info(`URL: ${verificationUri}`);
90-
log.info(`Code: ${userCode}`);
91-
const stdin = process.stdin;
92-
const copyHint = stdin.isTTY ? ` ${muted("(c to copy)")}` : "";
93-
log.info(
94-
`Browser didn't open? Use the url above to sign in${copyHint}`
126+
// Show the complete URL (with user_code embedded) and optional copy hint
127+
const displayLines = buildDeviceFlowDisplay(
128+
userCode,
129+
verificationUriComplete,
130+
browserOpened,
131+
process.stdin.isTTY ?? false
95132
);
133+
for (const line of displayLines) {
134+
log.log(line);
135+
}
96136

97137
// Use a spinner for the "waiting" state instead of raw polling dots
98138
log.start("Waiting for authorization...");
99139

100140
// Setup keyboard listener for 'c' to copy URL
101-
if (stdin.isTTY) {
141+
if (process.stdin.isTTY) {
102142
keyListener.cleanup = setupCopyKeyListener(
103-
stdin as NodeJS.ReadStream & { fd: 0 },
143+
process.stdin as NodeJS.ReadStream & { fd: 0 },
104144
() => urlToCopy
105145
);
106146
}

‎test/commands/cli/upgrade.test.ts‎

Lines changed: 13 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -268,9 +268,8 @@ describe("sentry cli upgrade", () => {
268268
);
269269

270270
const combined = getOutput();
271-
expect(combined).toContain("Installation method: curl");
272-
expect(combined).toContain(`Current version: ${CLI_VERSION}`);
273-
expect(combined).toContain(`Latest version: ${CLI_VERSION}`);
271+
expect(combined).toContain("| curl |");
272+
expect(combined).toContain(CLI_VERSION);
274273
expect(combined).toContain("You are already on the target version");
275274
});
276275

@@ -289,7 +288,7 @@ describe("sentry cli upgrade", () => {
289288
);
290289

291290
const combined = getOutput();
292-
expect(combined).toContain("Latest version: 99.99.99");
291+
expect(combined).toContain("99.99.99");
293292
expect(combined).toContain("Run 'sentry cli upgrade' to update.");
294293
});
295294

@@ -308,7 +307,7 @@ describe("sentry cli upgrade", () => {
308307
);
309308

310309
const combined = getOutput();
311-
expect(combined).toContain("Target version: 88.88.88");
310+
expect(combined).toContain("88.88.88");
312311
expect(combined).toContain(
313312
"Run 'sentry cli upgrade 88.88.88' to update."
314313
);
@@ -363,8 +362,8 @@ describe("sentry cli upgrade", () => {
363362
);
364363

365364
const combined = getOutput();
366-
expect(combined).toContain("Installation method: brew");
367-
expect(combined).toContain("Latest version: 99.99.99");
365+
expect(combined).toContain("| brew |");
366+
expect(combined).toContain("99.99.99");
368367
expect(combined).toContain("Run 'sentry cli upgrade' to update.");
369368
});
370369
});
@@ -500,7 +499,7 @@ describe("sentry cli upgrade — nightly channel", () => {
500499
);
501500

502501
const combined = getOutput();
503-
expect(combined).toContain("Channel: nightly");
502+
expect(combined).toContain("| nightly |");
504503
});
505504

506505
test("'stable' positional sets channel to stable", async () => {
@@ -519,7 +518,7 @@ describe("sentry cli upgrade — nightly channel", () => {
519518
);
520519

521520
const combined = getOutput();
522-
expect(combined).toContain("Channel: stable");
521+
expect(combined).toContain("| stable |");
523522
});
524523

525524
test("without positional, uses persisted channel", async () => {
@@ -538,7 +537,7 @@ describe("sentry cli upgrade — nightly channel", () => {
538537
);
539538

540539
const combined = getOutput();
541-
expect(combined).toContain("Channel: nightly");
540+
expect(combined).toContain("| nightly |");
542541
});
543542
});
544543

@@ -593,8 +592,8 @@ describe("sentry cli upgrade — nightly channel", () => {
593592
);
594593

595594
const combined = getOutput();
596-
expect(combined).toContain("Channel: nightly");
597-
expect(combined).toContain(`Latest version: ${CLI_VERSION}`);
595+
expect(combined).toContain("| nightly |");
596+
expect(combined).toContain(CLI_VERSION);
598597
expect(combined).toContain("You are already on the target version");
599598
});
600599

@@ -613,8 +612,8 @@ describe("sentry cli upgrade — nightly channel", () => {
613612
);
614613

615614
const combined = getOutput();
616-
expect(combined).toContain("Channel: nightly");
617-
expect(combined).toContain("Latest version: 0.99.0-dev.9999999999");
615+
expect(combined).toContain("| nightly |");
616+
expect(combined).toContain("0.99.0-dev.9999999999");
618617
expect(combined).toContain("Run 'sentry cli upgrade' to update.");
619618
});
620619
});

‎test/lib/formatters/human.test.ts‎

Lines changed: 64 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ import {
1414
formatIssueSubtitle,
1515
formatProjectCreated,
1616
formatShortId,
17+
formatUpgradeResult,
1718
formatUserIdentity,
1819
type IssueTableRow,
1920
type ProjectCreatedResult,
@@ -738,3 +739,66 @@ describe("formatDashboardView", () => {
738739
expect(result).toContain("No widgets.");
739740
});
740741
});
742+
743+
describe("formatUpgradeResult", () => {
744+
// Force plain output so we get raw markdown (no ANSI codes)
745+
const origPlain = process.env.SENTRY_PLAIN_OUTPUT;
746+
function withPlain(fn: () => void) {
747+
process.env.SENTRY_PLAIN_OUTPUT = "1";
748+
try {
749+
fn();
750+
} finally {
751+
if (origPlain === undefined) {
752+
delete process.env.SENTRY_PLAIN_OUTPUT;
753+
} else {
754+
process.env.SENTRY_PLAIN_OUTPUT = origPlain;
755+
}
756+
}
757+
}
758+
759+
test("renders metadata table with method and channel", () => {
760+
withPlain(() => {
761+
const result = formatUpgradeResult({
762+
action: "upgraded",
763+
currentVersion: "0.5.0",
764+
targetVersion: "0.6.0",
765+
channel: "stable",
766+
method: "curl",
767+
forced: false,
768+
});
769+
expect(result).toContain("| **Method** | curl |");
770+
expect(result).toContain("| **Channel** | stable |");
771+
});
772+
});
773+
774+
test("renders nightly channel in metadata table", () => {
775+
withPlain(() => {
776+
const result = formatUpgradeResult({
777+
action: "checked",
778+
currentVersion: "0.5.0",
779+
targetVersion: "0.6.0-dev.123",
780+
channel: "nightly",
781+
method: "npm",
782+
forced: false,
783+
});
784+
expect(result).toContain("| **Method** | npm |");
785+
expect(result).toContain("| **Channel** | nightly |");
786+
});
787+
});
788+
789+
test("up-to-date action includes metadata table", () => {
790+
withPlain(() => {
791+
const result = formatUpgradeResult({
792+
action: "up-to-date",
793+
currentVersion: "0.5.0",
794+
targetVersion: "0.5.0",
795+
channel: "stable",
796+
method: "brew",
797+
forced: false,
798+
});
799+
expect(result).toContain("Already up to date");
800+
expect(result).toContain("| **Method** | brew |");
801+
expect(result).toContain("| **Channel** | stable |");
802+
});
803+
});
804+
});

‎test/lib/interactive-login.test.ts‎

Lines changed: 69 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,69 @@
1+
/**
2+
* Tests for buildDeviceFlowDisplay — the extracted display logic from the
3+
* interactive login flow's onUserCode callback.
4+
*
5+
* Uses SENTRY_PLAIN_OUTPUT=1 to get predictable raw markdown output
6+
* (no ANSI codes) for string assertions.
7+
*/
8+
9+
import { afterAll, beforeAll, describe, expect, test } from "bun:test";
10+
import { buildDeviceFlowDisplay } from "../../src/lib/interactive-login.js";
11+
12+
// Force plain output for predictable string matching
13+
let origPlain: string | undefined;
14+
beforeAll(() => {
15+
origPlain = process.env.SENTRY_PLAIN_OUTPUT;
16+
process.env.SENTRY_PLAIN_OUTPUT = "1";
17+
});
18+
afterAll(() => {
19+
if (origPlain === undefined) {
20+
delete process.env.SENTRY_PLAIN_OUTPUT;
21+
} else {
22+
process.env.SENTRY_PLAIN_OUTPUT = origPlain;
23+
}
24+
});
25+
26+
describe("buildDeviceFlowDisplay", () => {
27+
const CODE = "ABCD-EFGH";
28+
const URL = "https://sentry.io/auth/device/?user_code=ABCD-EFGH";
29+
30+
test("includes complete URL with embedded code", () => {
31+
const lines = buildDeviceFlowDisplay(CODE, URL, true, false);
32+
const joined = lines.join("\n");
33+
expect(joined).toContain(URL);
34+
});
35+
36+
test("includes user code as inline code span", () => {
37+
const lines = buildDeviceFlowDisplay(CODE, URL, true, false);
38+
const joined = lines.join("\n");
39+
expect(joined).toContain(`\`${CODE}\``);
40+
});
41+
42+
test("omits copy hint when browser opened", () => {
43+
const lines = buildDeviceFlowDisplay(CODE, URL, true, true);
44+
const joined = lines.join("\n");
45+
expect(joined).not.toContain("Copy the URL above");
46+
expect(joined).not.toContain("to copy URL");
47+
});
48+
49+
test("shows copy hint when browser did not open (TTY)", () => {
50+
const lines = buildDeviceFlowDisplay(CODE, URL, false, true);
51+
const joined = lines.join("\n");
52+
expect(joined).toContain("Copy the URL above to sign in.");
53+
expect(joined).toContain("to copy URL");
54+
});
55+
56+
test("shows copy hint without keyboard shortcut in non-TTY", () => {
57+
const lines = buildDeviceFlowDisplay(CODE, URL, false, false);
58+
const joined = lines.join("\n");
59+
expect(joined).toContain("Copy the URL above to sign in.");
60+
expect(joined).not.toContain("to copy URL");
61+
});
62+
63+
test("returns more lines when browser did not open", () => {
64+
const withBrowser = buildDeviceFlowDisplay(CODE, URL, true, false);
65+
const withoutBrowser = buildDeviceFlowDisplay(CODE, URL, false, false);
66+
// Without browser: extra copy-hint line + blank line
67+
expect(withoutBrowser.length).toBeGreaterThan(withBrowser.length);
68+
});
69+
});

0 commit comments

Comments
 (0)