diff --git a/src/core/push.ts b/src/core/push.ts index 2f0eea4..f310bf2 100644 --- a/src/core/push.ts +++ b/src/core/push.ts @@ -14,6 +14,18 @@ import { Pushers, PushResults } from "../lib/pushers/orchestrate-pushers"; import { Pull } from "./pull"; import { preflightReport } from "../lib/preflight/preflight-report"; +/** + * PROD-2310: Decide whether a set of auto-publish errors should fail the sync exit code. + * Real publish failures ("publish") and a fatal auto-publish crash ("fatal") are + * unambiguous failures that CI must detect. "mapping"/"refresh" entries are post-publish + * bookkeeping that PROD-2311 may reclassify, so they are NOT treated as hard failures here. + */ +export function hasBlockingAutoPublishErrors( + autoPublishErrors: Array<{ locale: string; type: string; error: string }> +): boolean { + return autoPublishErrors.some((e) => e.type === "publish" || e.type === "fatal"); +} + export class Push { private pushers: Pushers; @@ -130,14 +142,16 @@ export class Push { } }); - // Calculate overall success - check both operation failures and item failures - const success = totalFailed === 0 && totalSyncFailures === 0; + // Calculate sync-phase success - check both operation failures and item failures. + // PROD-2310: this reflects only the sync/push phase. Auto-publish runs AFTER this + // point, so its errors are folded into the final `success` below (before we return). + const syncSuccess = totalFailed === 0 && totalSyncFailures === 0; // Use the orchestrator summary function to handle completion logic // But DON'T show log files yet - we'll show them at the very end const logger = getLogger(); if (logger) { - logger.orchestratorSummary(results, totalElapsedTime, success, []); // Empty array = no log files shown yet + logger.orchestratorSummary(results, totalElapsedTime, syncSuccess, []); // Empty array = no log files shown yet } finalizeLogger(); // Finalize global logger if it exists @@ -151,6 +165,14 @@ export class Push { autoPublishErrors = await this.executeAutoPublish(results, autoPublish); } + // PROD-2310: Fold auto-publish failures into the returned success signal so the + // entry point can exit non-zero. Real publish failures ("publish") and a fatal + // auto-publish crash ("fatal") are unambiguous failures that CI must detect; they + // previously never affected the exit code. "mapping"/"refresh" entries are + // post-publish bookkeeping that PROD-2311 may reclassify, so they are deliberately + // NOT treated as hard failures here (see PROD-2311 follow-up). + const success = syncSuccess && !hasBlockingAutoPublishErrors(autoPublishErrors); + // Final error summary - show if there were ANY failures (sync or auto-publish) const hasFailures = totalSyncFailures > 0 || syncErrors.length > 0 || autoPublishErrors.length > 0; diff --git a/src/core/tests/push.test.ts b/src/core/tests/push.test.ts index 824945a..03504dd 100644 --- a/src/core/tests/push.test.ts +++ b/src/core/tests/push.test.ts @@ -1,4 +1,4 @@ -import { Push } from "../push"; +import { Push, hasBlockingAutoPublishErrors } from "../push"; import { resetState, setState } from "../state"; beforeEach(() => { @@ -40,3 +40,39 @@ describe("Push.pushInstances", () => { } }); }); + +// ─── PROD-2310: auto-publish errors feed the exit code ─────────────────────── + +describe("hasBlockingAutoPublishErrors", () => { + it("returns false for an empty error set", () => { + expect(hasBlockingAutoPublishErrors([])).toBe(false); + }); + + it("returns true when a real publish failure is present", () => { + expect( + hasBlockingAutoPublishErrors([{ locale: "en-us", type: "publish", error: "boom" }]) + ).toBe(true); + }); + + it("returns true when a fatal auto-publish error is present", () => { + expect(hasBlockingAutoPublishErrors([{ locale: "all", type: "fatal", error: "crash" }])).toBe(true); + }); + + it("does not treat post-publish bookkeeping (mapping/refresh) as blocking", () => { + expect( + hasBlockingAutoPublishErrors([ + { locale: "en-us", type: "mapping", error: "stale" }, + { locale: "en-us", type: "refresh", error: "skipped" }, + ]) + ).toBe(false); + }); + + it("returns true when a real failure is mixed with bookkeeping errors", () => { + expect( + hasBlockingAutoPublishErrors([ + { locale: "en-us", type: "mapping", error: "stale" }, + { locale: "en-us", type: "publish", error: "boom" }, + ]) + ).toBe(true); + }); +}); diff --git a/src/index.ts b/src/index.ts index d2a012a..614cbc1 100644 --- a/src/index.ts +++ b/src/index.ts @@ -208,11 +208,26 @@ yargs.command({ // Validate sync command requirements const isValidCommand = await auth.validateCommand("push"); if (!isValidCommand) { - return; + // PROD-2310: a failed precondition (e.g. a requested locale missing on the target) + // is an abort with zero work done — it must exit non-zero so CI can detect it, + // instead of returning silently with exit code 0. + process.exit(1); } const push = new Push(); - await push.pushInstances(); + try { + // PROD-2310: honor the sync result. The handler previously ignored the returned + // { success }, so a sync with failed items or failed auto-publish still exited 0. + const result = await push.pushInstances(); + if (!result.success) { + process.exit(1); + } + } catch (error) { + // PROD-2310: a hard-stop abort (e.g. model-validation failure) throws out of + // pushInstances. The error is already logged inside pushInstances; exit non-zero + // here rather than relying on unhandled-rejection behavior for the exit code. + process.exit(1); + } }, });