resolve phase 1 audit: WallpaperManager filename collisions and EventMonitor start leak

@@ -52,3 +52,6 @@5252 # Koh artifacts5353 *.bundle5454 *.koh.offer55+56+# Claude worktrees (git worktrees used for isolated agent sessions)57+.claude/worktrees/

@@ -37,6 +37,12 @@3737   }3838 3939   /// Downloads the file to a temporary location and returns the local URL.40+  ///41+  /// Each call writes to a fresh, unique temp filename so that distinct42+  /// wallpapers sharing a `lastPathComponent` cannot overwrite one another43+  /// before they are consumed (e.g. before `setDesktopImage` or44+  /// `saveToFolder` reads the temp file). The caller owns the returned file45+  /// and is responsible for removing it when no longer needed.4046   func download(from url: URL) async throws -> URL {4147     let (data, response) = try await session.data(from: url)4248     guard let response = response as? HTTPURLResponse,@@ -45,14 +51,23 @@4551       throw WallhavenError.apiError("Wallpaper download failed")4652     }4753 48-    let filename = url.lastPathComponent49-    let tempDir = FileManager.default.temporaryDirectory50-    let fileURL = tempDir.appendingPathComponent(filename)51-54+    let fileURL = Self.makeTempURL(for: url)5255     try data.write(to: fileURL)5356     return fileURL5457   }5558 59+  /// Returns a temp URL unique to this call. The original `url.pathExtension`60+  /// is preserved when present so downstream consumers can identify the kind61+  /// of file, but the stem is a fresh UUID — never derived from the input62+  /// filename — to guarantee no two concurrent or sequential downloads of63+  /// distinct wallpapers collide in the shared temp directory.64+  static func makeTempURL(for url: URL) -> URL {65+    let fileExtension = url.pathExtension.isEmpty ? "img" : url.pathExtension66+    let stem = UUID().uuidString67+    let filename = "haven-\(stem).\(fileExtension)"68+    return FileManager.default.temporaryDirectory.appendingPathComponent(filename)69+  }70+5671   /// Sets the desktop image from a local file URL.5772   @MainActor5873   func setDesktopImage(from fileURL: URL, screen: NSScreen? = nil) throws {@@ -68,15 +83,26 @@6883   }6984 7085   /// Copies the file to the specified destination directory.86+  ///87+  /// Policy for an existing destination with the same name:88+  /// - if its contents match the source, leave it untouched (idempotent89+  ///   re-save of the same logical wallpaper, preserves modification time);90+  /// - if its contents differ, replace it with the source bytes.91+  /// Either branch avoids the prior behavior of unconditionally deleting the92+  /// destination before copying, which would destroy the source if the caller93+  /// passed the same path for both arguments.7194   func saveToFolder(from sourceURL: URL, to folderURL: URL) throws {7295     // Ensure folder exists7396     try FileManager.default.createDirectory(at: folderURL, withIntermediateDirectories: true)7497 7598     let destinationURL = folderURL.appendingPathComponent(sourceURL.lastPathComponent)7699 77-    // If file exists, maybe don't overwrite or handle gracefully?78-    // For now, we'll remove it if it exists to ensure we have the fresh one, or just skip.79100     if FileManager.default.fileExists(atPath: destinationURL.path) {101+      let sourceData = try Data(contentsOf: sourceURL)102+      let existingData = try Data(contentsOf: destinationURL)103+      if sourceData == existingData {104+        return105+      }80106       try FileManager.default.removeItem(at: destinationURL)81107     }82108 

@@ -73,10 +73,20 @@7373   private var monitor: Any?7474   private let mask: NSEvent.EventTypeMask7575   let handler: (NSEvent?) -> Void76+  private let addMonitor: (NSEvent.EventTypeMask, @escaping (NSEvent?) -> Void) -> Any?77+  private let removeMonitor: (Any) -> Void7678 77-  init(mask: NSEvent.EventTypeMask, handler: @escaping (NSEvent?) -> Void) {79+  init(80+    mask: NSEvent.EventTypeMask,81+    handler: @escaping (NSEvent?) -> Void,82+    addMonitor: @escaping (NSEvent.EventTypeMask, @escaping (NSEvent?) -> Void) -> Any? =83+      NSEvent.addGlobalMonitorForEvents,84+    removeMonitor: @escaping (Any) -> Void = NSEvent.removeMonitor85+  ) {7886     self.mask = mask7987     self.handler = handler88+    self.addMonitor = addMonitor89+    self.removeMonitor = removeMonitor8090   }8191 8292   deinit {@@ -84,12 +94,20 @@8494   }8595 8696   func start() {87-    monitor = NSEvent.addGlobalMonitorForEvents(matching: mask, handler: handler)97+    // Remove any existing monitor before installing a new one. Without this,98+    // repeated start() calls leaked monitors inside AppKit (each99+    // addGlobalMonitorForEvents registers a fresh handler, and overwriting100+    // `self.monitor` discarded the only reference to the prior handle, so101+    // removeMonitor could no longer reach it).102+    if monitor != nil {103+      stop()104+    }105+    monitor = addMonitor(mask, handler)88106   }89107 90108   func stop() {91109     if let monitor = monitor {92-      NSEvent.removeMonitor(monitor)110+      removeMonitor(monitor)93111       self.monitor = nil94112     }95113   }

@@ -229,7 +229,12 @@229229     try await manager.setWallpaper(from: remote, screen: screen)230230     defer { if let receivedURL { try? FileManager.default.removeItem(at: receivedURL) } }231231 232-    #expect(receivedURL?.lastPathComponent == remote.lastPathComponent)232+    // Temp file uses a UUID stem (to avoid collisions) but preserves the233+    // original extension so downstream consumers can identify the format.234+    #expect(receivedURL?.pathExtension == remote.pathExtension)235+    #expect(receivedURL?.lastPathComponent.hasPrefix("haven-") == true)236+    #expect(receivedURL?.lastPathComponent != remote.lastPathComponent)237+    #expect(try Data(contentsOf: receivedURL!) == Data("wallpaper".utf8))233238   }234239 235240   @Test("Saving creates the destination folder and copies the file")@@ -264,6 +269,88 @@264269 265270     let copied = try Data(contentsOf: destination.appendingPathComponent("wallpaper.jpg"))266271     #expect(copied == Data("new".utf8))272+  }273+274+  @Test("Saving the same logical wallpaper leaves the existing file untouched")275+  func saveSameContentIsIdempotent() throws {276+    let root = FileManager.default.temporaryDirectory.appendingPathComponent(UUID().uuidString)277+    defer { try? FileManager.default.removeItem(at: root) }278+    let source = root.appendingPathComponent("source/wallpaper.jpg")279+    let destination = root.appendingPathComponent("destination")280+    try FileManager.default.createDirectory(281+      at: source.deletingLastPathComponent(), withIntermediateDirectories: true)282+    try FileManager.default.createDirectory(at: destination, withIntermediateDirectories: true)283+    try Data("identical".utf8).write(to: source)284+    let existingURL = destination.appendingPathComponent("wallpaper.jpg")285+    try Data("identical".utf8).write(to: existingURL)286+    let originalAttributes = try FileManager.default.attributesOfItem(atPath: existingURL.path)287+288+    try WallpaperManager.shared.saveToFolder(from: source, to: destination)289+290+    let updatedAttributes = try FileManager.default.attributesOfItem(atPath: existingURL.path)291+    #expect(try Data(contentsOf: existingURL) == Data("identical".utf8))292+    #expect(updatedAttributes[.modificationDate] as? Date == originalAttributes[.modificationDate] as? Date)293+  }294+295+  @Test("Saving into the same path as the source does not destroy data")296+  func saveSamePathAsSource() throws {297+    let root = FileManager.default.temporaryDirectory.appendingPathComponent(UUID().uuidString)298+    defer { try? FileManager.default.removeItem(at: root) }299+    try FileManager.default.createDirectory(at: root, withIntermediateDirectories: true)300+    let source = root.appendingPathComponent("wallpaper.jpg")301+    try Data("payload".utf8).write(to: source)302+303+    try WallpaperManager.shared.saveToFolder(from: source, to: root)304+305+    #expect(try Data(contentsOf: source) == Data("payload".utf8))306+  }307+308+  @Test("Distinct URLs sharing a lastPathComponent produce distinct temp files")309+  func distinctDownloadsDoNotCollide() async throws {310+    let manager = WallpaperManager(session: managerSession())311+    let first = URL(string: "https://example.com/aaa/wallpaper.jpg")!312+    let second = URL(string: "https://example.com/bbb/wallpaper.jpg")!313+314+    let firstFile = try await manager.download(from: first)315+    let secondFile = try await manager.download(from: second)316+    defer {317+      try? FileManager.default.removeItem(at: firstFile)318+      try? FileManager.default.removeItem(at: secondFile)319+    }320+321+    #expect(firstFile.path != secondFile.path)322+    #expect(firstFile.lastPathComponent != secondFile.lastPathComponent)323+    #expect(try Data(contentsOf: firstFile) == Data("wallpaper".utf8))324+    #expect(try Data(contentsOf: secondFile) == Data("wallpaper".utf8))325+  }326+327+  @Test("Repeated downloads of the same URL produce independent temp files")328+  func repeatedDownloadsAreIndependent() async throws {329+    let manager = WallpaperManager(session: managerSession())330+    let remote = URL(string: "https://example.com/\(UUID().uuidString).jpg")!331+332+    let first = try await manager.download(from: remote)333+    let second = try await manager.download(from: remote)334+    defer {335+      try? FileManager.default.removeItem(at: first)336+      try? FileManager.default.removeItem(at: second)337+    }338+339+    #expect(first.path != second.path)340+    #expect(try Data(contentsOf: first) == Data("wallpaper".utf8))341+    #expect(try Data(contentsOf: second) == Data("wallpaper".utf8))342+  }343+344+  @Test("makeTempURL preserves the source extension when present")345+  func makeTempURLPreservesExtension() {346+    let withExtension = URL(string: "https://example.com/foo/wallpaper.png")!347+    let withoutExtension = URL(string: "https://example.com/foo/wallpaper")!348+349+    #expect(WallpaperManager.makeTempURL(for: withExtension).pathExtension == "png")350+    #expect(WallpaperManager.makeTempURL(for: withoutExtension).pathExtension == "img")351+    #expect(352+      WallpaperManager.makeTempURL(for: withExtension).path353+        != WallpaperManager.makeTempURL(for: withExtension).path)267354   }268355 }269356 

@@ -91,6 +91,37 @@9191     #expect(!eventSeen)9292   }9393 94+  @Test("Repeated start does not leak AppKit event monitors")95+  func eventMonitorRepeatedStartDoesNotLeak() {96+    final class RegistrationCounter {97+      var adds = 098+      var removes = 099+    }100+    let counter = RegistrationCounter()101+    let opaqueHandle = NSObject()102+    let monitor = EventMonitor(103+      mask: [.leftMouseDown],104+      handler: { _ in },105+      addMonitor: { _, _ in106+        counter.adds += 1107+        return opaqueHandle108+      },109+      removeMonitor: { _ in counter.removes += 1 }110+    )111+112+    monitor.start()113+    monitor.start()114+    monitor.start()115+    monitor.stop()116+    monitor.stop()117+118+    // The first two extra start() calls must each have removed the previous119+    // monitor before installing the next one, so adds and removes stay in120+    // lockstep and no AppKit handle outlives its owner.121+    #expect(counter.adds == 3)122+    #expect(counter.removes == 3)123+  }124+94125   @Test("Screen manager exposes AppKit screen metadata")95126   func screenManager() {96127     let manager = ScreenManager.shared