Reuse the shared SFTP download planner

This commit is contained in:
Kaede Akino committed 2026-09-16 12:21:27 +08:00
1 parent 2d6b3c0e14
commit 995ec975a0
4 files changed
+138 -182

No files matched your search

+4
View File
@@ -152,6 +152,10 @@ recorded only after a fresh successful listing and files only after download.
Failed, cancelled, stale, missing, or type-changed operations do not update it.
Transfers own dedicated connections and outlive browser closure; host deletion and
background expiration cancel them, and nothing is silently replayed.
Desktop and mobile upload/download preparation and enqueueing use the Remote
Infrastructure planners. Download batches retain their destination independently of
file jobs, so directory-only selections still create their empty directory trees.
App sessions retain pending-batch presentation, cancellation, and completion handling.
## Callback isolation
+11
View File
@@ -94,3 +94,14 @@ This English-only document is the durable record of completed redundancy removal
- SwiftData historical schemas remain frozen migration contracts, small structurally similar repositories and views remain separate when abstraction would not reduce code, and `RuntimeVersions` stays platform-local to avoid introducing an invalid Ghostty dependency.
The 2026-09-02 completion and re-verification scans found no further independent dead-code or exact-duplication change that met the backlog's reduction and boundary-preservation rules; later work should be appended only after a fresh reference scan.
## Fresh Refactoring Pass (2026-09-16)
- **Reuse the shared SFTP download planner — completed.** Evidence: `rg -n 'PreparedJob|PreparedBatch|pendingBatch|keepBothLocalURL' TFM TFI Frameworks TFMTests TFITests` found only one desktop `PreparedJob`/`PreparedBatch` construction path, in `SFTPSessionModel.prepareDownload`, with direction fixed to `.download` and both remote-directory collections empty. Uploads already used `SFTPLocalUploadPlanner`. `SFTPRemoteDownloadPlanner` already owned the equivalent download traversal, local conflict policies, scoped destination access, directory creation, and transfer enqueueing for mobile. Adopted that planner in the desktop session and removed the obsolete mixed-direction batch and local helpers, retaining desktop pending-state/error handling, remote-copy policy, and transfer lifetimes. Estimated production reduction: 180 lines; actual reduction: 167 lines, or 59 lines including the added regression tests. Directory-only downloads now create their empty directory trees because the shared batch retains its destination independently of file jobs. The architecture record documents that ownership.
Verification completed on 2026-09-16:
- All 75 SFTP and architecture tests passed, including three new parameterized/behavioral tests covering ten file/directory conflict, subtree, empty-directory, and cancellation cases. The empty-directory regression fails against the pre-refactor `SFTPSessionModel` and passes with the final implementation.
- The full pinned TFM test action built successfully and passed 1,337 of 1,343 tests. Four failures also reproduce with the pre-refactor production source: `SettingsSyncTests.providedDatabaseCopyMigratesWithoutReset` (expects schema 15, current schema is 17), `ModelCapabilityResolverTests.reportedFieldsWinOverCodexAutoReviewOverride`, `ScrubberWebClientTests.fetchPreflightNeverEscalatesPrivateHTTP`, and `ChatComposerStateTests.aFailureNoticeIsNotAnAnswerWorthNaming`. `MetricsJourneyTests.offlineRestoredMetricsReconcileOnMainActor` passed on rerun. `SSHImportTests.indexedIdentityPlanningHandlesTheImportFileLimit` exceeded its two-second wall-clock limit when run concurrently, but passed in isolation against both versions (0.100 seconds with the final source).
- The full pinned TFI test action on iPhone 17 Pro / iOS 26.5 built successfully and passed 426 of 432 tests, including all 205 framework tests. The unchanged mobile graph retained five failures on isolated rerun: `MobileVaultFeatureTests.importRequiresAnExplicitUsernameWhenTheConfigurationOmitsIt` and four `MobileAssistantTests` cases (`completeRegistryIncludesRemoteAndBuiltInToolsOnlyForCapableModels`, `firstNewConversationSendPromotesDraftAndStartsRun`, `attachmentOnlyFirstSendPromotesTheConversation`, and `existingConversationSendStillUsesTheDraftExecutionPath`). `MobileEditorToolbarTests.iconButtonsKeepTheirNamesAndSaveLifecycle` passed on rerun.
- Strict Swift formatting across all production/test targets, `Scripts/check-zh-localization.sh`, the removed-symbol reference scan, and `git diff --check` passed. Both full test actions retained `-onlyUsePackageVersionsFromResolvedFile`; no dependency, localization, or shared production source changed.
+15 -182
View File
@@ -86,26 +86,6 @@ private struct SFTPDirectoryPresentation {
@Observable
final class SFTPSessionModel: Identifiable {
struct PreparedJob {
var direction: SFTPTransfer.Direction
var localURL: URL
var remotePath: String
var byteCount: Int64
var scopedRoot: URL
var hasConflict: Bool
var conflictingTargetIsDirectory: Bool
}
struct PreparedBatch {
var id = UUID()
var jobs: [PreparedJob]
var remoteDirectories: [String]
var remoteDirectoryConflicts: Set<String>
var localDirectories: [URL]
var localDirectoryConflicts: Set<URL>
var conflictCount: Int
}
struct PreparedRemoteCopyJob {
var sourcePath: String
var destinationPath: String
@@ -187,7 +167,7 @@ final class SFTPSessionModel: Identifiable {
var pendingAuthentication: SSHAuthenticationPrompt?
let connection = RemoteSessionConnectionController()
var connectionJourney: ConnectionJourney { connection.journey }
var pendingBatch: PreparedBatch?
private var pendingDownloadBatch: SFTPRemoteDownloadBatch?
var pendingUploadBatch: SFTPLocalUploadBatch?
var pendingRemoteCopyBatch: PreparedRemoteCopyBatch?
var operationError: String?
@@ -253,11 +233,11 @@ final class SFTPSessionModel: Identifiable {
var conflictCount: Int {
pendingRemoteCopyBatch?.conflictCount
?? pendingUploadBatch?.conflictCount
?? pendingBatch?.conflictCount
?? pendingDownloadBatch?.conflictCount
?? 0
}
var hasPendingBatch: Bool {
pendingUploadBatch != nil || pendingBatch != nil || pendingRemoteCopyBatch != nil
pendingUploadBatch != nil || pendingDownloadBatch != nil || pendingRemoteCopyBatch != nil
}
/// What the list draws. Already filtered, sorted, and formatted.
var visibleRows: [SFTPRow] { directoryPresentation.visibleRows }
@@ -831,7 +811,7 @@ final class SFTPSessionModel: Identifiable {
func cancelPendingBatch() {
pendingUploadBatch = nil
pendingBatch = nil
pendingDownloadBatch = nil
pendingRemoteCopyBatch = nil
}
@@ -843,7 +823,7 @@ final class SFTPSessionModel: Identifiable {
destination: path,
client: client
)
pendingBatch = nil
pendingDownloadBatch = nil
pendingRemoteCopyBatch = nil
pendingUploadBatch = batch.isEmpty ? nil : batch
} catch {
@@ -854,48 +834,13 @@ final class SFTPSessionModel: Identifiable {
func prepareDownload(_ files: [RemoteFile], to directory: URL) async {
guard let client else { return }
var jobs: [PreparedJob] = []
var directories: [URL] = []
var directoryConflicts: Set<URL> = []
var conflicts = 0
do {
pendingUploadBatch = nil
func collect(_ remote: RemoteFile, local: URL) async throws {
if remote.isDirectory {
directories.append(local)
for child in try await client.listDirectory(at: remote.path) {
try await collect(child, local: local.appendingPathComponent(child.name))
}
} else {
var isDirectory: ObjCBool = false
let exists = FileManager.default.fileExists(atPath: local.path, isDirectory: &isDirectory)
if exists { conflicts += 1 }
jobs.append(
PreparedJob(
direction: .download, localURL: local, remotePath: remote.path,
byteCount: remote.size, scopedRoot: directory, hasConflict: exists,
conflictingTargetIsDirectory: exists && isDirectory.boolValue
))
}
}
for file in files {
try await collect(file, local: directory.appendingPathComponent(file.name))
}
for directory in directories {
var isDirectory: ObjCBool = false
if FileManager.default.fileExists(atPath: directory.path, isDirectory: &isDirectory),
!isDirectory.boolValue
{
directoryConflicts.insert(directory)
conflicts += 1
}
}
pendingRemoteCopyBatch = nil
pendingBatch = PreparedBatch(
jobs: jobs, remoteDirectories: [], remoteDirectoryConflicts: [],
localDirectories: directories, localDirectoryConflicts: directoryConflicts,
conflictCount: conflicts
let batch = try await SFTPRemoteDownloadPlanner.prepare(
files, destination: directory, client: client
)
pendingRemoteCopyBatch = nil
pendingDownloadBatch = batch
} catch { operationError = error.localizedDescription }
}
@@ -935,7 +880,7 @@ final class SFTPSessionModel: Identifiable {
jobs[index].conflictingTargetIsDirectory = kind
}
pendingBatch = nil
pendingDownloadBatch = nil
pendingRemoteCopyBatch = PreparedRemoteCopyBatch(
source: manifest.source, jobs: jobs,
remoteDirectories: preflight.directories,
@@ -1082,113 +1027,15 @@ final class SFTPSessionModel: Identifiable {
}
return
}
guard let batch = pendingBatch, let connection = transferConnection, let client else {
guard let batch = pendingDownloadBatch, let connection = transferConnection, client != nil else {
return
}
pendingBatch = nil
pendingDownloadBatch = nil
do {
let remoteResolution = try await resolveRemoteDirectories(
batch.remoteDirectories, conflicts: batch.remoteDirectoryConflicts,
policy: policy, client: client
try SFTPRemoteDownloadPlanner.enqueue(
batch, policy: policy, hostID: hostID, connection: connection,
homeDirectory: homeResolution.resolvedDirectory, coordinator: coordinator
)
let localRoots = Set(batch.jobs.filter { $0.direction == .download }.map(\.scopedRoot))
var skippedLocalPrefixes: [URL] = []
var localMappings: [(URL, URL)] = []
for root in localRoots {
let scoped = root.startAccessingSecurityScopedResource()
defer { if scoped { root.stopAccessingSecurityScopedResource() } }
for originalDirectory in batch.localDirectories where originalDirectory.path.hasPrefix(root.path) {
if skippedLocalPrefixes.contains(where: {
originalDirectory.path == $0.path
|| originalDirectory.path.hasPrefix($0.path + "/")
}) {
continue
}
var directory = originalDirectory
for mapping in localMappings
where directory.path == mapping.0.path || directory.path.hasPrefix(mapping.0.path + "/") {
directory = URL(
fileURLWithPath: mapping.1.path + directory.path.dropFirst(mapping.0.path.count))
}
if batch.localDirectoryConflicts.contains(originalDirectory) {
switch policy {
case .skip:
skippedLocalPrefixes.append(originalDirectory)
continue
case .replace:
try FileManager.default.removeItem(at: originalDirectory)
case .keepBoth:
let candidate = Self.keepBothLocalURL(originalDirectory)
localMappings.append((originalDirectory, candidate))
directory = candidate
}
}
try FileManager.default.createDirectory(at: directory, withIntermediateDirectories: true)
}
}
for var job in batch.jobs {
if remoteResolution.skipped.contains(where: {
job.remotePath == $0 || job.remotePath.hasPrefix($0 + "/")
})
|| skippedLocalPrefixes.contains(where: {
job.localURL.path == $0.path || job.localURL.path.hasPrefix($0.path + "/")
})
{
continue
}
for mapping in remoteResolution.mappings
where job.remotePath == mapping.0 || job.remotePath.hasPrefix(mapping.0 + "/") {
job.remotePath = mapping.1 + job.remotePath.dropFirst(mapping.0.count)
job.hasConflict = false
}
for mapping in localMappings
where job.localURL.path == mapping.0.path || job.localURL.path.hasPrefix(mapping.0.path + "/") {
let suffix = job.localURL.path.dropFirst(mapping.0.path.count)
job.localURL = URL(fileURLWithPath: mapping.1.path + suffix)
job.hasConflict = false
try FileManager.default.createDirectory(
at: job.localURL.deletingLastPathComponent(), withIntermediateDirectories: true
)
}
if job.hasConflict {
if job.direction == .upload {
guard
let resolved = try await resolveRemoteConflict(
path: job.remotePath,
conflictingTargetIsDirectory: job.conflictingTargetIsDirectory,
policy: policy, client: client
)
else { continue }
job.remotePath = resolved.path
job.hasConflict = resolved.overwrite
} else {
switch policy {
case .skip: continue
case .replace:
if job.conflictingTargetIsDirectory {
try FileManager.default.removeItem(at: job.localURL)
}
case .keepBoth:
job.localURL = Self.keepBothLocalURL(job.localURL)
}
}
}
let completion: ((SFTPTransfer.State) -> Void)? =
job.direction == .upload
? { [weak self] state in
if state == .completed { self?.scheduleSilentRefresh() }
}
: nil
coordinator.enqueue(
direction: job.direction, batchID: batch.id, localURL: job.localURL,
remotePath: job.remotePath, byteCount: job.byteCount, hostID: hostID,
connection: connection, scopedRoot: job.scopedRoot,
overwrite: job.hasConflict && policy == .replace,
homeDirectory: homeResolution.resolvedDirectory,
completion: completion
)
}
if remoteResolution.changed { scheduleSilentRefresh() }
} catch { operationError = error.localizedDescription }
}
@@ -1429,20 +1276,6 @@ final class SFTPSessionModel: Identifiable {
return result
}
private static func keepBothLocalURL(_ url: URL) -> URL {
let ext = url.pathExtension
let base = url.deletingPathExtension()
var index = 2
while true {
let name =
ext.isEmpty
? "\(base.lastPathComponent).\(index)"
: "\(base.lastPathComponent).\(index).\(ext)"
let candidate = base.deletingLastPathComponent().appendingPathComponent(name)
if !FileManager.default.fileExists(atPath: candidate.path) { return candidate }
index += 1
}
}
}
// MARK: - Demo data
+108
View File
@@ -791,6 +791,114 @@ struct SFTPFeatureTests {
#expect(batch.conflictCount == 0)
}
@Test(arguments: [TransferConflictPolicy.skip, .replace, .keepBoth], [false, true])
func downloadingAFileResolvesExistingFilesAndDirectories(
policy: TransferConflictPolicy, existingIsDirectory: Bool
) async throws {
let root = FileManager.default.temporaryDirectory.appending(path: UUID().uuidString)
try FileManager.default.createDirectory(at: root, withIntermediateDirectories: true)
defer { try? FileManager.default.removeItem(at: root) }
let original = root.appending(path: "report.txt")
if existingIsDirectory {
try FileManager.default.createDirectory(at: original, withIntermediateDirectories: true)
}
let preserved = existingIsDirectory ? original.appending(path: "keep.txt") : original
try Data("original".utf8).write(to: preserved)
let client = WritingSFTPClient()
let session = SFTPSessionModel(
hostID: UUID(), initialPath: "/source", client: client, configuration: testConfiguration
)
let coordinator = SFTPTransferCoordinator(store: HostLibraryModel()) { client }
await session.prepareDownload([remoteFile(path: "/source/report.txt", kind: .file)], to: root)
#expect(session.conflictCount == 1)
await session.enqueuePending(policy: policy, coordinator: coordinator)
for _ in 0..<200 {
guard coordinator.sftpTransfers.contains(where: { $0.state == .queued || $0.state == .running }) else {
break
}
try await Task.sleep(for: .milliseconds(10))
}
#expect(session.operationError == nil)
#expect(!session.hasPendingBatch)
if policy != .replace { #expect(try Data(contentsOf: preserved) == Data("original".utf8)) }
if policy == .skip {
#expect(coordinator.sftpTransfers.isEmpty)
} else {
let destination = policy == .keepBoth ? root.appending(path: "report.2.txt") : original
#expect(coordinator.sftpTransfers.map(\.state) == [.completed])
#expect(try Data(contentsOf: destination) == Data("termind".utf8))
}
}
@Test(arguments: [TransferConflictPolicy.skip, .replace, .keepBoth])
func downloadingAConflictingDirectoryAppliesThePolicyToItsWholeSubtree(
policy: TransferConflictPolicy
) async throws {
let root = FileManager.default.temporaryDirectory.appending(path: UUID().uuidString)
try FileManager.default.createDirectory(at: root, withIntermediateDirectories: true)
defer { try? FileManager.default.removeItem(at: root) }
let original = root.appending(path: "bundle")
try Data("original".utf8).write(to: original)
let bundle = remoteFile(path: "/source/bundle", kind: .directory)
let nested = remoteFile(path: "/source/bundle/nested", kind: .directory)
let file = remoteFile(path: "/source/bundle/nested/payload.txt", kind: .file)
let sibling = remoteFile(path: "/source/bundle-other", kind: .directory)
let client = TreeSFTPClient(listings: [bundle.path: [nested], nested.path: [file]])
let session = SFTPSessionModel(
hostID: UUID(), initialPath: "/source", client: client, configuration: testConfiguration
)
let coordinator = SFTPTransferCoordinator(store: HostLibraryModel()) { client }
await session.prepareDownload([bundle, sibling], to: root)
#expect(session.conflictCount == 1)
await session.enqueuePending(policy: policy, coordinator: coordinator)
#expect(session.operationError == nil)
#expect(!session.hasPendingBatch)
#expect(try root.appending(path: "bundle-other").resourceValues(forKeys: [.isDirectoryKey]).isDirectory == true)
if policy != .replace { #expect(try Data(contentsOf: original) == Data("original".utf8)) }
if policy == .skip {
#expect(coordinator.sftpTransfers.isEmpty)
} else {
let directory = policy == .keepBoth ? root.appending(path: "bundle.2") : original
let destination = directory.appending(path: "nested/payload.txt")
#expect(coordinator.sftpTransfers.map(\.localPath) == [destination.path])
#expect(coordinator.sftpTransfers.map(\.remotePath) == [file.path])
#expect(coordinator.sftpTransfers.map(\.direction) == [.download])
#expect(
try directory.appending(path: "nested").resourceValues(forKeys: [.isDirectoryKey]).isDirectory == true)
}
}
@Test
func downloadingOnlyEmptyDirectoriesCreatesThemUnlessTheBatchWasCancelled() async throws {
let root = FileManager.default.temporaryDirectory.appending(path: UUID().uuidString)
defer { try? FileManager.default.removeItem(at: root) }
let bundle = remoteFile(path: "/source/bundle", kind: .directory)
let empty = remoteFile(path: "/source/bundle/empty", kind: .directory)
let client = TreeSFTPClient(listings: [bundle.path: [empty]])
let session = SFTPSessionModel(
hostID: UUID(), initialPath: "/source", client: client, configuration: testConfiguration
)
let coordinator = SFTPTransferCoordinator(store: HostLibraryModel()) { client }
await session.prepareDownload([bundle], to: root)
#expect(session.hasPendingBatch)
session.cancelPendingBatch()
await session.enqueuePending(policy: .skip, coordinator: coordinator)
#expect(!FileManager.default.fileExists(atPath: root.path))
await session.prepareDownload([bundle], to: root)
await session.enqueuePending(policy: .skip, coordinator: coordinator)
#expect(session.operationError == nil)
#expect(!session.hasPendingBatch)
#expect(coordinator.sftpTransfers.isEmpty)
#expect(try root.appending(path: "bundle/empty").resourceValues(forKeys: [.isDirectoryKey]).isDirectory == true)
}
@Test
func remoteCopyPreflightFindsTargetConflicts() async {
let sourceHostID = UUID()