From 08612fe5664237302e1c4b6c9b27e5aa286a6239 Mon Sep 17 00:00:00 2001 From: Bruno Guidolim Date: Wed, 25 Mar 2026 13:00:14 +0100 Subject: [PATCH] Improve update checker: cache, parallelism, credential suppression (#275) - Respect 24-hour cache in sync/doctor/hook instead of always hitting network; check-updates (explicit) still forces fresh - Parallelize git ls-remote calls with DispatchQueue.concurrentPerform - Suppress keychain/credential prompts via GIT_TERMINAL_PROMPT=0 and GIT_ASKPASS="" - Show detailed per-pack output with SHA diffs and "up to date" message --- CLAUDE.md | 4 +- .../mcs/Commands/CheckUpdatesCommand.swift | 8 +-- Sources/mcs/Commands/DoctorCommand.swift | 2 +- Sources/mcs/Commands/SyncCommand.swift | 2 +- Sources/mcs/Core/UpdateChecker.swift | 60 ++++++++++++------- Tests/MCSTests/UpdateCheckerTests.swift | 43 ++++++++++--- docs/cli.md | 14 ++--- 7 files changed, 91 insertions(+), 42 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index dea3f9e..1844202 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -44,7 +44,7 @@ mcs export --identifier id # Set pack identifier (prompted if omitted) mcs export --non-interactive # Include everything without prompts mcs export --dry-run # Preview what would be exported mcs check-updates # Check for pack and CLI updates -mcs check-updates --hook # Run as SessionStart hook (7-day cooldown, respects config) +mcs check-updates --hook # Run as SessionStart hook (24-hour cooldown, respects config) mcs check-updates --json # Machine-readable JSON output mcs config list # Show all settings with current values mcs config get # Get a specific setting value @@ -112,7 +112,7 @@ mcs config set # Set a configuration value (true/false) - `CleanupCommand.swift` — backup file management with --force flag - `PackCommand.swift` — `mcs pack add/remove/list/update` subcommands; uses `PackSourceResolver` for 3-tier input detection (URL schemes → filesystem paths → GitHub shorthand) - `ExportCommand.swift` — export wizard: reads live configuration and generates a reusable tech pack directory; supports `--global`, `--identifier`, `--non-interactive`, `--dry-run` -- `CheckUpdatesCommand.swift` — lightweight update checker for packs (`git ls-remote`) and CLI version (`git ls-remote --tags`); respects config keys and 7-day cooldown +- `CheckUpdatesCommand.swift` — lightweight update checker for packs (`git ls-remote`) and CLI version (`git ls-remote --tags`); respects config keys and 24-hour cooldown - `ConfigCommand.swift` — `mcs config list/get/set` for managing user preferences; `set` immediately syncs the SessionStart hook in `~/.claude/settings.json` ### Export (`Sources/mcs/Export/`) diff --git a/Sources/mcs/Commands/CheckUpdatesCommand.swift b/Sources/mcs/Commands/CheckUpdatesCommand.swift index 0e33804..d41e0c5 100644 --- a/Sources/mcs/Commands/CheckUpdatesCommand.swift +++ b/Sources/mcs/Commands/CheckUpdatesCommand.swift @@ -7,7 +7,7 @@ struct CheckUpdatesCommand: ParsableCommand { abstract: "Check for tech pack and CLI updates" ) - @Flag(name: .long, help: "Run as a Claude Code SessionStart hook (respects 7-day cooldown and config)") + @Flag(name: .long, help: "Run as a Claude Code SessionStart hook (respects 24-hour cooldown and config)") var hook: Bool = false @Flag(name: .long, help: "Output results as JSON") @@ -47,15 +47,15 @@ struct CheckUpdatesCommand: ParsableCommand { let checker = UpdateChecker(environment: env, shell: shell) let result = checker.performCheck( entries: relevantEntries, - isHook: hook, + forceRefresh: !hook, checkPacks: checkPacks, checkCLI: checkCLI ) if json { printJSON(result) - } else { - UpdateChecker.printResult(result, output: output, isHook: hook) + } else if !UpdateChecker.printResult(result, output: output, isHook: hook), !hook { + output.success("Everything is up to date.") } } diff --git a/Sources/mcs/Commands/DoctorCommand.swift b/Sources/mcs/Commands/DoctorCommand.swift index f40dbeb..c7f7f90 100644 --- a/Sources/mcs/Commands/DoctorCommand.swift +++ b/Sources/mcs/Commands/DoctorCommand.swift @@ -41,7 +41,7 @@ struct DoctorCommand: LockedCommand { ) try runner.run() - // Always check for updates in doctor — it's a diagnostic tool + // Check for updates (respects 24-hour cache) UpdateChecker.checkAndPrint(env: env, shell: shell, output: output) } } diff --git a/Sources/mcs/Commands/SyncCommand.swift b/Sources/mcs/Commands/SyncCommand.swift index 759a674..987420e 100644 --- a/Sources/mcs/Commands/SyncCommand.swift +++ b/Sources/mcs/Commands/SyncCommand.swift @@ -64,7 +64,7 @@ struct SyncCommand: LockedCommand { try performProject(env: env, output: output, shell: shell, registry: registry) } - // Always check for updates after sync — user explicitly ran the command + // Check for updates after sync (respects 24-hour cache) if !dryRun { UpdateChecker.checkAndPrint(env: env, shell: shell, output: output) } diff --git a/Sources/mcs/Core/UpdateChecker.swift b/Sources/mcs/Core/UpdateChecker.swift index 600728a..fb493ba 100644 --- a/Sources/mcs/Core/UpdateChecker.swift +++ b/Sources/mcs/Core/UpdateChecker.swift @@ -9,8 +9,15 @@ struct UpdateChecker { let environment: Environment let shell: ShellRunner - /// Default cooldown interval: 7 days. - static let cooldownInterval: TimeInterval = 604_800 + /// Default cooldown interval: 24 hours. + static let cooldownInterval: TimeInterval = 86400 + + /// Environment variables to suppress credential prompts during read-only git checks. + /// GIT_TERMINAL_PROMPT=0 prevents terminal-based prompts; GIT_ASKPASS="" disables GUI credential helpers. + private static let gitNoPromptEnv: [String: String] = [ + "GIT_TERMINAL_PROMPT": "0", + "GIT_ASKPASS": "", + ] // MARK: - SessionStart Hook (single source of truth) @@ -51,7 +58,8 @@ struct UpdateChecker { } } - /// Run an update check and print results. Used by sync and doctor (user-invoked, no cooldown). + /// Run an update check and print results. Used by sync and doctor. + /// Respects the 24-hour cache cooldown — only does network checks when cache is stale. static func checkAndPrint(env: Environment, shell: ShellRunner, output: CLIOutput) { let packRegistry = PackRegistryFile(path: env.packsRegistry) let allEntries: [PackRegistryFile.PackEntry] @@ -160,36 +168,41 @@ struct UpdateChecker { // MARK: - Pack Checks /// Check each git pack for remote updates via `git ls-remote`. - /// Local packs are skipped. Network failures are silently ignored per-pack. + /// Local packs are skipped. Checks run in parallel. Network failures are silently ignored per-pack. func checkPackUpdates(entries: [PackRegistryFile.PackEntry]) -> [PackUpdate] { - var updates: [PackUpdate] = [] + let gitEntries = entries.filter { !$0.isLocalPack } + guard !gitEntries.isEmpty else { return [] } - for entry in entries { - if entry.isLocalPack { continue } + // Each index is written by exactly one iteration — no data race. + // nonisolated(unsafe) is needed because concurrentPerform's closure is @Sendable. + nonisolated(unsafe) var results = [PackUpdate?](repeating: nil, count: gitEntries.count) + DispatchQueue.concurrentPerform(iterations: gitEntries.count) { index in + let entry = gitEntries[index] let ref = entry.ref ?? "HEAD" let result = shell.run( environment.gitPath, - arguments: ["ls-remote", entry.sourceURL, ref] + arguments: ["ls-remote", entry.sourceURL, ref], + additionalEnvironment: Self.gitNoPromptEnv ) guard result.succeeded, let remoteSHA = Self.parseRemoteSHA(from: result.stdout) else { - continue + return } if remoteSHA != entry.commitSHA { - updates.append(PackUpdate( + results[index] = PackUpdate( identifier: entry.identifier, displayName: entry.displayName, localSHA: entry.commitSHA, remoteSHA: remoteSHA - )) + ) } } - return updates + return results.compactMap(\.self) } // MARK: - CLI Version Check @@ -199,7 +212,8 @@ struct UpdateChecker { func checkCLIVersion(currentVersion: String) -> CLIUpdate? { let result = shell.run( environment.gitPath, - arguments: ["ls-remote", "--tags", "--refs", Constants.MCSRepo.url] + arguments: ["ls-remote", "--tags", "--refs", Constants.MCSRepo.url], + additionalEnvironment: Self.gitNoPromptEnv ) guard result.succeeded, @@ -218,16 +232,16 @@ struct UpdateChecker { // MARK: - Combined Check /// Run all enabled checks. - /// - `isHook: true` — SessionStart hook: returns cached results if fresh, otherwise checks + caches - /// - `isHook: false` (default) — user-invoked: always does network checks + updates cache + /// - `forceRefresh: false` (default) — returns cached results if fresh (within 24h), otherwise network + cache + /// - `forceRefresh: true` — always does network checks + updates cache (for explicit `mcs check-updates`) func performCheck( entries: [PackRegistryFile.PackEntry], - isHook: Bool = false, + forceRefresh: Bool = false, checkPacks: Bool, checkCLI: Bool ) -> CheckResult { - // Hook mode: serve cached results if still fresh (single disk read) - if isHook, let cached = loadCache(), + // Serve cached results if still fresh (single disk read), unless explicitly forced + if !forceRefresh, let cached = loadCache(), let lastCheck = ISO8601DateFormatter().date(from: cached.timestamp), Date().timeIntervalSince(lastCheck) < Self.cooldownInterval { return cached.result @@ -279,8 +293,14 @@ struct UpdateChecker { ) } if !result.packUpdates.isEmpty { - let noun = result.packUpdates.count == 1 ? "pack has" : "packs have" - output.info("\(result.packUpdates.count) \(noun) updates available. Run 'mcs pack update' to update.") + let noun = result.packUpdates.count == 1 ? "pack update" : "pack updates" + output.info("\(result.packUpdates.count) \(noun) available:") + for pack in result.packUpdates { + let local = String(pack.localSHA.prefix(7)) + let remote = String(pack.remoteSHA.prefix(7)) + output.plain(" \u{2022} \(pack.displayName) (\(local) \u{2192} \(remote))") + } + output.plain(" Run 'mcs pack update' to update.") } } diff --git a/Tests/MCSTests/UpdateCheckerTests.swift b/Tests/MCSTests/UpdateCheckerTests.swift index e345fec..9c729af 100644 --- a/Tests/MCSTests/UpdateCheckerTests.swift +++ b/Tests/MCSTests/UpdateCheckerTests.swift @@ -149,8 +149,8 @@ struct UpdateCheckerPerformCheckTests { try data.write(to: env.updateCheckCacheFile, options: .atomic) } - @Test("Hook mode returns cached result when cache is fresh") - func hookModeReturnsCachedResult() throws { + @Test("Returns cached result when cache is fresh") + func cachedResultReturnedByDefault() throws { let tmpDir = try makeTmpDir() defer { try? FileManager.default.removeItem(at: tmpDir) } @@ -164,15 +164,15 @@ struct UpdateCheckerPerformCheckTests { try writeFreshCache(at: env, result: cachedResult) let checker = makeChecker(home: tmpDir) - let result = checker.performCheck(entries: [], isHook: true, checkPacks: true, checkCLI: false) + let result = checker.performCheck(entries: [], checkPacks: true, checkCLI: false) // Should return cached result, not do network calls (entries is empty so network would return nothing) #expect(result.packUpdates.count == 1) #expect(result.packUpdates.first?.identifier == "cached-pack") } - @Test("User mode always does fresh check regardless of cache") - func userModeIgnoresCache() throws { + @Test("forceRefresh bypasses cache and does fresh check") + func forceRefreshBypassesCache() throws { let tmpDir = try makeTmpDir() defer { try? FileManager.default.removeItem(at: tmpDir) } @@ -186,13 +186,42 @@ struct UpdateCheckerPerformCheckTests { try writeFreshCache(at: env, result: cachedResult) let checker = makeChecker(home: tmpDir) - // User mode (isHook: false, default) with no entries — should do fresh check, returning empty - let result = checker.performCheck(entries: [], checkPacks: true, checkCLI: false) + let result = checker.performCheck(entries: [], forceRefresh: true, checkPacks: true, checkCLI: false) // Should NOT return cached "stale" pack — should do fresh check with empty entries #expect(result.packUpdates.isEmpty) } + @Test("Stale cache triggers fresh check") + func staleCacheTriggersFreshCheck() throws { + let tmpDir = try makeTmpDir() + defer { try? FileManager.default.removeItem(at: tmpDir) } + + let env = Environment(home: tmpDir) + let staleResult = UpdateChecker.CheckResult( + packUpdates: [UpdateChecker.PackUpdate( + identifier: "old-pack", displayName: "Old", localSHA: "aaa", remoteSHA: "bbb" + )], + cliUpdate: nil + ) + // Write cache with timestamp older than 24 hours + let staleTimestamp = Date().addingTimeInterval(-90000) + let cached = UpdateChecker.CachedResult( + timestamp: ISO8601DateFormatter().string(from: staleTimestamp), + result: staleResult + ) + let dir = env.updateCheckCacheFile.deletingLastPathComponent() + try FileManager.default.createDirectory(at: dir, withIntermediateDirectories: true) + let data = try JSONEncoder().encode(cached) + try data.write(to: env.updateCheckCacheFile, options: .atomic) + + let checker = makeChecker(home: tmpDir) + let result = checker.performCheck(entries: [], checkPacks: true, checkCLI: false) + + // Cache is stale (>24h), so should do fresh check with empty entries → empty result + #expect(result.packUpdates.isEmpty) + } + @Test("performCheck saves cache after network check") func savesCache() throws { let tmpDir = try makeTmpDir() diff --git a/docs/cli.md b/docs/cli.md index ae23ed4..5b1d1bc 100644 --- a/docs/cli.md +++ b/docs/cli.md @@ -134,25 +134,25 @@ The export wizard discovers MCP servers, hooks, skills, commands, agents, plugin Check for available tech pack and CLI updates. Designed to be lightweight and non-intrusive. ```bash -mcs check-updates # Check for updates (always runs) -mcs check-updates --hook # Run as SessionStart hook (respects 7-day cooldown and config) +mcs check-updates # Check for updates (always fetches from remote) +mcs check-updates --hook # Run as SessionStart hook (respects 24-hour cooldown and config) mcs check-updates --json # Machine-readable JSON output ``` | Flag | Description | |------|-------------| -| `--hook` | Run as a Claude Code SessionStart hook. Respects the 7-day cooldown and config keys. Without this flag, checks always run. | +| `--hook` | Run as a Claude Code SessionStart hook. Respects the 24-hour cooldown and config keys. Without this flag, always fetches from remote. | | `--json` | Output results as JSON instead of human-readable text. | **How it works:** - **Pack checks**: Runs `git ls-remote` per pack to compare the remote HEAD against the local commit SHA. Local packs are skipped. - **CLI version check**: Queries `git ls-remote --tags` on the mcs repository and compares the latest CalVer tag against the installed version. -- **Cache**: Results are cached in `~/.mcs/update-check.json` (timestamp + results). In `--hook` mode, the hook serves cached results if the cache is less than 7 days old (no network request). When the cache is stale, a fresh network check runs and the results are cached for subsequent sessions. User-invoked checks always refresh the cache. +- **Cache**: Results are cached in `~/.mcs/update-check.json` (timestamp + results). Cached results are served if less than 24 hours old (no network request). When the cache is stale, a fresh network check runs and the results are cached. `mcs check-updates` (without `--hook`) always forces a fresh check; `mcs sync` and `mcs doctor` respect the cache. - **Cache invalidation**: `mcs pack update` deletes the cache so the next hook re-checks. CLI version cache self-invalidates when the user upgrades mcs. - **Scope**: Checks global packs plus packs configured in the current project (detected via project root). Packs not relevant to the current context are skipped. - **Offline resilience**: Network failures are silently ignored — the command never errors on connectivity issues. -**Note:** `mcs sync` and `mcs doctor` always check for updates regardless of config — they are user-initiated commands. The config keys below only control the **automatic** `SessionStart` hook that runs in the background when you start a Claude Code session. +**Note:** `mcs sync` and `mcs doctor` check for updates at the end of execution, respecting the 24-hour cache. The config keys below only control the **automatic** `SessionStart` hook that runs when you start a Claude Code session. ## `mcs config` @@ -173,8 +173,8 @@ mcs config set # Set a value (true/false) These keys control a `SessionStart` hook in `~/.claude/settings.json` that runs `mcs check-updates` when you start a Claude Code session. The hook's output is injected into Claude's context so Claude can inform you about available updates. -- **Enabled (either key `true`)**: A synchronous `SessionStart` hook is registered. It respects the 7-day cooldown. -- **Disabled (both keys `false`)**: No hook is registered. You can still check manually with `mcs check-updates` or rely on `mcs sync` / `mcs doctor` which always check. +- **Enabled (either key `true`)**: A synchronous `SessionStart` hook is registered. It respects the 24-hour cooldown. +- **Disabled (both keys `false`)**: No hook is registered. You can still check manually with `mcs check-updates` or rely on `mcs sync` / `mcs doctor` which check using the 24-hour cache. When either key changes, `mcs config set` immediately adds or removes the hook from `~/.claude/settings.json` — no re-sync needed. The same hook is also converged during `mcs sync`.