#337: Type PackUpdater update results so failures surface a non-zero exit

Sources/mcs/Commands/PackCommand.swift

@@ -652,6 +652,8 @@ struct UpdatePack: LockedCommand {
 
         var updatedData = registryData
         var updatedCount = 0
+        var attemptedCount = 0
+        var failedPacks: [String] = []
 
         for entry in packsToUpdate {
             if entry.isLocalPack {
@@ -663,10 +665,12 @@ struct UpdatePack: LockedCommand {
                 continue
             }
 
+            attemptedCount += 1
             ctx.output.info("Checking \(entry.displayName)...")
 
             guard let packPath = entry.resolvedPath(packsDirectory: ctx.env.packsDirectory) else {
                 ctx.output.error("Pack '\(entry.identifier)' has an invalid path — skipping")
+                failedPacks.append(entry.identifier)
                 continue
             }
 
@@ -678,8 +682,11 @@ struct UpdatePack: LockedCommand {
                 ctx.registry.register(updatedEntry, in: &updatedData)
                 updatedCount += 1
                 ctx.output.success("\(entry.displayName): updated (\(updatedEntry.shortSHA))")
-            case let .skipped(reason):
-                ctx.output.warn("\(entry.identifier): \(reason)")
+            case .trustDeclined:
+                ctx.output.info("\(entry.identifier): \(result.reason ?? "trust not granted") (will re-prompt next run)")
+            case .fetchFailed, .manifestInvalid, .internalError:
+                ctx.output.warn("\(entry.identifier): \(result.reason ?? "update failed")")
+                failedPacks.append(entry.identifier)
             }
         }
 
@@ -694,6 +701,14 @@ struct UpdatePack: LockedCommand {
             ctx.output.plain("")
             ctx.output.info("Run 'mcs update' to apply updates across all configured scopes.")
         }
+
+        if PackUpdater.shouldExitNonZero(
+            failedCount: failedPacks.count,
+            attemptedCount: attemptedCount,
+            isInteractive: ctx.output.hasInteractiveStdin
+        ) {
+            throw ExitCode.failure
+        }
     }
 }
 

Sources/mcs/Commands/SyncCommand.swift

@@ -58,6 +58,10 @@ struct SyncCommand: LockedCommand {
                     + "combine with 'mcs config set generate-lockfile true' if you want a lockfile."
             )
             let lockOps = LockfileOperations(environment: env, output: output, shell: shell)
+            // On a non-interactive hard failure this throws before the sync phase below, so
+            // healthy packs are fetched (and their SHAs saved) but not re-applied this run —
+            // unlike `mcs update`, which re-applies first and signals failure last. Accepted
+            // for this deprecated path; `mcs update` is the supported fetch-and-apply command.
             try lockOps.updatePacks()
         }
 

Sources/mcs/Commands/UpdateCommand.swift

@@ -65,7 +65,7 @@ struct UpdateCommand: LockedCommand {
         let registryFile = PackRegistryFile(path: env.packsRegistry)
         let registryData = try registryFile.load()
 
-        let (updatedRegistryData, anyUpdated, skippedPackIDs) = try runUpdatePhase(
+        let updatePhase = try runUpdatePhase(
             packIDsToUpdate: configuredAcrossScopes,
             registryFile: registryFile,
             registryData: registryData,
@@ -74,8 +74,8 @@ struct UpdateCommand: LockedCommand {
             output: output
         )
 
-        if !dryRun, anyUpdated {
-            try registryFile.save(updatedRegistryData)
+        if !dryRun, updatePhase.anyUpdated {
+            try registryFile.save(updatePhase.data)
         }
 
         let techPackRegistry = TechPackRegistry.loadWithExternalPacks(
@@ -85,7 +85,7 @@ struct UpdateCommand: LockedCommand {
 
         try runReapplyPhase(
             runs: runs,
-            skippedPackIDs: skippedPackIDs,
+            skippedPackIDs: updatePhase.skipped,
             registry: techPackRegistry,
             env: env,
             shell: shell,
@@ -97,6 +97,17 @@ struct UpdateCommand: LockedCommand {
         if !dryRun {
             UpdateChecker.checkAndPrint(env: env, shell: shell, output: output)
         }
+
+        // Reapply already ran for the packs that succeeded; signal failure last so a partial
+        // outage still re-applies the healthy packs.
+        if PackUpdater.shouldExitNonZero(
+            failedCount: updatePhase.failed.count,
+            attemptedCount: updatePhase.attempted,
+            isInteractive: output.hasInteractiveStdin
+        ) {
+            output.error("Failed to update: \(updatePhase.failed.sorted().joined(separator: ", "))")
+            throw ExitCode.failure
+        }
     }
 
     // MARK: - Helpers
@@ -215,21 +226,29 @@ struct UpdateCommand: LockedCommand {
         env: Environment,
         shell: ShellRunner,
         output: CLIOutput
-    ) throws -> (data: PackRegistryFile.RegistryData, anyUpdated: Bool, skipped: Set<String>) {
+    ) throws -> UpdatePhaseOutcome {
         var updatedData = registryData
         var anyUpdated = false
+        // `skipped` is excluded from reapply (every non-updated pack lands here so we never
+        // reapply a stale/broken checkout). `failed` is the hard-failure subset of `skipped`,
+        // used only for the exit-code decision in `perform()`. `attempted` counts the non-local
+        // packs we tried, so "every attempted pack failed" is an accurate trigger.
         var skipped: Set<String> = []
+        var failed: Set<String> = []
+        var attempted = 0
 
         let entries = registryData.packs.filter { packIDsToUpdate.contains($0.identifier) }
-        guard !entries.isEmpty else { return (updatedData, anyUpdated, skipped) }
+        guard !entries.isEmpty else {
+            return UpdatePhaseOutcome(data: updatedData, anyUpdated: anyUpdated, skipped: skipped, failed: failed, attempted: attempted)
+        }
 
         output.header("Updating packs")
 
         if dryRun {
             for entry in entries {
                 output.dimmed("  \(entry.displayName): would check for updates")
             }
-            return (updatedData, anyUpdated, skipped)
+            return UpdatePhaseOutcome(data: updatedData, anyUpdated: anyUpdated, skipped: skipped, failed: failed, attempted: attempted)
         }
 
         let updater = PackUpdater(
@@ -245,9 +264,11 @@ struct UpdateCommand: LockedCommand {
                 continue
             }
 
+            attempted += 1
             guard let packPath = entry.resolvedPath(packsDirectory: env.packsDirectory) else {
                 output.warn("  \(entry.identifier): invalid path — skipping")
                 skipped.insert(entry.identifier)
+                failed.insert(entry.identifier)
                 continue
             }
 
@@ -259,13 +280,27 @@ struct UpdateCommand: LockedCommand {
                 registryFile.register(updatedEntry, in: &updatedData)
                 anyUpdated = true
                 output.success("  \(entry.displayName): \(entry.shortSHA) → \(updatedEntry.shortSHA)")
-            case let .skipped(reason):
-                output.warn("  \(entry.identifier): \(reason) (will re-prompt on next 'mcs update')")
+            case .trustDeclined:
+                output.info("  \(entry.identifier): \(result.reason ?? "trust not granted") (will re-prompt on next 'mcs update')")
                 skipped.insert(entry.identifier)
+            case .fetchFailed, .manifestInvalid, .internalError:
+                output.warn("  \(entry.identifier): \(result.reason ?? "update failed")")
+                skipped.insert(entry.identifier)
+                failed.insert(entry.identifier)
             }
         }
 
-        return (updatedData, anyUpdated, skipped)
+        return UpdatePhaseOutcome(data: updatedData, anyUpdated: anyUpdated, skipped: skipped, failed: failed, attempted: attempted)
+    }
+
+    /// Result of the fetch/trust update pass, before reapply. `skipped` packs are excluded
+    /// from reapply; `failed` is the hard-failure subset used for the process exit code.
+    private struct UpdatePhaseOutcome {
+        let data: PackRegistryFile.RegistryData
+        let anyUpdated: Bool
+        let skipped: Set<String>
+        let failed: Set<String>
+        let attempted: Int
     }
 
     private func runReapplyPhase(

Sources/mcs/Sync/LockfileOperations.swift

@@ -100,14 +100,18 @@ struct LockfileOperations {
         )
 
         var updatedData = registryData
+        var attemptedCount = 0
+        var failedPacks: [String] = []
         for entry in registryData.packs {
             if entry.isLocalPack {
                 output.dimmed("  \(entry.identifier): local pack (skipped)")
                 continue
             }
 
+            attemptedCount += 1
             guard let packPath = entry.resolvedPath(packsDirectory: environment.packsDirectory) else {
                 output.warn("  \(entry.identifier): invalid path — skipping")
+                failedPacks.append(entry.identifier)
                 continue
             }
 
@@ -118,12 +122,24 @@ struct LockfileOperations {
             case let .updated(updatedEntry):
                 registryFile.register(updatedEntry, in: &updatedData)
                 output.success("  \(entry.identifier): updated (\(updatedEntry.shortSHA))")
-            case let .skipped(reason):
-                output.warn("  \(entry.identifier): \(reason)")
+            case .trustDeclined:
+                output.info("  \(entry.identifier): \(result.reason ?? "trust not granted") (will re-prompt next run)")
+            case .fetchFailed, .manifestInvalid, .internalError:
+                output.warn("  \(entry.identifier): \(result.reason ?? "update failed")")
+                failedPacks.append(entry.identifier)
             }
         }
 
         try registryFile.save(updatedData)
+
+        if PackUpdater.shouldExitNonZero(
+            failedCount: failedPacks.count,
+            attemptedCount: attemptedCount,
+            isInteractive: output.hasInteractiveStdin
+        ) {
+            output.error("Failed to update: \(failedPacks.joined(separator: ", "))")
+            throw ExitCode.failure
+        }
     }
 
     /// Write the lockfile after a successful sync.

Sources/mcs/Sync/PackUpdater.swift

@@ -9,10 +9,17 @@ struct PackUpdater {
     let output: CLIOutput
 
     /// Result of attempting to update a single git pack.
+    ///
+    /// Cases split by *which operation failed*, not by parsing git stderr — a transport
+    /// outage and a trust-decline must be distinguishable so callers can choose an exit code
+    /// (a systemic failure should be non-zero; a user trust-decline is a zero-exit outcome).
     enum UpdateResult {
         case alreadyUpToDate
         case updated(PackRegistryFile.PackEntry)
-        case skipped(String)
+        case fetchFailed(underlying: Error) // git fetch/pull or HEAD read failed (network/transport/ref/broken checkout)
+        case manifestInvalid(underlying: Error) // fetched revision's techpack.yaml is unusable
+        case trustDeclined // user declined the trust prompt — expected, zero exit
+        case internalError(underlying: Error) // script analysis / trust prompt crashed
     }
 
     /// Fetch, validate, and re-trust a single git pack entry.
@@ -26,16 +33,19 @@ struct PackUpdater {
         do {
             fetchResult = try fetcher.update(packPath: packPath, ref: entry.ref)
         } catch {
-            return .skipped("fetch failed — \(error.localizedDescription)")
+            return .fetchFailed(underlying: error)
         }
 
         guard let result = fetchResult else {
             // Disk may be ahead of registry if trust was denied on a previous update.
+            // A failure to read HEAD here is the same class of git-layer error as a failed
+            // fetch (and `fetcher.update` itself reads HEAD before fetching, so a broken
+            // checkout already surfaces as `.fetchFailed`) — keep them one case.
             let diskSHA: String
             do {
                 diskSHA = try fetcher.currentCommit(at: packPath)
             } catch {
-                return .skipped("could not read current commit at \(packPath.path) — \(error.localizedDescription)")
+                return .fetchFailed(underlying: error)
             }
             if diskSHA != entry.commitSHA {
                 return validateAndTrust(
@@ -62,7 +72,7 @@ struct PackUpdater {
         do {
             manifest = try loader.validate(at: packPath)
         } catch {
-            return .skipped("updated but manifest is invalid — \(error.localizedDescription)")
+            return .manifestInvalid(underlying: error)
         }
 
         var scriptHashes = entry.trustedScriptHashes
@@ -74,7 +84,7 @@ struct PackUpdater {
                 manifest: manifest
             )
         } catch {
-            return .skipped("could not analyze scripts — \(error.localizedDescription)")
+            return .internalError(underlying: error)
         }
 
         if !newItems.isEmpty {
@@ -87,10 +97,10 @@ struct PackUpdater {
                     items: newItems
                 )
             } catch {
-                return .skipped("trust verification failed — \(error.localizedDescription)")
+                return .internalError(underlying: error)
             }
             guard decision.approved else {
-                return .skipped("update skipped (trust not granted)")
+                return .trustDeclined
             }
             for (path, hash) in decision.scriptHashes {
                 scriptHashes[path] = hash
@@ -122,3 +132,44 @@ struct PackUpdater {
         return .updated(updatedEntry)
     }
 }
+
+extension PackUpdater {
+    /// Exit-code policy shared by every multi-pack update caller (`mcs pack update`,
+    /// `mcs sync --update`, `mcs update`): a hard failure exits non-zero when running
+    /// non-interactively (CI), or when every attempted pack failed. A lone trust-decline is
+    /// not a failure. Centralized so the three callers can't drift out of contract.
+    static func shouldExitNonZero(failedCount: Int, attemptedCount: Int, isInteractive: Bool) -> Bool {
+        failedCount > 0 && (!isInteractive || failedCount == attemptedCount)
+    }
+}
+
+extension PackUpdater.UpdateResult {
+    /// Outcomes that should contribute to a non-zero exit in non-interactive use. A user
+    /// trust-decline (`.trustDeclined`) is intentionally *not* a hard failure — declining is
+    /// an expected choice, re-prompted on the next update.
+    var isHardFailure: Bool {
+        switch self {
+        case .fetchFailed, .manifestInvalid, .internalError:
+            true
+        case .alreadyUpToDate, .updated, .trustDeclined:
+            false
+        }
+    }
+
+    /// Human-readable line for non-success outcomes; `nil` when there is nothing to print
+    /// (the success cases render their own pack-specific messages at the call site).
+    var reason: String? {
+        switch self {
+        case .alreadyUpToDate, .updated:
+            nil
+        case let .fetchFailed(error):
+            "fetch failed — \(error.localizedDescription)"
+        case let .manifestInvalid(error):
+            "updated but manifest is invalid — \(error.localizedDescription)"
+        case .trustDeclined:
+            "update skipped (trust not granted)"
+        case let .internalError(error):
+            "could not verify pack scripts — \(error.localizedDescription)"
+        }
+    }
+}

Tests/MCSTests/LockfileOperationsTests.swift

@@ -511,6 +511,34 @@ struct LockfileOperationsTests {
         try ops.updatePacks()
     }
 
+    @Test("updatePacks exits non-zero when a pack fails in a non-interactive process")
+    func updatePacksThrowsOnFailureNonInteractive() throws {
+        let home = try makeTmpDir()
+        defer { try? FileManager.default.removeItem(at: home) }
+
+        // A git pack whose checkout doesn't exist on disk — the real fetch fails, producing
+        // a hard failure. The test process has no TTY, so updatePacks must surface a non-zero
+        // exit rather than silently warning.
+        let ghost = PackRegistryFile.PackEntry(
+            identifier: "ghost",
+            displayName: "Ghost Pack",
+            author: nil,
+            sourceURL: "file:///fake/ghost.git",
+            ref: nil,
+            commitSHA: "abcdef0123",
+            localPath: "ghost",
+            addedAt: "2026-03-21T00:00:00Z",
+            trustedScriptHashes: [:],
+            isLocal: nil
+        )
+        try writeRegistry([ghost], home: home)
+
+        let ops = makeOperations(home: home)
+        #expect(throws: ExitCode.self) {
+            try ops.updatePacks()
+        }
+    }
+
     // MARK: - checkoutLockedCommits: Git operations (mock-based)
 
     /// Set up a locked pack with a real pack directory and lockfile, returning