From 1fcf1bbdc2e2f9597d149832b2833bc4812614ba Mon Sep 17 00:00:00 2001 From: Kevin Date: Mon, 13 Jul 2026 09:59:26 -0400 Subject: [PATCH] PROD-2310: fail sync exit code on item failures, auto-publish errors, and precondition aborts The sync/push command handler ignored the { success } returned by pushInstances, so a sync could exit 0 despite failed items, failed auto-publish, or a hard-stop abort. Additionally, `success` was computed before auto-publish ran, so auto-publish errors never fed the exit code. - index.ts: sync/push handler now checks result.success and exits 1; a failed precondition (validateCommand false, e.g. missing target locale) exits 1 instead of returning silently; a thrown abort (model-validation hard-stop) is caught and exits 1. - push.ts: fold real auto-publish failures into the returned success via new exported helper hasBlockingAutoPublishErrors (publish/fatal count; mapping/refresh left for PROD-2311). - push.test.ts: focused tests for hasBlockingAutoPublishErrors. Co-Authored-By: Claude Opus 4.8 (1M context) --- src/core/push.ts | 28 ++++++++++++++++++++++++--- src/core/tests/push.test.ts | 38 ++++++++++++++++++++++++++++++++++++- src/index.ts | 19 +++++++++++++++++-- 3 files changed, 79 insertions(+), 6 deletions(-) diff --git a/src/core/push.ts b/src/core/push.ts index 2f0eea4c..f310bf2d 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 824945ae..03504dde 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 d2a012a0..614cbc1d 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); + } }, });