From e19329a2a4cd4271bab43f0c27cecbccc31e2016 Mon Sep 17 00:00:00 2001
From: Bruno Guidolim <bguidolim@gmail.com>
Date: Fri, 29 May 2026 00:12:56 +0200
Subject: [PATCH 1/3] #337: Type PackUpdater update results so failures surface
 a non-zero exit

- Replace the single skipped(String) outcome with typed cases (fetch failure, broken checkout, invalid manifest, trust-declined, internal error)
- Pack-update commands now exit non-zero when non-interactive (CI) or when every attempted pack failed; a trust-decline stays zero-exit
- Centralize the exit-code decision so all three update callers stay in contract
---
 Sources/mcs/Commands/PackCommand.swift       |  19 ++-
 Sources/mcs/Commands/UpdateCommand.swift     |  55 +++++++--
 Sources/mcs/Sync/LockfileOperations.swift    |  20 ++-
 Sources/mcs/Sync/PackUpdater.swift           |  65 ++++++++--
 Tests/MCSTests/LockfileOperationsTests.swift |  28 +++++
 Tests/MCSTests/PackUpdaterTests.swift        | 121 +++++++++++++++++--
 6 files changed, 280 insertions(+), 28 deletions(-)

diff --git a/Sources/mcs/Commands/PackCommand.swift b/Sources/mcs/Commands/PackCommand.swift
index acf09111..c18a2678 100644
--- a/Sources/mcs/Commands/PackCommand.swift
+++ b/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, .localCheckoutBroken, .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
+        }
     }
 }
 
diff --git a/Sources/mcs/Commands/UpdateCommand.swift b/Sources/mcs/Commands/UpdateCommand.swift
index 40efbcf9..870bf50f 100644
--- a/Sources/mcs/Commands/UpdateCommand.swift
+++ b/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,13 +226,21 @@ 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")
 
@@ -229,7 +248,7 @@ struct UpdateCommand: LockedCommand {
             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, .localCheckoutBroken, .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(
diff --git a/Sources/mcs/Sync/LockfileOperations.swift b/Sources/mcs/Sync/LockfileOperations.swift
index 20c5eea4..5d6c6e51 100644
--- a/Sources/mcs/Sync/LockfileOperations.swift
+++ b/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, .localCheckoutBroken, .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.
diff --git a/Sources/mcs/Sync/PackUpdater.swift b/Sources/mcs/Sync/PackUpdater.swift
index c0dacdb7..9e5fe04b 100644
--- a/Sources/mcs/Sync/PackUpdater.swift
+++ b/Sources/mcs/Sync/PackUpdater.swift
@@ -9,10 +9,18 @@ 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 failed (network/transport/ref)
+        case localCheckoutBroken(underlying: Error) // could not read the local checkout's HEAD
+        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 (bug)
     }
 
     /// Fetch, validate, and re-trust a single git pack entry.
@@ -26,7 +34,7 @@ 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 {
@@ -35,7 +43,7 @@ struct PackUpdater {
             do {
                 diskSHA = try fetcher.currentCommit(at: packPath)
             } catch {
-                return .skipped("could not read current commit at \(packPath.path) — \(error.localizedDescription)")
+                return .localCheckoutBroken(underlying: error)
             }
             if diskSHA != entry.commitSHA {
                 return validateAndTrust(
@@ -62,7 +70,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 +82,7 @@ struct PackUpdater {
                 manifest: manifest
             )
         } catch {
-            return .skipped("could not analyze scripts — \(error.localizedDescription)")
+            return .internalError(underlying: error)
         }
 
         if !newItems.isEmpty {
@@ -87,10 +95,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 +130,46 @@ 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, .localCheckoutBroken, .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 .localCheckoutBroken(error):
+            "local checkout broken — \(error.localizedDescription)"
+        case let .manifestInvalid(error):
+            "updated but manifest is invalid — \(error.localizedDescription)"
+        case .trustDeclined:
+            "update skipped (trust not granted)"
+        case let .internalError(error):
+            "internal error (please report) — \(error.localizedDescription)"
+        }
+    }
+}
diff --git a/Tests/MCSTests/LockfileOperationsTests.swift b/Tests/MCSTests/LockfileOperationsTests.swift
index 958513f7..605c8768 100644
--- a/Tests/MCSTests/LockfileOperationsTests.swift
+++ b/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
diff --git a/Tests/MCSTests/PackUpdaterTests.swift b/Tests/MCSTests/PackUpdaterTests.swift
index da9fef2a..7c0b3029 100644
--- a/Tests/MCSTests/PackUpdaterTests.swift
+++ b/Tests/MCSTests/PackUpdaterTests.swift
@@ -141,6 +141,21 @@ struct PackUpdaterTests {
         return shaResult.stdout
     }
 
+    /// Replace techpack.yaml with the given content and push it as a new commit. Used to
+    /// drive the post-fetch validate/trust paths (manifest-invalid, trust-decline).
+    private func pushManifest(fixture: Fixture, manifest: String) throws {
+        let workDir = fixture.tmpDir.appendingPathComponent("work")
+        let shell = ShellRunner(environment: Environment(home: fixture.tmpDir))
+
+        try manifest.write(
+            to: workDir.appendingPathComponent("techpack.yaml"),
+            atomically: true, encoding: .utf8
+        )
+        try git(shell, ["-C", workDir.path, "add", "."], context: "git add")
+        try git(shell, ["-C", workDir.path, "commit", "-m", "update manifest"], context: "git commit")
+        try git(shell, ["-C", workDir.path, "push"], context: "git push")
+    }
+
     // MARK: - Tests
 
     @Test("returns alreadyUpToDate when disk SHA matches registry SHA")
@@ -260,7 +275,7 @@ struct PackUpdaterTests {
         #expect(FileManager.default.fileExists(atPath: fix.env.updateCheckCacheFile.path))
     }
 
-    @Test("returns skipped when fetch fails")
+    @Test("returns fetchFailed when fetch fails")
     func fetchFailure() throws {
         let fix = try makeFixture()
         defer { fix.cleanup() }
@@ -274,14 +289,15 @@ struct PackUpdaterTests {
             entry: entry, packPath: brokenPath, registry: fix.registry
         )
 
-        guard case .skipped = result else {
-            Issue.record("Expected .skipped, got \(result)")
+        guard case .fetchFailed = result else {
+            Issue.record("Expected .fetchFailed, got \(result)")
             return
         }
+        #expect(result.isHardFailure)
     }
 
-    @Test("skipped result leaves the update-check cache intact")
-    func skippedResultDoesNotInvalidateCache() throws {
+    @Test("fetchFailed result leaves the update-check cache intact")
+    func fetchFailedDoesNotInvalidateCache() throws {
         let fix = try makeFixture()
         defer { fix.cleanup() }
 
@@ -294,10 +310,101 @@ struct PackUpdaterTests {
             entry: entry, packPath: brokenPath, registry: fix.registry
         )
 
-        guard case .skipped = result else {
-            Issue.record("Expected .skipped, got \(result)")
+        guard case .fetchFailed = result else {
+            Issue.record("Expected .fetchFailed, got \(result)")
             return
         }
         #expect(FileManager.default.fileExists(atPath: fix.env.updateCheckCacheFile.path))
     }
+
+    @Test("returns trustDeclined when the user declines new scripts (non-interactive)")
+    func trustDeclined() throws {
+        let fix = try makeFixture()
+        defer { fix.cleanup() }
+
+        let entry = makeEntry(commitSHA: fix.initialSHA)
+        let packPath = fix.packsDir.appendingPathComponent("test-pack")
+
+        // Push a manifest that introduces a shell command (a trustable script). The trust
+        // prompt's askYesNo returns its `false` default in the non-interactive test
+        // environment, so the update is declined without any mocking.
+        try pushManifest(
+            fixture: fix,
+            manifest: """
+            schemaVersion: 1
+            identifier: test-pack
+            displayName: Test Pack
+            description: A test pack with a shell command
+            components:
+              - id: greet
+                description: Greet during install
+                type: skill
+                shell: "echo hello"
+            """
+        )
+
+        let result = fix.updater.updateGitPack(
+            entry: entry, packPath: packPath, registry: fix.registry
+        )
+
+        guard case .trustDeclined = result else {
+            Issue.record("Expected .trustDeclined, got \(result)")
+            return
+        }
+        #expect(!result.isHardFailure)
+    }
+
+    @Test("returns manifestInvalid when the fetched revision has a broken manifest")
+    func manifestInvalid() throws {
+        let fix = try makeFixture()
+        defer { fix.cleanup() }
+
+        let entry = makeEntry(commitSHA: fix.initialSHA)
+        let packPath = fix.packsDir.appendingPathComponent("test-pack")
+
+        // Push a techpack.yaml missing the required identifier/displayName fields.
+        try pushManifest(
+            fixture: fix,
+            manifest: """
+            schemaVersion: 1
+            description: Missing identifier and displayName
+            """
+        )
+
+        let result = fix.updater.updateGitPack(
+            entry: entry, packPath: packPath, registry: fix.registry
+        )
+
+        guard case .manifestInvalid = result else {
+            Issue.record("Expected .manifestInvalid, got \(result)")
+            return
+        }
+        #expect(result.isHardFailure)
+    }
+
+    // MARK: - Helper property contract
+
+    @Test("isHardFailure and reason classify every case correctly")
+    func resultClassification() {
+        let entry = makeEntry(commitSHA: "abc1234")
+        let dummy = TestSetupError(message: "boom")
+
+        // Non-failures
+        #expect(!PackUpdater.UpdateResult.alreadyUpToDate.isHardFailure)
+        #expect(PackUpdater.UpdateResult.alreadyUpToDate.reason == nil)
+        #expect(!PackUpdater.UpdateResult.updated(entry).isHardFailure)
+        #expect(PackUpdater.UpdateResult.updated(entry).reason == nil)
+        #expect(!PackUpdater.UpdateResult.trustDeclined.isHardFailure)
+        #expect(PackUpdater.UpdateResult.trustDeclined.reason?.contains("trust not granted") == true)
+
+        // Hard failures carry a reason
+        #expect(PackUpdater.UpdateResult.fetchFailed(underlying: dummy).isHardFailure)
+        #expect(PackUpdater.UpdateResult.fetchFailed(underlying: dummy).reason?.contains("fetch failed") == true)
+        #expect(PackUpdater.UpdateResult.localCheckoutBroken(underlying: dummy).isHardFailure)
+        #expect(PackUpdater.UpdateResult.localCheckoutBroken(underlying: dummy).reason?.contains("local checkout broken") == true)
+        #expect(PackUpdater.UpdateResult.manifestInvalid(underlying: dummy).isHardFailure)
+        #expect(PackUpdater.UpdateResult.manifestInvalid(underlying: dummy).reason?.contains("manifest is invalid") == true)
+        #expect(PackUpdater.UpdateResult.internalError(underlying: dummy).isHardFailure)
+        #expect(PackUpdater.UpdateResult.internalError(underlying: dummy).reason?.contains("internal error") == true)
+    }
 }

From e4e299fca0aeb72ed750544e5bfb12aabdda0386 Mon Sep 17 00:00:00 2001
From: Bruno Guidolim <bguidolim@gmail.com>
Date: Fri, 29 May 2026 00:32:34 +0200
Subject: [PATCH 2/3] Address review feedback on typed PackUpdater results

- Fold .localCheckoutBroken into .fetchFailed (a broken checkout already surfaces as fetchFailed since update() reads HEAD first)
- Reword .internalError reason to "could not verify pack scripts" (avoid mis-attributing user-environment errors to bugs)
- Add shouldExitNonZero truth-table test; document deprecated sync --update pre-sync throw
---
 Sources/mcs/Commands/PackCommand.swift    |  2 +-
 Sources/mcs/Commands/SyncCommand.swift    |  4 ++++
 Sources/mcs/Commands/UpdateCommand.swift  |  2 +-
 Sources/mcs/Sync/LockfileOperations.swift |  2 +-
 Sources/mcs/Sync/PackUpdater.swift        | 16 +++++++--------
 Tests/MCSTests/PackUpdaterTests.swift     | 25 ++++++++++++++++++++---
 6 files changed, 37 insertions(+), 14 deletions(-)

diff --git a/Sources/mcs/Commands/PackCommand.swift b/Sources/mcs/Commands/PackCommand.swift
index c18a2678..fdfd3f6f 100644
--- a/Sources/mcs/Commands/PackCommand.swift
+++ b/Sources/mcs/Commands/PackCommand.swift
@@ -684,7 +684,7 @@ struct UpdatePack: LockedCommand {
                 ctx.output.success("\(entry.displayName): updated (\(updatedEntry.shortSHA))")
             case .trustDeclined:
                 ctx.output.info("\(entry.identifier): \(result.reason ?? "trust not granted") (will re-prompt next run)")
-            case .fetchFailed, .localCheckoutBroken, .manifestInvalid, .internalError:
+            case .fetchFailed, .manifestInvalid, .internalError:
                 ctx.output.warn("\(entry.identifier): \(result.reason ?? "update failed")")
                 failedPacks.append(entry.identifier)
             }
diff --git a/Sources/mcs/Commands/SyncCommand.swift b/Sources/mcs/Commands/SyncCommand.swift
index 70e28553..2e15895a 100644
--- a/Sources/mcs/Commands/SyncCommand.swift
+++ b/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()
         }
 
diff --git a/Sources/mcs/Commands/UpdateCommand.swift b/Sources/mcs/Commands/UpdateCommand.swift
index 870bf50f..baca35b7 100644
--- a/Sources/mcs/Commands/UpdateCommand.swift
+++ b/Sources/mcs/Commands/UpdateCommand.swift
@@ -283,7 +283,7 @@ struct UpdateCommand: LockedCommand {
             case .trustDeclined:
                 output.info("  \(entry.identifier): \(result.reason ?? "trust not granted") (will re-prompt on next 'mcs update')")
                 skipped.insert(entry.identifier)
-            case .fetchFailed, .localCheckoutBroken, .manifestInvalid, .internalError:
+            case .fetchFailed, .manifestInvalid, .internalError:
                 output.warn("  \(entry.identifier): \(result.reason ?? "update failed")")
                 skipped.insert(entry.identifier)
                 failed.insert(entry.identifier)
diff --git a/Sources/mcs/Sync/LockfileOperations.swift b/Sources/mcs/Sync/LockfileOperations.swift
index 5d6c6e51..a8925ab6 100644
--- a/Sources/mcs/Sync/LockfileOperations.swift
+++ b/Sources/mcs/Sync/LockfileOperations.swift
@@ -124,7 +124,7 @@ struct LockfileOperations {
                 output.success("  \(entry.identifier): updated (\(updatedEntry.shortSHA))")
             case .trustDeclined:
                 output.info("  \(entry.identifier): \(result.reason ?? "trust not granted") (will re-prompt next run)")
-            case .fetchFailed, .localCheckoutBroken, .manifestInvalid, .internalError:
+            case .fetchFailed, .manifestInvalid, .internalError:
                 output.warn("  \(entry.identifier): \(result.reason ?? "update failed")")
                 failedPacks.append(entry.identifier)
             }
diff --git a/Sources/mcs/Sync/PackUpdater.swift b/Sources/mcs/Sync/PackUpdater.swift
index 9e5fe04b..091c5b81 100644
--- a/Sources/mcs/Sync/PackUpdater.swift
+++ b/Sources/mcs/Sync/PackUpdater.swift
@@ -16,11 +16,10 @@ struct PackUpdater {
     enum UpdateResult {
         case alreadyUpToDate
         case updated(PackRegistryFile.PackEntry)
-        case fetchFailed(underlying: Error) // git fetch/pull failed (network/transport/ref)
-        case localCheckoutBroken(underlying: Error) // could not read the local checkout's HEAD
+        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 (bug)
+        case internalError(underlying: Error) // script analysis / trust prompt crashed
     }
 
     /// Fetch, validate, and re-trust a single git pack entry.
@@ -39,11 +38,14 @@ struct PackUpdater {
 
         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 .localCheckoutBroken(underlying: error)
+                return .fetchFailed(underlying: error)
             }
             if diskSHA != entry.commitSHA {
                 return validateAndTrust(
@@ -147,7 +149,7 @@ extension PackUpdater.UpdateResult {
     /// an expected choice, re-prompted on the next update.
     var isHardFailure: Bool {
         switch self {
-        case .fetchFailed, .localCheckoutBroken, .manifestInvalid, .internalError:
+        case .fetchFailed, .manifestInvalid, .internalError:
             true
         case .alreadyUpToDate, .updated, .trustDeclined:
             false
@@ -162,14 +164,12 @@ extension PackUpdater.UpdateResult {
             nil
         case let .fetchFailed(error):
             "fetch failed — \(error.localizedDescription)"
-        case let .localCheckoutBroken(error):
-            "local checkout broken — \(error.localizedDescription)"
         case let .manifestInvalid(error):
             "updated but manifest is invalid — \(error.localizedDescription)"
         case .trustDeclined:
             "update skipped (trust not granted)"
         case let .internalError(error):
-            "internal error (please report) — \(error.localizedDescription)"
+            "could not verify pack scripts — \(error.localizedDescription)"
         }
     }
 }
diff --git a/Tests/MCSTests/PackUpdaterTests.swift b/Tests/MCSTests/PackUpdaterTests.swift
index 7c0b3029..2b4c78a7 100644
--- a/Tests/MCSTests/PackUpdaterTests.swift
+++ b/Tests/MCSTests/PackUpdaterTests.swift
@@ -400,11 +400,30 @@ struct PackUpdaterTests {
         // Hard failures carry a reason
         #expect(PackUpdater.UpdateResult.fetchFailed(underlying: dummy).isHardFailure)
         #expect(PackUpdater.UpdateResult.fetchFailed(underlying: dummy).reason?.contains("fetch failed") == true)
-        #expect(PackUpdater.UpdateResult.localCheckoutBroken(underlying: dummy).isHardFailure)
-        #expect(PackUpdater.UpdateResult.localCheckoutBroken(underlying: dummy).reason?.contains("local checkout broken") == true)
         #expect(PackUpdater.UpdateResult.manifestInvalid(underlying: dummy).isHardFailure)
         #expect(PackUpdater.UpdateResult.manifestInvalid(underlying: dummy).reason?.contains("manifest is invalid") == true)
         #expect(PackUpdater.UpdateResult.internalError(underlying: dummy).isHardFailure)
-        #expect(PackUpdater.UpdateResult.internalError(underlying: dummy).reason?.contains("internal error") == true)
+        #expect(PackUpdater.UpdateResult.internalError(underlying: dummy).reason?.contains("verify pack scripts") == true)
+    }
+
+    // MARK: - Exit-code policy
+
+    @Test("shouldExitNonZero truth table", arguments: [
+        // No failures → never exit non-zero, regardless of interactivity.
+        (failed: 0, attempted: 0, interactive: true, expected: false),
+        (failed: 0, attempted: 3, interactive: false, expected: false),
+        // Partial failure: interactive tolerates it (human sees the warnings), CI does not.
+        (failed: 1, attempted: 3, interactive: true, expected: false),
+        (failed: 1, attempted: 3, interactive: false, expected: true),
+        // Every attempted pack failed → exit non-zero even interactively.
+        (failed: 3, attempted: 3, interactive: true, expected: true),
+        (failed: 3, attempted: 3, interactive: false, expected: true),
+    ])
+    func exitPolicy(failed: Int, attempted: Int, interactive: Bool, expected: Bool) {
+        #expect(
+            PackUpdater.shouldExitNonZero(
+                failedCount: failed, attemptedCount: attempted, isInteractive: interactive
+            ) == expected
+        )
     }
 }

From 09efe1993fad72112097bb4dc89713004cef0977 Mon Sep 17 00:00:00 2001
From: Bruno Guidolim <bguidolim@gmail.com>
Date: Fri, 29 May 2026 00:36:50 +0200
Subject: [PATCH 3/3] Document non-zero exit behavior for mcs update / pack
 update

- Add exit-status notes to docs/cli.md: hard failures exit non-zero in CI or when all packs fail; trust-declines stay zero
---
 docs/cli.md | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/docs/cli.md b/docs/cli.md
index 79db3b36..78163b05 100644
--- a/docs/cli.md
+++ b/docs/cli.md
@@ -64,6 +64,8 @@ mcs update --dry-run             # Preview without making changes
 
 **Prompt value reuse:** unlike `mcs sync`, `mcs update` does not show the interactive *"Reuse these values? [Y/n]"* gate when a pack's prompts already have stored answers. Refresh implies "keep what I have," so existing values are reused silently. **New** prompts introduced by a pack update still execute normally. To revisit stored values, use `mcs sync` (answer "No" at the gate to re-enter values one by one, with the existing answer as the default) or `mcs sync --customize` (always re-asks every prompt, no gate).
 
+**Exit status:** a hard failure (failed fetch, unreadable checkout, invalid fetched manifest) exits non-zero when running non-interactively (CI), or when *every* attempted pack failed — so a transient outage in CI is a detectable failure, not a silent zero exit. In an interactive terminal a partial failure exits zero (the per-pack warnings are visible); the packs that updated cleanly are still re-applied first. A declined trust prompt is **not** a failure and always exits zero (it re-prompts next run).
+
 ## `mcs pack`
 
 Manage registered tech packs.
@@ -114,6 +116,8 @@ Fetches the latest commits from the remote and updates the local checkout. Local
 
 This is a low-level fetch — useful for pack authors testing upstream changes without applying them, or in CI workflows that handle the apply step separately. For most users, [`mcs update`](#mcs-update) is the right command: it does the same fetch *and* re-applies across every configured scope.
 
+Exits non-zero on a hard failure when running non-interactively (CI) or when every attempted pack failed, matching [`mcs update`](#mcs-update). A declined trust prompt is not a failure.
+
 ### `mcs pack validate [source]`
 
 Validate a tech pack for structural correctness and best practices. Designed for pack authors to catch issues locally before submitting to the registry.