From 87a4ee5c1c6886e5510580f8d8b5c7813991c219 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 9 Oct 2026 04:35:51 +0000 Subject: [PATCH 1/9] fix(settings): keep spaces in remote folder names in the folder chooser The profile editor's remote folder dropdown split `rclone lsd` output on whitespace and kept the last token, so "My Folder" was offered as "Folder" (colliding with a real folder of that name) and "My songs" as "songs". Present since the view was written, not a regression. List folders with `rclone lsf --dirs-only` (one name per line, no columns) through a shared RcloneConfigService.parseDirectoryListing used by the chooser, the setup wizard and the Sync Only These Folders browser. stderr is now read separately so rclone notices can't show up as folders. Covered by ConfigSelfTest AC-RF1. Claude-Session: https://claude.ai/code/session_01DXZWhCBFdN1o8jnSB8gd7t --- SyncTray/Services/ConfigSelfTest.swift | 30 ++++++++++++ SyncTray/Services/RcloneConfigService.swift | 47 +++++++++---------- .../Views/Settings/ProfileDetailView.swift | 31 ++++-------- 3 files changed, 61 insertions(+), 47 deletions(-) diff --git a/SyncTray/Services/ConfigSelfTest.swift b/SyncTray/Services/ConfigSelfTest.swift index 8c7cce94..b676b183 100644 --- a/SyncTray/Services/ConfigSelfTest.swift +++ b/SyncTray/Services/ConfigSelfTest.swift @@ -117,6 +117,7 @@ enum ConfigSelfTest { testRCAPIAuthenticated, testShimQuotesExecutablePath, testMountModeFileNotInSharedTmp, + testRemoteFolderListingKeepsSpaces, testCacheOnlyUnionConfig, testCacheOnlyUnionBehaviour, testMountModeSelection, @@ -3423,6 +3424,35 @@ enum ConfigSelfTest { return report(name, slug, true) } + // MARK: - AC-RF1 — remote folder names with spaces survive the folder chooser + + /// The profile editor's folder dropdown used to split `rclone lsd` output on whitespace + /// and keep the last token, so "My Folder" was offered as "Folder" — a different folder + /// when one by that name also exists. Asserts the shared `lsf --dirs-only` parser keeps + /// every name verbatim, and that the chooser and the wizard both list with it. + private static func testRemoteFolderListingKeepsSpaces() -> Bool { + let name = "AC-RF1", slug = "remote-folder-listing-keeps-spaces" + let output = "Folder/\nMy Folder/\nMy songs/\n trailing space /\r\n\n" + let parsed = RcloneConfigService.parseDirectoryListing(output) + let expected = [" trailing space ", "Folder", "My songs", "My Folder"] + guard parsed == expected else { + return report(name, slug, false, "(parsed \(parsed), expected \(expected))") + } + guard let detailSource = readSourceFile("Views/Settings/ProfileDetailView.swift"), + let configSource = readSourceFile("Services/RcloneConfigService.swift"), + let chooser = extractFunctionBody(startingAt: "private func loadRemoteFolders()", in: detailSource), + let wizard = extractFunctionBody(startingAt: "func listFolders(remote: String)", in: configSource) else { + return report(name, slug, false, "(could not read the folder-listing call sites)") + } + for (site, body) in [("loadRemoteFolders", chooser), ("listFolders", wizard)] { + guard body.contains("\"--dirs-only\""), body.contains("parseDirectoryListing("), + !body.contains("\"lsd\"") else { + return report(name, slug, false, "(\(site) does not list with lsf + parseDirectoryListing)") + } + } + return report(name, slug, true) + } + // MARK: - AC-SEC4 — the mount-mode file never lives in the shared /tmp /// The sync script writes the mode token with a plain `echo > "$MOUNT_MODE_PATH"`, which diff --git a/SyncTray/Services/RcloneConfigService.swift b/SyncTray/Services/RcloneConfigService.swift index a2c3f8e3..9b8614d8 100644 --- a/SyncTray/Services/RcloneConfigService.swift +++ b/SyncTray/Services/RcloneConfigService.swift @@ -527,6 +527,18 @@ final class RcloneConfigService { // MARK: - List Remote Contents /// List folders at the root of a remote + /// Folder names from `rclone lsf --dirs-only` output: one name per line, each ending in `/`. + /// Uses `lsf` rather than `lsd` because `lsd` prints names in a whitespace-padded column, + /// and splitting that on whitespace turned "My Folder" into "Folder". Names are kept + /// verbatim, so leading and trailing spaces survive. + static func parseDirectoryListing(_ output: String) -> [String] { + output.components(separatedBy: "\n") + .map { $0.hasSuffix("\r") ? String($0.dropLast()) : $0 } + .filter { $0.hasSuffix("/") && $0.count > 1 } + .map { String($0.dropLast()) } + .sorted() + } + func listFolders(remote: String) async -> Result<[String], ConfigError> { guard let rclonePath = findRclonePath() else { return .failure(.rcloneNotFound) @@ -536,49 +548,32 @@ final class RcloneConfigService { DispatchQueue.global(qos: .userInitiated).async { let process = Process() let pipe = Pipe() + let errPipe = Pipe() process.executableURL = URL(fileURLWithPath: rclonePath) let remotePath = remote.hasSuffix(":") ? remote : "\(remote):" let remoteName = remote.replacingOccurrences(of: ":", with: "") let skipCert = self.readRemoteConfig(name: remoteName)?.values["no_check_certificate"] == "true" - var args = ["lsd", remotePath] + var args = ["lsf", remotePath, "--dirs-only"] if skipCert { args.append("--no-check-certificate") } process.arguments = args process.standardOutput = pipe - process.standardError = pipe + process.standardError = errPipe do { try process.run() + // Read before waiting: a large listing would otherwise fill the pipe and block rclone. + let data = pipe.fileHandleForReading.readDataToEndOfFile() + let errData = errPipe.fileHandleForReading.readDataToEndOfFile() process.waitUntilExit() if process.terminationStatus == 0 { - let data = pipe.fileHandleForReading.readDataToEndOfFile() - if let output = String(data: data, encoding: .utf8) { - // Parse lsd output format: " -1 2000-01-01 01:00:00 -1 FolderName" - // Format: size date time count name (with variable whitespace) - let folders = output.components(separatedBy: "\n") - .compactMap { line -> String? in - let trimmed = line.trimmingCharacters(in: .whitespaces) - guard !trimmed.isEmpty else { return nil } - // Match: -1 YYYY-MM-DD HH:MM:SS -1 FolderName - // (size) (date) (time) (count) (name) - let pattern = #"^-?\d+\s+\d{4}-\d{2}-\d{2}\s+\d{2}:\d{2}:\d{2}\s+-?\d+\s+(.+)$"# - if let regex = try? NSRegularExpression(pattern: pattern), - let match = regex.firstMatch(in: trimmed, range: NSRange(trimmed.startIndex..., in: trimmed)), - let folderRange = Range(match.range(at: 1), in: trimmed) { - return String(trimmed[folderRange]) - } - return nil - } - continuation.resume(returning: .success(folders)) - } else { - continuation.resume(returning: .success([])) - } + let output = String(decoding: data, as: UTF8.self) + continuation.resume(returning: .success(Self.parseDirectoryListing(output))) } else { - let data = pipe.fileHandleForReading.readDataToEndOfFile() - let error = String(data: data, encoding: .utf8) ?? "Unknown error" + let error = String(data: errData, encoding: .utf8) ?? "Unknown error" continuation.resume(returning: .failure(.connectionFailed(error))) } } catch { diff --git a/SyncTray/Views/Settings/ProfileDetailView.swift b/SyncTray/Views/Settings/ProfileDetailView.swift index a5bea647..d68b4863 100644 --- a/SyncTray/Views/Settings/ProfileDetailView.swift +++ b/SyncTray/Views/Settings/ProfileDetailView.swift @@ -2625,6 +2625,7 @@ struct ProfileDetailView: View { DispatchQueue.global(qos: .userInitiated).async { let process = Process() let pipe = Pipe() + let errPipe = Pipe() let rclonePath = RcloneLocator.resolve() @@ -2639,21 +2640,23 @@ struct ProfileDetailView: View { process.executableURL = URL(fileURLWithPath: path) let remoteName = rcloneRemote.replacingOccurrences(of: ":", with: "") let skipCert = RcloneConfigService.shared.readRemoteConfig(name: remoteName)?.values["no_check_certificate"] == "true" - var args = ["lsd", "\(rcloneRemote):"] + var args = ["lsf", "\(rcloneRemote):", "--dirs-only"] if skipCert { args.append("--no-check-certificate") } process.arguments = args process.standardOutput = pipe - process.standardError = pipe + process.standardError = errPipe do { try process.run() + // Read before waiting: a large listing would otherwise fill the pipe and block rclone. + let data = pipe.fileHandleForReading.readDataToEndOfFile() + let errData = errPipe.fileHandleForReading.readDataToEndOfFile() process.waitUntilExit() guard process.terminationStatus == 0 else { - let data = pipe.fileHandleForReading.readDataToEndOfFile() - let errorOutput = String(data: data, encoding: .utf8) ?? "Unknown error" + let errorOutput = String(data: errData, encoding: .utf8) ?? "Unknown error" DispatchQueue.main.async { isLoadingFolders = false foldersError = "Failed to list folders: \(errorOutput)" @@ -2661,20 +2664,10 @@ struct ProfileDetailView: View { return } - let data = pipe.fileHandleForReading.readDataToEndOfFile() - let output = String(data: data, encoding: .utf8) ?? "" - - let folders = output - .components(separatedBy: .newlines) - .compactMap { line -> String? in - let trimmed = line.trimmingCharacters(in: .whitespaces) - guard !trimmed.isEmpty else { return nil } - let components = trimmed.components(separatedBy: .whitespaces).filter { !$0.isEmpty } - return components.last - } + let folders = RcloneConfigService.parseDirectoryListing(String(decoding: data, as: UTF8.self)) DispatchQueue.main.async { - availableFolders = folders.sorted() + availableFolders = folders isLoadingFolders = false if folders.isEmpty { foldersError = "No folders found on remote (or remote is empty)" @@ -4894,11 +4887,7 @@ struct RemoteFolderBrowserSheet: View { } return } - let dirs = out.components(separatedBy: .newlines) - .map { $0.trimmingCharacters(in: .whitespaces) } - .filter { !$0.isEmpty } - .map { $0.hasSuffix("/") ? String($0.dropLast()) : $0 } - .sorted() + let dirs = RcloneConfigService.parseDirectoryListing(out) DispatchQueue.main.async { self.folders = dirs; self.isLoading = false } } catch { DispatchQueue.main.async { self.isLoading = false; self.errorMessage = error.localizedDescription } From fb685989fb124071c3526c2aba157cbd06029118 Mon Sep 17 00:00:00 2001 From: mthines Date: Fri, 9 Oct 2026 08:02:30 +0200 Subject: [PATCH 2/9] feat(cli): add `synctray remote folders` to list a remote's folders as the app sees them Lists a remote's folders through the same rclone arguments and parser as the profile editor's folder chooser and the setup wizard, so a folder-name problem like the "My Folder" shown as "Folder" bug can be reproduced and reported from a terminal. Needs no profile; --json makes leading and trailing spaces visible. The listing arguments now live in RcloneConfigService.folderListingArguments, shared by the chooser, the wizard and the CLI. Covered by ConfigSelfTest AC-CLI12. --- .claude/rules/telemetry.md | 2 +- CLAUDE.md | 3 +- SyncTray/CLI/SyncTrayCLI.swift | 52 +++++++++++++++++++ SyncTray/Services/ConfigSelfTest.swift | 49 ++++++++++++++++- SyncTray/Services/RcloneConfigService.swift | 12 +++-- .../Views/Settings/ProfileDetailView.swift | 6 +-- 6 files changed, 111 insertions(+), 13 deletions(-) diff --git a/.claude/rules/telemetry.md b/.claude/rules/telemetry.md index a020c690..aa303822 100644 --- a/.claude/rules/telemetry.md +++ b/.claude/rules/telemetry.md @@ -230,7 +230,7 @@ rollout. No event is emitted on a fresh install. | `synctray.mount.reinstall_detach` | Counter | Pre-uninstall volume detach attempts for mount-mode profiles, reached on every `SyncSetupService.uninstall` (including the settings-save reinstall path) — `mount.result`: `not_mounted`/`success`/`success_forced`/`failure` | | `synctray.mount.reinstall_detach.duration` | Histogram | Time taken to gracefully detach a mounted volume before reinstall (seconds) — regression signal for the "settings change freezes an active Stream mount" failure mode; only recorded when the profile was actually mounted | | `synctray.mount.cache_only_fallback` | Counter | Cache Only was requested (`streamCacheOnly`) but the mount settled on streaming — the silent script fallback when the partial-file exclude list can't be built. The "I enabled Cache Only but it's still streaming/downloading" case; invisible without this. Emitted once at the mode-change edge by `SyncManager.reconcileMountStatesOffMain`, guarded by `resumingFromCacheOnly` so a legitimate drain-and-resume flip isn't counted | -| `synctray.cli.invoked` | Counter | Headless `synctray` CLI invocations (`cli.command`: bounded verb — doctor/status/offline-status/profiles/logs/test-remote/listremotes/sync/mount/unmount/reinstall/install/cache-move/profile-create/profile-show/profile-set/profile-enable/profile-disable/profile-delete/profile-list/help, or `(other)`; `cli.result`: ok/error). Never carries args, paths, profile names, or remotes. Recorded once per run by `runMeasured`, then flushed via `flushForExit` before the process exits | +| `synctray.cli.invoked` | Counter | Headless `synctray` CLI invocations (`cli.command`: bounded verb — doctor/status/offline-status/profiles/logs/test-remote/listremotes/remote-folders/sync/mount/unmount/reinstall/install/cache-move/profile-create/profile-show/profile-set/profile-enable/profile-disable/profile-delete/profile-list/help, or `(other)`; `cli.result`: ok/error). Never carries args, paths, profile names, or remotes. Recorded once per run by `runMeasured`, then flushed via `flushForExit` before the process exits | | `synctray.offline.warm.duration` | Histogram | Duration of an offline-file warming run, seconds (`warm.trigger`: manual/startup/finder_pin; `warm.outcome`: completed/cancelled) | | `synctray.offline.warm.throughput` | Histogram | Average read throughput of a warming run, MB/s — the primary signal for slow-fallback diagnosis (the SFTP fallback caps aggregate throughput) | | `synctray.offline.warm.files` | Counter | Files warmed into the VFS content cache (`warm.outcome`: completed/cancelled) | diff --git a/CLAUDE.md b/CLAUDE.md index 3383edbd..2b6f4c37 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -1127,6 +1127,7 @@ Covered by `ConfigSelfTest` AC-CLI4 and AC-SEC3. | `synctray logs [--follow]` | Print (or `tail -f`) that profile's sync log. | | `synctray test-remote ` | Probe one profile's remote with `rclone lsd` under a hard timeout; prints `reachable: ` or the real rclone stderr. | | `synctray listremotes` | `rclone listremotes`, passthrough. | +| `synctray remote folders [:path] [--json]` | List a remote's folders exactly as the profile editor's folder chooser and the setup wizard show them — same `rclone lsf --dirs-only` arguments (`RcloneConfigService.folderListingArguments`, including the remote's `no_check_certificate`) and the same parser (`parseDirectoryListing`), so names with spaces come out verbatim. Needs no profile. `--json` prints a JSON array, which makes leading/trailing spaces visible. Covered by `ConfigSelfTest` AC-CLI12 (and AC-RF1 for the parser). | **Configure** (mutating — headless-capable, no running app required): @@ -1641,7 +1642,7 @@ open ~/Library/Developer/Xcode/DerivedData/SyncTray-*/Build/Products/Debug/SyncT | `SettingsView.swift` | Main settings UI with profile editing | | `ProfileStore.swift` | File-backed profile persistence — authoritative `{shortId}.profile.json` per profile, write-only blob mirror (see "File-Backed Configuration") | | `ConfigFileWatcher.swift` | Live-apply watcher for `~/.config/synctray` (profiles + settings); routes an unknown-id `.profile.json` to create-via-file | -| `SyncTrayCLI.swift` | Headless `synctray` CLI: inspect (`doctor`/`status`/`profiles`/`profile show`/`logs`/`test-remote`/`listremotes`), configure (`profile create`/`set`/`enable`/`disable`/`delete`, `install`/`reinstall`), operate (`sync`/`mount`/`unmount`/`cache move`); dispatched from `SyncTrayApp.init` (see "Agent-Editable Configuration & CLI") | +| `SyncTrayCLI.swift` | Headless `synctray` CLI: inspect (`doctor`/`status`/`profiles`/`profile show`/`logs`/`test-remote`/`listremotes`/`remote folders`), configure (`profile create`/`set`/`enable`/`disable`/`delete`, `install`/`reinstall`), operate (`sync`/`mount`/`unmount`/`cache move`); dispatched from `SyncTrayApp.init` (see "Agent-Editable Configuration & CLI") | | `CLIShimInstaller.swift` | Installs the `~/.local/bin/synctray` shim (`~/.local/bin` must be on `PATH`) | | `SyncLogPatterns` | Centralized log message pattern matching (includes `isOutOfSyncError`) | | `TelemetryService.swift` | OTel singleton — traces, metrics, logs via OTLP/HTTP | diff --git a/SyncTray/CLI/SyncTrayCLI.swift b/SyncTray/CLI/SyncTrayCLI.swift index 53390a41..f0bbea1d 100644 --- a/SyncTray/CLI/SyncTrayCLI.swift +++ b/SyncTray/CLI/SyncTrayCLI.swift @@ -26,6 +26,7 @@ enum CLICommand: Equatable { case testRemote(String) case logs(target: String, follow: Bool) case listRemotes + case remoteFolders(target: String, json: Bool) case profiles case status(target: String?, json: Bool, wait: StatusWait?) case offlineStatus(target: String?, json: Bool) @@ -158,6 +159,9 @@ struct CLIEnvironment { /// network. Backs `offline status`; injected so the self-test drives it with /// fixtures. Defaults to `VFSCacheService.shared.estimateWarmWork`. var warmEstimate: (_ dir: String, _ profile: SyncProfile) -> VFSCacheService.WarmEstimate + /// Whether rclone.conf sets `no_check_certificate = true` for this remote, + /// so `remote folders` lists with the same flags the app's folder chooser uses. + var remoteSkipsCertCheck: (_ remoteName: String) -> Bool /// Pause between polls in `status --wait` / `mount`. Injected so the /// self-test's wait loops run instantly. var sleep: (TimeInterval) -> Void @@ -196,6 +200,8 @@ enum SyncTrayCLI { logs [--follow] Print or tail a profile's sync log test-remote Probe a profile's remote reachability listremotes List configured rclone remotes + remote folders [:path] [--json] + List the folders the app's folder chooser shows for a remote Configure: profile create --from Create a profile from a .profile.json file @@ -293,6 +299,10 @@ enum SyncTrayCLI { let sub = argv.dropFirst().first(where: { !$0.hasPrefix("-") }) ?? "" return sub == "move" ? "cache-move" : "(other)" } + if first == "remote" { + let sub = argv.dropFirst().first(where: { !$0.hasPrefix("-") }) ?? "" + return sub == "folders" ? "remote-folders" : "(other)" + } if first == "offline" { let sub = argv.dropFirst().first(where: { !$0.hasPrefix("-") }) ?? "" return sub == "status" ? "offline-status" : "(other)" @@ -328,6 +338,13 @@ enum SyncTrayCLI { case "listremotes": return .success(.listRemotes) + case "remote": + let flags = parseFlags(rest, valueFlags: []) + guard flags.positionals.first == "folders", flags.positionals.count == 2 else { + return .failure(CLIUsageError(message: "usage: synctray remote folders [:path] [--json]")) + } + return .success(.remoteFolders(target: flags.positionals[1], json: rest.contains("--json"))) + case "profiles": return .success(.profiles) @@ -542,6 +559,8 @@ enum SyncTrayCLI { return runLogs(target, follow: follow, env: env) case .listRemotes: return runListRemotes(env: env) + case .remoteFolders(let target, let json): + return runRemoteFolders(target, json: json, env: env) case .profiles: return runProfiles(env: env) case .status(let target, let json, let wait): @@ -729,6 +748,36 @@ enum SyncTrayCLI { } } + // MARK: - remote folders + + /// Lists a remote's folders through the same rclone arguments and parser as the + /// profile editor's folder chooser and the setup wizard, so what this prints is + /// what the app offers. Needs no profile. + private static func runRemoteFolders(_ target: String, json: Bool, env: CLIEnvironment) -> Int32 { + let remoteName = target.split(separator: ":", maxSplits: 1).first.map(String.init) ?? "" + guard !remoteName.isEmpty else { + env.stderr("usage: synctray remote folders [:path] [--json]\n") + return 64 + } + let fullTarget = target.contains(":") ? target : "\(target):" + let args = RcloneConfigService.folderListingArguments( + fullTarget, skipCertCheck: env.remoteSkipsCertCheck(remoteName) + ) + let (exit, out, err) = env.runRclone(args, 30) + guard exit == 0 else { + env.stderr(err.isEmpty ? "error: rclone exited \(exit)\n" : err) + return 1 + } + let folders = RcloneConfigService.parseDirectoryListing(out) + if json { + let data = (try? JSONSerialization.data(withJSONObject: folders, options: [.prettyPrinted])) ?? Data("[]".utf8) + env.stdout(String(decoding: data, as: UTF8.self) + "\n") + } else { + for folder in folders { env.stdout(folder + "\n") } + } + return 0 + } + // MARK: - profiles private static func runProfiles(env: CLIEnvironment) -> Int32 { @@ -1667,6 +1716,9 @@ extension CLIEnvironment { readFile: { try? String(contentsOfFile: $0, encoding: .utf8) }, probeMount: { profile in CLIEnvironment.probeMountProcess(profile) }, warmEstimate: { dir, profile in VFSCacheService.shared.estimateWarmWork(dir, for: profile) }, + remoteSkipsCertCheck: { name in + RcloneConfigService.shared.readRemoteConfig(name: name)?.values["no_check_certificate"] == "true" + }, sleep: { Thread.sleep(forTimeInterval: $0) }, stdout: { FileHandle.standardOutput.write(Data($0.utf8)) }, stderr: { FileHandle.standardError.write(Data($0.utf8)) }, diff --git a/SyncTray/Services/ConfigSelfTest.swift b/SyncTray/Services/ConfigSelfTest.swift index b676b183..31e076d5 100644 --- a/SyncTray/Services/ConfigSelfTest.swift +++ b/SyncTray/Services/ConfigSelfTest.swift @@ -118,6 +118,7 @@ enum ConfigSelfTest { testShimQuotesExecutablePath, testMountModeFileNotInSharedTmp, testRemoteFolderListingKeepsSpaces, + testCLIRemoteFolders, testCacheOnlyUnionConfig, testCacheOnlyUnionBehaviour, testMountModeSelection, @@ -1477,6 +1478,7 @@ enum ConfigSelfTest { warmEstimate: @escaping (String, SyncProfile) -> VFSCacheService.WarmEstimate = { _, _ in VFSCacheService.WarmEstimate(files: 0, bytes: 0, cachedFiles: 0, cachedBytes: 0) }, + remoteSkipsCertCheck: @escaping (String) -> Bool = { _ in false }, sleep: @escaping (TimeInterval) -> Void = { _ in }, stdout: @escaping (String) -> Void = { _ in }, stderr: @escaping (String) -> Void = { _ in }, @@ -1506,6 +1508,7 @@ enum ConfigSelfTest { readFile: readFile, probeMount: probeMount, warmEstimate: warmEstimate, + remoteSkipsCertCheck: remoteSkipsCertCheck, sleep: sleep, stdout: stdout, stderr: stderr, @@ -3445,7 +3448,7 @@ enum ConfigSelfTest { return report(name, slug, false, "(could not read the folder-listing call sites)") } for (site, body) in [("loadRemoteFolders", chooser), ("listFolders", wizard)] { - guard body.contains("\"--dirs-only\""), body.contains("parseDirectoryListing("), + guard body.contains("folderListingArguments("), body.contains("parseDirectoryListing("), !body.contains("\"lsd\"") else { return report(name, slug, false, "(\(site) does not list with lsf + parseDirectoryListing)") } @@ -3453,6 +3456,50 @@ enum ConfigSelfTest { return report(name, slug, true) } + // MARK: - AC-CLI12 — `remote folders` prints what the folder chooser shows + + /// `synctray remote folders` must list with the chooser's own rclone arguments + /// (cert flag included) and parser, keep spaces in names, work with no profile, + /// and surface rclone's error. + private static func testCLIRemoteFolders() -> Bool { + let name = "AC-CLI12", slug = "cli-remote-folders" + guard case .success(.remoteFolders("nas:Music", true)) = SyncTrayCLI.parse(["remote", "folders", "nas:Music", "--json"]), + case .failure = SyncTrayCLI.parse(["remote", "folders"]), + case .failure = SyncTrayCLI.parse(["remote", "list", "nas"]), + SyncTrayCLI.telemetryVerb(for: ["remote", "folders", "nas"]) == "remote-folders" else { + return report(name, slug, false, "(parse or telemetry verb wrong)") + } + + var seenArgs: [[String]] = [] + var out = "" + let env = fakeCLIEnvironment( + runRclone: { args, _ in seenArgs.append(args); return (0, "Folder/\nMy Folder/\n", "") }, + remoteSkipsCertCheck: { $0 == "nas" }, + stdout: { out += $0 } + ) + guard SyncTrayCLI.run(.remoteFolders(target: "nas", json: false), env: env) == 0, + seenArgs.last == ["lsf", "nas:", "--dirs-only", "--no-check-certificate"], + out == "Folder\nMy Folder\n" else { + return report(name, slug, false, "(text run: args \(seenArgs), out \(out.debugDescription))") + } + out = "" + guard SyncTrayCLI.run(.remoteFolders(target: "other:A/B", json: true), env: env) == 0, + seenArgs.last == ["lsf", "other:A/B", "--dirs-only"], + let data = out.data(using: .utf8), + (try? JSONSerialization.jsonObject(with: data)) as? [String] == ["Folder", "My Folder"] else { + return report(name, slug, false, "(json run: args \(seenArgs.last ?? []), out \(out.debugDescription))") + } + + var err = "" + let failing = fakeCLIEnvironment(runRclone: { _, _ in (1, "", "didn't find section in config file\n") }, + stderr: { err += $0 }) + guard SyncTrayCLI.run(.remoteFolders(target: "nope", json: false), env: failing) == 1, + err.contains("didn't find section") else { + return report(name, slug, false, "(rclone failure not surfaced: \(err.debugDescription))") + } + return report(name, slug, true) + } + // MARK: - AC-SEC4 — the mount-mode file never lives in the shared /tmp /// The sync script writes the mode token with a plain `echo > "$MOUNT_MODE_PATH"`, which diff --git a/SyncTray/Services/RcloneConfigService.swift b/SyncTray/Services/RcloneConfigService.swift index 9b8614d8..cba2b371 100644 --- a/SyncTray/Services/RcloneConfigService.swift +++ b/SyncTray/Services/RcloneConfigService.swift @@ -539,6 +539,12 @@ final class RcloneConfigService { .sorted() } + /// The `rclone` arguments every folder listing uses — the folder chooser, the + /// setup wizard and `synctray remote folders` — paired with `parseDirectoryListing`. + static func folderListingArguments(_ target: String, skipCertCheck: Bool) -> [String] { + ["lsf", target, "--dirs-only"] + (skipCertCheck ? ["--no-check-certificate"] : []) + } + func listFolders(remote: String) async -> Result<[String], ConfigError> { guard let rclonePath = findRclonePath() else { return .failure(.rcloneNotFound) @@ -554,11 +560,7 @@ final class RcloneConfigService { let remotePath = remote.hasSuffix(":") ? remote : "\(remote):" let remoteName = remote.replacingOccurrences(of: ":", with: "") let skipCert = self.readRemoteConfig(name: remoteName)?.values["no_check_certificate"] == "true" - var args = ["lsf", remotePath, "--dirs-only"] - if skipCert { - args.append("--no-check-certificate") - } - process.arguments = args + process.arguments = Self.folderListingArguments(remotePath, skipCertCheck: skipCert) process.standardOutput = pipe process.standardError = errPipe diff --git a/SyncTray/Views/Settings/ProfileDetailView.swift b/SyncTray/Views/Settings/ProfileDetailView.swift index d68b4863..fbbb402c 100644 --- a/SyncTray/Views/Settings/ProfileDetailView.swift +++ b/SyncTray/Views/Settings/ProfileDetailView.swift @@ -2640,11 +2640,7 @@ struct ProfileDetailView: View { process.executableURL = URL(fileURLWithPath: path) let remoteName = rcloneRemote.replacingOccurrences(of: ":", with: "") let skipCert = RcloneConfigService.shared.readRemoteConfig(name: remoteName)?.values["no_check_certificate"] == "true" - var args = ["lsf", "\(rcloneRemote):", "--dirs-only"] - if skipCert { - args.append("--no-check-certificate") - } - process.arguments = args + process.arguments = RcloneConfigService.folderListingArguments("\(rcloneRemote):", skipCertCheck: skipCert) process.standardOutput = pipe process.standardError = errPipe From 1120507ff8bae10a89c5f71a1713a020b75ca197 Mon Sep 17 00:00:00 2001 From: mthines Date: Fri, 9 Oct 2026 08:21:18 +0200 Subject: [PATCH 3/9] feat(cli): ask for usage-data consent once and share it with the app The CLI now honours the same telemetry setting as the app instead of silently following whatever the app last stored: - First run asks once ("Share usage data? [y/N]") with the banner's own copy, default No. Only on an interactive terminal outside CI, never for help or telemetry itself, so scripts and agents are never blocked. - `synctray telemetry [status|on|off] [--json]` reads/changes it, and writes settings.json (that key only) so a running app applies it live. - CLI telemetry carries synctray.process.kind=cli (app sends app); service.name stays synctray so existing queries keep working. - Exporter diagnostics are silent in CLI mode. Covered by ConfigSelfTest AC-CLI13. --- .claude/rules/telemetry.md | 6 +- CLAUDE.md | 25 +++- SyncTray/CLI/SyncTrayCLI.swift | 126 ++++++++++++++++++ SyncTray/Models/Settings.swift | 4 + SyncTray/Services/AppSettingsFileStore.swift | 35 +++++ SyncTray/Services/ConfigSelfTest.swift | 111 +++++++++++++++ SyncTray/Services/TelemetryService.swift | 10 ++ .../Views/Settings/TelemetryOptInBanner.swift | 2 +- 8 files changed, 312 insertions(+), 7 deletions(-) diff --git a/.claude/rules/telemetry.md b/.claude/rules/telemetry.md index aa303822..a0abe15e 100644 --- a/.claude/rules/telemetry.md +++ b/.claude/rules/telemetry.md @@ -138,6 +138,10 @@ For operations with real duration (like syncs), use the `activeSyncSpans` patter - `service.name` = synctray - `service.namespace` = synctray - `service.instance.id` = random UUID per installation (changes on reinstall) +- `synctray.process.kind` = `app` for the menu-bar app, `cli` for a headless `synctray` run + (`TelemetryService.processKind`, set by `SyncTrayCLI.dispatch`). Same `service.name` for + both — filter on this to separate CLI-originated events (a `cache move`, a `mount`) from + the app's - `enduser.id` = HMAC-SHA256 of hardware UUID (stable across reinstalls, not reversible) - `service.version` = deployment-precise version (see below) - `deployment.environment.name` = `development` for DEBUG builds, `production` for Release @@ -230,7 +234,7 @@ rollout. No event is emitted on a fresh install. | `synctray.mount.reinstall_detach` | Counter | Pre-uninstall volume detach attempts for mount-mode profiles, reached on every `SyncSetupService.uninstall` (including the settings-save reinstall path) — `mount.result`: `not_mounted`/`success`/`success_forced`/`failure` | | `synctray.mount.reinstall_detach.duration` | Histogram | Time taken to gracefully detach a mounted volume before reinstall (seconds) — regression signal for the "settings change freezes an active Stream mount" failure mode; only recorded when the profile was actually mounted | | `synctray.mount.cache_only_fallback` | Counter | Cache Only was requested (`streamCacheOnly`) but the mount settled on streaming — the silent script fallback when the partial-file exclude list can't be built. The "I enabled Cache Only but it's still streaming/downloading" case; invisible without this. Emitted once at the mode-change edge by `SyncManager.reconcileMountStatesOffMain`, guarded by `resumingFromCacheOnly` so a legitimate drain-and-resume flip isn't counted | -| `synctray.cli.invoked` | Counter | Headless `synctray` CLI invocations (`cli.command`: bounded verb — doctor/status/offline-status/profiles/logs/test-remote/listremotes/remote-folders/sync/mount/unmount/reinstall/install/cache-move/profile-create/profile-show/profile-set/profile-enable/profile-disable/profile-delete/profile-list/help, or `(other)`; `cli.result`: ok/error). Never carries args, paths, profile names, or remotes. Recorded once per run by `runMeasured`, then flushed via `flushForExit` before the process exits | +| `synctray.cli.invoked` | Counter | Headless `synctray` CLI invocations (`cli.command`: bounded verb — doctor/status/offline-status/profiles/logs/test-remote/listremotes/remote-folders/telemetry-status/telemetry-on/telemetry-off/sync/mount/unmount/reinstall/install/cache-move/profile-create/profile-show/profile-set/profile-enable/profile-disable/profile-delete/profile-list/help, or `(other)`; `cli.result`: ok/error). Never carries args, paths, profile names, or remotes. Recorded once per run by `runMeasured`, then flushed via `flushForExit` before the process exits | | `synctray.offline.warm.duration` | Histogram | Duration of an offline-file warming run, seconds (`warm.trigger`: manual/startup/finder_pin; `warm.outcome`: completed/cancelled) | | `synctray.offline.warm.throughput` | Histogram | Average read throughput of a warming run, MB/s — the primary signal for slow-fallback diagnosis (the SFTP fallback caps aggregate throughput) | | `synctray.offline.warm.files` | Counter | Files warmed into the VFS content cache (`warm.outcome`: completed/cancelled) | diff --git a/CLAUDE.md b/CLAUDE.md index 2b6f4c37..52c1324f 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -1128,6 +1128,7 @@ Covered by `ConfigSelfTest` AC-CLI4 and AC-SEC3. | `synctray test-remote ` | Probe one profile's remote with `rclone lsd` under a hard timeout; prints `reachable: ` or the real rclone stderr. | | `synctray listremotes` | `rclone listremotes`, passthrough. | | `synctray remote folders [:path] [--json]` | List a remote's folders exactly as the profile editor's folder chooser and the setup wizard show them — same `rclone lsf --dirs-only` arguments (`RcloneConfigService.folderListingArguments`, including the remote's `no_check_certificate`) and the same parser (`parseDirectoryListing`), so names with spaces come out verbatim. Needs no profile. `--json` prints a JSON array, which makes leading/trailing spaces visible. Covered by `ConfigSelfTest` AC-CLI12 (and AC-RF1 for the parser). | +| `synctray telemetry [status\|on\|off] [--json]` | Show or change whether usage data is shared — the same setting as the app's telemetry toggle (see "Privacy and consent" below). `status --json` prints `{"asked":…,"enabled":…}`. `on`/`off` count as answering the first-run question. | **Configure** (mutating — headless-capable, no running app required): @@ -1188,10 +1189,24 @@ process/filesystem/launchd touched. Every real `rclone` invocation runs through hard-timeout watchdog (mirrors `RcloneLocator`'s login-shell probe), since SMB/WebDAV remotes can hang past their own timeouts. -**Privacy.** The CLI's output goes to the invoking terminal, not telemetry — -`test-remote`/`profiles` printing a remote name to stdout is fine; CLI mode -never calls `TelemetryService.configure()`, so nothing from a CLI invocation -is ever sent anywhere. +**Privacy and consent — one setting for the app and the CLI.** The CLI's output +goes to the invoking terminal, not telemetry — `test-remote`/`profiles` printing a +remote name to stdout is fine. What the CLI *sends* is only the bounded +`synctray.cli.invoked` event above (plus the ordinary events of the operations it +drives, e.g. a `cache move`), and only when usage data is on. That is the SAME +`telemetryEnabled` setting the app's banner and Settings toggle use — the CLI is the +app's own binary, so it reads the same `UserDefaults`, and `synctray telemetry on|off` +also rewrites `telemetryEnabled` in `settings.json` (`AppSettingsFileStore.updateSettingsFile`, +which changes only that key and never `launchAtLogin`), so a running app applies it live. +A first run that has never been answered — in the app or the CLI — asks once +(`promptForTelemetryIfNeeded`, before the command runs, on stderr, with the banner's own +copy `SyncTraySettings.telemetryConsentSummary`), defaulting to **No** (`[y/N]`; an empty +line or EOF is No, Ctrl-C leaves it unanswered). It only asks when stdin AND stderr are a +terminal and `CI` is unset, never for `help` or `telemetry` itself, so scripts and agents +are never blocked. CLI telemetry carries the resource attribute `synctray.process.kind=cli` +(the app sends `app`); `service.name` stays `synctray` for both, so existing queries keep +working. Exporter diagnostics are silenced in CLI mode so they never clutter a terminal. +Covered by `ConfigSelfTest` AC-CLI13. ## Data Flow @@ -1642,7 +1657,7 @@ open ~/Library/Developer/Xcode/DerivedData/SyncTray-*/Build/Products/Debug/SyncT | `SettingsView.swift` | Main settings UI with profile editing | | `ProfileStore.swift` | File-backed profile persistence — authoritative `{shortId}.profile.json` per profile, write-only blob mirror (see "File-Backed Configuration") | | `ConfigFileWatcher.swift` | Live-apply watcher for `~/.config/synctray` (profiles + settings); routes an unknown-id `.profile.json` to create-via-file | -| `SyncTrayCLI.swift` | Headless `synctray` CLI: inspect (`doctor`/`status`/`profiles`/`profile show`/`logs`/`test-remote`/`listremotes`/`remote folders`), configure (`profile create`/`set`/`enable`/`disable`/`delete`, `install`/`reinstall`), operate (`sync`/`mount`/`unmount`/`cache move`); dispatched from `SyncTrayApp.init` (see "Agent-Editable Configuration & CLI") | +| `SyncTrayCLI.swift` | Headless `synctray` CLI: inspect (`doctor`/`status`/`profiles`/`profile show`/`logs`/`test-remote`/`listremotes`/`remote folders`/`telemetry`), configure (`profile create`/`set`/`enable`/`disable`/`delete`, `install`/`reinstall`), operate (`sync`/`mount`/`unmount`/`cache move`); dispatched from `SyncTrayApp.init` (see "Agent-Editable Configuration & CLI") | | `CLIShimInstaller.swift` | Installs the `~/.local/bin/synctray` shim (`~/.local/bin` must be on `PATH`) | | `SyncLogPatterns` | Centralized log message pattern matching (includes `isOutOfSyncError`) | | `TelemetryService.swift` | OTel singleton — traces, metrics, logs via OTLP/HTTP | diff --git a/SyncTray/CLI/SyncTrayCLI.swift b/SyncTray/CLI/SyncTrayCLI.swift index f0bbea1d..1c8ba1c9 100644 --- a/SyncTray/CLI/SyncTrayCLI.swift +++ b/SyncTray/CLI/SyncTrayCLI.swift @@ -1,4 +1,5 @@ import Foundation +import ServiceManagement /// Where `profile create` reads the profile JSON from. enum CreateSource: Equatable { @@ -20,6 +21,19 @@ struct StatusWait: Equatable { let timeout: TimeInterval } +/// The shared telemetry opt-in as the CLI sees it: `answered` is true once the user +/// has said yes or no in the app (banner, wizard) or the CLI, for the current +/// consent version. +struct TelemetryConsentState: Equatable { + let enabled: Bool + let answered: Bool +} + +enum TelemetryAction: Equatable { + case status(json: Bool) + case set(enabled: Bool) +} + /// Parsed CLI subcommand — the pure, testable result of `SyncTrayCLI.parse`. enum CLICommand: Equatable { case doctor @@ -27,6 +41,7 @@ enum CLICommand: Equatable { case logs(target: String, follow: Bool) case listRemotes case remoteFolders(target: String, json: Bool) + case telemetry(TelemetryAction) case profiles case status(target: String?, json: Bool, wait: StatusWait?) case offlineStatus(target: String?, json: Bool) @@ -162,6 +177,17 @@ struct CLIEnvironment { /// Whether rclone.conf sets `no_check_certificate = true` for this remote, /// so `remote folders` lists with the same flags the app's folder chooser uses. var remoteSkipsCertCheck: (_ remoteName: String) -> Bool + /// The shared telemetry opt-in (`SyncTraySettings`, the same store the app reads). + var telemetryConsent: () -> TelemetryConsentState + /// Record the user's telemetry answer in every place the app reads it — the + /// setting, the consent version (so the app's banner doesn't ask again) and + /// `settings.json` (so a running app applies it live). + var setTelemetryEnabled: (Bool) -> Void + /// Whether a person is at the keyboard: stdin and stderr are terminals and `CI` + /// is unset. The first-run telemetry question is only ever asked then. + var isInteractiveTerminal: () -> Bool + /// Read one line of the user's answer. `nil` on end of input. + var readLine: () -> String? /// Pause between polls in `status --wait` / `mount`. Injected so the /// self-test's wait loops run instantly. var sleep: (TimeInterval) -> Void @@ -202,6 +228,8 @@ enum SyncTrayCLI { listremotes List configured rclone remotes remote folders [:path] [--json] List the folders the app's folder chooser shows for a remote + telemetry [status|on|off] [--json] + Show or change usage-data sharing (shared with the app) Configure: profile create --from Create a profile from a .profile.json file @@ -259,6 +287,7 @@ enum SyncTrayCLI { return nil // other flags → GUI/self-test path } // `-`-prefixed `-h`/`--help`, or any bare token, IS a subcommand. + TelemetryService.processKind = "cli" return runMeasured(argv, env: .production()) } @@ -268,6 +297,7 @@ enum SyncTrayCLI { /// touches `TelemetryService`. Telemetry is gated on the user's opt-in, so /// this is a no-op — no setup, no network, no stdout — when disabled. private static func runMeasured(_ argv: [String], env: CLIEnvironment) -> Int32 { + promptForTelemetryIfNeeded(argv, env: env) let start = env.now() let code = execute(argv, env: env) let elapsed = env.now().timeIntervalSince(start) @@ -280,6 +310,29 @@ enum SyncTrayCLI { return code } + /// The CLI's first-run telemetry question — the same consent the app's banner + /// asks for, recorded in the same place. Asked at most once (until the consent + /// version is bumped), only when a person is at a terminal, never for `help` or + /// `telemetry` itself. Defaults to No; Ctrl-C leaves it unanswered. + static func promptForTelemetryIfNeeded(_ argv: [String], env: CLIEnvironment) { + guard let first = argv.first, !["help", "-h", "--help", "telemetry"].contains(first) else { return } + let state = env.telemetryConsent() + guard !state.enabled, !state.answered, env.isInteractiveTerminal() else { return } + + env.stderr(""" + Help improve SyncTray by sharing usage data? + \(SyncTraySettings.telemetryConsentSummary) + The SyncTray app uses the same setting. Change it any time: synctray telemetry on|off + + """ + "Share usage data? [y/N] ") + let answer = env.readLine()?.trimmingCharacters(in: .whitespaces).lowercased() + let enabled = answer == "y" || answer == "yes" + env.setTelemetryEnabled(enabled) + env.stderr(enabled + ? "Usage data is on. Thank you!\n\n" + : "\(answer == nil ? "\n" : "")Usage data stays off.\n\n") + } + /// Map an argv to a BOUNDED command verb for telemetry — never args, paths, /// profile names, or remotes. An unrecognised first token (or a bad /// `profile `) collapses to `(other)` so cardinality stays fixed. @@ -299,6 +352,10 @@ enum SyncTrayCLI { let sub = argv.dropFirst().first(where: { !$0.hasPrefix("-") }) ?? "" return sub == "move" ? "cache-move" : "(other)" } + if first == "telemetry" { + let sub = argv.dropFirst().first(where: { !$0.hasPrefix("-") }) ?? "status" + return ["status", "on", "off"].contains(sub) ? "telemetry-\(sub)" : "(other)" + } if first == "remote" { let sub = argv.dropFirst().first(where: { !$0.hasPrefix("-") }) ?? "" return sub == "folders" ? "remote-folders" : "(other)" @@ -338,6 +395,21 @@ enum SyncTrayCLI { case "listremotes": return .success(.listRemotes) + case "telemetry": + let flags = parseFlags(rest, valueFlags: []) + switch flags.positionals.first { + case nil, "status": + guard flags.positionals.count <= 1 else { break } + return .success(.telemetry(.status(json: rest.contains("--json")))) + case "on" where flags.positionals.count == 1: + return .success(.telemetry(.set(enabled: true))) + case "off" where flags.positionals.count == 1: + return .success(.telemetry(.set(enabled: false))) + default: + break + } + return .failure(CLIUsageError(message: "usage: synctray telemetry [status|on|off] [--json]")) + case "remote": let flags = parseFlags(rest, valueFlags: []) guard flags.positionals.first == "folders", flags.positionals.count == 2 else { @@ -561,6 +633,8 @@ enum SyncTrayCLI { return runListRemotes(env: env) case .remoteFolders(let target, let json): return runRemoteFolders(target, json: json, env: env) + case .telemetry(let action): + return runTelemetry(action, env: env) case .profiles: return runProfiles(env: env) case .status(let target, let json, let wait): @@ -778,6 +852,26 @@ enum SyncTrayCLI { return 0 } + // MARK: - telemetry + + private static func runTelemetry(_ action: TelemetryAction, env: CLIEnvironment) -> Int32 { + switch action { + case .status(let json): + let state = env.telemetryConsent() + if json { + env.stdout("{\"asked\":\(state.answered),\"enabled\":\(state.enabled)}\n") + } else if state.enabled { + env.stdout("Usage data: on. Turn it off with: synctray telemetry off\n") + } else { + env.stdout("Usage data: off\(state.answered ? "" : " (not asked yet)"). Turn it on with: synctray telemetry on\n") + } + case .set(let enabled): + env.setTelemetryEnabled(enabled) + env.stdout("Usage data: \(enabled ? "on" : "off"). The SyncTray app uses the same setting.\n") + } + return 0 + } + // MARK: - profiles private static func runProfiles(env: CLIEnvironment) -> Int32 { @@ -1719,6 +1813,18 @@ extension CLIEnvironment { remoteSkipsCertCheck: { name in RcloneConfigService.shared.readRemoteConfig(name: name)?.values["no_check_certificate"] == "true" }, + telemetryConsent: { + TelemetryConsentState( + enabled: SyncTraySettings.telemetryEnabled, + answered: SyncTraySettings.telemetryBannerDismissed + ) + }, + setTelemetryEnabled: { enabled in CLIEnvironment.setTelemetryEnabledProcess(enabled) }, + isInteractiveTerminal: { + isatty(STDIN_FILENO) == 1 && isatty(STDERR_FILENO) == 1 + && (ProcessInfo.processInfo.environment["CI"] ?? "").isEmpty + }, + readLine: { Swift.readLine() }, sleep: { Thread.sleep(forTimeInterval: $0) }, stdout: { FileHandle.standardOutput.write(Data($0.utf8)) }, stderr: { FileHandle.standardError.write(Data($0.utf8)) }, @@ -1726,6 +1832,26 @@ extension CLIEnvironment { ) } + /// Real implementation of `setTelemetryEnabled`: the same writes the app's + /// toggle and banner make, plus `settings.json` so a running app applies it live. + /// The opt-out event is sent before telemetry goes off, as the app does. + fileprivate static func setTelemetryEnabledProcess(_ enabled: Bool) { + if !enabled, SyncTraySettings.telemetryEnabled { + TelemetryService.shared.recordSettingChanged(name: "telemetry", enabled: false) + TelemetryService.shared.flushForExit() + } + let changed = SyncTraySettings.telemetryEnabled != enabled + SyncTraySettings.telemetryEnabled = enabled + SyncTraySettings.telemetryBannerDismissedVersion = SyncTraySettings.currentTelemetryConsentVersion + AppSettingsFileStore.updateSettingsFile( + .telemetryEnabled, to: enabled, + isLoginItemEnabled: SMAppService.mainApp.status == .enabled + ) + if enabled, changed { + TelemetryService.shared.recordSettingChanged(name: "telemetry", enabled: true) + } + } + /// Real implementation of `probeMount`. The process check matches `ps` output /// by substring rather than a `pgrep` regex, because a mount path can contain /// regex metacharacters (parentheses, dots) that would silently mis-match. diff --git a/SyncTray/Models/Settings.swift b/SyncTray/Models/Settings.swift index 2336677b..175beb6f 100644 --- a/SyncTray/Models/Settings.swift +++ b/SyncTray/Models/Settings.swift @@ -84,6 +84,10 @@ struct SyncTraySettings { return uuid } + /// The one-line description of what telemetry shares, shown by the app's opt-in + /// banner and the CLI's first-run question, so the two always say the same thing. + static let telemetryConsentSummary = "Usage data — sync results, error types, feature usage, profile names. No file names, remote names, or credentials." + /// The current consent banner version. Bump this to re-show the banner to all users, /// even those who previously dismissed it. Independent of the app version. static let currentTelemetryConsentVersion = 1 diff --git a/SyncTray/Services/AppSettingsFileStore.swift b/SyncTray/Services/AppSettingsFileStore.swift index 8a81eef0..ab34bf92 100644 --- a/SyncTray/Services/AppSettingsFileStore.swift +++ b/SyncTray/Services/AppSettingsFileStore.swift @@ -43,6 +43,41 @@ enum AppSettingsFileStore { ] } + /// Set one key in `settings.json`, keeping every other key as the file has it. + /// For a writer outside the app (the CLI): rewriting the whole file from its own + /// view could flip `launchAtLogin`, which the running app applies on any change. + /// A missing file is created from `currentSafeSettings`. + @discardableResult + static func updateSettingsFile( + _ key: SafeKey, + to value: Bool, + isLoginItemEnabled: Bool, + directory: String = AppSettingsFileStore.defaultDirectory + ) -> Bool { + let path = "\(directory)/settings.json" + var payload: [String: Any] + if let data = FileManager.default.contents(atPath: path), + let existing = (try? JSONSerialization.jsonObject(with: data)) as? [String: Any] { + payload = existing + } else { + payload = ["$schema": schemaRef] + for (k, v) in currentSafeSettings(isLoginItemEnabled: isLoginItemEnabled) { + payload[k.rawValue] = v + } + } + payload[key.rawValue] = value + guard let data = try? JSONSerialization.data(withJSONObject: payload, options: [.prettyPrinted, .sortedKeys]) else { + return false + } + do { + try FileManager.default.createDirectory(atPath: directory, withIntermediateDirectories: true) + try data.write(to: URL(fileURLWithPath: path), options: .atomic) + return true + } catch { + return false + } + } + /// Write `settings.json` (safe keys + `$schema`) into `directory` (defaults /// to the real config directory; overridable for self-test isolation). /// Notes the write's content hash in `ConfigSelfWriteRegistry` so diff --git a/SyncTray/Services/ConfigSelfTest.swift b/SyncTray/Services/ConfigSelfTest.swift index 31e076d5..86c2005c 100644 --- a/SyncTray/Services/ConfigSelfTest.swift +++ b/SyncTray/Services/ConfigSelfTest.swift @@ -119,6 +119,7 @@ enum ConfigSelfTest { testMountModeFileNotInSharedTmp, testRemoteFolderListingKeepsSpaces, testCLIRemoteFolders, + testCLITelemetryConsent, testCacheOnlyUnionConfig, testCacheOnlyUnionBehaviour, testMountModeSelection, @@ -1479,6 +1480,10 @@ enum ConfigSelfTest { VFSCacheService.WarmEstimate(files: 0, bytes: 0, cachedFiles: 0, cachedBytes: 0) }, remoteSkipsCertCheck: @escaping (String) -> Bool = { _ in false }, + telemetryConsent: @escaping () -> TelemetryConsentState = { TelemetryConsentState(enabled: false, answered: true) }, + setTelemetryEnabled: @escaping (Bool) -> Void = { _ in }, + isInteractiveTerminal: @escaping () -> Bool = { false }, + readLine: @escaping () -> String? = { nil }, sleep: @escaping (TimeInterval) -> Void = { _ in }, stdout: @escaping (String) -> Void = { _ in }, stderr: @escaping (String) -> Void = { _ in }, @@ -1509,6 +1514,10 @@ enum ConfigSelfTest { probeMount: probeMount, warmEstimate: warmEstimate, remoteSkipsCertCheck: remoteSkipsCertCheck, + telemetryConsent: telemetryConsent, + setTelemetryEnabled: setTelemetryEnabled, + isInteractiveTerminal: isInteractiveTerminal, + readLine: readLine, sleep: sleep, stdout: stdout, stderr: stderr, @@ -3500,6 +3509,108 @@ enum ConfigSelfTest { return report(name, slug, true) } + // MARK: - AC-CLI13 — the CLI asks for the app's telemetry consent once, shares it + + /// The CLI's first-run question must only appear for a person at a terminal who + /// hasn't answered in the app or the CLI, default to No, and record the answer + /// through the shared setter; `telemetry on|off|status` must use the same state; + /// a CLI write to settings.json must not touch the other keys (a running app + /// would apply a flipped `launchAtLogin`); and CLI telemetry must carry + /// `synctray.process.kind`. + private static func testCLITelemetryConsent() -> Bool { + let name = "AC-CLI13", slug = "cli-telemetry-consent" + let unanswered = TelemetryConsentState(enabled: false, answered: false) + + func prompt(_ argv: [String], state: TelemetryConsentState, interactive: Bool, answer: String?) -> (asked: Bool, set: Bool?, err: String) { + var asked = false, set: Bool? = nil, err = "" + let env = fakeCLIEnvironment( + telemetryConsent: { state }, + setTelemetryEnabled: { set = $0 }, + isInteractiveTerminal: { interactive }, + readLine: { asked = true; return answer }, + stderr: { err += $0 } + ) + SyncTrayCLI.promptForTelemetryIfNeeded(argv, env: env) + return (asked, set, err) + } + + let yes = prompt(["status"], state: unanswered, interactive: true, answer: " Y ") + guard yes.asked, yes.set == true, yes.err.contains(SyncTraySettings.telemetryConsentSummary), yes.err.contains("[y/N]") else { + return report(name, slug, false, "(yes answer not recorded or copy differs: \(yes))") + } + for answer in ["", "n", "maybe", nil] as [String?] { + let no = prompt(["status"], state: unanswered, interactive: true, answer: answer) + guard no.asked, no.set == false else { + return report(name, slug, false, "(answer \(answer.debugDescription) did not default to No: \(no))") + } + } + let silent: [([String], TelemetryConsentState, Bool)] = [ + (["status"], unanswered, false), // piped / agent / CI + (["status"], TelemetryConsentState(enabled: false, answered: true), true), // said no in the app + (["status"], TelemetryConsentState(enabled: true, answered: true), true), // already on + (["help"], unanswered, true), + (["telemetry", "on"], unanswered, true), + ] + for (argv, state, interactive) in silent { + let r = prompt(argv, state: state, interactive: interactive, answer: "y") + guard !r.asked, r.set == nil, r.err.isEmpty else { + return report(name, slug, false, "(prompted when it must not: \(argv) \(state) interactive=\(interactive))") + } + } + + guard case .success(.telemetry(.status(json: false))) = SyncTrayCLI.parse(["telemetry"]), + case .success(.telemetry(.status(json: true))) = SyncTrayCLI.parse(["telemetry", "status", "--json"]), + case .success(.telemetry(.set(enabled: true))) = SyncTrayCLI.parse(["telemetry", "on"]), + case .success(.telemetry(.set(enabled: false))) = SyncTrayCLI.parse(["telemetry", "off"]), + case .failure = SyncTrayCLI.parse(["telemetry", "maybe"]), + SyncTrayCLI.telemetryVerb(for: ["telemetry"]) == "telemetry-status", + SyncTrayCLI.telemetryVerb(for: ["telemetry", "off"]) == "telemetry-off" else { + return report(name, slug, false, "(telemetry parse or verb wrong)") + } + var set: Bool? = nil, out = "" + let env = fakeCLIEnvironment( + telemetryConsent: { TelemetryConsentState(enabled: true, answered: true) }, + setTelemetryEnabled: { set = $0 }, + stdout: { out += $0 } + ) + guard SyncTrayCLI.run(.telemetry(.set(enabled: false)), env: env) == 0, set == false else { + return report(name, slug, false, "(telemetry off did not use the shared setter)") + } + out = "" + guard SyncTrayCLI.run(.telemetry(.status(json: true)), env: env) == 0, + let data = out.data(using: .utf8), + let json = (try? JSONSerialization.jsonObject(with: data)) as? [String: Bool], + json == ["asked": true, "enabled": true] else { + return report(name, slug, false, "(status --json wrong: \(out.debugDescription))") + } + + let dir = "\(selfTestRoot)/cli-telemetry-settings" + try? FileManager.default.createDirectory(atPath: dir, withIntermediateDirectories: true) + let seed = #"{"autoFixSyncIssues":false,"debugLoggingEnabled":true,"launchAtLogin":true,"telemetryEnabled":false}"# + FileManager.default.createFile(atPath: "\(dir)/settings.json", contents: Data(seed.utf8)) + guard AppSettingsFileStore.updateSettingsFile(.telemetryEnabled, to: true, isLoginItemEnabled: false, directory: dir), + let written = FileManager.default.contents(atPath: "\(dir)/settings.json"), + let settings = (try? JSONSerialization.jsonObject(with: written)) as? [String: Bool], + settings == ["autoFixSyncIssues": false, "debugLoggingEnabled": true, "launchAtLogin": true, "telemetryEnabled": true] else { + return report(name, slug, false, "(settings.json update changed other keys)") + } + + guard let cliSource = readSourceFile("CLI/SyncTrayCLI.swift"), + let measured = extractFunctionBody(startingAt: "private static func runMeasured(", in: cliSource), + let promptAt = measured.range(of: "promptForTelemetryIfNeeded("), + let executeAt = measured.range(of: "execute(argv"), + promptAt.lowerBound < executeAt.lowerBound, + let dispatch = extractFunctionBody(startingAt: "static func dispatch(arguments:", in: cliSource), + dispatch.contains("TelemetryService.processKind = \"cli\""), + let telemetrySource = readSourceFile("Services/TelemetryService.swift"), + telemetrySource.contains("\"synctray.process.kind\": .string(Self.processKind)"), + let banner = readSourceFile("Views/Settings/TelemetryOptInBanner.swift"), + banner.contains("SyncTraySettings.telemetryConsentSummary") else { + return report(name, slug, false, "(prompt not before execute, process kind unset, or banner copy not shared)") + } + return report(name, slug, true) + } + // MARK: - AC-SEC4 — the mount-mode file never lives in the shared /tmp /// The sync script writes the mode token with a plain `echo > "$MOUNT_MODE_PATH"`, which diff --git a/SyncTray/Services/TelemetryService.swift b/SyncTray/Services/TelemetryService.swift index 13bab552..f7421d83 100644 --- a/SyncTray/Services/TelemetryService.swift +++ b/SyncTray/Services/TelemetryService.swift @@ -35,6 +35,12 @@ import OpenTelemetryProtocolExporterHttp final class TelemetryService { static let shared = TelemetryService() + /// Which process is reporting — `app` or `cli` — emitted as the + /// `synctray.process.kind` resource attribute. Both share `service.name` (one + /// binary, one release); this keeps a CLI `cache move` or `mount` apart from the + /// app's own. Set by `SyncTrayCLI.dispatch` before any telemetry is set up. + static var processKind = "app" + // MARK: - Configuration // // Lookup priority (first non-empty wins): @@ -2128,6 +2134,7 @@ final class TelemetryService { ResourceAttributes.serviceVersion.rawValue: .string(version), "service.instance.id": .string(SyncTraySettings.installationId), "enduser.id": .string(SyncTraySettings.anonymousUserId), + "synctray.process.kind": .string(Self.processKind), ResourceAttributes.osType.rawValue: .string("darwin"), ResourceAttributes.osVersion.rawValue: .string(osVersion), ] @@ -2788,7 +2795,10 @@ final class TelemetryService { /// Setup diagnostics go to stderr, never stdout: the headless CLI configures /// telemetry in-process, and a stray stdout line breaks every agent that /// parses `status --json` / `profile show`. + /// Silent in the CLI: its stderr is the user's terminal, where exporter setup noise + /// on every command would bury the real output. private static func diagnostic(_ message: String) { + guard processKind == "app" else { return } FileHandle.standardError.write(Data("[SyncTray][Telemetry] \(message)\n".utf8)) } diff --git a/SyncTray/Views/Settings/TelemetryOptInBanner.swift b/SyncTray/Views/Settings/TelemetryOptInBanner.swift index ea822ebe..8bd14ee3 100644 --- a/SyncTray/Views/Settings/TelemetryOptInBanner.swift +++ b/SyncTray/Views/Settings/TelemetryOptInBanner.swift @@ -24,7 +24,7 @@ struct TelemetryOptInBanner: View { VStack(alignment: .leading, spacing: 2) { Text("Help shape SyncTray") .font(.subheadline.weight(.medium)) - Text("Usage data — sync results, error types, feature usage, profile names. No file names, remote names, or credentials.") + Text(SyncTraySettings.telemetryConsentSummary) .font(.caption) .foregroundStyle(.secondary) .fixedSize(horizontal: false, vertical: true) From 612b0501466ae1fa2ac60e6c50b78478915bce3e Mon Sep 17 00:00:00 2001 From: mthines Date: Fri, 9 Oct 2026 08:53:55 +0200 Subject: [PATCH 4/9] fix(cli): only ask for usage-data consent when the command parses Addresses pr-reviewer comment https://github.com/mthines/sync-tray/pull/102#discussion_r4227457860 --- SyncTray/CLI/SyncTrayCLI.swift | 2 ++ SyncTray/Services/ConfigSelfTest.swift | 1 + 2 files changed, 3 insertions(+) diff --git a/SyncTray/CLI/SyncTrayCLI.swift b/SyncTray/CLI/SyncTrayCLI.swift index 1c8ba1c9..cd962744 100644 --- a/SyncTray/CLI/SyncTrayCLI.swift +++ b/SyncTray/CLI/SyncTrayCLI.swift @@ -316,6 +316,8 @@ enum SyncTrayCLI { /// `telemetry` itself. Defaults to No; Ctrl-C leaves it unanswered. static func promptForTelemetryIfNeeded(_ argv: [String], env: CLIEnvironment) { guard let first = argv.first, !["help", "-h", "--help", "telemetry"].contains(first) else { return } + // A command that doesn't parse only prints a usage error — no point asking. + guard case .success = parse(argv) else { return } let state = env.telemetryConsent() guard !state.enabled, !state.answered, env.isInteractiveTerminal() else { return } diff --git a/SyncTray/Services/ConfigSelfTest.swift b/SyncTray/Services/ConfigSelfTest.swift index 86c2005c..9ca5e328 100644 --- a/SyncTray/Services/ConfigSelfTest.swift +++ b/SyncTray/Services/ConfigSelfTest.swift @@ -3550,6 +3550,7 @@ enum ConfigSelfTest { (["status"], TelemetryConsentState(enabled: true, answered: true), true), // already on (["help"], unanswered, true), (["telemetry", "on"], unanswered, true), + (["stauts"], unanswered, true), // typo: usage error only ] for (argv, state, interactive) in silent { let r = prompt(argv, state: state, interactive: interactive, answer: "y") From 5ef09b652abf0fe965194106534c954b779e8bd5 Mon Sep 17 00:00:00 2001 From: mthines Date: Fri, 9 Oct 2026 08:54:05 +0200 Subject: [PATCH 5/9] fix(settings): drain rclone pipes before waiting in the remote folder browser Addresses pr-reviewer report (Less certain) --- SyncTray/Views/Settings/ProfileDetailView.swift | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/SyncTray/Views/Settings/ProfileDetailView.swift b/SyncTray/Views/Settings/ProfileDetailView.swift index fbbb402c..f2df748d 100644 --- a/SyncTray/Views/Settings/ProfileDetailView.swift +++ b/SyncTray/Views/Settings/ProfileDetailView.swift @@ -4870,10 +4870,14 @@ struct RemoteFolderBrowserSheet: View { proc.standardOutput = pipe; proc.standardError = errPipe do { try proc.run() + // Drain both pipes before waiting: a listing larger than the pipe + // buffer would otherwise block rclone and never exit. + let outData = pipe.fileHandleForReading.readDataToEndOfFile() + let errData = errPipe.fileHandleForReading.readDataToEndOfFile() proc.waitUntilExit() - let out = String(decoding: pipe.fileHandleForReading.readDataToEndOfFile(), as: UTF8.self) + let out = String(decoding: outData, as: UTF8.self) if proc.terminationStatus != 0 { - let e = String(decoding: errPipe.fileHandleForReading.readDataToEndOfFile(), as: UTF8.self) + let e = String(decoding: errData, as: UTF8.self) .trimmingCharacters(in: .whitespacesAndNewlines) DispatchQueue.main.async { self.isLoading = false From 0bf17825ab52cc8e1e1d260a643d82f23c6169c1 Mon Sep 17 00:00:00 2001 From: mthines Date: Fri, 9 Oct 2026 08:54:12 +0200 Subject: [PATCH 6/9] docs(claude): note the one-time consent question in the CLI telemetry paragraph Addresses pr-reviewer report (Less certain) --- CLAUDE.md | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/CLAUDE.md b/CLAUDE.md index 52c1324f..9f5379ba 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -1177,7 +1177,8 @@ counter + `CLI invoked` structured log (bounded command verb + `ok`/`error` + exit code + duration — never args, paths, profile names, or remotes) and flushes before exit (`runMeasured` → `TelemetryService.recordCLIInvocation` + `flushForExit`). It is gated on the user's telemetry opt-in, so a disabled CLI -does no setup, no network, and prints nothing extra. The self-test path +does no setup and no network; its only extra output is the one-time consent +question on an interactive terminal (see "Privacy and consent" below). The self-test path (`execute` with a fake `CLIEnvironment`) never touches telemetry. **Pure core / impure shell.** `SyncTrayCLI.parse`/`execute`/`run`/`doctorChecks` From e07777eaae44059c0235f2e5ccb83b3bcaaa1e76 Mon Sep 17 00:00:00 2001 From: mthines Date: Fri, 9 Oct 2026 09:04:54 +0200 Subject: [PATCH 7/9] fix(settings): drain rclone's stdout and stderr concurrently when listing folders Reading stdout to EOF before stderr deadlocks once rclone fills the unread stderr pipe's ~64 KB buffer. Add RcloneConfigService.drainPipes, which reads stderr on a background queue while reading stdout, and use it at all three folder-listing sites. AC-RF1 now runs a child writing 300 KB to both pipes under a 10 s timeout and pins drainPipes at each site. Addresses https://github.com/mthines/sync-tray/pull/102#discussion_r4227528802 --- SyncTray/Services/ConfigSelfTest.swift | 43 +++++++++++++++++++ SyncTray/Services/RcloneConfigService.swift | 19 ++++++-- .../Views/Settings/ProfileDetailView.swift | 9 +--- 3 files changed, 61 insertions(+), 10 deletions(-) diff --git a/SyncTray/Services/ConfigSelfTest.swift b/SyncTray/Services/ConfigSelfTest.swift index 9ca5e328..c5d3921c 100644 --- a/SyncTray/Services/ConfigSelfTest.swift +++ b/SyncTray/Services/ConfigSelfTest.swift @@ -3462,9 +3462,52 @@ enum ConfigSelfTest { return report(name, slug, false, "(\(site) does not list with lsf + parseDirectoryListing)") } } + guard let browser = extractFunctionBody(startingAt: "private func loadFolders()", in: detailSource) else { + return report(name, slug, false, "(could not read RemoteFolderBrowserSheet.loadFolders)") + } + for (site, body) in [("loadRemoteFolders", chooser), ("listFolders", wizard), ("RemoteFolderBrowserSheet.loadFolders", browser)] { + guard body.contains("drainPipes(") else { + return report(name, slug, false, "(\(site) does not drain rclone's pipes concurrently)") + } + } + if let failure = drainPipesFailure() { + return report(name, slug, false, "(\(failure))") + } return report(name, slug, true) } + /// Runs a child that writes 300 KB to stderr and then 300 KB to stdout — far past + /// the ~64 KB pipe buffer — and checks `drainPipes` returns both intact. A drain + /// that reads one pipe to EOF first never returns; the 10 s wait turns that hang + /// into a failure. Returns nil on success. + private static func drainPipesFailure() -> String? { + let size = 300_000 + let proc = Process() + proc.executableURL = URL(fileURLWithPath: "/bin/sh") + proc.arguments = ["-c", "head -c \(size) /dev/zero | tr '\\0' e >&2; head -c \(size) /dev/zero | tr '\\0' o"] + let outPipe = Pipe(), errPipe = Pipe() + proc.standardOutput = outPipe + proc.standardError = errPipe + do { try proc.run() } catch { return "could not start /bin/sh: \(error)" } + + var drained: (out: Data, err: Data)? + let done = DispatchSemaphore(value: 0) + DispatchQueue.global(qos: .userInitiated).async { + drained = RcloneConfigService.drainPipes(stdout: outPipe, stderr: errPipe) + done.signal() + } + guard done.wait(timeout: .now() + 10) == .success, let result = drained else { + proc.terminate() + return "drainPipes hung on a child writing >64 KB to both pipes" + } + proc.waitUntilExit() + guard result.out == Data(repeating: UInt8(ascii: "o"), count: size), + result.err == Data(repeating: UInt8(ascii: "e"), count: size) else { + return "drainPipes lost data (stdout \(result.out.count) B, stderr \(result.err.count) B)" + } + return nil + } + // MARK: - AC-CLI12 — `remote folders` prints what the folder chooser shows /// `synctray remote folders` must list with the chooser's own rclone arguments diff --git a/SyncTray/Services/RcloneConfigService.swift b/SyncTray/Services/RcloneConfigService.swift index cba2b371..8657eff9 100644 --- a/SyncTray/Services/RcloneConfigService.swift +++ b/SyncTray/Services/RcloneConfigService.swift @@ -545,6 +545,21 @@ final class RcloneConfigService { ["lsf", target, "--dirs-only"] + (skipCertCheck ? ["--no-check-certificate"] : []) } + /// Reads a child's stdout and stderr to EOF at the same time. Reading one pipe + /// to EOF before the other deadlocks once the child fills the unread pipe's + /// ~64 KB buffer: it blocks writing, so the pipe being read never closes. + /// Call before `waitUntilExit()`. + static func drainPipes(stdout: Pipe, stderr: Pipe) -> (out: Data, err: Data) { + var err = Data() + let errGroup = DispatchGroup() + DispatchQueue.global(qos: .utility).async(group: errGroup) { + err = stderr.fileHandleForReading.readDataToEndOfFile() + } + let out = stdout.fileHandleForReading.readDataToEndOfFile() + errGroup.wait() + return (out, err) + } + func listFolders(remote: String) async -> Result<[String], ConfigError> { guard let rclonePath = findRclonePath() else { return .failure(.rcloneNotFound) @@ -566,9 +581,7 @@ final class RcloneConfigService { do { try process.run() - // Read before waiting: a large listing would otherwise fill the pipe and block rclone. - let data = pipe.fileHandleForReading.readDataToEndOfFile() - let errData = errPipe.fileHandleForReading.readDataToEndOfFile() + let (data, errData) = Self.drainPipes(stdout: pipe, stderr: errPipe) process.waitUntilExit() if process.terminationStatus == 0 { diff --git a/SyncTray/Views/Settings/ProfileDetailView.swift b/SyncTray/Views/Settings/ProfileDetailView.swift index f2df748d..70470099 100644 --- a/SyncTray/Views/Settings/ProfileDetailView.swift +++ b/SyncTray/Views/Settings/ProfileDetailView.swift @@ -2646,9 +2646,7 @@ struct ProfileDetailView: View { do { try process.run() - // Read before waiting: a large listing would otherwise fill the pipe and block rclone. - let data = pipe.fileHandleForReading.readDataToEndOfFile() - let errData = errPipe.fileHandleForReading.readDataToEndOfFile() + let (data, errData) = RcloneConfigService.drainPipes(stdout: pipe, stderr: errPipe) process.waitUntilExit() guard process.terminationStatus == 0 else { @@ -4870,10 +4868,7 @@ struct RemoteFolderBrowserSheet: View { proc.standardOutput = pipe; proc.standardError = errPipe do { try proc.run() - // Drain both pipes before waiting: a listing larger than the pipe - // buffer would otherwise block rclone and never exit. - let outData = pipe.fileHandleForReading.readDataToEndOfFile() - let errData = errPipe.fileHandleForReading.readDataToEndOfFile() + let (outData, errData) = RcloneConfigService.drainPipes(stdout: pipe, stderr: errPipe) proc.waitUntilExit() let out = String(decoding: outData, as: UTF8.self) if proc.terminationStatus != 0 { From 7ee2ce42f4be3f8fdf9f536d14b5f32ed96bceda Mon Sep 17 00:00:00 2001 From: mthines Date: Fri, 9 Oct 2026 09:05:00 +0200 Subject: [PATCH 8/9] docs(rclone): put listFolders' doc comment back on listFolders Addresses https://github.com/mthines/sync-tray/pull/102#discussion_r4227528809 --- SyncTray/Services/RcloneConfigService.swift | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/SyncTray/Services/RcloneConfigService.swift b/SyncTray/Services/RcloneConfigService.swift index 8657eff9..be079687 100644 --- a/SyncTray/Services/RcloneConfigService.swift +++ b/SyncTray/Services/RcloneConfigService.swift @@ -526,7 +526,6 @@ final class RcloneConfigService { // MARK: - List Remote Contents - /// List folders at the root of a remote /// Folder names from `rclone lsf --dirs-only` output: one name per line, each ending in `/`. /// Uses `lsf` rather than `lsd` because `lsd` prints names in a whitespace-padded column, /// and splitting that on whitespace turned "My Folder" into "Folder". Names are kept @@ -560,6 +559,7 @@ final class RcloneConfigService { return (out, err) } + /// List folders at the root of a remote func listFolders(remote: String) async -> Result<[String], ConfigError> { guard let rclonePath = findRclonePath() else { return .failure(.rcloneNotFound) From a1831f809de23c7e2a8d9520281ceda922ea493d Mon Sep 17 00:00:00 2001 From: mthines Date: Fri, 9 Oct 2026 09:07:37 +0200 Subject: [PATCH 9/9] chore: simplify pass (reuse drainPipes in the CLI's rclone runner) --- SyncTray/CLI/SyncTrayCLI.swift | 16 ++++------------ 1 file changed, 4 insertions(+), 12 deletions(-) diff --git a/SyncTray/CLI/SyncTrayCLI.swift b/SyncTray/CLI/SyncTrayCLI.swift index cd962744..42e88611 100644 --- a/SyncTray/CLI/SyncTrayCLI.swift +++ b/SyncTray/CLI/SyncTrayCLI.swift @@ -2014,18 +2014,10 @@ extension CLIEnvironment { let watchdog = DispatchWorkItem { if proc.isRunning { proc.terminate() } } DispatchQueue.global(qos: .utility).asyncAfter(deadline: .now() + timeout, execute: watchdog) - // Drain stdout and stderr CONCURRENTLY. Reading one to EOF before the - // other deadlocks when the child fills the still-unread pipe's ~64 KB - // buffer — exactly what an unreachable remote does to stderr, the very - // buffer test-remote/doctor need. (RcloneLocator sidesteps this by - // nulling stderr; here we need it, so we drain both at once.) - var errData = Data() - let errGroup = DispatchGroup() - DispatchQueue.global(qos: .utility).async(group: errGroup) { - errData = stderrPipe.fileHandleForReading.readDataToEndOfFile() - } - let outData = stdoutPipe.fileHandleForReading.readDataToEndOfFile() - errGroup.wait() + // An unreachable remote fills stderr — the very output test-remote/doctor + // need — so both pipes are drained at once. (RcloneLocator sidesteps this + // by nulling stderr.) + let (outData, errData) = RcloneConfigService.drainPipes(stdout: stdoutPipe, stderr: stderrPipe) proc.waitUntilExit() watchdog.cancel()