Skip to content

Commit b13b810

Browse files
committed
refactor: convert Tier 1 commands to return-based output
Convert 5 remaining Tier 1 commands to the return-based CommandOutput<T> pattern introduced in #380. Commands return { data, hint? } and declare an OutputConfig with json: true and a human formatter — the framework handles JSON serialization and human-readable rendering. Commands converted: - event/view: formatEventView + buildEventData - issue/view: formatIssueView + buildIssueData - issue/plan: formatPlanOutput + buildPlanData (removed outputSolution) - project/create: formatProjectCreatedView - trace/view: formatTraceView + buildTraceData Additional cleanup from review feedback: - Unify hint and footer into single hint field on CommandOutput<T> - Replace normalizePlatform stderr.write with consola logger - Remove stdout parameter from openInBrowser/openOrShowUrl (now uses process.stdout directly) — all 7 command callers updated - Remove unused Writer imports and stdout destructuring from commands
1 parent 35ca172 commit b13b810

21 files changed

Lines changed: 327 additions & 375 deletions

File tree

src/commands/event/view.ts

Lines changed: 31 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,7 @@ import {
2222
import { openInBrowser } from "../../lib/browser.js";
2323
import { buildCommand } from "../../lib/command.js";
2424
import { ContextError, ResolutionError } from "../../lib/errors.js";
25-
import { formatEventDetails, writeJson } from "../../lib/formatters/index.js";
25+
import { formatEventDetails } from "../../lib/formatters/index.js";
2626
import {
2727
applyFreshFlag,
2828
FRESH_ALIASES,
@@ -40,7 +40,7 @@ import {
4040
} from "../../lib/sentry-url-parser.js";
4141
import { buildEventSearchUrl } from "../../lib/sentry-urls.js";
4242
import { getSpanTreeLines } from "../../lib/span-tree.js";
43-
import type { SentryEvent, Writer } from "../../types/index.js";
43+
import type { SentryEvent } from "../../types/index.js";
4444

4545
type ViewFlags = {
4646
readonly json: boolean;
@@ -50,30 +50,29 @@ type ViewFlags = {
5050
readonly fields?: string[];
5151
};
5252

53-
type HumanOutputOptions = {
53+
/** Return type for event view — includes all data both renderers need */
54+
type EventViewData = {
5455
event: SentryEvent;
55-
detectedFrom?: string;
56+
trace: { traceId: string; spans: unknown[] } | null;
57+
/** Pre-formatted span tree lines for human output (not serialized) */
5658
spanTreeLines?: string[];
5759
};
5860

5961
/**
60-
* Write human-readable event output to stdout.
62+
* Format event view data for human-readable terminal output.
6163
*
62-
* @param stdout - Output stream
63-
* @param options - Output options including event, detectedFrom, and spanTreeLines
64+
* Renders event details and optional span tree.
6465
*/
65-
function writeHumanOutput(stdout: Writer, options: HumanOutputOptions): void {
66-
const { event, detectedFrom, spanTreeLines } = options;
66+
function formatEventView(data: EventViewData): string {
67+
const parts: string[] = [];
6768

68-
stdout.write(`${formatEventDetails(event, `Event ${event.eventID}`)}\n`);
69+
parts.push(formatEventDetails(data.event, `Event ${data.event.eventID}`));
6970

70-
if (spanTreeLines && spanTreeLines.length > 0) {
71-
stdout.write(`${spanTreeLines.join("\n")}\n`);
71+
if (data.spanTreeLines && data.spanTreeLines.length > 0) {
72+
parts.push(data.spanTreeLines.join("\n"));
7273
}
7374

74-
if (detectedFrom) {
75-
stdout.write(`\nDetected from ${detectedFrom}\n`);
76-
}
75+
return parts.join("\n");
7776
}
7877

7978
/** Usage hint for ContextError messages */
@@ -303,7 +302,11 @@ export const viewCommand = buildCommand({
303302
" sentry event view <org>/<proj> <event-id> # explicit org and project\n" +
304303
" sentry event view <project> <event-id> # find project across all orgs",
305304
},
306-
output: "json",
305+
output: {
306+
json: true,
307+
human: formatEventView,
308+
jsonExclude: ["spanTreeLines"],
309+
},
307310
parameters: {
308311
positional: {
309312
kind: "array",
@@ -325,13 +328,9 @@ export const viewCommand = buildCommand({
325328
},
326329
aliases: { ...FRESH_ALIASES, w: "web" },
327330
},
328-
async func(
329-
this: SentryContext,
330-
flags: ViewFlags,
331-
...args: string[]
332-
): Promise<void> {
331+
async func(this: SentryContext, flags: ViewFlags, ...args: string[]) {
333332
applyFreshFlag(flags);
334-
const { stdout, cwd } = this;
333+
const { cwd } = this;
335334

336335
const log = logger.withTag("event.view");
337336

@@ -360,11 +359,7 @@ export const viewCommand = buildCommand({
360359
}
361360

362361
if (flags.web) {
363-
await openInBrowser(
364-
stdout,
365-
buildEventSearchUrl(target.org, eventId),
366-
"event"
367-
);
362+
await openInBrowser(buildEventSearchUrl(target.org, eventId), "event");
368363
return;
369364
}
370365

@@ -381,18 +376,15 @@ export const viewCommand = buildCommand({
381376
? await getSpanTreeLines(target.org, event, flags.spans)
382377
: undefined;
383378

384-
if (flags.json) {
385-
const trace = spanTreeResult?.success
386-
? { traceId: spanTreeResult.traceId, spans: spanTreeResult.spans }
387-
: null;
388-
writeJson(stdout, { event, trace }, flags.fields);
389-
return;
390-
}
379+
const trace = spanTreeResult?.success
380+
? { traceId: spanTreeResult.traceId, spans: spanTreeResult.spans }
381+
: null;
391382

392-
writeHumanOutput(stdout, {
393-
event,
394-
detectedFrom: target.detectedFrom,
395-
spanTreeLines: spanTreeResult?.lines,
396-
});
383+
return {
384+
data: { event, trace, spanTreeLines: spanTreeResult?.lines },
385+
hint: target.detectedFrom
386+
? `Detected from ${target.detectedFrom}`
387+
: undefined,
388+
};
397389
},
398390
});

src/commands/issue/explain.ts

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -73,7 +73,7 @@ export const explainCommand = buildCommand({
7373
},
7474
async func(this: SentryContext, flags: ExplainFlags, issueArg: string) {
7575
applyFreshFlag(flags);
76-
const { stderr, cwd } = this;
76+
const { cwd } = this;
7777

7878
// Declare org outside try block so it's accessible in catch for error messages
7979
let resolvedOrg: string | undefined;
@@ -91,7 +91,6 @@ export const explainCommand = buildCommand({
9191
const state = await ensureRootCauseAnalysis({
9292
org,
9393
issueId: numericId,
94-
stderr,
9594
json: flags.json,
9695
force: flags.force,
9796
});

src/commands/issue/list.ts

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -756,7 +756,6 @@ async function fetchOrgAllIssues(
756756
/** Options for {@link handleOrgAllIssues}. */
757757
type OrgAllIssuesOptions = {
758758
stdout: Writer;
759-
stderr: Writer;
760759
org: string;
761760
flags: ListFlags;
762761
setContext: (orgs: string[], projects: string[]) => void;
@@ -769,7 +768,7 @@ type OrgAllIssuesOptions = {
769768
* never accidentally reused.
770769
*/
771770
async function handleOrgAllIssues(options: OrgAllIssuesOptions): Promise<void> {
772-
const { stdout, stderr, org, flags, setContext } = options;
771+
const { stdout, org, flags, setContext } = options;
773772
// Encode sort + query in context key so cursors from different searches don't collide.
774773
const contextKey = buildPaginationContextKey("org", org, {
775774
sort: flags.sort,
@@ -781,7 +780,7 @@ async function handleOrgAllIssues(options: OrgAllIssuesOptions): Promise<void> {
781780
setContext([org], []);
782781

783782
const { issues, nextCursor } = await withProgress(
784-
{ stderr, message: `Fetching issues (up to ${flags.limit})...` },
783+
{ message: `Fetching issues (up to ${flags.limit})...` },
785784
(setMessage) =>
786785
fetchOrgAllIssues(org, flags, cursor, (fetched, limit) =>
787786
setMessage(
@@ -927,7 +926,7 @@ async function handleResolvedTargets(
927926
: "Fetching issues";
928927

929928
const { results, hasMore } = await withProgress(
930-
{ stderr, message: `${baseMessage} (up to ${flags.limit})...` },
929+
{ message: `${baseMessage} (up to ${flags.limit})...` },
931930
(setMessage) =>
932931
fetchWithBudget(
933932
activeTargets,
@@ -1266,7 +1265,6 @@ export const listCommand = buildListCommand("issue", {
12661265
"org-all": (ctx) =>
12671266
handleOrgAllIssues({
12681267
stdout: ctx.stdout,
1269-
stderr,
12701268
org: ctx.parsed.org,
12711269
flags,
12721270
setContext,

src/commands/issue/plan.ts

Lines changed: 36 additions & 60 deletions
Original file line numberDiff line numberDiff line change
@@ -9,8 +9,6 @@ import type { SentryContext } from "../../context.js";
99
import { triggerSolutionPlanning } from "../../lib/api-client.js";
1010
import { buildCommand, numberParser } from "../../lib/command.js";
1111
import { ApiError, ValidationError } from "../../lib/errors.js";
12-
import { muted } from "../../lib/formatters/colors.js";
13-
import { writeJson } from "../../lib/formatters/index.js";
1412
import {
1513
formatSolution,
1614
handleSeerApiError,
@@ -20,7 +18,7 @@ import {
2018
FRESH_ALIASES,
2119
FRESH_FLAG,
2220
} from "../../lib/list-command.js";
23-
import type { Writer } from "../../types/index.js";
21+
import { logger } from "../../lib/logger.js";
2422
import {
2523
type AutofixState,
2624
extractRootCauses,
@@ -108,39 +106,36 @@ function validateCauseSelection(
108106
return causeId;
109107
}
110108

111-
type OutputSolutionOptions = {
112-
stdout: Writer;
113-
stderr: Writer;
114-
solution: SolutionArtifact | null;
115-
state: AutofixState;
116-
json: boolean;
117-
fields?: string[];
109+
/** Return type for issue plan — includes state metadata and solution */
110+
type PlanData = {
111+
run_id: number;
112+
status: string;
113+
solution: SolutionArtifact["data"] | null;
118114
};
119115

120116
/**
121-
* Output a solution artifact to stdout.
117+
* Format solution plan data for human-readable terminal output.
118+
*
119+
* Returns the formatted solution or a "no solution" message.
122120
*/
123-
function outputSolution(options: OutputSolutionOptions): void {
124-
const { stdout, stderr, solution, state, json, fields } = options;
125-
126-
if (json) {
127-
writeJson(
128-
stdout,
129-
{
130-
run_id: state.run_id,
131-
status: state.status,
132-
solution: solution?.data ?? null,
133-
},
134-
fields
135-
);
136-
return;
121+
function formatPlanOutput(data: PlanData): string {
122+
if (data.solution) {
123+
// Wrap in the SolutionArtifact shape expected by formatSolution
124+
return formatSolution({ data: data.solution } as SolutionArtifact);
137125
}
126+
return "No solution found. Check the Sentry web UI for details.";
127+
}
138128

139-
if (solution) {
140-
stdout.write(`${formatSolution(solution)}\n`);
141-
} else {
142-
stderr.write("No solution found. Check the Sentry web UI for details.\n");
143-
}
129+
/**
130+
* Build the plan data object from autofix state.
131+
*/
132+
function buildPlanData(state: AutofixState): PlanData {
133+
const solution = extractSolution(state);
134+
return {
135+
run_id: state.run_id,
136+
status: state.status,
137+
solution: solution?.data ?? null,
138+
};
144139
}
145140

146141
export const planCommand = buildCommand({
@@ -171,7 +166,10 @@ export const planCommand = buildCommand({
171166
" sentry issue plan cli-G --cause 0\n" +
172167
" sentry issue plan 123456789 --force",
173168
},
174-
output: "json",
169+
output: {
170+
json: true,
171+
human: formatPlanOutput,
172+
},
175173
parameters: {
176174
positional: issueIdPositional,
177175
flags: {
@@ -190,13 +188,9 @@ export const planCommand = buildCommand({
190188
},
191189
aliases: FRESH_ALIASES,
192190
},
193-
async func(
194-
this: SentryContext,
195-
flags: PlanFlags,
196-
issueArg: string
197-
): Promise<void> {
191+
async func(this: SentryContext, flags: PlanFlags, issueArg: string) {
198192
applyFreshFlag(flags);
199-
const { stdout, stderr, cwd } = this;
193+
const { cwd } = this;
200194

201195
// Declare org outside try block so it's accessible in catch for error messages
202196
let resolvedOrg: string | undefined;
@@ -214,7 +208,6 @@ export const planCommand = buildCommand({
214208
const state = await ensureRootCauseAnalysis({
215209
org,
216210
issueId: numericId,
217-
stderr,
218211
json: flags.json,
219212
});
220213

@@ -229,23 +222,16 @@ export const planCommand = buildCommand({
229222
if (!flags.force) {
230223
const existingSolution = extractSolution(state);
231224
if (existingSolution) {
232-
outputSolution({
233-
stdout,
234-
stderr,
235-
solution: existingSolution,
236-
state,
237-
json: flags.json,
238-
fields: flags.fields,
239-
});
240-
return;
225+
return { data: buildPlanData(state) };
241226
}
242227
}
243228

244229
// No solution exists, trigger planning
245230
if (!flags.json) {
246-
stderr.write(`Creating plan for cause #${causeId}...\n`);
231+
const log = logger.withTag("issue.plan");
232+
log.info(`Creating plan for cause #${causeId}...`);
247233
if (selectedCause) {
248-
stderr.write(`${muted(`"${selectedCause.description}"`)}\n\n`);
234+
log.info(`"${selectedCause.description}"`);
249235
}
250236
}
251237

@@ -255,7 +241,6 @@ export const planCommand = buildCommand({
255241
const finalState = await pollAutofixState({
256242
orgSlug: org,
257243
issueId: numericId,
258-
stderr,
259244
json: flags.json,
260245
timeoutMessage:
261246
"Plan creation timed out after 6 minutes. Try again or check the issue in Sentry web UI.",
@@ -272,16 +257,7 @@ export const planCommand = buildCommand({
272257
throw new Error("Plan creation was cancelled.");
273258
}
274259

275-
// Extract and output solution
276-
const solution = extractSolution(finalState);
277-
outputSolution({
278-
stdout,
279-
stderr,
280-
solution,
281-
state: finalState,
282-
json: flags.json,
283-
fields: flags.fields,
284-
});
260+
return { data: buildPlanData(finalState) };
285261
} catch (error) {
286262
// Handle API errors with friendly messages
287263
if (error instanceof ApiError) {

0 commit comments

Comments
 (0)