diff --git a/Sources/mcs/Core/UpdateChecker.swift b/Sources/mcs/Core/UpdateChecker.swift index ae974e1..0047756 100644 --- a/Sources/mcs/Core/UpdateChecker.swift +++ b/Sources/mcs/Core/UpdateChecker.swift @@ -7,7 +7,10 @@ import Foundation /// remotes produce no output, matching the design goal of non-intrusive checks. struct UpdateChecker { let environment: Environment - let shell: ShellRunner + /// `any ShellRunning` so tests can inject `MockShellRunner` for the new + /// post-SHA-change git fetch/diff calls. Production call sites pass a concrete + /// `ShellRunner`, which upcasts implicitly. `ShellRunning: Sendable`. + let shell: any ShellRunning /// Default cooldown interval: 24 hours. static let cooldownInterval: TimeInterval = 86400 @@ -168,42 +171,173 @@ struct UpdateChecker { // MARK: - Pack Checks + /// Classification of an upstream commit-SHA change. Drives whether the noise filter + /// suppresses the update notification (issue #338). + /// + /// **Never-hide invariant:** `.suppressed` is the narrow special case. Every error + /// path (fetch fail, diff fail, missing clone, parse error) must fall through to + /// `.unknown`, which surfaces the notification as today. Do not invert this polarity. + enum UpstreamChange: Equatable { + case suppressed + case material([String]) + case unknown + } + + /// Pure classifier for the changed-path list returned by `git diff --name-only`. + /// No I/O — unit-testable in isolation. The orchestrator (`classifyUpstreamChange`) + /// runs git and calls into this. + /// + /// Rules, applied in order: + /// 1. If any path equals `techpack.yaml` → `.material` (supply-chain invariant — + /// manifest edits can swap hook scripts, MCP commands, install surface). + /// 2. A path is "noise" if its leading segment matches `PackHeuristics.ignoredDirectories` + /// (e.g. `.github/workflows/ci.yml`) OR its basename matches + /// `PackHeuristics.infrastructureFilesForUpdateCheck` (e.g. `README.md`, `LICENSE`). + /// 3. If all paths are noise → `.suppressed`. Any survivor → `.material(survivors)`. + static func classifyDiffPaths(_ paths: [String]) -> UpstreamChange { + let cleaned = paths + .map { $0.trimmingCharacters(in: .whitespaces) } + .filter { !$0.isEmpty } + guard !cleaned.isEmpty else { return .suppressed } + + if cleaned.contains(Constants.ExternalPacks.manifestFilename) { + return .material([Constants.ExternalPacks.manifestFilename]) + } + + var material: [String] = [] + for path in cleaned where !isNoisePath(path) { + material.append(path) + } + return material.isEmpty ? .suppressed : .material(material) + } + + private static func isNoisePath(_ path: String) -> Bool { + let leadingSegment = path.split(separator: "/", maxSplits: 1).first.map(String.init) ?? path + if PackHeuristics.ignoredDirectories.contains(leadingSegment) { + return true + } + // Basename match only when the path is a single segment — a file called `README.md` + // inside `hooks/` (i.e. `hooks/README.md`) is not suppressed; only pack-root infra is. + if !path.contains("/"), PackHeuristics.infrastructureFilesForUpdateCheck.contains(path) { + return true + } + return false + } + + /// Orchestrator — runs `git fetch` + `git diff --name-only` in the pack clone + /// and feeds the diff into `classifyDiffPaths`. Every error path falls through + /// to `.unknown` per the never-hide invariant. + func classifyUpstreamChange(entry: PackRegistryFile.PackEntry) -> UpstreamChange { + guard let workDirURL = entry.resolvedPath(packsDirectory: environment.packsDirectory) else { + return .unknown + } + let workDir = workDirURL.path + let ref = entry.ref ?? "HEAD" + + let fetchResult = shell.run( + environment.gitPath, + arguments: ["fetch", "--depth", "1", "origin", ref], + workingDirectory: workDir, + additionalEnvironment: Self.gitNoPromptEnv + ) + guard fetchResult.succeeded else { return .unknown } + + let diffResult = shell.run( + environment.gitPath, + arguments: ["diff", "--name-only", "HEAD", "FETCH_HEAD"], + workingDirectory: workDir, + additionalEnvironment: Self.gitNoPromptEnv + ) + guard diffResult.succeeded else { return .unknown } + + let paths = diffResult.stdout.split(separator: "\n").map(String.init) + return Self.classifyDiffPaths(paths) + } + + /// Per-iteration result of `checkPackUpdates`: an optional notification to emit + /// and an optional registry baseline advance (applied post-loop to serialize writes). + private struct PackCheckOutcome { + let update: PackUpdate? + let advance: SHAAdvance? + } + + private struct SHAAdvance { + let identifier: String + let newSHA: String + } + /// Check each git pack for remote updates via `git ls-remote`. /// Local packs are skipped. Checks run in parallel. Network failures are silently ignored per-pack. + /// + /// When a remote SHA differs, runs a noise filter (`classifyUpstreamChange`) that may + /// suppress the notification for README/CI/infra-only commits and advance the registry + /// baseline so the same commits don't re-trigger (issue #338). func checkPackUpdates(entries: [PackRegistryFile.PackEntry]) -> [PackUpdate] { let gitEntries = entries.filter { !$0.isLocalPack } guard !gitEntries.isEmpty else { return [] } // 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) + nonisolated(unsafe) var results = [PackCheckOutcome?](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( + let lsRemote = shell.run( environment.gitPath, arguments: ["ls-remote", entry.sourceURL, ref], additionalEnvironment: Self.gitNoPromptEnv ) - guard result.succeeded, - let remoteSHA = Self.parseRemoteSHA(from: result.stdout) + guard lsRemote.succeeded, + let remoteSHA = Self.parseRemoteSHA(from: lsRemote.stdout), + remoteSHA != entry.commitSHA else { return } - if remoteSHA != entry.commitSHA { - results[index] = PackUpdate( - identifier: entry.identifier, - displayName: entry.displayName, - localSHA: entry.commitSHA, - remoteSHA: remoteSHA + let pendingUpdate = PackUpdate( + identifier: entry.identifier, + displayName: entry.displayName, + localSHA: entry.commitSHA, + remoteSHA: remoteSHA + ) + + switch classifyUpstreamChange(entry: entry) { + case .suppressed: + results[index] = PackCheckOutcome( + update: nil, + advance: SHAAdvance(identifier: entry.identifier, newSHA: remoteSHA) ) + case .material, .unknown: + // .unknown → never-hide: surface the notification unfiltered. + results[index] = PackCheckOutcome(update: pendingUpdate, advance: nil) } } - return results.compactMap(\.self) + let advances = results.compactMap { $0?.advance } + if !advances.isEmpty { + applyRegistryAdvances(advances) + } + + return results.compactMap { $0?.update } + } + + /// Apply collected SHA advances to `registry.yaml` in one load→mutate→save round-trip. + /// Serializes the writes that were collected from the parallel `concurrentPerform` loop. + /// Silent on failure: next check will re-classify (same pattern as `saveCache`). + private func applyRegistryAdvances(_ advances: [SHAAdvance]) { + let registry = PackRegistryFile(path: environment.packsRegistry) + do { + var data = try registry.load() + for advance in advances { + guard let existing = registry.pack(identifier: advance.identifier, in: data) else { continue } + registry.register(existing.withCommitSHA(advance.newSHA), in: &data) + } + try registry.save(data) + } catch { + // Registry write failure is non-fatal — next check will classify again. + } } // MARK: - CLI Version Check diff --git a/Sources/mcs/ExternalPack/PackHeuristics.swift b/Sources/mcs/ExternalPack/PackHeuristics.swift index a4fd818..905c265 100644 --- a/Sources/mcs/ExternalPack/PackHeuristics.swift +++ b/Sources/mcs/ExternalPack/PackHeuristics.swift @@ -79,7 +79,9 @@ enum PackHeuristics { } /// Directories at the pack root that are infrastructure, not pack content. - private static let ignoredDirectories: Set = [ + /// Shared by `checkUnreferencedFiles` and `UpdateChecker`'s noise filter — a path + /// whose leading segment is in this set is never material to a pack install. + static let ignoredDirectories: Set = [ ".git", ".github", ".gitlab", ".vscode", "node_modules", "__pycache__", ".build", ] @@ -168,6 +170,9 @@ enum PackHeuristics { } /// Files at the pack root that are expected infrastructure, not content. + /// Used by `checkRootLevelContentFiles` to avoid warning about these files. + /// Intentionally includes `techpack.yaml` — see `infrastructureFilesForUpdateCheck` + /// below for the (deliberately different) set the update-check filter uses. private static let infrastructureFiles: Set = [ Constants.ExternalPacks.manifestFilename, "README.md", "README", "LICENSE", "LICENSE.md", "CHANGELOG.md", "CONTRIBUTING.md", ".gitignore", ".editorconfig", @@ -175,6 +180,14 @@ enum PackHeuristics { "Makefile", "Dockerfile", ".dockerignore", ] + /// Files treated as non-material by `UpdateChecker`'s noise filter (issue #338). + /// **Intentionally distinct from `infrastructureFiles`**: `techpack.yaml` is + /// excluded here because a manifest edit can change the entire install surface + /// (new hook scripts, different MCP commands). Silently suppressing manifest-only + /// commits would be a supply-chain attack vector. Do not deduplicate these sets. + static let infrastructureFilesForUpdateCheck: Set = + infrastructureFiles.subtracting([Constants.ExternalPacks.manifestFilename]) + private static func checkRootLevelContentFiles( manifest: ExternalPackManifest, packPath: URL diff --git a/Sources/mcs/ExternalPack/PackRegistryFile.swift b/Sources/mcs/ExternalPack/PackRegistryFile.swift index cc8d97f..0071480 100644 --- a/Sources/mcs/ExternalPack/PackRegistryFile.swift +++ b/Sources/mcs/ExternalPack/PackRegistryFile.swift @@ -31,6 +31,24 @@ struct PackRegistryFile { } return PathContainment.safePath(relativePath: localPath, within: packsDirectory) } + + /// Return a copy with `commitSHA` replaced. + /// Used when `UpdateChecker`'s noise filter advances the baseline without + /// updating the working-tree checkout (see issue #338). + func withCommitSHA(_ sha: String) -> Self { + PackEntry( + identifier: identifier, + displayName: displayName, + author: author, + sourceURL: sourceURL, + ref: ref, + commitSHA: sha, + localPath: localPath, + addedAt: addedAt, + trustedScriptHashes: trustedScriptHashes, + isLocal: isLocal + ) + } } struct RegistryData: Codable { diff --git a/Tests/MCSTests/PackHeuristicsTests.swift b/Tests/MCSTests/PackHeuristicsTests.swift index 6db046b..7e512df 100644 --- a/Tests/MCSTests/PackHeuristicsTests.swift +++ b/Tests/MCSTests/PackHeuristicsTests.swift @@ -684,4 +684,32 @@ struct PackHeuristicsTests { #expect(!dirWarnings.isEmpty) #expect(dirWarnings.allSatisfy { $0.severity == .warning }) } + + // MARK: - infrastructureFilesForUpdateCheck (issue #338) + + @Test("infrastructureFilesForUpdateCheck excludes techpack.yaml (supply-chain invariant)") + func updateCheckSetExcludesManifest() { + #expect(!PackHeuristics.infrastructureFilesForUpdateCheck.contains( + Constants.ExternalPacks.manifestFilename + )) + } + + @Test("infrastructureFilesForUpdateCheck still contains the other infra files") + func updateCheckSetContainsInfraFiles() { + let set = PackHeuristics.infrastructureFilesForUpdateCheck + #expect(set.contains("README.md")) + #expect(set.contains("LICENSE")) + #expect(set.contains("CHANGELOG.md")) + #expect(set.contains(".gitignore")) + #expect(set.contains("Makefile")) + } + + @Test("ignoredDirectories is accessible and contains expected entries") + func ignoredDirsAccessible() { + let dirs = PackHeuristics.ignoredDirectories + #expect(dirs.contains(".git")) + #expect(dirs.contains(".github")) + #expect(dirs.contains("node_modules")) + #expect(dirs.contains(".build")) + } } diff --git a/Tests/MCSTests/UpdateCheckerTests.swift b/Tests/MCSTests/UpdateCheckerTests.swift index 202e99e..35ad62b 100644 --- a/Tests/MCSTests/UpdateCheckerTests.swift +++ b/Tests/MCSTests/UpdateCheckerTests.swift @@ -237,6 +237,311 @@ struct UpdateCheckerPerformCheckTests { } } +// MARK: - Noise Filter Tests (issue #338) + +struct UpdateCheckerClassifyDiffPathsTests { + @Test("Empty diff → suppressed") + func emptyIsSuppressed() { + #expect(UpdateChecker.classifyDiffPaths([]) == .suppressed) + #expect(UpdateChecker.classifyDiffPaths(["", " "]) == .suppressed) + } + + @Test("README-only diff → suppressed") + func readmeOnlySuppressed() { + #expect(UpdateChecker.classifyDiffPaths(["README.md"]) == .suppressed) + } + + @Test("All-infra root files → suppressed") + func infraFilesSuppressed() { + let paths = ["README.md", "LICENSE", "CHANGELOG.md", ".gitignore", "Makefile"] + #expect(UpdateChecker.classifyDiffPaths(paths) == .suppressed) + } + + @Test("techpack.yaml change → always material (supply-chain invariant)") + func manifestAlwaysMaterial() { + #expect(UpdateChecker.classifyDiffPaths(["techpack.yaml"]) + == .material(["techpack.yaml"])) + // Even mixed with noise, manifest short-circuits to material. + #expect(UpdateChecker.classifyDiffPaths(["README.md", "techpack.yaml"]) + == .material(["techpack.yaml"])) + } + + @Test("Ignored-dir leading segment → suppressed") + func ignoredDirsSuppressed() { + #expect(UpdateChecker.classifyDiffPaths([".github/workflows/ci.yml"]) == .suppressed) + #expect(UpdateChecker.classifyDiffPaths(["node_modules/foo/bar.js"]) == .suppressed) + #expect(UpdateChecker.classifyDiffPaths([".build/debug/output"]) == .suppressed) + } + + @Test("Hook script change → material") + func hookScriptMaterial() { + let result = UpdateChecker.classifyDiffPaths(["hooks/session-start.sh"]) + #expect(result == .material(["hooks/session-start.sh"])) + } + + @Test("Mixed material + noise → material (only material paths)") + func mixedReturnsMaterialOnly() { + let result = UpdateChecker.classifyDiffPaths([ + "README.md", "hooks/session-start.sh", ".github/workflows/ci.yml", + ]) + #expect(result == .material(["hooks/session-start.sh"])) + } + + @Test("Infra basename inside subdir → material (basename match only applies to root)") + func nonRootInfraNotSuppressed() { + // `hooks/README.md` is NOT suppressed — only the pack-root README is. + let result = UpdateChecker.classifyDiffPaths(["hooks/README.md"]) + #expect(result == .material(["hooks/README.md"])) + } + + @Test("Unknown root dir (docs/) → material (Phase 1 has no author ignore: yet)") + func unknownDirMaterial() { + let result = UpdateChecker.classifyDiffPaths(["docs/guide.md"]) + #expect(result == .material(["docs/guide.md"])) + } + + @Test("Whitespace and empty lines are stripped") + func whitespaceStripped() { + let result = UpdateChecker.classifyDiffPaths([ + " README.md ", "", " ", "LICENSE", + ]) + #expect(result == .suppressed) + } +} + +struct UpdateCheckerOrchestratorTests { + private func makeEntry(identifier: String = "pack-a", commitSHA: String = "old123") + -> PackRegistryFile.PackEntry { + makeRegistryEntry(identifier: identifier, commitSHA: commitSHA) + } + + private func makeChecker(home: URL, mock: MockShellRunner) -> UpdateChecker { + let env = Environment(home: home) + return UpdateChecker(environment: env, shell: mock) + } + + private func preparePackDir(home: URL, identifier: String) throws { + let dir = home.appendingPathComponent(".mcs/packs/\(identifier)") + try FileManager.default.createDirectory(at: dir, withIntermediateDirectories: true) + } + + @Test("fetch + diff succeed with noise → suppressed") + func fetchDiffNoiseSuppressed() throws { + let tmpDir = try makeTmpDir(label: "classify") + defer { try? FileManager.default.removeItem(at: tmpDir) } + try preparePackDir(home: tmpDir, identifier: "pack-a") + + let mock = MockShellRunner(environment: Environment(home: tmpDir)) + mock.runResults = [ + ShellResult(exitCode: 0, stdout: "", stderr: ""), // fetch + ShellResult(exitCode: 0, stdout: "README.md\nLICENSE\n", stderr: ""), // diff + ] + + let checker = makeChecker(home: tmpDir, mock: mock) + let result = checker.classifyUpstreamChange(entry: makeEntry()) + + #expect(result == .suppressed) + #expect(mock.runCalls.count == 2) + #expect(mock.runCalls[0].arguments.contains("fetch")) + #expect(mock.runCalls[1].arguments.contains("diff")) + } + + @Test("fetch + diff succeed with material → material") + func fetchDiffMaterial() throws { + let tmpDir = try makeTmpDir(label: "classify") + defer { try? FileManager.default.removeItem(at: tmpDir) } + try preparePackDir(home: tmpDir, identifier: "pack-a") + + let mock = MockShellRunner(environment: Environment(home: tmpDir)) + mock.runResults = [ + ShellResult(exitCode: 0, stdout: "", stderr: ""), + ShellResult(exitCode: 0, stdout: "hooks/session-start.sh\n", stderr: ""), + ] + + let checker = makeChecker(home: tmpDir, mock: mock) + let result = checker.classifyUpstreamChange(entry: makeEntry()) + + #expect(result == .material(["hooks/session-start.sh"])) + } + + @Test("fetch fails → unknown (never-hide, diff not called)") + func fetchFailsNeverHide() throws { + let tmpDir = try makeTmpDir(label: "classify") + defer { try? FileManager.default.removeItem(at: tmpDir) } + try preparePackDir(home: tmpDir, identifier: "pack-a") + + let mock = MockShellRunner(environment: Environment(home: tmpDir)) + mock.runResults = [ + ShellResult(exitCode: 128, stdout: "", stderr: "fatal: unable to access"), + ] + + let checker = makeChecker(home: tmpDir, mock: mock) + let result = checker.classifyUpstreamChange(entry: makeEntry()) + + #expect(result == .unknown) + #expect(mock.runCalls.count == 1) // diff should not be called after fetch fails + } + + @Test("diff fails → unknown (never-hide)") + func diffFailsNeverHide() throws { + let tmpDir = try makeTmpDir(label: "classify") + defer { try? FileManager.default.removeItem(at: tmpDir) } + try preparePackDir(home: tmpDir, identifier: "pack-a") + + let mock = MockShellRunner(environment: Environment(home: tmpDir)) + mock.runResults = [ + ShellResult(exitCode: 0, stdout: "", stderr: ""), + ShellResult(exitCode: 128, stdout: "", stderr: "fatal: bad revision"), + ] + + let checker = makeChecker(home: tmpDir, mock: mock) + let result = checker.classifyUpstreamChange(entry: makeEntry()) + + #expect(result == .unknown) + } + + @Test("git commands pass GIT_TERMINAL_PROMPT=0 to avoid credential prompts") + func credentialSuppression() throws { + let tmpDir = try makeTmpDir(label: "classify") + defer { try? FileManager.default.removeItem(at: tmpDir) } + try preparePackDir(home: tmpDir, identifier: "pack-a") + + let mock = MockShellRunner(environment: Environment(home: tmpDir)) + mock.runResults = [ + ShellResult(exitCode: 0, stdout: "", stderr: ""), + ShellResult(exitCode: 0, stdout: "README.md\n", stderr: ""), + ] + + let checker = makeChecker(home: tmpDir, mock: mock) + _ = checker.classifyUpstreamChange(entry: makeEntry()) + + for call in mock.runCalls { + #expect(call.additionalEnvironment["GIT_TERMINAL_PROMPT"] == "0") + } + } +} + +struct UpdateCheckerPackUpdatesTests { + private func writeRegistry(at env: Environment, entries: [PackRegistryFile.PackEntry]) throws { + let registry = PackRegistryFile(path: env.packsRegistry) + var data = PackRegistryFile.RegistryData() + for entry in entries { + registry.register(entry, in: &data) + } + try registry.save(data) + } + + private func preparePackDir(home: URL, identifier: String) throws { + let dir = home.appendingPathComponent(".mcs/packs/\(identifier)") + try FileManager.default.createDirectory(at: dir, withIntermediateDirectories: true) + } + + @Test("Noise-only upstream commit → no PackUpdate, registry commitSHA advances") + func noiseSuppressedAndBaselineAdvances() throws { + let tmpDir = try makeTmpDir(label: "checkPackUpdates") + defer { try? FileManager.default.removeItem(at: tmpDir) } + try preparePackDir(home: tmpDir, identifier: "pack-a") + + let env = Environment(home: tmpDir) + let entry = makeRegistryEntry(identifier: "pack-a", commitSHA: "old123") + try writeRegistry(at: env, entries: [entry]) + + let mock = MockShellRunner(environment: env) + mock.runResults = [ + // ls-remote reports a new SHA + ShellResult(exitCode: 0, stdout: "new456\tHEAD\n", stderr: ""), + // fetch succeeds + ShellResult(exitCode: 0, stdout: "", stderr: ""), + // diff reports README-only (noise) + ShellResult(exitCode: 0, stdout: "README.md\n", stderr: ""), + ] + + let checker = UpdateChecker(environment: env, shell: mock) + let updates = checker.checkPackUpdates(entries: [entry]) + + #expect(updates.isEmpty, "Noise-only upstream commit must not produce a notification") + + // Registry baseline advanced to the new SHA + let registry = PackRegistryFile(path: env.packsRegistry) + let data = try registry.load() + let updated = registry.pack(identifier: "pack-a", in: data) + #expect(updated?.commitSHA == "new456") + } + + @Test("Material upstream commit → PackUpdate emitted, registry NOT advanced") + func materialEmitsUpdateAndPreservesBaseline() throws { + let tmpDir = try makeTmpDir(label: "checkPackUpdates") + defer { try? FileManager.default.removeItem(at: tmpDir) } + try preparePackDir(home: tmpDir, identifier: "pack-a") + + let env = Environment(home: tmpDir) + let entry = makeRegistryEntry(identifier: "pack-a", commitSHA: "old123") + try writeRegistry(at: env, entries: [entry]) + + let mock = MockShellRunner(environment: env) + mock.runResults = [ + ShellResult(exitCode: 0, stdout: "new456\tHEAD\n", stderr: ""), + ShellResult(exitCode: 0, stdout: "", stderr: ""), + ShellResult(exitCode: 0, stdout: "hooks/session-start.sh\n", stderr: ""), + ] + + let checker = UpdateChecker(environment: env, shell: mock) + let updates = checker.checkPackUpdates(entries: [entry]) + + #expect(updates.count == 1) + #expect(updates.first?.identifier == "pack-a") + #expect(updates.first?.remoteSHA == "new456") + + // Registry preserved at the old SHA — `mcs pack update` is responsible for advancing it. + let registry = PackRegistryFile(path: env.packsRegistry) + let data = try registry.load() + #expect(registry.pack(identifier: "pack-a", in: data)?.commitSHA == "old123") + } + + @Test("fetch failure during classification → never-hide (PackUpdate emitted)") + func fetchFailureSurfaces() throws { + let tmpDir = try makeTmpDir(label: "checkPackUpdates") + defer { try? FileManager.default.removeItem(at: tmpDir) } + try preparePackDir(home: tmpDir, identifier: "pack-a") + + let env = Environment(home: tmpDir) + let entry = makeRegistryEntry(identifier: "pack-a", commitSHA: "old123") + try writeRegistry(at: env, entries: [entry]) + + let mock = MockShellRunner(environment: env) + mock.runResults = [ + ShellResult(exitCode: 0, stdout: "new456\tHEAD\n", stderr: ""), + ShellResult(exitCode: 128, stdout: "", stderr: "fatal: unable to access"), + ] + + let checker = UpdateChecker(environment: env, shell: mock) + let updates = checker.checkPackUpdates(entries: [entry]) + + #expect(updates.count == 1, "fetch failure must never hide a real upstream change") + } + + @Test("ls-remote SHA matches local → no classifier invocation, no update") + func noChangeSkipsClassifier() throws { + let tmpDir = try makeTmpDir(label: "checkPackUpdates") + defer { try? FileManager.default.removeItem(at: tmpDir) } + + let env = Environment(home: tmpDir) + let entry = makeRegistryEntry(identifier: "pack-a", commitSHA: "same789") + try writeRegistry(at: env, entries: [entry]) + + let mock = MockShellRunner(environment: env) + mock.runResults = [ + ShellResult(exitCode: 0, stdout: "same789\tHEAD\n", stderr: ""), + ] + + let checker = UpdateChecker(environment: env, shell: mock) + let updates = checker.checkPackUpdates(entries: [entry]) + + #expect(updates.isEmpty) + #expect(mock.runCalls.count == 1) // only ls-remote; no fetch/diff + } +} + // MARK: - Parsing Tests struct UpdateCheckerParsingTests { diff --git a/docs/architecture.md b/docs/architecture.md index 509f1b6..fdc8319 100644 --- a/docs/architecture.md +++ b/docs/architecture.md @@ -109,6 +109,12 @@ Wraps `claude mcp add/remove` and `claude plugin install/remove` CLI commands. M - **`project`**: team-shared — stored in `.mcp.json` in the project directory - **`user`**: cross-project — stored in `~/.claude.json` globally +### UpdateChecker (`Core/UpdateChecker.swift`) + +Detects upstream pack and CLI updates via `git ls-remote`, with a **noise filter** so non-material upstream commits (README, LICENSE, CI, `.github/`, etc.) don't trigger spurious notifications. When the remote SHA differs from the registry baseline, the checker does a shallow `git fetch` + `git diff --name-only` in the local pack clone and classifies the changed-path list against a built-in deny-list. If every path is infrastructure, the notification is suppressed and the registry `commitSHA` advances so the same commits don't re-trigger. `techpack.yaml` is always treated as material — manifest edits can swap the install surface entirely, and silently suppressing those commits would be a supply-chain attack vector. Filter failures (offline, fetch error) fall through to surfacing the notification unfiltered — the filter can only suppress, never manufacture silence. + +Results are cached in `~/.mcs/update-check.json` with a 24-hour cooldown. The SessionStart hook serves cached results on every session start; only `mcs check-updates` (without `--hook`) forces a fresh network check. + ## External Pack System External packs are directories containing a `techpack.yaml` manifest — either Git repositories cloned into `~/.mcs/packs/` or local directories registered in-place. The system has these layers: diff --git a/docs/cli.md b/docs/cli.md index aca15b3..1a080b0 100644 --- a/docs/cli.md +++ b/docs/cli.md @@ -174,6 +174,9 @@ mcs check-updates --json # Machine-readable JSON output **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. +- **Noise filter**: When the remote SHA differs, runs a shallow `git fetch` + `git diff --name-only` in the local pack clone and classifies the changed-path list. If every path is deny-listed infrastructure (README, LICENSE, CHANGELOG, `.github/`, `node_modules/`, `.build/`, `Makefile`, `.gitignore`, etc.), the notification is suppressed and `~/.mcs/registry.yaml`'s `commitSHA` advances to the new remote SHA — the same infrastructure-only commits won't re-trigger on future cooldown windows. Any non-deny-listed path makes the update material and surfaces the notification. +- **`techpack.yaml` always surfaces**: manifest edits can change the install surface entirely (new hook scripts, swapped MCP server commands, different component actions), so the filter never suppresses a commit that touches the manifest — supply-chain safety takes precedence over noise reduction. +- **Never-hide on filter failure**: if `git fetch` or `git diff` errors (offline, access revoked, repo corrupt), the notification surfaces unfiltered. The filter can only suppress; it can't manufacture silence on error. - **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). 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.