diff --git a/packages/cli/src/lib/init/wizard-runner.ts b/packages/cli/src/lib/init/wizard-runner.ts index 2165c79a0..abe14eb2f 100644 --- a/packages/cli/src/lib/init/wizard-runner.ts +++ b/packages/cli/src/lib/init/wizard-runner.ts @@ -1343,6 +1343,39 @@ function syncWorkflowStepStatuses( } } +type WorkflowFailure = { + message: string; + resultForDisplay: WorkflowRunResult; + workflowCode: number | undefined; +}; + +function getWorkflowFailure( + result: WorkflowRunResult +): WorkflowFailure | undefined { + const workflowCode = result.result?.exitCode; + if (result.status === "success" && workflowCode === 0) { + return; + } + + const missingExitCodeMessage = + result.status === "success" && workflowCode === undefined + ? "Workflow reported success without an explicit exit code" + : undefined; + const message = + missingExitCodeMessage ?? + result.error ?? + result.result?.message ?? + "Workflow returned an error"; + + return { + message, + resultForDisplay: missingExitCodeMessage + ? { ...result, error: message } + : result, + workflowCode, + }; +} + // biome-ignore lint/nursery/useMaxParams: cwd and sentryProject are optional trailing extensions export async function handleFinalResult( result: WorkflowRunResult, @@ -1352,26 +1385,22 @@ export async function handleFinalResult( cwd?: string, sentryProject?: SentryProjectIdentity ): Promise { - const hasError = result.status !== "success" || result.result?.exitCode; + const failure = getWorkflowFailure(result); - if (hasError) { + if (failure) { if (spinState.running) { spin.stop("Failed", 1); spinState.running = false; } - formatError(result, ui); + formatError(failure.resultForDisplay, ui); // Map workflow-internal exit codes to semantic EXIT.* constants - const workflowCode = result.result?.exitCode; - const exitCode = mapWorkflowExitCode(workflowCode); + const exitCode = mapWorkflowExitCode(failure.workflowCode); setTag("wizard.outcome", "errored"); - if (workflowCode !== undefined) { - setTag("wizard.exit_code", workflowCode); + if (failure.workflowCode !== undefined) { + setTag("wizard.exit_code", failure.workflowCode); } - throw new WizardError( - result.error ?? result.result?.message ?? "Workflow returned an error", - { exitCode } - ); + throw new WizardError(failure.message, { exitCode }); } // Run verification before printing the final summary so the user diff --git a/packages/cli/test/lib/init/wizard-runner.test.ts b/packages/cli/test/lib/init/wizard-runner.test.ts index b6304215d..5e8d9bcf5 100644 --- a/packages/cli/test/lib/init/wizard-runner.test.ts +++ b/packages/cli/test/lib/init/wizard-runner.test.ts @@ -165,7 +165,10 @@ beforeEach(() => { savedPlainOutput = process.env.SENTRY_PLAIN_OUTPUT; process.env.SENTRY_PLAIN_OUTPUT = "0"; - mockStartResult = { status: "success", result: { platform: "React" } }; + mockStartResult = { + status: "success", + result: { exitCode: 0, platform: "React" }, + }; mockResumeResults = []; resumeCallCount = 0; mockRunByIdResult = new Error("runById not configured"); @@ -236,6 +239,7 @@ beforeEach(() => { sharedResumeAsyncMock = vi.fn(() => { const result = mockResumeResults[resumeCallCount] ?? { status: "success", + result: { exitCode: 0 }, }; resumeCallCount += 1; return Promise.resolve(result); @@ -610,7 +614,7 @@ describe("runWizard", () => { "apply-codemods": { suspendPayload: payload }, }, }; - mockResumeResults = [{ status: "success" }]; + mockResumeResults = [{ status: "success", result: { exitCode: 0 } }]; await runWizard(makeOptions()); @@ -639,7 +643,7 @@ describe("runWizard", () => { "apply-codemods": { suspendPayload: protocolPayload }, }, }; - mockResumeResults = [{ status: "success" }]; + mockResumeResults = [{ status: "success", result: { exitCode: 0 } }]; await runWizard(makeOptions()); @@ -672,7 +676,7 @@ describe("runWizard", () => { }, }, }; - mockResumeResults = [{ status: "success" }]; + mockResumeResults = [{ status: "success", result: { exitCode: 0 } }]; await runWizard(makeOptions()); @@ -710,7 +714,7 @@ describe("runWizard", () => { }, }, }; - mockResumeResults = [{ status: "success" }]; + mockResumeResults = [{ status: "success", result: { exitCode: 0 } }]; await runWizard(makeOptions()); @@ -736,7 +740,7 @@ describe("runWizard", () => { }, }, }; - mockResumeResults = [{ status: "success" }]; + mockResumeResults = [{ status: "success", result: { exitCode: 0 } }]; await runWizard(makeOptions({ dryRun: true })); @@ -907,7 +911,7 @@ describe("runWizard", () => { }, }, }; - mockResumeResults = [{ status: "success" }]; + mockResumeResults = [{ status: "success", result: { exitCode: 0 } }]; await runWizard(makeOptions()); @@ -979,7 +983,7 @@ describe("runWizard", () => { message: "Using existing project", data: {}, }); - mockResumeResults = [{ status: "success" }]; + mockResumeResults = [{ status: "success", result: { exitCode: 0 } }]; await runWizard(makeOptions()); @@ -1013,7 +1017,7 @@ describe("runWizard", () => { }, }; executeToolSpy.mockResolvedValue({ ok: true, data: identity }); - mockResumeResults = [{ status: "success" }]; + mockResumeResults = [{ status: "success", result: { exitCode: 0 } }]; await runWizard(makeOptions()); @@ -1132,7 +1136,9 @@ describe("runWizard — MastraClient lifecycle", () => { createRun: vi.fn(() => Promise.resolve({ startAsync: startAsyncMock, - resumeAsync: vi.fn(() => Promise.resolve({ status: "success" })), + resumeAsync: vi.fn(() => + Promise.resolve({ status: "success", result: { exitCode: 0 } }) + ), }) ), } as any; @@ -1149,6 +1155,15 @@ describe("runWizard — MastraClient lifecycle", () => { // ─── Additional coverage tests ─────────────────────────────────────────────── describe("runWizard — workflow exit codes", () => { + test("rejects workflow success without an explicit exit code", async () => { + mockStartResult = { status: "success", result: { platform: "React" } }; + + const error = await runWizard(makeOptions()).catch((caught) => caught); + + expect(error).toBeInstanceOf(WizardError); + expect((error as WizardError).exitCode).not.toBe(0); + }); + // handleFinalResult calls mapWorkflowExitCode when the workflow result // carries a non-zero exitCode. Each case maps a server-internal code to // the CLI's semantic EXIT constant. @@ -1270,7 +1285,7 @@ describe("runWizard — resumeWithRetry stale-step recovery", () => { let capturedResume: Record | undefined; makeStaleStepRun((args) => { capturedResume = args.resumeData as Record; - return Promise.resolve({ status: "success" }); + return Promise.resolve({ status: "success", result: { exitCode: 0 } }); }); await runWizard(makeOptions()); @@ -1299,7 +1314,10 @@ describe("runWizard — resumeWithRetry stale-step recovery", () => { status: "suspended", suspendPayload: { ...protocolPayload, detail: "new display text" }, }) - .mockResolvedValueOnce({ status: "success" }); + .mockResolvedValueOnce({ + status: "success", + result: { exitCode: 0 }, + }); let resumeCount = 0; makeStaleStepRun(() => { resumeCount += 1; @@ -1322,6 +1340,7 @@ describe("runWizard — resumeWithRetry stale-step recovery", () => { }; const currentRunState: WorkflowRunResult = { status: "success", + result: { exitCode: 0 }, suspended: [], }; runByIdMock.mockImplementation( @@ -1335,7 +1354,7 @@ describe("runWizard — resumeWithRetry stale-step recovery", () => { if (resumeCount === 1) { return Promise.reject(staleStepError(409)); } - return Promise.resolve({ status: "success" }); + return Promise.resolve({ status: "success", result: { exitCode: 0 } }); }); await runWizard(makeOptions()); @@ -1368,6 +1387,7 @@ describe("runWizard — resumeWithRetry stale-step recovery", () => { }; runByIdMock.mockResolvedValue({ status: "success", + result: { exitCode: 0 }, suspended: [], }); let resumeCount = 0; @@ -1394,7 +1414,10 @@ describe("runWizard — resumeWithRetry stale-step recovery", () => { status: "suspended", suspendPayload: toolPayload, }) - .mockResolvedValueOnce({ status: "success" }); + .mockResolvedValueOnce({ + status: "success", + result: { exitCode: 0 }, + }); let resumeCount = 0; makeStaleStepRun(() => { @@ -1445,7 +1468,7 @@ describe("runWizard — resumeWithRetry stale-step recovery", () => { if (resumeCount === 1) { return Promise.reject(staleStepError()); } - return Promise.resolve({ status: "success" }); + return Promise.resolve({ status: "success", result: { exitCode: 0 } }); }); await runWizard(makeOptions()); @@ -1518,7 +1541,7 @@ describe("runWizard — resumeWithRetry stale-step recovery", () => { suspended: [["tool-step"]], steps: { "tool-step": { suspendPayload: toolPayload } }, }; - mockRunByIdResult = { status: "success" }; + mockRunByIdResult = { status: "success", result: { exitCode: 0 } }; let resumeCount = 0; makeStaleStepRun(() => { @@ -1606,7 +1629,7 @@ describe("runWizard — resumeWithRetry stale-step recovery", () => { steps: { "apply-codemods": { suspendPayload: applyPayload } }, }); } - return Promise.resolve({ status: "success" }); + return Promise.resolve({ status: "success", result: { exitCode: 0 } }); }); await runWizard(makeOptions()); @@ -1793,7 +1816,7 @@ describe("runWizard — additional coverage", () => { "step-b": { suspendPayload: payload }, }, }; - mockResumeResults = [{ status: "success" }]; + mockResumeResults = [{ status: "success", result: { exitCode: 0 } }]; await expect(runWizard(makeOptions())).rejects.toThrow(WizardError); @@ -1813,7 +1836,7 @@ describe("runWizard — additional coverage", () => { "step-b": { suspendPayload: payload }, }, }; - mockResumeResults = [{ status: "success" }]; + mockResumeResults = [{ status: "success", result: { exitCode: 0 } }]; await runWizard(makeOptions()); @@ -1845,7 +1868,7 @@ describe("runWizard — additional coverage", () => { suspended: [["detect-platform"]], steps: { "detect-platform": { suspendPayload: payloadB } }, }, - { status: "success" }, + { status: "success", result: { exitCode: 0 } }, ]; await runWizard(makeOptions()); @@ -1900,6 +1923,7 @@ describe("runWizard — additional coverage", () => { mockResumeResults = [ { status: "success", + result: { exitCode: 0 }, steps: { "discover-context": { status: "success" }, "detect-platform": { status: "success" }, @@ -1986,7 +2010,7 @@ describe("runWizard — additional coverage", () => { }, }, }; - mockResumeResults = [{ status: "success" }]; + mockResumeResults = [{ status: "success", result: { exitCode: 0 } }]; await runWizard(makeOptions()); @@ -2069,7 +2093,7 @@ describe("runWizard — progress rotation for long-running steps", () => { ).toBe(true); // Resolve the resume and let the wizard finish - resolveResume({ status: "success" }); + resolveResume({ status: "success", result: { exitCode: 0 } }); await vi.advanceTimersByTimeAsync(100); await runPromise; }); @@ -2126,7 +2150,7 @@ describe("runWizard — progress rotation for long-running steps", () => { // After exhausting messages, should show elapsed time expect(messages.some((m) => /\(\d+s\)/.test(m))).toBe(true); - resolveResume({ status: "success" }); + resolveResume({ status: "success", result: { exitCode: 0 } }); await vi.advanceTimersByTimeAsync(100); await runPromise; }); @@ -2182,7 +2206,7 @@ describe("runWizard — progress rotation for long-running steps", () => { // No new messages should have been added by the rotation timer expect(messagesAfter).toBe(messagesBefore); - resolveResume({ status: "success" }); + resolveResume({ status: "success", result: { exitCode: 0 } }); await vi.advanceTimersByTimeAsync(100); await runPromise; }); diff --git a/packages/cli/test/lib/wizard-runner-handle-final-result.mocked.test.ts b/packages/cli/test/lib/wizard-runner-handle-final-result.mocked.test.ts index d433f5e5e..8d80e2c72 100644 --- a/packages/cli/test/lib/wizard-runner-handle-final-result.mocked.test.ts +++ b/packages/cli/test/lib/wizard-runner-handle-final-result.mocked.test.ts @@ -47,16 +47,20 @@ function makeSpinState(running = false) { /** Minimal WizardUI stub — only the methods formatError touches. */ function makeUI() { - const noop = () => null; return { - log: { error: noop, warn: noop, info: noop, message: noop }, - cancel: noop, - feedback: noop, - summary: noop, - outro: noop, - intro: noop, - setStep: noop, - markFilesAnalyzed: noop, + log: { + error: vi.fn(), + warn: vi.fn(), + info: vi.fn(), + message: vi.fn(), + }, + cancel: vi.fn(), + feedback: vi.fn(), + summary: vi.fn(), + outro: vi.fn(), + intro: vi.fn(), + setStep: vi.fn(), + markFilesAnalyzed: vi.fn(), } as any; } @@ -80,6 +84,41 @@ beforeEach(() => { }); describe("handleFinalResult", () => { + describe("success contract", () => { + test.each([ + { status: "success" } as WorkflowRunResult, + { status: "success", result: {} } as WorkflowRunResult, + ])("rejects a successful workflow without an explicit zero exit", async (result) => { + const ui = makeUI(); + const error = await handleFinalResult( + result, + makeSpinnerHandle(), + makeSpinState(), + ui + ).catch((caught) => caught); + + expect(error).toBeInstanceOf(WizardError); + expect((error as WizardError).exitCode).not.toBe(0); + expect((error as WizardError).message).toBe( + "Workflow reported success without an explicit exit code" + ); + expect(ui.log.error).toHaveBeenCalledWith( + "Workflow reported success without an explicit exit code" + ); + }); + + test("accepts a successful workflow with exit code zero", async () => { + await expect( + handleFinalResult( + { status: "success", result: { exitCode: 0 } }, + makeSpinnerHandle(), + makeSpinState(), + makeUI() + ) + ).resolves.toBeUndefined(); + }); + }); + describe("WizardError message", () => { test("uses bail message from result.result.message when present", async () => { const result = makeBailResult({