diff --git a/supacode/Features/Settings/BusinessLogic/RepositoryLocalSettingsPersistence.swift b/supacode/Features/Settings/BusinessLogic/RepositoryLocalSettingsPersistence.swift index ca357f17..0bc98275 100644 --- a/supacode/Features/Settings/BusinessLogic/RepositoryLocalSettingsPersistence.swift +++ b/supacode/Features/Settings/BusinessLogic/RepositoryLocalSettingsPersistence.swift @@ -10,11 +10,12 @@ nonisolated enum RepositoryLocalSettingsStorageKey: DependencyKey { static var liveValue: RepositoryLocalSettingsStorage { RepositoryLocalSettingsStorage( load: { try Data(contentsOf: $0) }, - save: { data, url in - let directory = url.deletingLastPathComponent() - try FileManager.default.createDirectory(at: directory, withIntermediateDirectories: true) - try data.write(to: url, options: [.atomic]) - } + // Per-repo settings live under `~/.prowl/repo//` (not inside the + // cloned repo), so they are user-owned config a dotfiles user may symlink + // — follow the link on write to preserve it (#478). Upstream keeps a + // non-following write for its in-repo `supacode.json`; that exception does + // not apply here because the fork stores these outside the repository. + save: { data, url in try SymlinkPreservingFileWriter.write(data, to: url) } ) } diff --git a/supacode/Features/Settings/BusinessLogic/SettingsFilePersistence.swift b/supacode/Features/Settings/BusinessLogic/SettingsFilePersistence.swift index d6c3d9da..262faccd 100644 --- a/supacode/Features/Settings/BusinessLogic/SettingsFilePersistence.swift +++ b/supacode/Features/Settings/BusinessLogic/SettingsFilePersistence.swift @@ -11,11 +11,11 @@ nonisolated enum SettingsFileStorageKey: DependencyKey { static var liveValue: SettingsFileStorage { SettingsFileStorage( load: { try Data(contentsOf: $0) }, - save: { data, url in - let directory = url.deletingLastPathComponent() - try FileManager.default.createDirectory(at: directory, withIntermediateDirectories: true) - try data.write(to: url, options: [.atomic]) - } + // Follows a symlinked destination to its real file so a user who links + // `~/.prowl/settings.json` (and the repository-entries / appearances + // files that share this closure) into a dotfiles repo keeps the link + // instead of having it replaced with a plain file on every save (#478). + save: { data, url in try SymlinkPreservingFileWriter.write(data, to: url) } ) } static var previewValue: SettingsFileStorage { .inMemory() } diff --git a/supacode/Features/Settings/BusinessLogic/SymlinkPreservingFileWriter.swift b/supacode/Features/Settings/BusinessLogic/SymlinkPreservingFileWriter.swift new file mode 100644 index 00000000..56591c80 --- /dev/null +++ b/supacode/Features/Settings/BusinessLogic/SymlinkPreservingFileWriter.swift @@ -0,0 +1,70 @@ +import Foundation + +nonisolated enum SymlinkPreservingFileWriterError: Error, Equatable { + /// The destination resolves through a symlink cycle, so there is no real file + /// to write without clobbering one of the links. + case symbolicLinkCycle(URL) + /// The chain exceeds the kernel's symlink-resolution limit, so the loader + /// could never follow it; refuse rather than write a file it can't read back. + case symbolicLinkChainTooDeep(URL) +} + +/// Atomic file writes that survive a symlinked destination. When the target is a +/// symlink (e.g. a `~/.prowl/settings.json` linked into a dotfiles repo), the +/// write follows the link to its real file so the temp+rename replaces the +/// target, leaving the link intact, instead of overwriting the link with a +/// regular file. +nonisolated enum SymlinkPreservingFileWriter { + /// Atomically writes `data` to `url`, following a symlink at `url` so the link + /// is preserved. Creates the destination's own parent directory when missing, + /// but never a symlink target's parent (a link into a missing directory fails + /// the write rather than fabricating a phantom tree there). + static func write(_ data: Data, to url: URL) throws { + let target = try resolvedTarget(for: url) + try FileManager.default.createDirectory( + at: url.deletingLastPathComponent(), + withIntermediateDirectories: true + ) + // The atomic temp+rename happens in the target's directory, so a symlink at + // `url` is written through and preserved instead of replaced. + try data.write(to: target, options: [.atomic]) + } + + /// macOS resolves at most MAXSYMLINKS (32) links before ELOOP, so a deeper + /// chain is one the loader's `Data(contentsOf:)` could never read back. + private static let maxFollowedSymbolicLinks = 32 + + /// Follows a symlink chain at `url` to its final real file. Returns `url` + /// unchanged when it is not a symlink (including a not-yet-created file). + /// Relative link targets resolve against the link's real directory so a link + /// under a symlinked parent still lands on the file the kernel would. Throws + /// on a cycle or an over-deep chain rather than silently overwriting a link in + /// the loop, and surfaces a real read error rather than misreading an + /// unreadable link as a plain file (which would clobber it). + private static func resolvedTarget(for url: URL) throws -> URL { + let fileManager = FileManager.default + var current = url + var visited: Set = [] + while true { + let isSymbolicLink: Bool + do { + isSymbolicLink = try current.resourceValues(forKeys: [.isSymbolicLinkKey]).isSymbolicLink ?? false + } catch CocoaError.fileReadNoSuchFile { + return current + } + guard isSymbolicLink else { return current } + let linkPath = current.path(percentEncoded: false) + guard visited.insert(linkPath).inserted else { + throw SymlinkPreservingFileWriterError.symbolicLinkCycle(url) + } + guard visited.count <= Self.maxFollowedSymbolicLinks else { + throw SymlinkPreservingFileWriterError.symbolicLinkChainTooDeep(url) + } + let destination = try fileManager.destinationOfSymbolicLink(atPath: linkPath) + // A relative target resolves against the link's real directory; an absolute one ignores the base. + let base = + destination.hasPrefix("/") ? nil : current.deletingLastPathComponent().resolvingSymlinksInPath() + current = URL(filePath: destination, directoryHint: .notDirectory, relativeTo: base).standardizedFileURL + } + } +} diff --git a/supacodeTests/SymlinkPreservingFileWriterTests.swift b/supacodeTests/SymlinkPreservingFileWriterTests.swift new file mode 100644 index 00000000..1fcf01aa --- /dev/null +++ b/supacodeTests/SymlinkPreservingFileWriterTests.swift @@ -0,0 +1,186 @@ +import Foundation +import Testing + +@testable import supacode + +struct SymlinkPreservingFileWriterTests { + private let fileManager = FileManager.default + + private func makeTempDir() throws -> URL { + let dir = fileManager.temporaryDirectory + .appending(path: "symlink-writer-test-\(UUID().uuidString)", directoryHint: .isDirectory) + try fileManager.createDirectory(at: dir, withIntermediateDirectories: true) + return dir + } + + private func isSymlink(_ url: URL) -> Bool { + (try? fileManager.destinationOfSymbolicLink(atPath: url.path(percentEncoded: false))) != nil + } + + @Test func writesPlainFileWithContent() throws { + let dir = try makeTempDir() + defer { try? fileManager.removeItem(at: dir) } + let url = dir.appending(path: "settings.json", directoryHint: .notDirectory) + let payload = Data("{\"a\":1}".utf8) + + try SymlinkPreservingFileWriter.write(payload, to: url) + + #expect(try Data(contentsOf: url) == payload) + #expect(!isSymlink(url)) + } + + @Test func createsMissingParentDirectories() throws { + let dir = try makeTempDir() + defer { try? fileManager.removeItem(at: dir) } + let url = dir.appending(path: "nested/deep/settings.json", directoryHint: .notDirectory) + let payload = Data("{}".utf8) + + try SymlinkPreservingFileWriter.write(payload, to: url) + + #expect(try Data(contentsOf: url) == payload) + } + + @Test func preservesAbsoluteSymlinkAndWritesThrough() throws { + let dir = try makeTempDir() + defer { try? fileManager.removeItem(at: dir) } + let target = dir.appending(path: "target.json", directoryHint: .notDirectory) + let link = dir.appending(path: "settings.json", directoryHint: .notDirectory) + try Data("old".utf8).write(to: target) + try fileManager.createSymbolicLink(at: link, withDestinationURL: target) + let payload = Data("new".utf8) + + try SymlinkPreservingFileWriter.write(payload, to: link) + + #expect(isSymlink(link)) + #expect(try Data(contentsOf: target) == payload) + #expect(try Data(contentsOf: link) == payload) + } + + @Test func preservesRelativeSymlinkAndWritesThrough() throws { + let dir = try makeTempDir() + defer { try? fileManager.removeItem(at: dir) } + let prowlDir = dir.appending(path: ".prowl", directoryHint: .isDirectory) + let dotfilesDir = dir.appending(path: "dotfiles", directoryHint: .isDirectory) + try fileManager.createDirectory(at: prowlDir, withIntermediateDirectories: true) + try fileManager.createDirectory(at: dotfilesDir, withIntermediateDirectories: true) + let target = dotfilesDir.appending(path: "settings.json", directoryHint: .notDirectory) + try Data("old".utf8).write(to: target) + let link = prowlDir.appending(path: "settings.json", directoryHint: .notDirectory) + try fileManager.createSymbolicLink( + atPath: link.path(percentEncoded: false), + withDestinationPath: "../dotfiles/settings.json" + ) + let payload = Data("new".utf8) + + try SymlinkPreservingFileWriter.write(payload, to: link) + + #expect(isSymlink(link)) + #expect(try Data(contentsOf: target) == payload) + } + + @Test func createsTargetForDanglingSymlinkWhenParentExists() throws { + let dir = try makeTempDir() + defer { try? fileManager.removeItem(at: dir) } + let prowlDir = dir.appending(path: ".prowl", directoryHint: .isDirectory) + let dotfilesDir = dir.appending(path: "dotfiles", directoryHint: .isDirectory) + try fileManager.createDirectory(at: prowlDir, withIntermediateDirectories: true) + try fileManager.createDirectory(at: dotfilesDir, withIntermediateDirectories: true) + let link = prowlDir.appending(path: "settings.json", directoryHint: .notDirectory) + try fileManager.createSymbolicLink( + atPath: link.path(percentEncoded: false), + withDestinationPath: "../dotfiles/settings.json" + ) + let payload = Data("new".utf8) + + try SymlinkPreservingFileWriter.write(payload, to: link) + + #expect(isSymlink(link)) + let target = dotfilesDir.appending(path: "settings.json", directoryHint: .notDirectory) + #expect(try Data(contentsOf: target) == payload) + } + + @Test func failsAndCreatesNoPhantomDirectoryForDanglingIntoMissingDir() throws { + let dir = try makeTempDir() + defer { try? fileManager.removeItem(at: dir) } + let prowlDir = dir.appending(path: ".prowl", directoryHint: .isDirectory) + try fileManager.createDirectory(at: prowlDir, withIntermediateDirectories: true) + let link = prowlDir.appending(path: "settings.json", directoryHint: .notDirectory) + try fileManager.createSymbolicLink( + atPath: link.path(percentEncoded: false), + withDestinationPath: "../missing/settings.json" + ) + + #expect(throws: (any Error).self) { + try SymlinkPreservingFileWriter.write(Data("new".utf8), to: link) + } + let missingDir = dir.appending(path: "missing", directoryHint: .isDirectory) + #expect(!fileManager.fileExists(atPath: missingDir.path(percentEncoded: false))) + #expect(isSymlink(link)) + } + + @Test func followsSymlinkChainToFinalTarget() throws { + let dir = try makeTempDir() + defer { try? fileManager.removeItem(at: dir) } + let real = dir.appending(path: "real.json", directoryHint: .notDirectory) + let mid = dir.appending(path: "mid.json", directoryHint: .notDirectory) + let link = dir.appending(path: "link.json", directoryHint: .notDirectory) + try Data("old".utf8).write(to: real) + try fileManager.createSymbolicLink(atPath: mid.path(percentEncoded: false), withDestinationPath: "real.json") + try fileManager.createSymbolicLink(atPath: link.path(percentEncoded: false), withDestinationPath: "mid.json") + let payload = Data("new".utf8) + + try SymlinkPreservingFileWriter.write(payload, to: link) + + #expect(isSymlink(link)) + #expect(isSymlink(mid)) + #expect(try Data(contentsOf: real) == payload) + } + + @Test func throwsOnSymlinkCycle() throws { + let dir = try makeTempDir() + defer { try? fileManager.removeItem(at: dir) } + let linkA = dir.appending(path: "a.json", directoryHint: .notDirectory) + let linkB = dir.appending(path: "b.json", directoryHint: .notDirectory) + try fileManager.createSymbolicLink(atPath: linkA.path(percentEncoded: false), withDestinationPath: "b.json") + try fileManager.createSymbolicLink(atPath: linkB.path(percentEncoded: false), withDestinationPath: "a.json") + + #expect(throws: SymlinkPreservingFileWriterError.self) { + try SymlinkPreservingFileWriter.write(Data("x".utf8), to: linkA) + } + #expect(isSymlink(linkA)) + #expect(isSymlink(linkB)) + } + + @Test func throwsOnOverlyDeepSymlinkChain() throws { + let dir = try makeTempDir() + defer { try? fileManager.removeItem(at: dir) } + // A 40-link chain (link0 -> link1 -> ... -> link40) exceeds the kernel's + // symlink-resolution limit, so the loader could never read it back. + for hop in 0..<40 { + let here = dir.appending(path: "link\(hop).json", directoryHint: .notDirectory) + try fileManager.createSymbolicLink( + atPath: here.path(percentEncoded: false), + withDestinationPath: "link\(hop + 1).json" + ) + } + let head = dir.appending(path: "link0.json", directoryHint: .notDirectory) + + #expect(throws: SymlinkPreservingFileWriterError.self) { + try SymlinkPreservingFileWriter.write(Data("x".utf8), to: head) + } + #expect(isSymlink(head)) + } + + @Test func writeOverwritesExistingRealFile() throws { + let dir = try makeTempDir() + defer { try? fileManager.removeItem(at: dir) } + let url = dir.appending(path: "settings.json", directoryHint: .notDirectory) + try Data("old".utf8).write(to: url) + let payload = Data("new".utf8) + + try SymlinkPreservingFileWriter.write(payload, to: url) + + #expect(try Data(contentsOf: url) == payload) + #expect(!isSymlink(url)) + } +}