From 815942cb82f645e86b9c26627ce3cb743275e4d2 Mon Sep 17 00:00:00 2001 From: Fred Rivett Date: Wed, 1 Jul 2026 14:44:04 +0100 Subject: [PATCH 1/2] Address review feedback on port conflict rescue MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - parsePort: only treat `--port=` (not any `--port`-prefixed flag such as `--port-range=`) as the long-form port, avoiding a wrong-port rescue. - Port-conflict detection now gates the error-log match on recency: a log last modified before the process's current run start (pm_uptime) is stale and ignored, so an old bind error can't prompt killing whatever legitimately holds the port now. - ProcessRowView: key the free-port confirmation to the specific port rather than a Bool, so a poll that changes portConflict mid-confirm can't authorize killing a different port's holder. - DemoData: freePort now transfers port ownership — the holder is stopped and loses the port, the rescued process comes online bound to it. Co-Authored-By: Claude Opus 4.8 (1M context) --- Sources/Reeve/Models/PM2Process.swift | 2 +- Sources/Reeve/Services/DemoData.swift | 16 +++++++++++++--- .../Reeve/Services/PM2Service+Parsing.swift | 13 ++++++++++++- Sources/Reeve/Services/PM2Service.swift | 2 +- Sources/Reeve/Views/ProcessRowView.swift | 18 +++++++++++------- Tests/reeveTests/ReeveTests.swift | 5 +++++ 6 files changed, 43 insertions(+), 13 deletions(-) diff --git a/Sources/Reeve/Models/PM2Process.swift b/Sources/Reeve/Models/PM2Process.swift index d8e62d4..ae1854a 100644 --- a/Sources/Reeve/Models/PM2Process.swift +++ b/Sources/Reeve/Models/PM2Process.swift @@ -173,7 +173,7 @@ extension PM2Process: Decodable { if arg == "--port" || arg == "-p", let next, let port = validPort(next[...]) { return port } - if let eq = arg.firstIndex(of: "="), arg.hasPrefix("--port") { + if arg.hasPrefix("--port="), let eq = arg.firstIndex(of: "=") { if let port = validPort(arg[arg.index(after: eq)...]) { return port } } // Bind targets look like `host:port`, `:port`, or `[::1]:port` — the diff --git a/Sources/Reeve/Services/DemoData.swift b/Sources/Reeve/Services/DemoData.swift index 5c4ff25..3681bd6 100644 --- a/Sources/Reeve/Services/DemoData.swift +++ b/Sources/Reeve/Services/DemoData.swift @@ -27,7 +27,7 @@ final class DemoData { var startedAt: Int64 // pm_uptime, ms — uptime ticks up naturally from here let createdAt: Int64 // ms var restartCount: Int - let ports: [Int] + var ports: [Int] let cwd: String let crashLoop: Bool // keep this process perpetually crash-looping var lastLog: Date? @@ -157,13 +157,23 @@ final class DemoData { envs[i].procs = [] } - /// Simulate freeing a port: any process that was blocked waiting for it - /// clears its conflict and comes online. + /// Simulate freeing a port, mirroring the real action: kill whatever holds + /// the port (it goes offline and loses the port), then bring the blocked + /// process online bound to the now-free port. func freePort(_ port: Int) { + for i in envs.indices { + for j in envs[i].procs.indices { + if envs[i].procs[j].portConflict != port, envs[i].procs[j].ports.contains(port) { + envs[i].procs[j].ports.removeAll { $0 == port } + envs[i].procs[j].status = "stopped" + } + } + } for i in envs.indices { for j in envs[i].procs.indices where envs[i].procs[j].portConflict == port { envs[i].procs[j].portConflict = nil envs[i].procs[j].status = "online" + envs[i].procs[j].ports = [port] envs[i].procs[j].startedAt = DemoData.nowMs() envs[i].procs[j].lastLog = Date() } diff --git a/Sources/Reeve/Services/PM2Service+Parsing.swift b/Sources/Reeve/Services/PM2Service+Parsing.swift index 7dac6f7..b524ee1 100644 --- a/Sources/Reeve/Services/PM2Service+Parsing.swift +++ b/Sources/Reeve/Services/PM2Service+Parsing.swift @@ -38,9 +38,20 @@ extension PM2Service { /// Read the tail of an error-log file and check for a bind-conflict message. /// Only the last few KB are read, so this stays cheap on the poll loop. - nonisolated static func errorLogReportsAddressInUse(atPath path: String) -> Bool { + /// + /// `runStartMs` is the process's current run start (`pm_uptime`): when > 0, + /// a log last modified *before* that start predates the current run, so its + /// bind error is stale and ignored — this stops an old conflict from + /// prompting the user to kill whatever legitimately holds the port now. + nonisolated static func errorLogReportsAddressInUse(atPath path: String, runStartMs: Int64) -> Bool { guard !path.isEmpty, let handle = FileHandle(forReadingAtPath: path) else { return false } defer { try? handle.close() } + if runStartMs > 0, + let attrs = try? FileManager.default.attributesOfItem(atPath: path), + let modified = attrs[.modificationDate] as? Date, + modified.timeIntervalSince1970 * 1000 < Double(runStartMs) { + return false + } let tailBytes: UInt64 = 8192 if let size = try? handle.seekToEnd(), size > tailBytes { try? handle.seek(toOffset: size - tailBytes) diff --git a/Sources/Reeve/Services/PM2Service.swift b/Sources/Reeve/Services/PM2Service.swift index d7206f8..7a3e095 100644 --- a/Sources/Reeve/Services/PM2Service.swift +++ b/Sources/Reeve/Services/PM2Service.swift @@ -495,7 +495,7 @@ public class PM2Service: ObservableObject { guard let port = processes[i].desiredPort, !processes[i].ports.contains(port), SocketScanner.isPortListening(port, in: sockets), - errorLogReportsAddressInUse(atPath: processes[i].errLogPath) + errorLogReportsAddressInUse(atPath: processes[i].errLogPath, runStartMs: processes[i].uptime) else { continue } processes[i].portConflict = port } diff --git a/Sources/Reeve/Views/ProcessRowView.swift b/Sources/Reeve/Views/ProcessRowView.swift index 0788c86..2fa89fb 100644 --- a/Sources/Reeve/Views/ProcessRowView.swift +++ b/Sources/Reeve/Views/ProcessRowView.swift @@ -9,7 +9,10 @@ struct ProcessRowView: View { @State private var isActing = false @State private var showCrashPopover = false @State private var copied = false - @State private var confirmingFreePort = false + /// The port awaiting a confirm click, or nil when not confirming. Keyed to + /// the port (not a Bool) so a poll that changes `portConflict` mid-confirm + /// can't let a stale confirmation authorize killing a different port's holder. + @State private var confirmingFreePort: Int? @State private var freePortResetTask: Task? private static let tooltipFormatter: DateFormatter = { @@ -176,21 +179,22 @@ struct ProcessRowView: View { .font(.system(size: 10)) .foregroundColor(.secondary) + let isConfirming = confirmingFreePort == port Button { - if confirmingFreePort { - confirmingFreePort = false + if isConfirming { + confirmingFreePort = nil freePortResetTask?.cancel() performAction { await pm2Service.freePort(port) await pm2Service.restart(process: process, environment: environment) } } else { - confirmingFreePort = true + confirmingFreePort = port freePortResetTask?.cancel() freePortResetTask = Task { try? await Task.sleep(nanoseconds: 3_000_000_000) if !Task.isCancelled { - await MainActor.run { confirmingFreePort = false } + await MainActor.run { confirmingFreePort = nil } } } } @@ -198,7 +202,7 @@ struct ProcessRowView: View { HStack(spacing: 3) { Image(systemName: "bolt.fill") .font(.system(size: 8)) - Text(confirmingFreePort ? "Confirm free port :\(String(port)) and restart service?" : "Free port :\(String(port)) and restart service") + Text(isConfirming ? "Confirm free port :\(String(port)) and restart service?" : "Free port :\(String(port)) and restart service") .font(.system(size: 10, weight: .medium)) } .padding(.horizontal, 8) @@ -209,7 +213,7 @@ struct ProcessRowView: View { .buttonStyle(.plain) .foregroundColor(.orange) .disabled(isActing) - .help(confirmingFreePort ? "Click again to confirm" : "Kill whatever is listening on :\(String(port)), then restart this process") + .help(isConfirming ? "Click again to confirm" : "Kill whatever is listening on :\(String(port)), then restart this process") .onHover { hovering in if hovering { NSCursor.pointingHand.push() } else { NSCursor.pop() } } diff --git a/Tests/reeveTests/ReeveTests.swift b/Tests/reeveTests/ReeveTests.swift index cd74603..cc2931d 100644 --- a/Tests/reeveTests/ReeveTests.swift +++ b/Tests/reeveTests/ReeveTests.swift @@ -1358,6 +1358,11 @@ struct ParsePortTests { func nonNumeric() { #expect(PM2Process.parsePort(fromArgs: ["--port", "auto"]) == nil) } + + @Test("Unrelated --port-prefixed flag is not mistaken for a port") + func portPrefixedFlag() { + #expect(PM2Process.parsePort(fromArgs: ["--port-range=9000"]) == nil) + } } // MARK: - Address-in-use log detection From 352399300afffb063ba79da07fae378c12ac3d28 Mon Sep 17 00:00:00 2001 From: Fred Rivett Date: Wed, 1 Jul 2026 14:50:28 +0100 Subject: [PATCH 2/2] Scope free-port confirm timeout to the port it armed for The 3s reset task cancels on re-arm, but if it passes its isCancelled check just as a newer confirmation is armed, its queued MainActor.run could still clear the fresh confirmation. Guard the clear on the port the timer armed for so a newer confirmation for a different port stands. Co-Authored-By: Claude Opus 4.8 (1M context) --- Sources/Reeve/Views/ProcessRowView.swift | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/Sources/Reeve/Views/ProcessRowView.swift b/Sources/Reeve/Views/ProcessRowView.swift index 2fa89fb..aadf868 100644 --- a/Sources/Reeve/Views/ProcessRowView.swift +++ b/Sources/Reeve/Views/ProcessRowView.swift @@ -194,7 +194,11 @@ struct ProcessRowView: View { freePortResetTask = Task { try? await Task.sleep(nanoseconds: 3_000_000_000) if !Task.isCancelled { - await MainActor.run { confirmingFreePort = nil } + // Only clear the confirmation this timer armed — a + // newer confirmation for a different port must stand. + await MainActor.run { + if confirmingFreePort == port { confirmingFreePort = nil } + } } } }