PROD-2310: sync exits non-zero on failures and precondition aborts

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;
 

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);
+  });
+});

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);
+    }
   },
 });