Skip to content

Commit bc50b58

Browse files
authored
fix(init): require explicit zero workflow exit (#1451)
## Summary Make `sentry init` accept a terminal workflow result as successful only when it explicitly reports `status: "success"` and `result.exitCode: 0`. Previously, a successful response with no `result` or no `exitCode` passed the truthiness check and could print the success handoff with process exit 0. The CLI now treats that as malformed, surfaces a specific `WizardError`, and exits nonzero. Existing nonzero workflow-to-CLI exit mappings are unchanged. This makes the process exit status a reliable black-box signal for the init smoke harness. ## Compatibility This intentionally stops accepting terminal success payloads that omit `exitCode`. The current wizard success output already returns `exitCode: 0`. The draft [#1406](#1406) also touches `handleFinalResult`; merge order will require rebasing whichever PR lands second. ## Test plan - focused wizard runner tests: 74 passed - CLI typecheck - Biome on the three changed files - `git diff --check` - native `darwin-arm64` binary build - real Express init against the local candidate API: exit 0 - invalid OpenRouter credential: CLI exit 61 and smoke harness exit 1 ## Rollout The paired API-owned smoke is [getsentry/cli-init-api#249](getsentry/cli-init-api#249). Once this lands, bump its pinned CLI SHA so malformed successful workflow responses cannot produce a false-green smoke result.
1 parent 8300222 commit bc50b58

3 files changed

Lines changed: 136 additions & 44 deletions

File tree

packages/cli/src/lib/init/wizard-runner.ts

Lines changed: 40 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1329,6 +1329,39 @@ function syncWorkflowStepStatuses(
13291329
}
13301330
}
13311331

1332+
type WorkflowFailure = {
1333+
message: string;
1334+
resultForDisplay: WorkflowRunResult;
1335+
workflowCode: number | undefined;
1336+
};
1337+
1338+
function getWorkflowFailure(
1339+
result: WorkflowRunResult
1340+
): WorkflowFailure | undefined {
1341+
const workflowCode = result.result?.exitCode;
1342+
if (result.status === "success" && workflowCode === 0) {
1343+
return;
1344+
}
1345+
1346+
const missingExitCodeMessage =
1347+
result.status === "success" && workflowCode === undefined
1348+
? "Workflow reported success without an explicit exit code"
1349+
: undefined;
1350+
const message =
1351+
missingExitCodeMessage ??
1352+
result.error ??
1353+
result.result?.message ??
1354+
"Workflow returned an error";
1355+
1356+
return {
1357+
message,
1358+
resultForDisplay: missingExitCodeMessage
1359+
? { ...result, error: message }
1360+
: result,
1361+
workflowCode,
1362+
};
1363+
}
1364+
13321365
// biome-ignore lint/nursery/useMaxParams: cwd and sentryProject are optional trailing extensions
13331366
export async function handleFinalResult(
13341367
result: WorkflowRunResult,
@@ -1338,26 +1371,22 @@ export async function handleFinalResult(
13381371
cwd?: string,
13391372
sentryProject?: SentryProjectIdentity
13401373
): Promise<void> {
1341-
const hasError = result.status !== "success" || result.result?.exitCode;
1374+
const failure = getWorkflowFailure(result);
13421375

1343-
if (hasError) {
1376+
if (failure) {
13441377
if (spinState.running) {
13451378
spin.stop("Failed", 1);
13461379
spinState.running = false;
13471380
}
1348-
formatError(result, ui);
1381+
formatError(failure.resultForDisplay, ui);
13491382

13501383
// Map workflow-internal exit codes to semantic EXIT.* constants
1351-
const workflowCode = result.result?.exitCode;
1352-
const exitCode = mapWorkflowExitCode(workflowCode);
1384+
const exitCode = mapWorkflowExitCode(failure.workflowCode);
13531385
setTag("wizard.outcome", "errored");
1354-
if (workflowCode !== undefined) {
1355-
setTag("wizard.exit_code", workflowCode);
1386+
if (failure.workflowCode !== undefined) {
1387+
setTag("wizard.exit_code", failure.workflowCode);
13561388
}
1357-
throw new WizardError(
1358-
result.error ?? result.result?.message ?? "Workflow returned an error",
1359-
{ exitCode }
1360-
);
1389+
throw new WizardError(failure.message, { exitCode });
13611390
}
13621391

13631392
// Run verification before printing the final summary so the user

packages/cli/test/lib/init/wizard-runner.test.ts

Lines changed: 48 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -218,7 +218,10 @@ beforeEach(() => {
218218
savedPlainOutput = process.env.SENTRY_PLAIN_OUTPUT;
219219
process.env.SENTRY_PLAIN_OUTPUT = "0";
220220

221-
mockStartResult = { status: "success", result: { platform: "React" } };
221+
mockStartResult = {
222+
status: "success",
223+
result: { exitCode: 0, platform: "React" },
224+
};
222225
mockResumeResults = [];
223226
resumeCallCount = 0;
224227
mockRunByIdResult = new Error("runById not configured");
@@ -293,6 +296,7 @@ beforeEach(() => {
293296
sharedResumeAsyncMock = vi.fn(() => {
294297
const result = mockResumeResults[resumeCallCount] ?? {
295298
status: "success",
299+
result: { exitCode: 0 },
296300
};
297301
resumeCallCount += 1;
298302
return Promise.resolve(withV1SuspendEnvelopes(result));
@@ -667,7 +671,7 @@ describe("runWizard", () => {
667671
"apply-codemods": { suspendPayload: payload },
668672
},
669673
};
670-
mockResumeResults = [{ status: "success" }];
674+
mockResumeResults = [{ status: "success", result: { exitCode: 0 } }];
671675

672676
await runWizard(makeOptions());
673677

@@ -696,7 +700,7 @@ describe("runWizard", () => {
696700
"apply-codemods": { suspendPayload: protocolPayload },
697701
},
698702
};
699-
mockResumeResults = [{ status: "success" }];
703+
mockResumeResults = [{ status: "success", result: { exitCode: 0 } }];
700704

701705
await runWizard(makeOptions());
702706

@@ -729,7 +733,7 @@ describe("runWizard", () => {
729733
},
730734
},
731735
};
732-
mockResumeResults = [{ status: "success" }];
736+
mockResumeResults = [{ status: "success", result: { exitCode: 0 } }];
733737

734738
await runWizard(makeOptions());
735739

@@ -767,7 +771,7 @@ describe("runWizard", () => {
767771
},
768772
},
769773
};
770-
mockResumeResults = [{ status: "success" }];
774+
mockResumeResults = [{ status: "success", result: { exitCode: 0 } }];
771775

772776
await runWizard(makeOptions());
773777

@@ -793,7 +797,7 @@ describe("runWizard", () => {
793797
},
794798
},
795799
};
796-
mockResumeResults = [{ status: "success" }];
800+
mockResumeResults = [{ status: "success", result: { exitCode: 0 } }];
797801

798802
await runWizard(makeOptions({ dryRun: true }));
799803

@@ -993,7 +997,7 @@ describe("runWizard", () => {
993997
},
994998
},
995999
};
996-
mockResumeResults = [{ status: "success" }];
1000+
mockResumeResults = [{ status: "success", result: { exitCode: 0 } }];
9971001

9981002
await runWizard(makeOptions());
9991003

@@ -1065,7 +1069,7 @@ describe("runWizard", () => {
10651069
message: "Using existing project",
10661070
data: {},
10671071
});
1068-
mockResumeResults = [{ status: "success" }];
1072+
mockResumeResults = [{ status: "success", result: { exitCode: 0 } }];
10691073

10701074
await runWizard(makeOptions());
10711075

@@ -1099,7 +1103,7 @@ describe("runWizard", () => {
10991103
},
11001104
};
11011105
executeToolSpy.mockResolvedValue({ ok: true, data: identity });
1102-
mockResumeResults = [{ status: "success" }];
1106+
mockResumeResults = [{ status: "success", result: { exitCode: 0 } }];
11031107

11041108
await runWizard(makeOptions());
11051109

@@ -1218,7 +1222,9 @@ describe("runWizard — MastraClient lifecycle", () => {
12181222
createRun: vi.fn(() =>
12191223
Promise.resolve({
12201224
startAsync: startAsyncMock,
1221-
resumeAsync: vi.fn(() => Promise.resolve({ status: "success" })),
1225+
resumeAsync: vi.fn(() =>
1226+
Promise.resolve({ status: "success", result: { exitCode: 0 } })
1227+
),
12221228
})
12231229
),
12241230
} as any;
@@ -1235,6 +1241,15 @@ describe("runWizard — MastraClient lifecycle", () => {
12351241
// ─── Additional coverage tests ───────────────────────────────────────────────
12361242

12371243
describe("runWizard — workflow exit codes", () => {
1244+
test("rejects workflow success without an explicit exit code", async () => {
1245+
mockStartResult = { status: "success", result: { platform: "React" } };
1246+
1247+
const error = await runWizard(makeOptions()).catch((caught) => caught);
1248+
1249+
expect(error).toBeInstanceOf(WizardError);
1250+
expect((error as WizardError).exitCode).not.toBe(0);
1251+
});
1252+
12381253
// handleFinalResult calls mapWorkflowExitCode when the workflow result
12391254
// carries a non-zero exitCode. Each case maps a server-internal code to
12401255
// the CLI's semantic EXIT constant.
@@ -1358,7 +1373,7 @@ describe("runWizard — resumeWithRetry stale-step recovery", () => {
13581373
let capturedResume: Record<string, unknown> | undefined;
13591374
makeStaleStepRun((args) => {
13601375
capturedResume = args.resumeData as Record<string, unknown>;
1361-
return Promise.resolve({ status: "success" });
1376+
return Promise.resolve({ status: "success", result: { exitCode: 0 } });
13621377
});
13631378

13641379
await runWizard(makeOptions());
@@ -1387,7 +1402,10 @@ describe("runWizard — resumeWithRetry stale-step recovery", () => {
13871402
status: "suspended",
13881403
suspendPayload: { ...protocolPayload, detail: "new display text" },
13891404
})
1390-
.mockResolvedValueOnce({ status: "success" });
1405+
.mockResolvedValueOnce({
1406+
status: "success",
1407+
result: { exitCode: 0 },
1408+
});
13911409
let resumeCount = 0;
13921410
makeStaleStepRun(() => {
13931411
resumeCount += 1;
@@ -1410,6 +1428,7 @@ describe("runWizard — resumeWithRetry stale-step recovery", () => {
14101428
};
14111429
const currentRunState: WorkflowRunResult = {
14121430
status: "success",
1431+
result: { exitCode: 0 },
14131432
suspended: [],
14141433
};
14151434
runByIdMock.mockImplementation(
@@ -1423,7 +1442,7 @@ describe("runWizard — resumeWithRetry stale-step recovery", () => {
14231442
if (resumeCount === 1) {
14241443
return Promise.reject(staleStepError(409));
14251444
}
1426-
return Promise.resolve({ status: "success" });
1445+
return Promise.resolve({ status: "success", result: { exitCode: 0 } });
14271446
});
14281447

14291448
await runWizard(makeOptions());
@@ -1456,6 +1475,7 @@ describe("runWizard — resumeWithRetry stale-step recovery", () => {
14561475
};
14571476
runByIdMock.mockResolvedValue({
14581477
status: "success",
1478+
result: { exitCode: 0 },
14591479
suspended: [],
14601480
});
14611481
let resumeCount = 0;
@@ -1487,7 +1507,10 @@ describe("runWizard — resumeWithRetry stale-step recovery", () => {
14871507
status: "suspended",
14881508
suspendPayload: protocolPayload,
14891509
})
1490-
.mockResolvedValueOnce({ status: "success" });
1510+
.mockResolvedValueOnce({
1511+
status: "success",
1512+
result: { exitCode: 0 },
1513+
});
14911514

14921515
let resumeCount = 0;
14931516
makeStaleStepRun(() => {
@@ -1538,7 +1561,7 @@ describe("runWizard — resumeWithRetry stale-step recovery", () => {
15381561
if (resumeCount === 1) {
15391562
return Promise.reject(staleStepError());
15401563
}
1541-
return Promise.resolve({ status: "success" });
1564+
return Promise.resolve({ status: "success", result: { exitCode: 0 } });
15421565
});
15431566

15441567
await runWizard(makeOptions());
@@ -1611,7 +1634,7 @@ describe("runWizard — resumeWithRetry stale-step recovery", () => {
16111634
suspended: [["tool-step"]],
16121635
steps: { "tool-step": { suspendPayload: toolPayload } },
16131636
};
1614-
mockRunByIdResult = { status: "success" };
1637+
mockRunByIdResult = { status: "success", result: { exitCode: 0 } };
16151638

16161639
let resumeCount = 0;
16171640
makeStaleStepRun(() => {
@@ -1708,7 +1731,7 @@ describe("runWizard — resumeWithRetry stale-step recovery", () => {
17081731
steps: { "apply-codemods": { suspendPayload: applyPayload } },
17091732
});
17101733
}
1711-
return Promise.resolve({ status: "success" });
1734+
return Promise.resolve({ status: "success", result: { exitCode: 0 } });
17121735
});
17131736

17141737
await runWizard(makeOptions());
@@ -1895,7 +1918,7 @@ describe("runWizard — additional coverage", () => {
18951918
"step-b": { suspendPayload: payload },
18961919
},
18971920
};
1898-
mockResumeResults = [{ status: "success" }];
1921+
mockResumeResults = [{ status: "success", result: { exitCode: 0 } }];
18991922

19001923
await expect(runWizard(makeOptions())).rejects.toThrow(WizardError);
19011924

@@ -1915,7 +1938,7 @@ describe("runWizard — additional coverage", () => {
19151938
"step-b": { suspendPayload: payload },
19161939
},
19171940
};
1918-
mockResumeResults = [{ status: "success" }];
1941+
mockResumeResults = [{ status: "success", result: { exitCode: 0 } }];
19191942

19201943
await runWizard(makeOptions());
19211944

@@ -1947,7 +1970,7 @@ describe("runWizard — additional coverage", () => {
19471970
suspended: [["detect-platform"]],
19481971
steps: { "detect-platform": { suspendPayload: payloadB } },
19491972
},
1950-
{ status: "success" },
1973+
{ status: "success", result: { exitCode: 0 } },
19511974
];
19521975

19531976
await runWizard(makeOptions());
@@ -2002,6 +2025,7 @@ describe("runWizard — additional coverage", () => {
20022025
mockResumeResults = [
20032026
{
20042027
status: "success",
2028+
result: { exitCode: 0 },
20052029
steps: {
20062030
"discover-context": { status: "success" },
20072031
"detect-platform": { status: "success" },
@@ -2088,7 +2112,7 @@ describe("runWizard — additional coverage", () => {
20882112
},
20892113
},
20902114
};
2091-
mockResumeResults = [{ status: "success" }];
2115+
mockResumeResults = [{ status: "success", result: { exitCode: 0 } }];
20922116

20932117
await runWizard(makeOptions());
20942118

@@ -2171,7 +2195,7 @@ describe("runWizard — progress rotation for long-running steps", () => {
21712195
).toBe(true);
21722196

21732197
// Resolve the resume and let the wizard finish
2174-
resolveResume({ status: "success" });
2198+
resolveResume({ status: "success", result: { exitCode: 0 } });
21752199
await vi.advanceTimersByTimeAsync(100);
21762200
await runPromise;
21772201
});
@@ -2228,7 +2252,7 @@ describe("runWizard — progress rotation for long-running steps", () => {
22282252
// After exhausting messages, should show elapsed time
22292253
expect(messages.some((m) => /\(\d+s\)/.test(m))).toBe(true);
22302254

2231-
resolveResume({ status: "success" });
2255+
resolveResume({ status: "success", result: { exitCode: 0 } });
22322256
await vi.advanceTimersByTimeAsync(100);
22332257
await runPromise;
22342258
});
@@ -2284,7 +2308,7 @@ describe("runWizard — progress rotation for long-running steps", () => {
22842308
// No new messages should have been added by the rotation timer
22852309
expect(messagesAfter).toBe(messagesBefore);
22862310

2287-
resolveResume({ status: "success" });
2311+
resolveResume({ status: "success", result: { exitCode: 0 } });
22882312
await vi.advanceTimersByTimeAsync(100);
22892313
await runPromise;
22902314
});

0 commit comments

Comments
 (0)