PackUpdater.UpdateResult.skipped flattens distinct error categories

[bguidolim]

Summary

PackUpdater.UpdateResult.skipped(String) collapses every non-success non-update outcome into a single case with a human-readable reason string. Callers cannot programmatically distinguish "skip this pack and continue" from "something is fundamentally broken, bail out."

// Sources/mcs/Sync/PackUpdater.swift
enum UpdateResult {
    case alreadyUpToDate
    case updated(PackRegistryFile.PackEntry)
    case skipped(String)
}

The .skipped case is reachable from four distinct sources, each with different remediation:

Source	What happened	Suggested handling
fetcher.update throws	Network, auth, transport, repository gone	Maybe retry; definitely not "user canceled"
fetcher.currentCommit fails	Local checkout corrupt	mcs pack remove && mcs pack add
detectNewScripts throws	Script analysis bug	Report as bug
promptForTrust throws	Trust prompt crashed	Report as bug
decision.approved == false	User declined trust	Zero-exit expected outcome

Filing separately since it's pre-existing and outside the #334 fix scope.

Why this matters

• LockfileOperations.updatePacks() iterates packs and prints output.warn per skipped pack. A trust-decline and a systemic git outage look identical at the CLI layer — both say <pack>: fetch failed — ... or <pack>: update skipped (trust not granted).
• mcs sync --update / mcs pack update always exit zero, even when every single pack failed with a transport error. CI calling mcs pack update has no signal.
• Future programmatic consumers (automated workflows, JSON output mode) can't filter by severity.

Proposed fix

Replace the single .skipped(String) with structured cases:

enum UpdateResult {
    case alreadyUpToDate
    case updated(PackRegistryFile.PackEntry)
    case fetchFailed(underlying: Error)       // network / ref-not-found / transport
    case localCheckoutBroken(underlying: Error)  // currentCommit failure
    case trustDeclined                        // user said no — expected, zero-exit
    case internalError(underlying: Error)     // detectNewScripts / promptForTrust crashes
}

Callers (UpdatePack.perform, LockfileOperations.updatePacks) then decide exit code policy:

• .trustDeclined / .alreadyUpToDate — zero-exit, no scary output
• .fetchFailed / .localCheckoutBroken / .internalError — non-zero if running non-interactively, or if all packs failed

Scope

• Sources/mcs/Sync/PackUpdater.swift — UpdateResult, updateGitPack, validateAndTrust
• Sources/mcs/Commands/PackCommand.swift — UpdatePack.perform switch handling
• Sources/mcs/Sync/LockfileOperations.swift — updatePacks switch handling
• Tests: update existing .skipped assertions in PackUpdaterTests, LockfileOperationsTests

Related

• Pack updates stuck for branch refs; sync --update doesn't invalidate update-check cache #334 — surfaced this during review (silent-failure-hunter agent, Finding D). The fix there made ref-fetch errors always produce .fetchFailed — clean typed error from PackFetcher, but it still gets flattened back into .skipped(String) at the PackUpdater layer.
• Support non-interactive mode for mcs pack update #323 — non-interactive mcs pack update would benefit from structured exit codes