diff --git a/Sources/OpenFreshrApp/AppViewModel.swift b/Sources/OpenFreshrApp/AppViewModel.swift index f389e31..5758016 100644 --- a/Sources/OpenFreshrApp/AppViewModel.swift +++ b/Sources/OpenFreshrApp/AppViewModel.swift @@ -729,6 +729,56 @@ public final class AppViewModel { } } + /// Whether ``retryViaPnpm(_:)`` can be offered for `package`: it is + /// npm-tracked, and pnpm is actually available right now. + /// + /// Exists for a verified real-world case, not a hypothetical one: a + /// corporate npm registry was found to refuse `npm install`'s own fetch + /// for a package while `pnpm add` succeeded for the exact same package, + /// version and registry, every time. Deliberately npm-specific — this is + /// not a generic "try a different tool" button, only the one pairing this + /// was verified for. + public func canRetryViaPnpm(_ package: OutdatedPackage) -> Bool { + package.ecosystem == .npm && ecosystemCoordinator.isAvailable(.pnpm) + } + + /// Retry `package` (an npm package whose own update just failed) via pnpm + /// instead. On a **confirmed** pnpm success — the coordinator's own + /// rescan, not pnpm's claim — the stale npm-tracked copy is removed + /// (best-effort; pnpm is already the source of truth for this package + /// regardless of whether that succeeds), so `npm outdated` stops + /// reporting a package that is verifiably current again, just through a + /// different tool. + public func retryViaPnpm(_ package: OutdatedPackage) async { + guard package.ecosystem == .npm else { return } + let key = package.id + guard !updateInFlight.contains(key) else { return } + updateInFlight.insert(key) + defer { updateInFlight.remove(key) } + + let pnpmPackage = OutdatedPackage( + ecosystem: .pnpm, name: package.name, installed: package.installed, + available: package.available, isMajor: package.isMajor, description: package.description) + + let ecosystems = self.ecosystemCoordinator + let results = await ecosystems.update([pnpmPackage]) + guard let result = results.first else { return } + + guard result.isVerified else { + updateOutcomes[key] = + result.stillOutdated == true + ? String(localized: "pnpm also reported success, but the package is still outdated.") + : (result.action.explanation ?? String(localized: "pnpm could not update this either.")) + return + } + + updateOutcomes[key] = Self.updatedMessage + _ = await Task.detached { ecosystems.uninstall(package) }.value + // Both ecosystems changed (npm lost a row, pnpm gained one); a full + // recheck reflects both rather than hand-patching two reports at once. + await checkForUpdates() + } + /// The ecosystem's check with `package` removed when the update was verified, /// or unchanged otherwise — a light local patch so the row disappears at once /// instead of waiting for the next full ``checkForUpdates()``. diff --git a/Sources/OpenFreshrApp/ContentView.swift b/Sources/OpenFreshrApp/ContentView.swift index 9f5226c..2f16b00 100644 --- a/Sources/OpenFreshrApp/ContentView.swift +++ b/Sources/OpenFreshrApp/ContentView.swift @@ -298,17 +298,27 @@ private struct PackageRow: View { if isUpdating { ProgressView().controlSize(.small) Text("Updating …").foregroundStyle(.secondary) - } else if canAutomaticallyUpdate { - Button("Update") { - Task { await viewModel.updatePackage(package) } - } - .buttonStyle(.borderedProminent) - .help("Install the new version of \(package.name)") } else { - Button("Open") { - openWhereToUpdate() + HStack(spacing: 8) { + if problem != nil, viewModel.canRetryViaPnpm(package) { + Button("Try via pnpm") { + Task { await viewModel.retryViaPnpm(package) } + } + .help("Retry updating \(package.name) through pnpm instead of npm") + } + if canAutomaticallyUpdate { + Button("Update") { + Task { await viewModel.updatePackage(package) } + } + .buttonStyle(.borderedProminent) + .help("Install the new version of \(package.name)") + } else { + Button("Open") { + openWhereToUpdate() + } + .help("Open where \(package.name) can be updated") + } } - .help("Open where \(package.name) can be updated") } } .padding(.vertical, 4) diff --git a/Sources/OpenFreshrApp/Resources/Localizable.xcstrings b/Sources/OpenFreshrApp/Resources/Localizable.xcstrings index 08e3691..e503bd3 100644 --- a/Sources/OpenFreshrApp/Resources/Localizable.xcstrings +++ b/Sources/OpenFreshrApp/Resources/Localizable.xcstrings @@ -2441,6 +2441,16 @@ } } }, + "Retry updating %@ through pnpm instead of npm" : { + "localizations" : { + "de" : { + "stringUnit" : { + "state" : "translated", + "value" : "%@ stattdessen über pnpm statt npm aktualisieren" + } + } + } + }, "Scanning …" : { "localizations" : { "de" : { @@ -2981,6 +2991,16 @@ } } }, + "Try via pnpm" : { + "localizations" : { + "de" : { + "stringUnit" : { + "state" : "translated", + "value" : "Über pnpm versuchen" + } + } + } + }, "Type" : { "localizations" : { "de" : { @@ -3511,6 +3531,26 @@ } } }, + "pnpm also reported success, but the package is still outdated." : { + "localizations" : { + "de" : { + "stringUnit" : { + "state" : "translated", + "value" : "pnpm hat ebenfalls Erfolg gemeldet, das Paket ist aber weiterhin veraltet." + } + } + } + }, + "pnpm could not update this either." : { + "localizations" : { + "de" : { + "stringUnit" : { + "state" : "translated", + "value" : "pnpm konnte dies ebenfalls nicht aktualisieren." + } + } + } + }, "recognized" : { "localizations" : { "de" : { diff --git a/Sources/OpenFreshrCore/Ecosystem/Ecosystem.swift b/Sources/OpenFreshrCore/Ecosystem/Ecosystem.swift index 1f17c60..4f323ee 100644 --- a/Sources/OpenFreshrCore/Ecosystem/Ecosystem.swift +++ b/Sources/OpenFreshrCore/Ecosystem/Ecosystem.swift @@ -124,6 +124,28 @@ public protocol EcosystemUpdating: Sendable { func update(_ package: OutdatedPackage) -> BackendActionResult } +/// An ``EcosystemUpdating`` that can additionally **remove** a package it +/// manages. Only meaningful where removing a stale tracked copy is itself +/// safe and well-understood (`npm uninstall`) — not every ecosystem needs +/// this, so it is a separate capability, mirroring ``UninstallingBackend``'s +/// relationship to ``PackageBackend``. +/// +/// Exists for exactly one purpose today: after ``PnpmEcosystem`` rescues an +/// npm package a corporate registry refuses to let `npm` itself fetch (see +/// ``AppViewModel/retryViaPnpm(_:)``), the stale npm-tracked copy is removed +/// so `npm outdated` stops reporting a package that is verifiably current +/// again, just through a different tool. +public protocol EcosystemUninstalling: EcosystemUpdating { + + /// The exact command that would remove `package`, or `nil` when the tool + /// is missing or the name fails validation. + func resolveUninstallCommand(for package: OutdatedPackage) -> ResolvedCommand? + + /// Remove `package`. A reported success is only a claim, like every other + /// action here. + func uninstall(_ package: OutdatedPackage) -> BackendActionResult +} + /// One ecosystem's check, paired with the ecosystem it belongs to. public struct EcosystemReport: Hashable, Sendable, Identifiable { public var kind: EcosystemKind @@ -191,6 +213,28 @@ public struct EcosystemCoordinator: Sendable { return ecosystem.resolveUpdateCommand(for: package) != nil } + /// Whether `kind`'s ecosystem is configured here and its tool is present + /// right now — checked fresh (a cheap file-exists probe, no process + /// spawn), not read from a stale cached report. + public func isAvailable(_ kind: EcosystemKind) -> Bool { + ecosystems.first(where: { $0.kind == kind })?.isAvailable() ?? false + } + + /// Remove `package` via its own ecosystem, when that ecosystem supports + /// removal (see ``EcosystemUninstalling``). `.failed(.toolUnavailable)` + /// when the ecosystem is not configured, or does not support removal at + /// all — a reported success is only a claim, exactly like every other + /// action here. + public func uninstall(_ package: OutdatedPackage) -> BackendActionResult { + guard + let ecosystem = ecosystems.first(where: { $0.kind == package.ecosystem }) + as? any EcosystemUninstalling + else { + return .failed(reason: .toolUnavailable(tool: package.ecosystem.label)) + } + return ecosystem.uninstall(package) + } + /// Updates the given packages one after another, then checks each affected /// ecosystem again so a reported success is confirmed rather than trusted. public func update(_ packages: [OutdatedPackage]) async -> [PackageUpdateResult] { diff --git a/Sources/OpenFreshrCore/Ecosystem/NpmEcosystem.swift b/Sources/OpenFreshrCore/Ecosystem/NpmEcosystem.swift index 74bbf45..ea282d5 100644 --- a/Sources/OpenFreshrCore/Ecosystem/NpmEcosystem.swift +++ b/Sources/OpenFreshrCore/Ecosystem/NpmEcosystem.swift @@ -8,7 +8,7 @@ import Foundation /// built with) puts `npm` somewhere else, and this ecosystem reports /// ``EcosystemCheck/unavailable`` rather than guess a path. A caller who knows the /// real path can pass it in `candidateNpmPaths`. -public struct NpmEcosystem: EcosystemUpdating { +public struct NpmEcosystem: EcosystemUninstalling { public let kind: EcosystemKind = .npm @@ -92,6 +92,41 @@ public struct NpmEcosystem: EcosystemUpdating { } } + /// The exact `npm uninstall --global -- ` command, or `nil` when the + /// tool is missing or the name fails validation. Verified directly: + /// `npm uninstall` accepts a `--` separator the same way `install` does. + public func resolveUninstallCommand(for package: OutdatedPackage) -> ResolvedCommand? { + guard package.ecosystem == .npm, Self.isValidPackageName(package.name), let npm = npmURL() else { + return nil + } + return ResolvedCommand( + executablePath: npm.path, + arguments: ["uninstall", "--global", "--", package.name] + ) + } + + /// Remove the globally installed npm package `package`. Exists for + /// ``AppViewModel/retryViaPnpm(_:)``: once pnpm has confirmed it can + /// manage a package npm itself could not fetch, the stale npm-tracked + /// copy is removed so `npm outdated` stops reporting it. + public func uninstall(_ package: OutdatedPackage) -> BackendActionResult { + guard package.ecosystem == .npm, Self.isValidPackageName(package.name) else { + return .failed(reason: .invalidIdentifier(package.name)) + } + guard let command = resolveUninstallCommand(for: package) else { + return .failed(reason: .toolUnavailable(tool: "npm")) + } + do { + let result = try processRunner.run( + executableURL: URL(fileURLWithPath: command.executablePath), arguments: command.arguments) + if result.didSucceed { return .succeeded(standardOutput: result.standardOutput) } + return .failed( + reason: .processFailed(exitCode: result.exitCode, standardError: result.standardError)) + } catch { + return .failed(reason: .launchFailed(message: error.localizedDescription)) + } + } + // MARK: - Parsing and validation private struct Entry: Decodable { diff --git a/Tests/OpenFreshrCoreTests/EcosystemTests.swift b/Tests/OpenFreshrCoreTests/EcosystemTests.swift index caacba4..faf6e93 100644 --- a/Tests/OpenFreshrCoreTests/EcosystemTests.swift +++ b/Tests/OpenFreshrCoreTests/EcosystemTests.swift @@ -49,6 +49,37 @@ private func package(_ name: String, _ kind: EcosystemKind = .npm) -> OutdatedPa OutdatedPackage(ecosystem: kind, name: name, installed: "1.0.0", available: "1.1.0", isMajor: false) } +/// Same programmed-answers shape as ``FakeEcosystem``, but additionally +/// conforming to ``EcosystemUninstalling``, so ``EcosystemCoordinator/uninstall(_:)`` +/// can be exercised against an ecosystem that actually supports removal. +private final class FakeUninstallingEcosystem: EcosystemUninstalling, @unchecked Sendable { + let kind: EcosystemKind + private let lock = NSLock() + private(set) var uninstallCalls: [String] = [] + private let uninstallResult: BackendActionResult + + init(kind: EcosystemKind, uninstallResult: BackendActionResult = .succeeded(standardOutput: "")) { + self.kind = kind + self.uninstallResult = uninstallResult + } + + func isAvailable() -> Bool { true } + func check() -> EcosystemCheck { .upToDate } + func resolveUpdateCommand(for package: OutdatedPackage) -> ResolvedCommand? { nil } + func update(_ package: OutdatedPackage) -> BackendActionResult { .succeeded(standardOutput: "") } + + func resolveUninstallCommand(for package: OutdatedPackage) -> ResolvedCommand? { + ResolvedCommand(executablePath: "/fake", arguments: ["uninstall", package.name]) + } + + func uninstall(_ package: OutdatedPackage) -> BackendActionResult { + lock.lock() + defer { lock.unlock() } + uninstallCalls.append(package.name) + return uninstallResult + } +} + @Suite("EcosystemCoordinator") struct EcosystemCoordinatorTests { @@ -109,6 +140,40 @@ struct EcosystemCoordinatorTests { let results = await coordinator.update([package("a")]) #expect(!results[0].action.didReportSuccess) } + + @Test("isAvailable reflects the configured ecosystem's own answer, and false when not configured at all") + func isAvailableReflectsTheEcosystem() { + let coordinator = EcosystemCoordinator(ecosystems: [ + FakeEcosystem(kind: .npm, available: true), + FakeEcosystem(kind: .pnpm, available: false), + ]) + #expect(coordinator.isAvailable(.npm)) + #expect(!coordinator.isAvailable(.pnpm)) + #expect(!coordinator.isAvailable(.pipx)) + } + + @Test("uninstall routes to an ecosystem that supports removal") + func uninstallRoutesToASupportingEcosystem() { + let uninstaller = FakeUninstallingEcosystem(kind: .npm) + let coordinator = EcosystemCoordinator(ecosystems: [uninstaller]) + let result = coordinator.uninstall(package("corepack")) + #expect(result.didReportSuccess) + #expect(uninstaller.uninstallCalls == ["corepack"]) + } + + @Test("uninstall fails without running anything when the ecosystem cannot remove packages") + func uninstallFailsForANonUninstallingEcosystem() { + let coordinator = EcosystemCoordinator(ecosystems: [FakeEcosystem(kind: .npm)]) + let result = coordinator.uninstall(package("corepack")) + #expect(!result.didReportSuccess) + } + + @Test("uninstall fails when the ecosystem is not configured at all") + func uninstallFailsForAnUnconfiguredEcosystem() { + let coordinator = EcosystemCoordinator(ecosystems: []) + let result = coordinator.uninstall(package("corepack")) + #expect(!result.didReportSuccess) + } } @Suite("HomebrewFormulaEcosystem") @@ -351,6 +416,46 @@ struct NpmEcosystemTests { } #expect(runner.invocations.isEmpty) } + + // MARK: - Uninstall (the pnpm-fallback cleanup step) + + @Test("The uninstall command removes exactly the one named package, with a -- separator, verified directly") + func uninstallCommand() { + let (ecosystem, runner) = ecosystem { _, _ in ProcessResult(exitCode: 0, standardOutput: "", standardError: "") + } + let plain = OutdatedPackage( + ecosystem: .npm, name: "corepack", installed: "0.34.6", available: "0.36.0", isMajor: false) + #expect( + ecosystem.resolveUninstallCommand(for: plain)?.arguments + == ["uninstall", "--global", "--", "corepack"]) + + let result = ecosystem.uninstall(plain) + #expect(result.didReportSuccess) + #expect(runner.invocations.last?.arguments == ["uninstall", "--global", "--", "corepack"]) + } + + @Test("A hostile name is refused before any uninstall process runs") + func uninstallRejectsHostileNames() { + let (ecosystem, runner) = ecosystem { _, _ in ProcessResult(exitCode: 0, standardOutput: "", standardError: "") + } + let hostile = OutdatedPackage( + ecosystem: .npm, name: "-g", installed: "1", available: "2", isMajor: false) + #expect(ecosystem.resolveUninstallCommand(for: hostile) == nil) + #expect(!ecosystem.uninstall(hostile).didReportSuccess) + #expect(runner.invocations.isEmpty) + } + + @Test("Without npm the uninstall command is nil and nothing runs") + func uninstallFailsCleanlyWithoutNpm() { + let (ecosystem, runner) = ecosystem(npmInstalled: false) { _, _ in + ProcessResult(exitCode: 0, standardOutput: "", standardError: "") + } + let plain = OutdatedPackage( + ecosystem: .npm, name: "corepack", installed: "1", available: "2", isMajor: false) + #expect(ecosystem.resolveUninstallCommand(for: plain) == nil) + #expect(!ecosystem.uninstall(plain).didReportSuccess) + #expect(runner.invocations.isEmpty) + } } /// Verified directly against real `pnpm 10.33.3` (see ``PnpmEcosystem``'s doc