diff --git a/TFI/Application/FeatureModels/MobileProviderEditorModel.swift b/TFI/Application/FeatureModels/MobileProviderEditorModel.swift index 895caefa..4ad4db06 100644 --- a/TFI/Application/FeatureModels/MobileProviderEditorModel.swift +++ b/TFI/Application/FeatureModels/MobileProviderEditorModel.swift @@ -12,12 +12,9 @@ final class MobileProviderEditorModel { let originalEndpoint: ProviderEndpoint var draft: ModelProvider var endpoint: ProviderEndpoint - let key = MobileSecretDraft() + let key = MobileSecretDraft(editingMode: .direct) let serviceAccount = MobileSecretDraft() - var keyAction: KeyAction { - get { key.action } - set { key.action = newValue } - } + var keyAction: KeyAction { key.action } var secret: String { get { key.text } set { key.text = newValue } diff --git a/TFI/Application/FeatureModels/MobileSecretDraft.swift b/TFI/Application/FeatureModels/MobileSecretDraft.swift index 70fe42c8..6b212053 100644 --- a/TFI/Application/FeatureModels/MobileSecretDraft.swift +++ b/TFI/Application/FeatureModels/MobileSecretDraft.swift @@ -7,21 +7,38 @@ import TermindPresentation @MainActor @Observable final class MobileSecretDraft { enum Action: String, CaseIterable { case preserve, replace, clear } + enum EditingMode { case explicitAction, direct } + let editingMode: EditingMode private var originalReference = "" private var originalSecret: String? private var readError: String? + init(editingMode: EditingMode = .explicitAction) { + self.editingMode = editingMode + } + func begin(in ai: AIConfigurationModel, reference: String) { originalReference = reference + originalSecret = nil + readError = nil if !reference.isEmpty { originalSecret = ai.secret(for: reference) readError = ai.secretStorageError } + if editingMode == .direct { text = originalSecret ?? "" } } var action = Action.preserve - var text = "" - var isDirty: Bool { action != .preserve || !text.isEmpty } + var text = "" { + didSet { + guard editingMode == .direct else { return } + action = !canEdit || text == (originalSecret ?? "") ? .preserve : text.isEmpty ? .clear : .replace + } + } + var canEdit: Bool { + editingMode != .direct || originalReference.isEmpty || (readError == nil && originalSecret != nil) + } + var isDirty: Bool { action != .preserve || (editingMode == .explicitAction && !text.isEmpty) } var isValid: Bool { action != .replace || !text.isEmpty } func effective(in ai: AIConfigurationModel, reference: String) throws -> String { @@ -45,8 +62,8 @@ final class MobileSecretDraft { } func discardPlaintext() { - text = "" originalSecret = nil + text = "" } func commit(in ai: AIConfigurationModel, reference: String, kind: String = "key", save: (String) -> Bool) throws { diff --git a/TFI/Features/Settings/MobileAISettingsView.swift b/TFI/Features/Settings/MobileAISettingsView.swift index 49414166..eb329a48 100644 --- a/TFI/Features/Settings/MobileAISettingsView.swift +++ b/TFI/Features/Settings/MobileAISettingsView.swift @@ -138,7 +138,16 @@ struct MobileSecretFields: View { var body: some View { Section(isToken ? AppStrings.Settings.bearerToken.swiftUIKey : AppStrings.Settings.apiKey.swiftUIKey) { - if hasExistingCredential { + if key.editingMode == .direct { + RevealableSecretField( + isToken ? AppStrings.Settings.bearerToken : AppStrings.Settings.apiKey, + text: $key.text, + prompt: Text(requiresCredential ? AppStrings.Vault.required : AppStrings.Common.optional) + ) + .textInputAutocapitalization(.never).autocorrectionDisabled() + .multilineTextAlignment(.leading) + .disabled(!key.canEdit) + } else if hasExistingCredential { Picker(AppStrings.Settings.credential.swiftUIKey, selection: $key.action) { Text(isToken ? AppStrings.Settings.keepExistingToken : AppStrings.Settings.keepExistingKey).tag( MobileSecretDraft.Action.preserve) diff --git a/TFITests/MobileAISettingsRenderingTests.swift b/TFITests/MobileAISettingsRenderingTests.swift index 34cde6a7..005e4868 100644 --- a/TFITests/MobileAISettingsRenderingTests.swift +++ b/TFITests/MobileAISettingsRenderingTests.swift @@ -12,6 +12,20 @@ import UIKit /// Uses the shipping views and a disposable in-memory app, without accounts or network requests. @Suite(.serialized) @MainActor struct MobileAISettingsRenderingTests { + @Test( + .enabled(if: ProcessInfo.processInfo.environment["TERMIND_AI_SCREENSHOTS"] != nil), + arguments: ["en", "zh-Hans", "zh-Hant"] + ) + func existingProviderKeyIsEditedDirectly(_ language: String) async throws { + let provider = ModelProvider( + name: "Model Provider", inferenceURL: "https://example.invalid/v1", credentialRef: "fixture-key") + let ai = AIConfigurationModel(modelProviders: [provider], persistsSecrets: false) + ai.setSecret("fixture-api-key", for: "fixture-key") + try await capture( + "provider-direct-key", language: language, + view: MobileProviderEditor(ai: ai, provider: provider).environment(\.locale, Locale(identifier: language))) + } + @Test(arguments: ["en", "zh-Hans", "zh-Hant"]) func anonymousProviderEditorShowsOptionalKey(_ language: String) async throws { let ai = AIConfigurationModel() @@ -202,6 +216,12 @@ struct MobileAISettingsRenderingTests { controller.view.setNeedsLayout() controller.view.layoutIfNeeded() try await Task.sleep(for: .milliseconds(700)) + if name == "provider-direct-key" { + let field = try #require( + firstSubview(of: UITextField.self, in: controller.view, matching: { $0.isSecureTextEntry })) + #expect(field.text == "fixture-api-key") + #expect(field.isEnabled) + } if scrollToBottom, let scrollView = firstSubview(of: UIScrollView.self, in: controller.view) { let offset = max( -scrollView.adjustedContentInset.top, @@ -232,10 +252,11 @@ struct MobileAISettingsRenderingTests { print("AI settings snapshot: \(directory.path)/\(device)-\(language)-\(name).png") } - private func firstSubview(of type: T.Type, in view: UIView) -> T? { - if let match = view as? T { return match } + private func firstSubview(of type: T.Type, in view: UIView, matching: (T) -> Bool = { _ in true }) -> T? + { + if let match = view as? T, matching(match) { return match } for subview in view.subviews { - if let match = firstSubview(of: type, in: subview) { return match } + if let match = firstSubview(of: type, in: subview, matching: matching) { return match } } return nil } diff --git a/TFITests/MobileAISettingsTests.swift b/TFITests/MobileAISettingsTests.swift index ea4e1ce5..cbd73e31 100644 --- a/TFITests/MobileAISettingsTests.swift +++ b/TFITests/MobileAISettingsTests.swift @@ -48,15 +48,103 @@ struct MobileAISettingsTests { #expect(!editor.canSave) } - @Test func missingStoredKeyRequiresExplicitClearBeforeAnonymousPreview() throws { + @Test(arguments: [false, true]) + func unavailableStoredKeyDisablesEditingAndPreservesCredential(_ failsToRead: Bool) throws { let provider = ModelProvider(name: "Missing", inferenceURL: "http://localhost/v1", credentialRef: "missing") - let ai = AIConfigurationModel(modelProviders: [provider], persistsSecrets: false) + let secrets = AIEditorTestSecrets() + secrets.failReads = failsToRead + let ai = AIConfigurationModel(modelProviders: [provider], aiSecrets: secrets) let editor = MobileProviderEditorModel(ai: ai, provider: provider) + #expect(!editor.key.canEdit) + #expect(editor.secret.isEmpty) + #expect(!editor.isDirty) #expect(throws: (any Error).self) { try editor.requestSecret() } - editor.keyAction = .clear - #expect(try editor.requestSecret() == "") + editor.draft.name = "Renamed" try editor.save() - #expect(ai.modelProvider(provider.id)?.credentialRef == "") + #expect(ai.modelProvider(provider.id)?.name == "Renamed") + #expect(ai.modelProvider(provider.id)?.credentialRef == "missing") + #expect(secrets.writeCount == 0) + } + + @Test func loadedKeyIsUnchangedAndMetadataSavePreservesANewerKey() throws { + var provider = provider() + provider.credentialRef = "key" + let secrets = AIEditorTestSecrets() + secrets.values["key"] = "original" + let ai = AIConfigurationModel(modelProviders: [provider], aiSecrets: secrets) + let editor = MobileProviderEditorModel(ai: ai, provider: provider) + + #expect(editor.key.canEdit) + #expect(editor.secret == "original") + #expect(!editor.isDirty) + #expect(editor.keyAction == .preserve) + secrets.values["key"] = "remote" + editor.draft.name = "Renamed" + try editor.save() + + #expect(ai.modelProvider(provider.id)?.name == "Renamed") + #expect(secrets.values["key"] == "remote") + #expect(secrets.writeCount == 0) + #expect(editor.secret.isEmpty) + } + + @Test(arguments: ["", "original"]) + func restoringOriginalKeyRemovesUnsavedChanges(_ original: String) { + var provider = provider() + provider.credentialRef = original.isEmpty ? "" : "key" + let secrets = AIEditorTestSecrets() + if !original.isEmpty { secrets.values["key"] = original } + let ai = AIConfigurationModel(modelProviders: [provider], aiSecrets: secrets) + let editor = MobileProviderEditorModel(ai: ai, provider: provider) + + editor.secret = "replacement" + #expect(editor.isDirty) + #expect(editor.keyAction == .replace) + editor.secret = original + #expect(!editor.isDirty) + #expect(editor.keyAction == .preserve) + #expect(secrets.writeCount == 0) + } + + @Test(arguments: ["replacement", ""]) + func editedKeyIsUsedForRequestsAndOnlyWrittenOnSave(_ replacement: String) throws { + var provider = provider() + provider.credentialRef = "key" + let secrets = AIEditorTestSecrets() + secrets.values["key"] = "original" + let ai = AIConfigurationModel(modelProviders: [provider], aiSecrets: secrets) + let editor = MobileProviderEditorModel(ai: ai, provider: provider) + + editor.secret = replacement + #expect(editor.isDirty) + #expect(editor.canSave) + #expect(editor.keyAction == (replacement.isEmpty ? .clear : .replace)) + #expect(try editor.requestSecret() == replacement) + #expect(secrets.values["key"] == "original") + #expect(secrets.writeCount == 0) + try editor.save() + + #expect(secrets.values["key"] == (replacement.isEmpty ? nil : replacement)) + #expect(ai.modelProvider(provider.id)?.credentialRef == (replacement.isEmpty ? "" : "key")) + #expect(editor.secret.isEmpty) + } + + @Test func discardingKeyEditsClearsPlaintextWithoutWritingCredentials() { + var provider = provider() + provider.credentialRef = "key" + let secrets = AIEditorTestSecrets() + secrets.values["key"] = "original" + let ai = AIConfigurationModel(modelProviders: [provider], aiSecrets: secrets) + let editor = MobileProviderEditorModel(ai: ai, provider: provider) + editor.secret = "replacement" + + editor.discardPlaintext() + + #expect(editor.secret.isEmpty) + #expect(editor.keyAction == .preserve) + #expect(secrets.values["key"] == "original") + #expect(secrets.writeCount == 0) + #expect(ai.modelProvider(provider.id) == provider) } private func provider() -> ModelProvider { @@ -146,12 +234,11 @@ struct MobileAISettingsTests { #expect(try editor.requestSecret() == "") editor.endpoint.url = "https://draft.invalid/v1" editor.endpoint.apiType = .responses - editor.keyAction = .replace editor.secret = "draft-key" #expect(editor.requestProvider.inferenceURL == "https://draft.invalid/v1/responses") #expect(try editor.requestSecret() == "draft-key") #expect(ai.modelProviders.isEmpty) - editor.keyAction = .clear + editor.secret = "" #expect(try editor.requestSecret() == "") } @@ -321,7 +408,6 @@ struct MobileAISettingsTests { secrets.values["shared-key"] = "original" let ai = AIConfigurationModel(modelProviders: [provider], aiSecrets: secrets) let editor = MobileProviderEditorModel(ai: ai, provider: provider) - editor.keyAction = .replace editor.secret = "local" secrets.values["shared-key"] = "remote" #expect(throws: SettingsDraftMerge.Conflict.self) { try editor.save() } @@ -431,8 +517,16 @@ private actor AIRequestLatch { @MainActor private final class AIEditorTestSecrets: AISecretStoring { var values: [String: String] = [:] - func string(for reference: String) throws -> String? { values[reference] } - func setString(_ value: String?, for reference: String) throws { values[reference] = value } + var failReads = false + var writeCount = 0 + func string(for reference: String) throws -> String? { + if failReads { throw CocoaError(.fileReadNoPermission) } + return values[reference] + } + func setString(_ value: String?, for reference: String) throws { + writeCount += 1 + values[reference] = value + } } @MainActor private final class AIEditorTestPersistence: AIConfigurationPersisting { diff --git a/TFITests/MobileSettingsPortTests.swift b/TFITests/MobileSettingsPortTests.swift index 642a631f..45e8cb0a 100644 --- a/TFITests/MobileSettingsPortTests.swift +++ b/TFITests/MobileSettingsPortTests.swift @@ -62,7 +62,6 @@ struct MobileSettingsPortTests { let editor = MobileProviderEditorModel(ai: ai, provider: ModelProvider(name: "Gateway")) editor.endpoint.url = "https://example.invalid/v1" editor.endpoint.apiType = .responses - editor.keyAction = .replace editor.secret = "test-key" try editor.save() let saved = try #require(ai.modelProvider(editor.original.id)) @@ -78,7 +77,6 @@ struct MobileSettingsPortTests { let ai = AIConfigurationModel(aiSecrets: secrets) let editor = MobileProviderEditorModel(ai: ai, provider: ModelProvider(name: "Gateway")) editor.endpoint.url = "https://example.invalid/v1" - editor.keyAction = .replace editor.secret = "test-key" #expect(throws: (any Error).self) { try editor.save() } #expect(ai.modelProviders.isEmpty) @@ -95,7 +93,6 @@ struct MobileSettingsPortTests { let ai = AIConfigurationModel(persistence: persistence, aiSecrets: secrets) let editor = MobileProviderEditorModel(ai: ai, provider: provider) editor.draft.name = "After" - editor.keyAction = .replace editor.secret = "after" persistence.failWrites = true #expect(throws: (any Error).self) { try editor.save() } @@ -112,7 +109,7 @@ struct MobileSettingsPortTests { let ai = AIConfigurationModel(modelProviders: [provider], aiSecrets: secrets) let editor = MobileProviderEditorModel(ai: ai, provider: provider) ai.addModel(AIModel(id: "new-model"), to: provider.id) - editor.keyAction = .clear + editor.secret = "" try editor.save() #expect(ai.modelProvider(provider.id)?.models.map(\.id) == ["new-model"]) #expect(ai.modelProvider(provider.id)?.credentialRef == "")