diff --git a/Sources/Services/RunnerEnvironment.swift b/Sources/Services/RunnerEnvironment.swift index 2090e46..595c6bf 100644 --- a/Sources/Services/RunnerEnvironment.swift +++ b/Sources/Services/RunnerEnvironment.swift @@ -55,4 +55,22 @@ enum RunnerEnvironment { try (path + "\n").write(to: pathFile, atomically: true, encoding: .utf8) } + + static func pathSnapshotNeedsRefresh(in runnerDirectory: String) -> Bool { + let pathFile = URL(fileURLWithPath: runnerDirectory) + .appendingPathComponent(".path") + + guard let snapshot = try? String(contentsOf: pathFile, encoding: .utf8) else { + return true + } + + let entries = Set( + snapshot + .trimmingCharacters(in: .whitespacesAndNewlines) + .split(separator: ":", omittingEmptySubsequences: true) + .map(String.init) + ) + + return !preferredPathEntries.allSatisfy { entries.contains($0) } + } } diff --git a/Sources/Services/RunnerManager.swift b/Sources/Services/RunnerManager.swift index 6c460bb..33f981b 100644 --- a/Sources/Services/RunnerManager.swift +++ b/Sources/Services/RunnerManager.swift @@ -389,7 +389,11 @@ class RunnerManager: ObservableObject { /// but whose processes are no longer alive (e.g., after app restart or crash). /// Only restarts runners that were in the auto-restart set. func autoRestartRunners() async { - guard !runnersToAutoRestart.isEmpty else { return } + if runnersToAutoRestart.isEmpty { + await restartRunnersWithStalePathSnapshots() + return + } + let ids = runnersToAutoRestart runnersToAutoRestart.removeAll() @@ -403,6 +407,8 @@ class RunnerManager: ObservableObject { } } } + + await restartRunnersWithStalePathSnapshots() } // MARK: - Runner Management @@ -824,6 +830,7 @@ class RunnerManager: ObservableObject { // group purely by `repo` string: an org-level runner and a repo-level runner can // share an identifier prefix, and the GitHub API endpoints differ by scope. let runnersByTarget = Dictionary(grouping: runners) { $0.target } + var becameIdleRunnerIDs = Set() for (target, runnersInTarget) in runnersByTarget { // Only check runners that are currently running @@ -849,6 +856,7 @@ class RunnerManager: ObservableObject { await handleJobStarted(for: runners[index]) } else { await handleJobCompleted(for: runners[index]) + becameIdleRunnerIDs.insert(runner.id) } } } @@ -859,6 +867,10 @@ class RunnerManager: ObservableObject { objectWillChange.send() } } + + if !becameIdleRunnerIDs.isEmpty { + await restartRunnersWithStalePathSnapshots(candidateIDs: becameIdleRunnerIDs) + } } // MARK: - Duplicate Runner @@ -1234,6 +1246,77 @@ class RunnerManager: ObservableObject { scheduledRestarts.removeAll() } + private func restartRunnersWithStalePathSnapshots(candidateIDs: Set? = nil) async { + var staleRunnerIDs: [UUID] = [] + let runnerSnapshot = runners + + for runner in runnerSnapshot { + guard runner.status == .running else { continue } + guard !runner.busy else { continue } + if let candidateIDs { + guard candidateIDs.contains(runner.id) else { continue } + } + + let isolation = runner.effectiveIsolationMode(global: currentSettings.isolationMode) + guard isolation != .container else { continue } + guard processManager.isProcessAlive(for: runner.id) else { continue } + guard let runnerDir = try? RunnerDirectory.path(for: runner.id, isolation: isolation) else { continue } + guard RunnerEnvironment.pathSnapshotNeedsRefresh(in: runnerDir) else { continue } + guard await runnerIsConfirmedIdle(runner) else { continue } + + staleRunnerIDs.append(runner.id) + } + + for id in staleRunnerIDs { + await restartRunnerForPathSnapshotRefresh(id) + } + } + + private func runnerIsConfirmedIdle(_ runner: Runner) async -> Bool { + guard let remoteRunners = try? await ghService.listRemoteRunners(for: runner.target), + let remoteRunner = remoteRunners.first(where: { $0.name == runner.name }) else { + return false + } + + if let index = runners.firstIndex(where: { $0.id == runner.id }) { + runners[index].busy = remoteRunner.busy + } + + return !remoteRunner.busy + } + + private func restartRunnerForPathSnapshotRefresh(_ id: UUID) async { + guard let index = runners.firstIndex(where: { $0.id == id }) else { return } + guard runners[index].status == .running else { return } + guard !runners[index].busy else { return } + + let runner = runners[index] + guard await runnerIsConfirmedIdle(runner) else { return } + + logRunnerEvent( + for: runner, + message: "Runner PATH snapshot is stale; restarting to apply current Homebrew tool paths." + ) + + do { + try await stopRunner(id) + try await startRunner(id) + + if let refreshedIndex = runners.firstIndex(where: { $0.id == id }) { + runners[refreshedIndex].lastRestartEvent = "Runner restarted to apply current Homebrew tool paths." + logRunnerEvent(for: runners[refreshedIndex], message: runners[refreshedIndex].lastRestartEvent ?? "") + saveConfiguration() + } + } catch { + if let refreshedIndex = runners.firstIndex(where: { $0.id == id }) { + runners[refreshedIndex].status = .error + runners[refreshedIndex].lastRestartEvent = "Failed to refresh runner PATH: \(error.localizedDescription)" + logRunnerEvent(for: runners[refreshedIndex], message: runners[refreshedIndex].lastRestartEvent ?? "") + saveConfiguration() + } + } + } + private func logRunnerEvent(for runner: Runner, message: String) { guard !message.isEmpty else { return } diff --git a/Tests/MacRunnerTests/MacRunnerTests.swift b/Tests/MacRunnerTests/MacRunnerTests.swift index 7648566..2a353de 100644 --- a/Tests/MacRunnerTests/MacRunnerTests.swift +++ b/Tests/MacRunnerTests/MacRunnerTests.swift @@ -206,6 +206,44 @@ final class MacRunnerTests: XCTestCase { ) } + func testRunnerEnvironmentDetectsMissingPathSnapshot() throws { + let temporaryDirectory = FileManager.default.temporaryDirectory + .appendingPathComponent(UUID().uuidString, isDirectory: true) + try FileManager.default.createDirectory(at: temporaryDirectory, withIntermediateDirectories: true) + defer { try? FileManager.default.removeItem(at: temporaryDirectory) } + + XCTAssertTrue(RunnerEnvironment.pathSnapshotNeedsRefresh(in: temporaryDirectory.path)) + } + + func testRunnerEnvironmentDetectsStalePathSnapshotMissingHomebrewEntries() throws { + let temporaryDirectory = FileManager.default.temporaryDirectory + .appendingPathComponent(UUID().uuidString, isDirectory: true) + try FileManager.default.createDirectory(at: temporaryDirectory, withIntermediateDirectories: true) + defer { try? FileManager.default.removeItem(at: temporaryDirectory) } + + try "/usr/bin:/bin:/usr/sbin:/sbin:/usr/local/bin\n".write( + to: temporaryDirectory.appendingPathComponent(".path"), + atomically: true, + encoding: .utf8 + ) + + XCTAssertTrue(RunnerEnvironment.pathSnapshotNeedsRefresh(in: temporaryDirectory.path)) + } + + func testRunnerEnvironmentAcceptsCurrentPathSnapshotWithPreferredEntries() throws { + let temporaryDirectory = FileManager.default.temporaryDirectory + .appendingPathComponent(UUID().uuidString, isDirectory: true) + try FileManager.default.createDirectory(at: temporaryDirectory, withIntermediateDirectories: true) + defer { try? FileManager.default.removeItem(at: temporaryDirectory) } + + try RunnerEnvironment.writePathSnapshot( + in: temporaryDirectory.path, + environment: ["PATH": "/usr/bin:/bin"] + ) + + XCTAssertFalse(RunnerEnvironment.pathSnapshotNeedsRefresh(in: temporaryDirectory.path)) + } + func testAdministratorAuthenticationProcessUsesInteractiveTerminalHandles() { let process = UserIsolationService.makeAdministratorAuthenticationProcess()