From e54f5c81c95bd0f016df3d9efedcda5eb4fe3f26 Mon Sep 17 00:00:00 2001 From: Kaede Akino Date: Wed, 16 Sep 2026 22:35:49 +0800 Subject: [PATCH] Reuse remote copy planning and trim copy-only tests --- .../CommandRiskClassifierTests.swift | 6 - .../WorkspaceUploadNoticeTests.swift | 8 +- .../Transfers/SFTPRemoteTransferPlanner.swift | 8 +- Memory/ARCHITECTURE.md | 10 +- Memory/REFACTOR.md | 13 + TFITests/MobileAISettingsTests.swift | 5 - .../MobileSFTPCopyAvailabilityTests.swift | 22 -- TFM/Features/SFTP/SFTPSessionModel.swift | 223 +----------------- TFMTests/AgentToolPresentationTests.swift | 11 - TFMTests/SFTPCopyToolTests.swift | 16 +- TFMTests/SFTPFeatureTests.swift | 99 +++++++- TFMTests/SSHHandshakeDiagnosticsTests.swift | 13 +- TFMTests/SSHImportTests.swift | 1 - TFMTests/WindowToolbarFindTests.swift | 1 - 14 files changed, 137 insertions(+), 299 deletions(-) diff --git a/FrameworkTests/CommandRiskClassifierTests.swift b/FrameworkTests/CommandRiskClassifierTests.swift index 18016cd0..70829fa3 100644 --- a/FrameworkTests/CommandRiskClassifierTests.swift +++ b/FrameworkTests/CommandRiskClassifierTests.swift @@ -11,12 +11,6 @@ struct CommandRiskCase: Sendable, CustomTestStringConvertible { @Suite struct CommandRiskClassifierTests { - @Test - func cautionLabelAlsoFitsUnknownCommands() { - #expect(CommandRisk.caution.label == String(localized: "Caution")) - #expect(CommandRiskClassifier.assess("vendorctl inspect").risk == .caution) - } - @Test( arguments: [ CommandRiskCase(command: "ls -la", expected: .safe), diff --git a/FrameworkTests/WorkspaceUploadNoticeTests.swift b/FrameworkTests/WorkspaceUploadNoticeTests.swift index 57fc46e0..e3ac6871 100644 --- a/FrameworkTests/WorkspaceUploadNoticeTests.swift +++ b/FrameworkTests/WorkspaceUploadNoticeTests.swift @@ -11,7 +11,7 @@ struct WorkspaceUploadNoticeTests { hasTransferConfiguration: false, isWorkingDirectoryPending: true, workingDirectory: nil - ) == String(localized: "The SSH session is not ready for file transfer.") + ) != nil ) #expect( TerminalUploadAvailability.problem( @@ -19,7 +19,7 @@ struct WorkspaceUploadNoticeTests { hasTransferConfiguration: true, isWorkingDirectoryPending: true, workingDirectory: nil - ) == String(localized: "Wait for the shell prompt to report its current directory.") + ) != nil ) #expect( TerminalUploadAvailability.problem( @@ -31,10 +31,8 @@ struct WorkspaceUploadNoticeTests { ) } - @Test func uploadNoticePresentationUsesOneCrossPlatformMapping() { + @Test func uploadNoticeRetainsItsDestinationAndRejectionState() { let queued = WorkspaceUploadNotice.queued(2, "/srv/project") - #expect(queued.symbol == "checkmark.circle.fill") - #expect(queued.title == String(localized: "Added 2 items to Transfers")) #expect(queued.detail == "/srv/project") #expect(!queued.isRejected) #expect(WorkspaceUploadNotice.rejected("Denied").isRejected) diff --git a/Frameworks/TermindRemoteInfrastructure/Transfers/SFTPRemoteTransferPlanner.swift b/Frameworks/TermindRemoteInfrastructure/Transfers/SFTPRemoteTransferPlanner.swift index 4311af07..d88f7be6 100644 --- a/Frameworks/TermindRemoteInfrastructure/Transfers/SFTPRemoteTransferPlanner.swift +++ b/Frameworks/TermindRemoteInfrastructure/Transfers/SFTPRemoteTransferPlanner.swift @@ -256,6 +256,8 @@ public nonisolated enum SFTPRemoteCopyPlanner { ) } + /// Returns whether resolving the batch changed remote directories, even without file jobs. + @discardableResult @MainActor public static func enqueue( _ batch: SFTPRemoteCopyBatch, policy: TransferConflictPolicy, @@ -265,7 +267,7 @@ public nonisolated enum SFTPRemoteCopyPlanner { client: any SFTPClientProtocol, coordinator: SFTPTransferCoordinator, completion: ((SFTPTransfer.State) -> Void)? = nil - ) async throws { + ) async throws -> Bool { let resolution = try await resolveDirectories( batch.directories, conflicts: batch.directoryConflicts, policy: policy, client: client @@ -304,6 +306,7 @@ public nonisolated enum SFTPRemoteCopyPlanner { completion: completion ) } + return resolution.changed } private struct Preflight { @@ -317,6 +320,7 @@ public nonisolated enum SFTPRemoteCopyPlanner { var skipped: [String] = [] var mappings: [(String, String)] = [] var created: [String] = [] + var changed = false } private static func preflight( @@ -367,6 +371,7 @@ public nonisolated enum SFTPRemoteCopyPlanner { continue case .replace: try await client.removeFile(at: original) + result.changed = true case .keepBoth: destination = try await availablePath(for: original, client: client) result.mappings.append((original, destination)) @@ -375,6 +380,7 @@ public nonisolated enum SFTPRemoteCopyPlanner { if try await client.attributes(at: destination) == nil { try await client.createDirectory(at: destination) result.created.append(destination) + result.changed = true } } return result diff --git a/Memory/ARCHITECTURE.md b/Memory/ARCHITECTURE.md index 5fd2f30c..e4193696 100644 --- a/Memory/ARCHITECTURE.md +++ b/Memory/ARCHITECTURE.md @@ -173,10 +173,12 @@ background expiration pauses uploads/downloads and cancels remote copies; borrow session retirement still cancels its transfers. macOS termination gives asynchronous cleanup at most three seconds, so an unreachable server cannot hold up quitting. 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. +Desktop and mobile upload/download and remote-copy preparation and enqueueing use +Remote Infrastructure planners. Download batches retain their destination independently +of file jobs, so directory-only selections still create their empty directory trees. +Remote-copy enqueueing reports directory mutations so desktop refreshes also cover +copies without file jobs. App sessions retain pending-batch presentation, cancellation, +error handling, and completion refresh; interactive browser deletion stays app-owned. `sftp_copy` is Termind-owned and acting-only in Assistant, with Ask/sequential policy. It copies regular files and directory trees into an existing directory; diff --git a/Memory/REFACTOR.md b/Memory/REFACTOR.md index e4b8e76a..7f3c4667 100644 --- a/Memory/REFACTOR.md +++ b/Memory/REFACTOR.md @@ -105,3 +105,16 @@ Verification completed on 2026-09-16: - 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. + +## Follow-up Reuse and Test Cleanup (2026-09-16) + +- **Reuse the shared remote-copy planner — implemented; full verification remains partial.** Evidence: `rg -n 'PreparedRemoteCopy|pendingRemoteCopyBatch|SFTPRemoteCopyPlanner.enqueue|resolveRemoteDirectories' TFM TFI Frameworks TFMTests TFITests -g '*.swift'` confirms the desktop-only batch types and duplicated preparation, conflict resolution, and enqueue loops in `SFTPSessionModel`; mobile already uses `SFTPRemoteCopyPlanner`. Replaced the desktop path with that planner and returned directory-mutation information so empty-directory copies still refresh the browser. Retained app-owned pending state, error handling, transfer completion refresh, and the interactive deletion lifecycle. Estimated production reduction: 220 lines; actual reduction: 193 lines. Added behavioral coverage for file/directory and subtree conflicts across all three policies, empty-directory refresh, and pending-copy cancellation; retained the planner's cancellation and validated recursive deletion checks. Upload conflict helpers remain a separate candidate rather than widening this change. +- **Remove copy-only and duplicated test expectations — implemented; full verification remains partial.** Evidence: `rg -n 'roleLabelsMatchDesktopDefaults|cautionLabelAlsoFitsUnknownCommands|copyApprovalLabelsAndOverwriteWarningUseAppResources|approvalDetailsLocalizeBothEndpointsAndDestructiveWarning|handshakeFailureDescriptionsDistinguishBannerConditions|uploadNoticePresentationUsesOneCrossPlatformMapping' FrameworkTests TFMTests TFITests -g '*.swift'` identifies role-label snapshots, a duplicate unknown-command case, repeated cross-platform approval copy checks, and mixed behavior/copy tests. Removed pure copy assertions and duplicate cases, retained unknown-command classification in its parameterized suite, and consolidated approval endpoint values and destructive-warning policy coverage across policies/locales. Preserved diagnostic context, upload gating, and notice payload/state coverage. Also dropped isolated fixed title/icon assertions from otherwise behavioral tests. Estimated test reduction: 50 lines; actual cleanup reduction: 59 lines, excluding 75 net lines of remote-copy regression coverage. Total Swift reduction: 177 lines. Retained architecture guards, language-selection/catalog validation, protocol strings, user-data round trips, and behavioral assertions that contain strings. + +Verification and remaining limitations (2026-09-16): + +- The pinned TFM focused test action passed all 200 tests, including SFTP/session/copy, architecture boundaries, tool presentation, handshake diagnostics, SSH import, toolbar behavior, app language, upload availability, and command classification. Result: `/tmp/termind-refactor-copy-focused-20260916.xcresult`. +- The pinned full TFM test action built successfully and passed 1,436 of 1,437 tests. The sole failure is the unchanged `SpeechWaveformViewTests.waveformRespondsToLevelsAndReduceMotionMakesItStable` bitmap equality assertion. An earlier backlog entry records this same test failure; no baseline or isolated rerun was performed in this pass. Result: `/tmp/termind-refactor-copy-macos-20260916.xcresult`. +- The full TFI action did not build or execute tests: the supplied simulator destination UUID was unavailable. This is not a passing iOS verification. A fresh device listing found an available booted iPhone 17 Pro, but the owner requested an immediate commit before rerunning either platform. +- Strict Swift formatting across all production/test targets, `Scripts/check-zh-localization.sh`, the removed desktop-helper reference scan, and `git diff --check` passed. Xcode successfully resolved modules that standalone SourceKit could not find. Existing generic SwiftLint file/type/function-size and complexity findings remain outside this independent refactor; no inline suppressions were added. Dependencies and localization catalogs are unchanged. +- Submitted at the owner's explicit request with these verification limitations retained rather than deleting unrelated behavioral tests to obtain a green result. diff --git a/TFITests/MobileAISettingsTests.swift b/TFITests/MobileAISettingsTests.swift index 481b531d..ca153244 100644 --- a/TFITests/MobileAISettingsTests.swift +++ b/TFITests/MobileAISettingsTests.swift @@ -8,11 +8,6 @@ import Testing @Suite @MainActor struct MobileAISettingsTests { - @Test func roleLabelsMatchDesktopDefaults() { - #expect(AIRole.chat.unsetLabel == AIRole.guardian.unsetLabel) - #expect(AIRole.commandGenerator.unsetLabel != AIRole.guardian.unsetLabel) - } - private func provider() -> ModelProvider { var value = ModelProvider(name: "Gateway", inferenceURL: "https://example.invalid/v1/responses") value.models = [AIModel(id: "one"), AIModel(id: "two")] diff --git a/TFITests/MobileSFTPCopyAvailabilityTests.swift b/TFITests/MobileSFTPCopyAvailabilityTests.swift index 9755ded5..f3e13170 100644 --- a/TFITests/MobileSFTPCopyAvailabilityTests.swift +++ b/TFITests/MobileSFTPCopyAvailabilityTests.swift @@ -27,28 +27,6 @@ struct MobileSFTPCopyAvailabilityTests { #expect(registry.available(in: .reviewing).descriptors.isEmpty) #expect(TermindAgentTools.presenter(for: descriptor)?.id == "builtin.sftp_copy") } - - @Test(arguments: ["en", "zh-Hans", "zh-Hant"]) - func copyApprovalLabelsAndOverwriteWarningUseAppResources(_ language: String) { - let items = SFTPCopyTool.presentArguments( - .init( - arguments: [ - "source_host_id": .string("source"), "destination_host_id": .string("destination"), - "conflict_policy": .string("replace"), - ], locale: Locale(identifier: language) - )) - let label = language == "en" ? "Source host" : language == "zh-Hant" ? "來源主機" : "源主机" - #expect( - items.contains { - if case .field(let field) = $0 { return field.label == label } - return false - }) - #expect( - items.contains { - if case .message(_, .warning) = $0 { return true } - return false - }) - } } @MainActor diff --git a/TFM/Features/SFTP/SFTPSessionModel.swift b/TFM/Features/SFTP/SFTPSessionModel.swift index 826de5a6..140c4cc2 100644 --- a/TFM/Features/SFTP/SFTPSessionModel.swift +++ b/TFM/Features/SFTP/SFTPSessionModel.swift @@ -86,30 +86,6 @@ private struct SFTPDirectoryPresentation { @Observable final class SFTPSessionModel: Identifiable { - struct PreparedRemoteCopyJob { - var sourcePath: String - var destinationPath: String - var byteCount: Int64 - var hasConflict: Bool - var conflictingTargetIsDirectory: Bool - } - - struct PreparedRemoteCopyBatch { - var id = UUID() - var source: SFTPRemoteCopySource - var jobs: [PreparedRemoteCopyJob] - var remoteDirectories: [String] - var remoteDirectoryConflicts: Set - var conflictCount: Int - } - - private struct RemoteConflictPreflight { - var directories: [String] - var directoryConflicts: Set - var targetKinds: [Bool?] - var conflictCount: Int - } - let id = UUID() let hostID: Host.ID let initialPath: String @@ -169,7 +145,7 @@ final class SFTPSessionModel: Identifiable { var connectionJourney: ConnectionJourney { connection.journey } private var pendingDownloadBatch: SFTPRemoteDownloadBatch? var pendingUploadBatch: SFTPLocalUploadBatch? - var pendingRemoteCopyBatch: PreparedRemoteCopyBatch? + var pendingRemoteCopyBatch: SFTPRemoteCopyBatch? var operationError: String? private(set) var canCancelOperation = false @@ -853,77 +829,16 @@ final class SFTPSessionModel: Identifiable { guard let client else { return } do { pendingUploadBatch = nil - let manifest = try await coordinator.remoteManifest(for: payload) - var jobs: [PreparedRemoteCopyJob] = [] - var directories: [String] = [] - for entry in manifest.entries { - let destination = RemotePath.join(path, entry.relativePath) - if entry.isDirectory { - directories.append(destination) - } else { - jobs.append( - PreparedRemoteCopyJob( - sourcePath: entry.sourcePath, - destinationPath: destination, - byteCount: entry.byteCount, - hasConflict: false, - conflictingTargetIsDirectory: false - )) - } - } - - let preflight = try await remoteConflictPreflight( - directories: directories, targets: jobs.map(\.destinationPath), client: client + let batch = try await SFTPRemoteCopyPlanner.prepare( + payload, destination: path, client: client, coordinator: coordinator ) - for (index, kind) in zip(jobs.indices, preflight.targetKinds) { - guard let kind else { continue } - jobs[index].hasConflict = true - jobs[index].conflictingTargetIsDirectory = kind - } - pendingDownloadBatch = nil - pendingRemoteCopyBatch = PreparedRemoteCopyBatch( - source: manifest.source, jobs: jobs, - remoteDirectories: preflight.directories, - remoteDirectoryConflicts: preflight.directoryConflicts, - conflictCount: preflight.conflictCount - ) + pendingRemoteCopyBatch = batch } catch { operationError = error.localizedDescription } } - private func remoteConflictPreflight( - directories: [String], targets: [String], client: any SFTPClientProtocol - ) async throws -> RemoteConflictPreflight { - let directories = directories.sorted { $0.count < $1.count } - var directoryConflicts: Set = [] - var targetKinds: [Bool?] = [] - var conflictCount = 0 - - for directory in directories { - guard !directoryConflicts.contains(where: { directory.hasPrefix($0 + "/") }) - else { continue } - if let existing = try await client.attributes(at: directory), !existing.isDirectory { - directoryConflicts.insert(directory) - conflictCount += 1 - } - } - for target in targets { - guard !directoryConflicts.contains(where: { target.hasPrefix($0 + "/") }) else { - targetKinds.append(nil) - continue - } - let kind = try await client.attributes(at: target)?.isDirectory - targetKinds.append(kind) - if kind != nil { conflictCount += 1 } - } - return RemoteConflictPreflight( - directories: directories, directoryConflicts: directoryConflicts, - targetKinds: targetKinds, conflictCount: conflictCount - ) - } - func open( _ file: RemoteFile, coordinator: SFTPTransferCoordinator, completion: @escaping (URL) -> Void @@ -1051,55 +966,15 @@ final class SFTPSessionModel: Identifiable { else { return } pendingRemoteCopyBatch = nil do { - let remoteResolution = try await resolveRemoteDirectories( - batch.remoteDirectories, conflicts: batch.remoteDirectoryConflicts, - policy: policy, client: client - ) - - for var job in batch.jobs { - if remoteResolution.skipped.contains(where: { - job.destinationPath == $0 || job.destinationPath.hasPrefix($0 + "/") - }) { - continue - } - for mapping in remoteResolution.mappings - where job.destinationPath == mapping.0 - || job.destinationPath.hasPrefix(mapping.0 + "/") - { - job.destinationPath = - mapping.1 - + job.destinationPath.dropFirst(mapping.0.count) - job.hasConflict = false - } - if job.hasConflict { - guard - let resolved = try await resolveRemoteConflict( - path: job.destinationPath, - conflictingTargetIsDirectory: job.conflictingTargetIsDirectory, - policy: policy, client: client - ) - else { continue } - job.destinationPath = resolved.path - job.hasConflict = resolved.overwrite - } - - coordinator.enqueueRemoteCopy( - batchID: batch.id, - sourcePath: job.sourcePath, - sourceHostID: batch.source.hostID, - sourceConnection: batch.source.connection, - destinationPath: job.destinationPath, - destinationHostID: hostID, - destinationConnection: destinationConnection, - byteCount: job.byteCount, - overwrite: job.hasConflict && policy == .replace, - sourceHomeDirectory: batch.source.homeDirectory, - destinationHomeDirectory: homeResolution.resolvedDirectory - ) { [weak self] state in - if state == .completed { self?.scheduleSilentRefresh() } - } + let changedDirectories = try await SFTPRemoteCopyPlanner.enqueue( + batch, policy: policy, destinationHostID: hostID, + destinationConnection: destinationConnection, + destinationHomeDirectory: homeResolution.resolvedDirectory, + client: client, coordinator: coordinator + ) { [weak self] state in + if state == .completed { self?.scheduleSilentRefresh() } } - if remoteResolution.changed { scheduleSilentRefresh() } + if changedDirectories { scheduleSilentRefresh() } } catch { operationError = error.localizedDescription } @@ -1203,80 +1078,6 @@ final class SFTPSessionModel: Identifiable { } } - private func availableRemotePath( - for path: String, client: any SFTPClientProtocol - ) async throws -> String { - var index = 2 - var candidate = RemotePath.keepBothCandidate(for: path, index: index) - while try await client.attributes(at: candidate) != nil { - index += 1 - candidate = RemotePath.keepBothCandidate(for: path, index: index) - } - return candidate - } - - private struct RemoteDirectoryResolution { - var skipped: [String] = [] - var mappings: [(String, String)] = [] - var changed = false - } - - private func resolveRemoteConflict( - path: String, conflictingTargetIsDirectory: Bool, - policy: TransferConflictPolicy, client: any SFTPClientProtocol - ) async throws -> (path: String, overwrite: Bool)? { - switch policy { - case .skip: - return nil - case .replace: - if conflictingTargetIsDirectory, - let existing = try await client.attributes(at: path) - { - try await deleteRecursively(existing, client: client) - } - return (path, true) - case .keepBoth: - return (try await availableRemotePath(for: path, client: client), false) - } - } - - private func resolveRemoteDirectories( - _ directories: [String], conflicts: Set, - policy: TransferConflictPolicy, client: any SFTPClientProtocol - ) async throws -> RemoteDirectoryResolution { - var result = RemoteDirectoryResolution() - for original in directories { - if result.skipped.contains(where: { - original == $0 || original.hasPrefix($0 + "/") - }) { - continue - } - var destination = original - for mapping in result.mappings - where destination == mapping.0 || destination.hasPrefix(mapping.0 + "/") { - destination = mapping.1 + destination.dropFirst(mapping.0.count) - } - if conflicts.contains(original) { - switch policy { - case .skip: - result.skipped.append(original) - continue - case .replace: - try await client.removeFile(at: original) - result.changed = true - case .keepBoth: - destination = try await availableRemotePath(for: original, client: client) - result.mappings.append((original, destination)) - } - } - if try await client.attributes(at: destination) == nil { - try await client.createDirectory(at: destination) - result.changed = true - } - } - return result - } - } // MARK: - Demo data diff --git a/TFMTests/AgentToolPresentationTests.swift b/TFMTests/AgentToolPresentationTests.swift index bf62a940..849ec74c 100644 --- a/TFMTests/AgentToolPresentationTests.swift +++ b/TFMTests/AgentToolPresentationTests.swift @@ -54,18 +54,8 @@ struct AgentToolPresentationTests { locale: Locale(identifier: "en") ) - #expect(presentation.symbol == "terminal") - #expect(presentation.title == "Run the SSH command on Production") #expect(!presentation.title.contains(command)) #expect(!presentation.title.contains(hostID.uuidString)) - #expect( - AgentToolPresentation.approval( - for: request, - presenter: SSHExecTool.presenter, - hostName: "Production", - locale: Locale(identifier: "zh-Hans") - ).title == "在Production上运行SSH命令" - ) #expect( presentation.detail.items == [ .text(.init(title: "Command", text: command, style: .monospaced)), @@ -97,7 +87,6 @@ struct AgentToolPresentationTests { let presentation = AgentToolPresentation.approval( for: request, locale: Locale(identifier: "en") ) - #expect(presentation.title == "Run remote tool") #expect(!presentation.title.contains("hunter2")) #expect( presentation.detail.items == [ diff --git a/TFMTests/SFTPCopyToolTests.swift b/TFMTests/SFTPCopyToolTests.swift index f16b93a9..e9f233fa 100644 --- a/TFMTests/SFTPCopyToolTests.swift +++ b/TFMTests/SFTPCopyToolTests.swift @@ -279,25 +279,25 @@ struct SFTPCopyToolTests { #expect(await fixture.backend.opens == 0) } - @Test(arguments: ["en", "zh-Hans", "zh-Hant"]) - func approvalDetailsLocalizeBothEndpointsAndDestructiveWarning(_ language: String) throws { + @Test(arguments: ["skip", "replace", "keep_both"], ["en", "zh-Hans", "zh-Hant"]) + func approvalDetailsRetainEndpointsAndWarnOnlyForReplacement(_ policy: String, _ language: String) throws { let fixture = CopyFixture() let items = SFTPCopyTool.presentArguments( .init( - arguments: try #require(fixture.invocation(policy: "replace").call.arguments.objectValue), + arguments: try #require(fixture.invocation(policy: policy).call.arguments.objectValue), locale: Locale(identifier: language) )) - let labels = items.compactMap { item -> String? in - if case .field(let field) = item { return field.label } + let values = items.compactMap { item -> String? in + if case .field(let field) = item { return field.value } return nil } - #expect(labels.contains(language == "en" ? "Source host" : language == "zh-Hant" ? "來源主機" : "源主机")) - #expect(labels.contains(language == "en" ? "Destination host" : language == "zh-Hant" ? "目標主機" : "目标主机")) + #expect(values.contains(fixture.sourceID.uuidString)) + #expect(values.contains(fixture.destinationID.uuidString)) #expect( items.contains { if case .message(_, .warning) = $0 { return true } return false - }) + } == (policy == "replace")) } } diff --git a/TFMTests/SFTPFeatureTests.swift b/TFMTests/SFTPFeatureTests.swift index 681da773..bcb48e20 100644 --- a/TFMTests/SFTPFeatureTests.swift +++ b/TFMTests/SFTPFeatureTests.swift @@ -899,8 +899,10 @@ struct SFTPFeatureTests { #expect(try root.appending(path: "bundle/empty").resourceValues(forKeys: [.isDirectoryKey]).isDirectory == true) } - @Test - func remoteCopyPreflightFindsTargetConflicts() async { + @Test(arguments: [TransferConflictPolicy.skip, .replace, .keepBoth], [RemoteFile.Kind.file, .directory]) + func remoteCopyResolvesExistingFilesAndDirectories( + policy: TransferConflictPolicy, kind: RemoteFile.Kind + ) async throws { let sourceHostID = UUID() let targetHostID = UUID() let bundle = remoteFile(path: "/source/bundle", kind: .directory) @@ -908,7 +910,7 @@ struct SFTPFeatureTests { let sourceClient = TreeSFTPClient(listings: [bundle.path: [readme]]) let targetClient = ManifestSFTPClient(entries: [ "/incoming/bundle/README.md": remoteFile( - path: "/incoming/bundle/README.md", kind: .file, size: 3 + path: "/incoming/bundle/README.md", kind: kind, size: 3 ) ]) let coordinator = SFTPTransferCoordinator(store: HostLibraryModel()) { sourceClient } @@ -925,10 +927,78 @@ struct SFTPFeatureTests { await target.prepareRemoteCopy(payload, coordinator: coordinator) - #expect(target.pendingRemoteCopyBatch?.remoteDirectories == ["/incoming/bundle"]) - #expect(target.pendingRemoteCopyBatch?.jobs.first?.destinationPath == "/incoming/bundle/README.md") - #expect(target.pendingRemoteCopyBatch?.jobs.first?.hasConflict == true) + #expect(target.pendingRemoteCopyBatch?.directoryCount == 1) + #expect(target.pendingRemoteCopyBatch?.transferCount == 1) #expect(target.conflictCount == 1) + await target.enqueuePendingRemoteCopy(policy: policy, coordinator: coordinator) + #expect(!target.hasPendingBatch) + #expect(target.operationError == nil) + let name = policy == .keepBoth ? "README.2.md" : "README.md" + #expect(coordinator.sftpTransfers.map(\.remotePath) == (policy == .skip ? [] : ["/incoming/bundle/\(name)"])) + #expect(coordinator.sftpTransfers.allSatisfy { $0.direction == .copy && $0.localPath == readme.path }) + let existing = try await targetClient.attributes(at: "/incoming/bundle/README.md") + #expect(existing?.kind == (policy == .replace && kind == .directory ? nil : kind)) + } + + @Test(arguments: [TransferConflictPolicy.skip, .replace, .keepBoth]) + func remoteCopyDirectoryConflictsApplyToTheWholeSubtree(policy: TransferConflictPolicy) async throws { + 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 source = TreeSFTPClient(listings: [bundle.path: [nested], nested.path: [file]]) + let destination = ManifestSFTPClient( + entries: ["/incoming/bundle": remoteFile(path: "/incoming/bundle", kind: .file)], + rejectedAttributePrefix: policy == .replace ? nil : "/incoming/bundle/" + ) + let coordinator = SFTPTransferCoordinator(store: HostLibraryModel()) { source } + let payload = coordinator.registerRemoteCopySource( + SFTPRemoteCopySource(hostID: UUID(), configuration: testConfiguration, files: [bundle, sibling])) + let session = SFTPSessionModel( + hostID: UUID(), initialPath: "/incoming", client: destination, configuration: testConfiguration + ) + + await session.prepareRemoteCopy(payload, coordinator: coordinator) + #expect(session.conflictCount == 1) + await session.enqueuePendingRemoteCopy(policy: policy, coordinator: coordinator) + + #expect(session.operationError == nil) + #expect(!session.hasPendingBatch) + #expect(await destination.rejectedAttributeRequestCount == 0) + #expect(try await destination.attributes(at: "/incoming/bundle-other")?.isDirectory == true) + let directory = policy == .keepBoth ? "/incoming/bundle.2" : "/incoming/bundle" + #expect( + coordinator.sftpTransfers.map(\.remotePath) == (policy == .skip ? [] : [directory + "/nested/payload.txt"])) + if policy != .replace { #expect(try await destination.attributes(at: "/incoming/bundle")?.kind == .file) } + } + + @Test + func remoteCopyOfEmptyDirectoriesRefreshesTheBrowserAndHonorsPendingCancellation() async throws { + let empty = remoteFile(path: "/source/empty", kind: .directory) + let source = TreeSFTPClient(listings: [:]) + let destination = ManifestSFTPClient(entries: [:]) + let coordinator = SFTPTransferCoordinator(store: HostLibraryModel()) { source } + let payload = coordinator.registerRemoteCopySource( + SFTPRemoteCopySource(hostID: UUID(), configuration: testConfiguration, files: [empty])) + let session = SFTPSessionModel( + hostID: UUID(), initialPath: "/incoming", client: destination, configuration: testConfiguration + ) + + await session.prepareRemoteCopy(payload, coordinator: coordinator) + session.cancelPendingBatch() + await session.enqueuePendingRemoteCopy(policy: .replace, coordinator: coordinator) + #expect(await destination.createdDirectories.isEmpty) + + await session.prepareRemoteCopy(payload, coordinator: coordinator) + await session.enqueuePendingRemoteCopy(policy: .replace, coordinator: coordinator) + for _ in 0..<200 { + if !session.files.isEmpty { break } + try await Task.sleep(for: .milliseconds(10)) + } + #expect(session.files.map(\.path) == ["/incoming/empty"]) + #expect(session.operationError == nil) + #expect(!session.hasPendingBatch) + #expect(coordinator.sftpTransfers.isEmpty) } @Test @@ -1058,7 +1128,7 @@ private func makeTestConfiguration(hostID: UUID, host: String) -> SSHConnectionC ) } -private func remoteFile( +private nonisolated func remoteFile( path: String, kind: RemoteFile.Kind, size: Int64 = 0, modifiedAt: Date? = nil ) -> RemoteFile { RemoteFile( @@ -1211,7 +1281,7 @@ private actor BorrowedSFTPTransferClient: SFTPClientProtocol { } private actor ManifestSFTPClient: SFTPClientProtocol { - private let entries: [String: RemoteFile] + private var entries: [String: RemoteFile] private let rejectedAttributePrefix: String? private let home: String private let rejectsHome: Bool @@ -1239,7 +1309,9 @@ private actor ManifestSFTPClient: SFTPClientProtocol { if rejectsHome { throw SFTPError.operation("Home unavailable") } return home } - func listDirectory(at path: String) async throws -> [RemoteFile] { [] } + func listDirectory(at path: String) async throws -> [RemoteFile] { + entries.values.filter { ($0.path as NSString).deletingLastPathComponent == path } + } func attributes(at path: String) async throws -> RemoteFile? { if let rejectedAttributePrefix, path.hasPrefix(rejectedAttributePrefix) { rejectedAttributeRequestCount += 1 @@ -1248,10 +1320,13 @@ private actor ManifestSFTPClient: SFTPClientProtocol { return entries[path] } func setPermissions(at path: String, to permissions: UInt32) async throws {} - func createDirectory(at path: String) async throws { createdDirectories.append(path) } + func createDirectory(at path: String) async throws { + createdDirectories.append(path) + entries[path] = remoteFile(path: path, kind: .directory) + } func rename(from source: String, to destination: String, overwrite: Bool) async throws {} - func removeFile(at path: String) async throws {} - func removeDirectory(at path: String) async throws {} + func removeFile(at path: String) async throws { entries[path] = nil } + func removeDirectory(at path: String) async throws { entries[path] = nil } func uploadFile( from localURL: URL, to remotePath: String, overwrite: Bool, cancellation: TransferCancellationToken, diff --git a/TFMTests/SSHHandshakeDiagnosticsTests.swift b/TFMTests/SSHHandshakeDiagnosticsTests.swift index 20266043..7f84d43c 100644 --- a/TFMTests/SSHHandshakeDiagnosticsTests.swift +++ b/TFMTests/SSHHandshakeDiagnosticsTests.swift @@ -84,23 +84,12 @@ struct SSHHandshakeDiagnosticsTests { #expect(server.acceptedConnections == 1) } - @Test func handshakeFailureDescriptionsDistinguishBannerConditions() { + @Test func handshakeDiagnosticsRetainThePeerEndpoint() { let peer = "203.0.113.4:22" let disconnected = SSHHandshakeFailure( code: -13, systemError: nil, phase: .identification, elapsedMilliseconds: 18, peer: peer ) - let invalid = SSHHandshakeFailure( - code: -2, systemError: nil, phase: .identification, - elapsedMilliseconds: 7, peer: peer - ) - let timedOut = SSHHandshakeFailure( - code: -30, systemError: ETIMEDOUT, phase: .identification, - elapsedMilliseconds: 20_000, peer: peer - ) - - #expect(invalid.localizedDescription.contains("invalid SSH identification")) - #expect(timedOut.localizedDescription.contains("timed out")) #expect(disconnected.diagnosticDescription.contains("peer \(peer)")) } diff --git a/TFMTests/SSHImportTests.swift b/TFMTests/SSHImportTests.swift index 92d6fde6..a72cd127 100644 --- a/TFMTests/SSHImportTests.swift +++ b/TFMTests/SSHImportTests.swift @@ -533,7 +533,6 @@ struct SSHImportTests { #expect(window.vaultSelection(for: .knownHosts) == [.knownHost(entry.id)]) #expect(window.vaultPages.knownHosts.query == "key") #expect(window.findKind == .knownHosts) - #expect(VaultPage.knownHosts.symbol == "touchid") #expect(VaultPage.allCases.last == .knownHosts) } } diff --git a/TFMTests/WindowToolbarFindTests.swift b/TFMTests/WindowToolbarFindTests.swift index a4240198..aacb74bf 100644 --- a/TFMTests/WindowToolbarFindTests.swift +++ b/TFMTests/WindowToolbarFindTests.swift @@ -160,7 +160,6 @@ struct WindowToolbarFindTests { #expect(actions.last?.startsGroup == true) let menu = window.creationMenu(for: page, locale: Locale(identifier: "en")) #expect(menu[menu.count - 2].isSeparator) - #expect(menu.last?.title == "Import .ssh Folder") } }