UpdateChecker.saveCache silently swallows all write errors

[bguidolim]

Summary

UpdateChecker.saveCache(_:) catches every error from the cache-write pipeline (directory creation, JSON encoding, file write) and drops them on the floor:

// Sources/mcs/Core/UpdateChecker.swift (in saveCache)
} catch {
    // Cache write failure is non-fatal — next check will just redo network calls
}

This was surfaced during #334 review. Filing separately since it's pre-existing and outside the #334 fix scope.

Why this matters

The inline justification ("non-fatal — next check will just redo network calls") is only partially true:

1. Permission/filesystem errors repeat forever. If createDirectory fails because ~/.mcs/ is read-only (recovery mode, disk full, bad ACL, sudo mcs aftermath), every session silently re-runs full network probes. No feedback to the user, no degraded-mode signal.
2. Encoding errors are programmer bugs, not transient. JSONEncoder failing on CachedResult means the type is broken — will never self-correct.
3. Silent-failure compounds with other silent paths. When invalidateCache later can't delete a file saveCache silently failed to write, the two failures chain with zero diagnostic trail. The PR author had to add a fileExists post-check in seedUpdateCheckCache test helper specifically to guard against this defense-in-depth hole.

CLAUDE.md's convention is explicit: "Never use try? to silently discard errors — use do/catch and surface the error (via output.warn(), logging, or propagation)." The same reasoning applies to empty catch {} blocks.

Proposed fix

Match the pattern used by invalidateCache after #334: return a Bool (or throw), and have the single caller (performCheck) decide how to surface the failure. Or simpler: accept a CLIOutput? parameter and warn at the source, preserving the underlying error in the message.

@discardableResult  // ← only if truly safe to ignore, which it isn't
func saveCache(_ result: CheckResult) -> Bool {
    // ... existing encode + write logic, with catch returning false
}

Callers that can't reasonably react (e.g., the SessionStart hook path) can _ = checker.saveCache(result) with an explicit comment — forces the decision to be auditable rather than implicit.

Scope

• Sources/mcs/Core/UpdateChecker.swift — saveCache(_:)
• Call sites: performCheck, --hook path
• Tests: UpdateCheckerCacheTests — add a permission-error case (chmod'd tmp dir)

Related

• Pack updates stuck for branch refs; sync --update doesn't invalidate update-check cache #334 — surfaced this during review; the @discardableResult removal on invalidateCache addresses the parallel issue in the delete path but the write path still silently fails
• Tests/MCSTests/PackUpdaterTests.swift seedUpdateCheckCache helper — uses fileExists after saveCache specifically to detect the silent failure