Skip to content

Commit 0ba3c5a

Browse files
MathurAditya724MathurAditya724
committed
fix(init): generate spinner messages from payload params instead of server detail
The server-provided detail string could show misleading file names (e.g. JS config files for a Python project). Replace it with CLI-generated messages derived from the actual payload params so the user always sees truthful info about which files are being read, written, or checked.
1 parent d5806b4 commit 0ba3c5a

File tree

2 files changed

+281
-16
lines changed

2 files changed

+281
-16
lines changed

‎src/lib/init/wizard-runner.ts‎

Lines changed: 89 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77
*/
88

99
import { randomBytes } from "node:crypto";
10+
import { basename } from "node:path";
1011
import {
1112
cancel,
1213
confirm,
@@ -49,6 +50,7 @@ import {
4950
tryGetExistingProject,
5051
} from "./local-ops.js";
5152
import type {
53+
LocalOpPayload,
5254
LocalOpResult,
5355
SuspendPayload,
5456
WizardOptions,
@@ -90,6 +92,92 @@ function truncateForTerminal(message: string): string {
9092
return `${message.slice(0, maxWidth - 1)}…`;
9193
}
9294

95+
/**
96+
* Build a human-readable spinner message from the actual payload params
97+
* instead of relying on the server-provided `detail` string. This ensures
98+
* the user always sees truthful information about which files are being
99+
* read, written, or checked — regardless of what the server sends.
100+
*/
101+
export function describeLocalOp(payload: LocalOpPayload): string {
102+
switch (payload.operation) {
103+
case "read-files": {
104+
const paths = payload.params.paths;
105+
return describeFilePaths("Reading", paths);
106+
}
107+
case "file-exists-batch": {
108+
const paths = payload.params.paths;
109+
return describeFilePaths("Checking", paths);
110+
}
111+
case "apply-patchset": {
112+
const patches = payload.params.patches;
113+
const first = patches[0];
114+
if (patches.length === 1 && first) {
115+
const verb = patchActionVerb(first.action);
116+
return `${verb} ${basename(first.path)}...`;
117+
}
118+
const counts = patchActionCounts(patches);
119+
return `Applying ${patches.length} file changes (${counts})...`;
120+
}
121+
case "run-commands": {
122+
const cmds = payload.params.commands;
123+
const first = cmds[0];
124+
if (cmds.length === 1 && first) {
125+
return `Running ${first}...`;
126+
}
127+
return `Running ${cmds.length} commands (${first ?? "..."}, ...)...`;
128+
}
129+
case "list-dir":
130+
return "Listing directory...";
131+
case "create-sentry-project":
132+
return `Creating project "${payload.params.name}" (${payload.params.platform})...`;
133+
default:
134+
return `${(payload as { operation: string }).operation}...`;
135+
}
136+
}
137+
138+
/** Format a file paths list into a human-readable message with a verb prefix. */
139+
function describeFilePaths(verb: string, paths: string[]): string {
140+
const first = paths[0];
141+
const second = paths[1];
142+
if (!first) {
143+
return `${verb} files...`;
144+
}
145+
if (paths.length === 1) {
146+
return `${verb} ${basename(first)}...`;
147+
}
148+
if (paths.length === 2 && second) {
149+
return `${verb} ${basename(first)}, ${basename(second)}...`;
150+
}
151+
return `${verb} ${paths.length} files (${basename(first)}${second ? `, ${basename(second)}` : ""}, ...)...`;
152+
}
153+
154+
/** Map a patch action to a user-facing verb. */
155+
function patchActionVerb(action: string): string {
156+
switch (action) {
157+
case "create":
158+
return "Creating";
159+
case "modify":
160+
return "Modifying";
161+
case "delete":
162+
return "Deleting";
163+
default:
164+
return "Updating";
165+
}
166+
}
167+
168+
/** Summarize patch actions into a compact string like "2 created, 1 modified". */
169+
function patchActionCounts(patches: Array<{ action: string }>): string {
170+
const counts = new Map<string, number>();
171+
for (const p of patches) {
172+
counts.set(p.action, (counts.get(p.action) ?? 0) + 1);
173+
}
174+
const parts: string[] = [];
175+
for (const [action, count] of counts) {
176+
parts.push(`${count} ${action === "modify" ? "modified" : `${action}d`}`);
177+
}
178+
return parts.join(", ");
179+
}
180+
93181
async function handleSuspendedStep(
94182
ctx: StepContext,
95183
stepPhases: Map<string, number>,
@@ -99,9 +187,7 @@ async function handleSuspendedStep(
99187
const label = STEP_LABELS[stepId] ?? stepId;
100188

101189
if (payload.type === "local-op") {
102-
const message = payload.detail
103-
? payload.detail
104-
: `${label} (${payload.operation})...`;
190+
const message = describeLocalOp(payload);
105191
spin.message(truncateForTerminal(message));
106192

107193
const localResult = await handleLocalOp(payload, options);

‎test/lib/init/wizard-runner.test.ts‎

Lines changed: 192 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -36,10 +36,14 @@ import * as inter from "../../../src/lib/init/interactive.js";
3636
// biome-ignore lint/performance/noNamespaceImport: spyOn requires object reference
3737
import * as ops from "../../../src/lib/init/local-ops.js";
3838
import type {
39+
LocalOpPayload,
3940
WizardOptions,
4041
WorkflowRunResult,
4142
} from "../../../src/lib/init/types.js";
42-
import { runWizard } from "../../../src/lib/init/wizard-runner.js";
43+
import {
44+
describeLocalOp,
45+
runWizard,
46+
} from "../../../src/lib/init/wizard-runner.js";
4347
// biome-ignore lint/performance/noNamespaceImport: spyOn requires object reference
4448
import * as sentryUrls from "../../../src/lib/sentry-urls.js";
4549

@@ -538,7 +542,7 @@ describe("runWizard", () => {
538542
expect(payload.operation).toBe("list-dir");
539543
});
540544

541-
test("uses detail field for spinner message when present", async () => {
545+
test("ignores server detail and generates message from payload params", async () => {
542546
mockStartResult = {
543547
status: "suspended",
544548
suspended: [["install-deps"]],
@@ -547,10 +551,10 @@ describe("runWizard", () => {
547551
suspendPayload: {
548552
type: "local-op",
549553
operation: "run-commands",
550-
detail: "npm install @sentry/nextjs @sentry/profiling-node",
554+
detail: "Server-provided message about next.config.js",
551555
cwd: "/app",
552556
params: {
553-
commands: ["npm install @sentry/nextjs @sentry/profiling-node"],
557+
commands: ["pip install sentry-sdk"],
554558
},
555559
},
556560
},
@@ -560,12 +564,13 @@ describe("runWizard", () => {
560564

561565
await runWizard(makeOptions());
562566

567+
// Should use CLI-generated message, NOT the server detail
563568
expect(spinnerMock.message).toHaveBeenCalledWith(
564-
"npm install @sentry/nextjs @sentry/profiling-node"
569+
"Running pip install sentry-sdk..."
565570
);
566571
});
567572

568-
test("falls back to generic message when detail is absent", async () => {
573+
test("generates message from payload when detail is absent", async () => {
569574
mockStartResult = {
570575
status: "suspended",
571576
suspended: [["install-deps"]],
@@ -585,19 +590,17 @@ describe("runWizard", () => {
585590
await runWizard(makeOptions());
586591

587592
expect(spinnerMock.message).toHaveBeenCalledWith(
588-
"Installing dependencies (run-commands)..."
593+
"Running npm install @sentry/nextjs..."
589594
);
590595
});
591596

592-
test("truncates detail message when terminal is narrow", async () => {
597+
test("truncates generated message when terminal is narrow", async () => {
593598
const origColumns = process.stdout.columns;
594599
Object.defineProperty(process.stdout, "columns", {
595600
value: 40,
596601
configurable: true,
597602
});
598603

599-
const longDetail =
600-
"npm install @sentry/nextjs @sentry/profiling-node @sentry/browser";
601604
mockStartResult = {
602605
status: "suspended",
603606
suspended: [["install-deps"]],
@@ -606,9 +609,12 @@ describe("runWizard", () => {
606609
suspendPayload: {
607610
type: "local-op",
608611
operation: "run-commands",
609-
detail: longDetail,
610612
cwd: "/app",
611-
params: { commands: [longDetail] },
613+
params: {
614+
commands: [
615+
"npm install @sentry/nextjs @sentry/profiling-node @sentry/browser",
616+
],
617+
},
612618
},
613619
},
614620
},
@@ -620,7 +626,7 @@ describe("runWizard", () => {
620626

621627
// 40 columns - 4 reserved = 36 max, truncated with "…"
622628
const call = spinnerMock.message.mock.calls.find((c: string[]) =>
623-
c[0]?.includes("npm install")
629+
c[0]?.includes("Running")
624630
) as string[] | undefined;
625631
expect(call).toBeDefined();
626632
const msg = call?.[0] ?? "";
@@ -887,3 +893,176 @@ describe("runWizard", () => {
887893
});
888894
});
889895
});
896+
897+
// ── describeLocalOp unit tests ──────────────────────────────────────────────
898+
899+
describe("describeLocalOp", () => {
900+
function payload(
901+
overrides: Partial<LocalOpPayload> &
902+
Pick<LocalOpPayload, "operation" | "params">
903+
): LocalOpPayload {
904+
return { type: "local-op", cwd: "/app", ...overrides } as LocalOpPayload;
905+
}
906+
907+
describe("read-files", () => {
908+
test("single file shows basename", () => {
909+
const msg = describeLocalOp(
910+
payload({
911+
operation: "read-files",
912+
params: { paths: ["src/settings.py"] },
913+
})
914+
);
915+
expect(msg).toBe("Reading settings.py...");
916+
});
917+
918+
test("two files shows both basenames", () => {
919+
const msg = describeLocalOp(
920+
payload({
921+
operation: "read-files",
922+
params: { paths: ["src/settings.py", "src/urls.py"] },
923+
})
924+
);
925+
expect(msg).toBe("Reading settings.py, urls.py...");
926+
});
927+
928+
test("three+ files shows count and first two basenames", () => {
929+
const msg = describeLocalOp(
930+
payload({
931+
operation: "read-files",
932+
params: {
933+
paths: ["a/one.py", "b/two.py", "c/three.py", "d/four.py"],
934+
},
935+
})
936+
);
937+
expect(msg).toBe("Reading 4 files (one.py, two.py, ...)...");
938+
});
939+
940+
test("empty paths array", () => {
941+
const msg = describeLocalOp(
942+
payload({ operation: "read-files", params: { paths: [] } })
943+
);
944+
expect(msg).toBe("Reading files...");
945+
});
946+
});
947+
948+
describe("file-exists-batch", () => {
949+
test("single file shows basename", () => {
950+
const msg = describeLocalOp(
951+
payload({
952+
operation: "file-exists-batch",
953+
params: { paths: ["requirements.txt"] },
954+
})
955+
);
956+
expect(msg).toBe("Checking requirements.txt...");
957+
});
958+
959+
test("multiple files shows count", () => {
960+
const msg = describeLocalOp(
961+
payload({
962+
operation: "file-exists-batch",
963+
params: { paths: ["a.py", "b.py", "c.py"] },
964+
})
965+
);
966+
expect(msg).toBe("Checking 3 files (a.py, b.py, ...)...");
967+
});
968+
});
969+
970+
describe("apply-patchset", () => {
971+
test("single create shows verb and basename", () => {
972+
const msg = describeLocalOp(
973+
payload({
974+
operation: "apply-patchset",
975+
params: {
976+
patches: [{ path: "src/sentry.py", action: "create", patch: "" }],
977+
},
978+
})
979+
);
980+
expect(msg).toBe("Creating sentry.py...");
981+
});
982+
983+
test("single modify shows verb and basename", () => {
984+
const msg = describeLocalOp(
985+
payload({
986+
operation: "apply-patchset",
987+
params: {
988+
patches: [{ path: "settings.py", action: "modify", patch: "" }],
989+
},
990+
})
991+
);
992+
expect(msg).toBe("Modifying settings.py...");
993+
});
994+
995+
test("single delete shows verb and basename", () => {
996+
const msg = describeLocalOp(
997+
payload({
998+
operation: "apply-patchset",
999+
params: {
1000+
patches: [{ path: "old.js", action: "delete", patch: "" }],
1001+
},
1002+
})
1003+
);
1004+
expect(msg).toBe("Deleting old.js...");
1005+
});
1006+
1007+
test("multiple patches shows count and breakdown", () => {
1008+
const msg = describeLocalOp(
1009+
payload({
1010+
operation: "apply-patchset",
1011+
params: {
1012+
patches: [
1013+
{ path: "a.py", action: "create", patch: "" },
1014+
{ path: "b.py", action: "create", patch: "" },
1015+
{ path: "c.py", action: "modify", patch: "" },
1016+
],
1017+
},
1018+
})
1019+
);
1020+
expect(msg).toBe("Applying 3 file changes (2 created, 1 modified)...");
1021+
});
1022+
});
1023+
1024+
describe("run-commands", () => {
1025+
test("single command shows the command", () => {
1026+
const msg = describeLocalOp(
1027+
payload({
1028+
operation: "run-commands",
1029+
params: { commands: ["pip install sentry-sdk"] },
1030+
})
1031+
);
1032+
expect(msg).toBe("Running pip install sentry-sdk...");
1033+
});
1034+
1035+
test("multiple commands shows count and first", () => {
1036+
const msg = describeLocalOp(
1037+
payload({
1038+
operation: "run-commands",
1039+
params: {
1040+
commands: ["pip install sentry-sdk", "python manage.py check"],
1041+
},
1042+
})
1043+
);
1044+
expect(msg).toBe("Running 2 commands (pip install sentry-sdk, ...)...");
1045+
});
1046+
});
1047+
1048+
describe("list-dir", () => {
1049+
test("shows generic listing message", () => {
1050+
const msg = describeLocalOp(
1051+
payload({ operation: "list-dir", params: { path: "." } })
1052+
);
1053+
expect(msg).toBe("Listing directory...");
1054+
});
1055+
});
1056+
1057+
describe("create-sentry-project", () => {
1058+
test("shows project name and platform", () => {
1059+
const msg = describeLocalOp(
1060+
payload({
1061+
operation: "create-sentry-project",
1062+
params: { name: "my-app", platform: "python-django" },
1063+
})
1064+
);
1065+
expect(msg).toBe('Creating project "my-app" (python-django)...');
1066+
});
1067+
});
1068+
});

0 commit comments

Comments
 (0)