From d3145bcccf34608961560a61b996004d4ddc6cf5 Mon Sep 17 00:00:00 2001 From: Kaede Akino Date: Mon, 14 Sep 2026 10:09:06 +0800 Subject: [PATCH] Fix Assistant approval interaction --- ...AssistantTranscriptPresentationTests.swift | 4 +- Frameworks/Resources/Localizable.xcstrings | 34 +++++- .../Agent/ConversationModel.swift | 4 +- .../AgentToolPresentation.swift | 55 ++++++++++ .../AssistantToolDetails.swift | 75 ++++++++++++- .../Agent/AgentBuiltInSFTPTools.swift | 102 +++++++++++++++++- .../Agent/AgentBuiltInSSHTools.swift | 32 +++++- .../Agent/AgentScratchTransferTools.swift | 22 +++- Memory/ARCHITECTURE.md | 5 +- .../AI/MobileAssistantInteraction.swift | 101 +++++++++++++---- TFITests/MobileAssistantTests.swift | 2 +- .../AI/ConversationTranscriptView.swift | 70 +++++++++--- TFM/Features/Assistant/AssistantView.swift | 24 ++++- TFMTests/AgentApprovalQueueTests.swift | 26 ++++- TFMTests/AgentToolPresentationTests.swift | 81 ++++++++++++++ TFMTests/ChatComposerStateTests.swift | 19 ++++ Termind.xcodeproj/project.pbxproj | 2 +- .../xcshareddata/swiftpm/Package.resolved | 4 +- 18 files changed, 601 insertions(+), 61 deletions(-) diff --git a/FrameworkTests/AssistantTranscriptPresentationTests.swift b/FrameworkTests/AssistantTranscriptPresentationTests.swift index 9994decf..a7eae2db 100644 --- a/FrameworkTests/AssistantTranscriptPresentationTests.swift +++ b/FrameworkTests/AssistantTranscriptPresentationTests.swift @@ -130,10 +130,10 @@ struct AssistantTranscriptPresentationTests { let request = AgentApprovalRequest( invocation: value.invocation, descriptor: value.descriptor, userIntent: "Inspect") let answer = Task { await conversation.requestApproval(request) } - defer { conversation.respondToApproval(allow: false) } + defer { conversation.respondToApproval(request.id, allow: false) } for _ in 0..<100 where conversation.pendingApprovals.isEmpty { await Task.yield() } _ = try #require(conversation.pendingApprovals.first) - conversation.respondToApproval(allow: false) + conversation.respondToApproval(request.id, allow: false) _ = await answer.value conversation.handle(.toolProposed(value.invocation, value.descriptor)) conversation.handle(.approvalRequested(request)) diff --git a/Frameworks/Resources/Localizable.xcstrings b/Frameworks/Resources/Localizable.xcstrings index 264a57dc..803c2037 100644 --- a/Frameworks/Resources/Localizable.xcstrings +++ b/Frameworks/Resources/Localizable.xcstrings @@ -2968,6 +2968,22 @@ } } }, + "Arguments" : { + "localizations" : { + "zh-Hans" : { + "stringUnit" : { + "state" : "translated", + "value" : "参数" + } + }, + "zh-Hant" : { + "stringUnit" : { + "state" : "translated", + "value" : "參數" + } + } + } + }, "Archive" : { "localizations" : { "zh-Hans" : { @@ -19038,6 +19054,22 @@ } } }, + "Safety details" : { + "localizations" : { + "zh-Hans" : { + "stringUnit" : { + "state" : "translated", + "value" : "安全详情" + } + }, + "zh-Hant" : { + "stringUnit" : { + "state" : "translated", + "value" : "安全詳情" + } + } + } + }, "Sampled at" : { "extractionState" : "stale", "localizations" : { @@ -26853,4 +26885,4 @@ } }, "version" : "1.2" -} \ No newline at end of file +} diff --git a/Frameworks/TermindApplication/Agent/ConversationModel.swift b/Frameworks/TermindApplication/Agent/ConversationModel.swift index 23767d89..91c56006 100644 --- a/Frameworks/TermindApplication/Agent/ConversationModel.swift +++ b/Frameworks/TermindApplication/Agent/ConversationModel.swift @@ -1942,8 +1942,8 @@ public final class ConversationModel: Identifiable { return await withCheckedContinuation { approvalWaiters[request.id] = $0 } } - public func respondToApproval(allow: Bool) { - guard !pendingApprovals.isEmpty else { return } + public func respondToApproval(_ requestID: UUID, allow: Bool) { + guard pendingApprovals.first?.id == requestID else { return } let request = pendingApprovals.removeFirst() let decision: AgentApprovalDecision = allow diff --git a/Frameworks/TermindPresentation/AgentToolPresentation.swift b/Frameworks/TermindPresentation/AgentToolPresentation.swift index 40dc2d3f..2a0d3d65 100644 --- a/Frameworks/TermindPresentation/AgentToolPresentation.swift +++ b/Frameworks/TermindPresentation/AgentToolPresentation.swift @@ -23,6 +23,34 @@ nonisolated public enum AgentToolPresentation { return card.descriptor.presentation?.symbol ?? "wrench.and.screwdriver" } + /// The live, user-only projection of a call waiting for approval. + /// + /// Approval is the one place where the person needs to inspect exactly + /// what will be sent. Its arguments therefore remain unredacted here; the + /// projection is derived from the in-memory request and is never attached + /// to the durable tool card or model transcript. + public static func approval( + for request: AgentApprovalRequest, + presenter: AgentToolDetailPresenter? = nil, + hostName: String? = nil, + locale: Locale = .current + ) -> AgentApprovalPresentation { + let card = AgentToolCard( + invocation: request.invocation, + descriptor: request.descriptor, + state: .waitingForApproval + ) + return AgentApprovalPresentation( + symbol: symbol(for: card), + title: summary(for: card, hostName: hostName, locale: locale), + detail: AgentToolDetail( + items: approvalArgumentItems( + request.invocation.call.arguments, presenter: presenter, + hidesHostID: hostName != nil, locale: locale) + ) + ) + } + /// While work is live, use the icon of the tool(s) still in flight. After /// completion, a homogeneous batch keeps its tool icon; mixed batches use /// the generic tools symbol. @@ -278,6 +306,33 @@ nonisolated public enum AgentToolPresentation { ) } + private static func approvalArgumentItems( + _ arguments: AgentJSONValue, + presenter: AgentToolDetailPresenter?, + hidesHostID: Bool, + locale: Locale + ) -> [AgentToolDetail.Item] { + guard let object = arguments.objectValue else { + return [ + .text( + .init( + title: localized("Arguments", locale: locale), + text: arguments.encodedString, + style: .monospaced + )) + ] + } + + var visible = object + if hidesHostID { visible.removeValue(forKey: "host_id") } + if let projected = presenter?.presentArguments( + .init(arguments: visible, locale: locale) + ) { + return projected + } + return AgentToolDetailFormatting.genericItems(.object(visible), locale: locale) + } + private static func renderedPreview( _ full: String, headEnd: String.Index, tailStart: String.Index ) -> String { diff --git a/Frameworks/TermindPresentation/AssistantToolDetails.swift b/Frameworks/TermindPresentation/AssistantToolDetails.swift index caf20b24..691ee6d1 100644 --- a/Frameworks/TermindPresentation/AssistantToolDetails.swift +++ b/Frameworks/TermindPresentation/AssistantToolDetails.swift @@ -5,9 +5,16 @@ public struct AssistantToolDetails: View { private let items: [AgentToolDetail.Item] private let isError: Bool private let copy: ((String) -> Void)? - public init(items: [AgentToolDetail.Item], isError: Bool = false, copy: ((String) -> Void)? = nil) { + private let shortensText: Bool + public init( + items: [AgentToolDetail.Item], + isError: Bool = false, + shortensText: Bool = true, + copy: ((String) -> Void)? = nil + ) { self.items = items self.isError = isError + self.shortensText = shortensText self.copy = copy } public var body: some View { @@ -31,12 +38,13 @@ public struct AssistantToolDetails: View { case .message(let text, let tint): Text(text).foregroundStyle(color(tint)) case .text(let block): if let title = block.title { Text(title).font(.caption.weight(.medium)).foregroundStyle(.secondary) } - let preview = AgentToolPresentation.preview(block.text) + let preview = shortensText ? AgentToolPresentation.preview(block.text) : nil ScrollView(.horizontal) { - Text(preview.displayed).font(block.style == .monospaced ? .caption.monospaced() : .caption) + Text(preview?.displayed ?? block.text) + .font(block.style == .monospaced ? .caption.monospaced() : .caption) .foregroundStyle(isError ? Color.red : .secondary) } - if preview.isShortened { + if let preview, preview.isShortened { HStack { Text("Output shortened for display").font(.caption2).foregroundStyle(.secondary) if let copy { @@ -90,3 +98,62 @@ public struct AssistantToolDetails: View { } } } + +/// The shared body of the macOS and mobile approval cards. +/// Tool arguments remain fully visible; only the safety explanation is folded. +public struct AssistantApprovalDetails: View { + private let detail: AgentToolDetail + private let reasons: [String] + @Binding private var showsSafetyDetails: Bool + + public init( + detail: AgentToolDetail, + reasons: [String], + showsSafetyDetails: Binding + ) { + self.detail = detail + self.reasons = reasons + _showsSafetyDetails = showsSafetyDetails + } + + public var body: some View { + VStack(alignment: .leading, spacing: 10) { + AssistantToolDetails(items: detail.items, shortensText: false) + if !reasons.isEmpty { + DisclosureGroup(isExpanded: $showsSafetyDetails) { + VStack(alignment: .leading, spacing: 4) { + ForEach(reasons, id: \.self) { reason in + Text(reason) + .font(.caption) + .foregroundStyle(.orange) + .textSelection(.enabled) + } + } + .padding(.top, 4) + } label: { + HStack(spacing: 6) { + Label("Safety details", systemImage: "exclamationmark.triangle") + Text(reasons.count, format: .number) + .monospacedDigit() + .foregroundStyle(.secondary) + } + .font(.caption.weight(.medium)) + } + .accessibilityIdentifier("AssistantApproval.SafetyDetails") + } + } + .accessibilityIdentifier("AssistantApproval.Details") + } +} + +public nonisolated struct AgentApprovalPresentation: Equatable, Sendable { + public let symbol: String + public let title: String + public let detail: AgentToolDetail + + public init(symbol: String, title: String, detail: AgentToolDetail) { + self.symbol = symbol + self.title = title + self.detail = detail + } +} diff --git a/Frameworks/TermindRemoteInfrastructure/Agent/AgentBuiltInSFTPTools.swift b/Frameworks/TermindRemoteInfrastructure/Agent/AgentBuiltInSFTPTools.swift index 787d7455..0d530672 100644 --- a/Frameworks/TermindRemoteInfrastructure/Agent/AgentBuiltInSFTPTools.swift +++ b/Frameworks/TermindRemoteInfrastructure/Agent/AgentBuiltInSFTPTools.swift @@ -161,7 +161,8 @@ nonisolated public struct SFTPWriteTool: SFTPToolDefinition { public static let extraProperties = ["content": string(max: 131_072), "overwrite": boolean()] public static let extraRequired = ["content"] public static let presenter = AgentToolDetailPresenter( - id: "builtin.sftp_write", present: present + id: "builtin.sftp_write", present: present, + presentArguments: presentArguments ) public static let presentation = AgentToolDescriptor.Presentation( symbol: "square.and.pencil", @@ -191,7 +192,8 @@ nonisolated public struct SFTPMkdirTool: SFTPToolDefinition { public static let summary = "Create a remote directory." public static let safety: AgentToolDescriptor.Safety = .requiresAuthorization public static let presenter = AgentToolDetailPresenter( - id: "builtin.sftp_mkdir", present: present + id: "builtin.sftp_mkdir", present: present, + presentArguments: presentArguments ) public static let presentation = AgentToolDescriptor.Presentation( symbol: "folder.badge.plus", @@ -221,7 +223,8 @@ nonisolated public struct SFTPMoveTool: SFTPToolDefinition { public static let extraProperties = ["destination": string(max: 4_096), "overwrite": boolean()] public static let extraRequired = ["destination"] public static let presenter = AgentToolDetailPresenter( - id: "builtin.sftp_move", present: present + id: "builtin.sftp_move", present: present, + presentArguments: presentArguments ) public static let presentation = AgentToolDescriptor.Presentation( symbol: "arrow.right", activity: .semanticArgument(key: "path", fallback: .remoteFile), @@ -257,7 +260,8 @@ nonisolated public struct SFTPChmodTool: SFTPToolDefinition { public static let extraProperties = ["mode": string(max: 4)] public static let extraRequired = ["mode"] public static let presenter = AgentToolDetailPresenter( - id: "builtin.sftp_chmod", present: present + id: "builtin.sftp_chmod", present: present, + presentArguments: presentArguments ) public static let presentation = AgentToolDescriptor.Presentation( symbol: "lock", activity: .semanticArgument(key: "path", fallback: .remoteFile), @@ -292,7 +296,8 @@ nonisolated public struct SFTPDeleteTool: SFTPToolDefinition { public static let extraProperties = ["kind": enumeration(["file", "directory"])] public static let extraRequired = ["kind"] public static let presenter = AgentToolDetailPresenter( - id: "builtin.sftp_delete", present: present + id: "builtin.sftp_delete", present: present, + presentArguments: presentArguments ) public static let presentation = AgentToolDescriptor.Presentation( symbol: "trash", activity: .semanticArgument(key: "path", fallback: .remoteFile), @@ -403,6 +408,20 @@ nonisolated extension SFTPWriteTool { return items } } + + public static func presentArguments( + _ input: AgentToolArgumentDetailInput + ) -> [AgentToolDetail.Item] { + typealias F = AgentToolDetailFormatting + var items = SFTPToolPresentation.pathArguments(input) + if let content = input.arguments["content"]?.stringValue { + items.append(F.text("Content", content, locale: input.locale)) + } + SFTPToolPresentation.appendBoolean( + &items, label: "Overwrite", value: input.arguments["overwrite"], + locale: input.locale) + return items + } } nonisolated extension SFTPMkdirTool { public static func present(_ input: AgentToolDetailInput) -> [AgentToolDetail.Item] { @@ -413,6 +432,12 @@ nonisolated extension SFTPMkdirTool { ) ) } + + public static func presentArguments( + _ input: AgentToolArgumentDetailInput + ) -> [AgentToolDetail.Item] { + SFTPToolPresentation.pathArguments(input) + } } nonisolated extension SFTPMoveTool { public static func present(_ input: AgentToolDetailInput) -> [AgentToolDetail.Item] { @@ -428,6 +453,23 @@ nonisolated extension SFTPMoveTool { return items } } + + public static func presentArguments( + _ input: AgentToolArgumentDetailInput + ) -> [AgentToolDetail.Item] { + typealias F = AgentToolDetailFormatting + var items: [AgentToolDetail.Item] = [] + F.appendField( + &items, "Source", input.arguments["path"]?.stringValue, + locale: input.locale, monospaced: true) + F.appendField( + &items, "Destination", input.arguments["destination"]?.stringValue, + locale: input.locale, monospaced: true) + SFTPToolPresentation.appendBoolean( + &items, label: "Overwrite", value: input.arguments["overwrite"], + locale: input.locale) + return items + } } nonisolated extension SFTPChmodTool { public static func present(_ input: AgentToolDetailInput) -> [AgentToolDetail.Item] { @@ -443,6 +485,17 @@ nonisolated extension SFTPChmodTool { return items } } + + public static func presentArguments( + _ input: AgentToolArgumentDetailInput + ) -> [AgentToolDetail.Item] { + typealias F = AgentToolDetailFormatting + var items = SFTPToolPresentation.pathArguments(input) + F.appendField( + &items, "Permissions", input.arguments["mode"]?.stringValue, + locale: input.locale, monospaced: true) + return items + } } nonisolated extension SFTPDeleteTool { public static func present(_ input: AgentToolDetailInput) -> [AgentToolDetail.Item] { @@ -464,9 +517,48 @@ nonisolated extension SFTPDeleteTool { return items } } + + public static func presentArguments( + _ input: AgentToolArgumentDetailInput + ) -> [AgentToolDetail.Item] { + typealias F = AgentToolDetailFormatting + var items = SFTPToolPresentation.pathArguments(input) + F.appendField( + &items, "Kind", + input.arguments["kind"]?.stringValue.map { + F.kindLabel($0, locale: input.locale) + }, locale: input.locale) + return items + } } nonisolated public enum SFTPToolPresentation { + public static func pathArguments( + _ input: AgentToolArgumentDetailInput + ) -> [AgentToolDetail.Item] { + typealias F = AgentToolDetailFormatting + var items: [AgentToolDetail.Item] = [] + F.appendField( + &items, "Path", input.arguments["path"]?.stringValue, + locale: input.locale, monospaced: true) + return items + } + + public static func appendBoolean( + _ items: inout [AgentToolDetail.Item], + label: String.LocalizationValue, + value: AgentJSONValue?, + locale: Locale + ) { + guard let value = value?.boolValue else { return } + items.append( + AgentToolDetailFormatting.field( + label, + AgentToolDetailFormatting.localized(value ? "Yes" : "No", locale: locale), + locale: locale + )) + } + public static func remoteFileRow( _ value: AgentJSONValue, locale: Locale ) -> AgentToolDetail.ListRow? { diff --git a/Frameworks/TermindRemoteInfrastructure/Agent/AgentBuiltInSSHTools.swift b/Frameworks/TermindRemoteInfrastructure/Agent/AgentBuiltInSSHTools.swift index 238006e8..29646e82 100644 --- a/Frameworks/TermindRemoteInfrastructure/Agent/AgentBuiltInSSHTools.swift +++ b/Frameworks/TermindRemoteInfrastructure/Agent/AgentBuiltInSSHTools.swift @@ -230,7 +230,10 @@ nonisolated extension AgentToolSupport { } nonisolated public struct SSHExecTool: AgentToolDefinition, AgentToolSchemaBuilding { - public static let presenter = AgentToolDetailPresenter(id: "builtin.ssh_exec", present: present) + public static let presenter = AgentToolDetailPresenter( + id: "builtin.ssh_exec", present: present, + presentArguments: presentArguments + ) public let authentication: AgentSSHAuthenticationRegistry public let pool: AgentConnectionPool public let background: AgentBackgroundCommands @@ -475,6 +478,33 @@ nonisolated extension SSHExecTool { public static func present(_ input: AgentToolDetailInput) -> [AgentToolDetail.Item] { SSHCommandPresentation.present(input) } + + public static func presentArguments( + _ input: AgentToolArgumentDetailInput + ) -> [AgentToolDetail.Item] { + typealias F = AgentToolDetailFormatting + var items: [AgentToolDetail.Item] = [] + if let command = input.arguments["command"]?.stringValue { + items.append(F.text("Command", command, locale: input.locale)) + } + F.appendField( + &items, "Working directory", input.arguments["cwd"]?.stringValue, + locale: input.locale, monospaced: true) + if let seconds = input.arguments["timeout_seconds"]?.integerValue { + items.append( + F.field("Timeout", "\(seconds)s", locale: input.locale, monospaced: true)) + } + if let standardInput = input.arguments["stdin"]?.stringValue { + items.append(F.text("Input", standardInput, locale: input.locale)) + } + F.appendField( + &items, "Secret handle", input.arguments["secret_handle"]?.stringValue, + locale: input.locale, monospaced: true) + F.appendField( + &items, "Purpose", input.arguments["stdin_purpose"]?.stringValue, + locale: input.locale) + return items + } } nonisolated public enum SSHCommandPresentation { diff --git a/Frameworks/TermindRemoteInfrastructure/Agent/AgentScratchTransferTools.swift b/Frameworks/TermindRemoteInfrastructure/Agent/AgentScratchTransferTools.swift index 59077852..63789837 100644 --- a/Frameworks/TermindRemoteInfrastructure/Agent/AgentScratchTransferTools.swift +++ b/Frameworks/TermindRemoteInfrastructure/Agent/AgentScratchTransferTools.swift @@ -81,7 +81,10 @@ nonisolated public struct ScratchPullTool: AgentToolDefinition, AgentToolSchemaB } nonisolated public struct ScratchPushTool: AgentToolDefinition, AgentToolSchemaBuilding { - public static let presenter = AgentToolDetailPresenter(id: "builtin.scratch_push", present: present) + public static let presenter = AgentToolDetailPresenter( + id: "builtin.scratch_push", present: present, + presentArguments: presentArguments + ) public let workspace: AgentScratchWorkspace public let pool: AgentConnectionPool public let authentication: AgentSSHAuthenticationRegistry @@ -221,4 +224,21 @@ nonisolated extension ScratchPushTool { return items } } + + public static func presentArguments( + _ input: AgentToolArgumentDetailInput + ) -> [AgentToolDetail.Item] { + typealias F = AgentToolDetailFormatting + var items: [AgentToolDetail.Item] = [] + F.appendField( + &items, "Scratch path", input.arguments["scratch_path"]?.stringValue, + locale: input.locale, monospaced: true) + F.appendField( + &items, "Remote path", input.arguments["remote_path"]?.stringValue, + locale: input.locale, monospaced: true) + SFTPToolPresentation.appendBoolean( + &items, label: "Overwrite", value: input.arguments["overwrite"], + locale: input.locale) + return items + } } diff --git a/Memory/ARCHITECTURE.md b/Memory/ARCHITECTURE.md index cacc3820..95070b74 100644 --- a/Memory/ARCHITECTURE.md +++ b/Memory/ARCHITECTURE.md @@ -188,7 +188,10 @@ library only after confirmation. Tools never receive the interactive PTY. SSH exec uses a dedicated channel and file operations use run-scoped SFTP. User-entered secrets are single-use handles bound to run/tool/host/purpose; plaintext never enters model input, reviews, events, logs, or -SwiftData. Fetch permits proven public HTTP(S) reads, revalidates DNS on every +SwiftData. A live approval is the deliberate user-review boundary: while it is pending, +the local UI projects the invocation's original arguments through the registered tool +presenter so the person can inspect exactly what will run. That projection is neither +recorded nor reused after the decision. Fetch permits proven public HTTP(S) reads, revalidates DNS on every redirect, reauthorizes entry into a restricted origin, follows explicit headers like `curl -H -L`, and redacts sensitive values. Remote MCP metadata can tighten but never relax local safety. diff --git a/TFI/Features/AI/MobileAssistantInteraction.swift b/TFI/Features/AI/MobileAssistantInteraction.swift index aee89f06..d200aa28 100644 --- a/TFI/Features/AI/MobileAssistantInteraction.swift +++ b/TFI/Features/AI/MobileAssistantInteraction.swift @@ -2,11 +2,14 @@ import AgentKit import SwiftUI import TermindApplication import TermindPresentation +import TermindRemoteInfrastructure struct MobileAssistantInteraction: View { @Bindable var conversation: ConversationModel let hosts: HostLibraryModel @FocusState private var focusedInput: InputFocus? + @Environment(\.locale) private var locale + @State private var expandedApprovalID: UUID? private enum InputFocus: Hashable { case text(String) @@ -19,25 +22,7 @@ struct MobileAssistantInteraction: View { if let approval = conversation.pendingApproval, conversation.pendingUserInteraction.map({ approval.invocation.createdAt <= $0.createdAt }) ?? true { - MobileAssistantAdaptiveHeight { - VStack(alignment: .leading, spacing: 8) { - Text("Approval Required").font(.headline) - Text(approval.descriptor.name).font(.callout.monospaced()) - Text(approval.invocation.call.arguments.displayText).font(.caption.monospaced()).textSelection( - .enabled - ) - .frame(maxWidth: .infinity, alignment: .leading) - ForEach(approval.localReasons, id: \.self) { Text($0).font(.caption) } - HStack { - Button("Deny", systemImage: "xmark", role: .cancel) { - conversation.respondToApproval(allow: false) - } - Spacer() - Button("Allow", systemImage: "checkmark") { conversation.respondToApproval(allow: true) } - .buttonStyle(.borderedProminent) - } - } - } + approvalCard(approval) } else if let interaction = conversation.pendingUserInteraction { switch interaction { case .sshAuthentication(let request): @@ -54,6 +39,84 @@ struct MobileAssistantInteraction: View { } } + private func approvalCard(_ approval: AgentApprovalRequest) -> some View { + let presentation = AgentToolPresentation.approval( + for: approval, + presenter: TermindAgentTools.presenter(for: approval.descriptor), + hostName: approval.invocation.targetHostID.map(hostName), + locale: locale + ) + return VStack(alignment: .leading, spacing: 12) { + HStack { + Label("Approval required", systemImage: "hand.raised.fill") + .font(.headline) + Spacer() + if conversation.pendingApprovals.count > 1 { + Text("1 of \(conversation.pendingApprovals.count)") + .font(.caption.monospacedDigit()) + .foregroundStyle(.secondary) + } + } + + HStack(alignment: .firstTextBaseline, spacing: 8) { + Image(systemName: presentation.symbol) + .foregroundStyle(.orange) + .accessibilityHidden(true) + Text(presentation.title) + .font(.callout.weight(.medium)) + .fixedSize(horizontal: false, vertical: true) + } + + ViewThatFits(in: .vertical) { + approvalDetails(presentation, approval: approval) + .fixedSize(horizontal: false, vertical: true) + ScrollView { + approvalDetails(presentation, approval: approval) + .frame(maxWidth: .infinity, alignment: .leading) + } + .scrollBounceBehavior(.basedOnSize) + } + + HStack { + Button("Deny", systemImage: "xmark", role: .cancel) { + conversation.respondToApproval(approval.id, allow: false) + } + .accessibilityIdentifier("AssistantApproval.Deny") + Spacer() + Button("Allow Once", systemImage: "checkmark") { + conversation.respondToApproval(approval.id, allow: true) + } + .buttonStyle(.borderedProminent) + .accessibilityIdentifier("AssistantApproval.AllowOnce") + } + } + .frame(maxWidth: .infinity, alignment: .topLeading) + .accessibilityElement(children: .contain) + .accessibilityIdentifier("AssistantApproval.Card") + } + + private func approvalDetails( + _ presentation: AgentApprovalPresentation, + approval: AgentApprovalRequest + ) -> some View { + AssistantApprovalDetails( + detail: presentation.detail, + reasons: approval.localReasons, + showsSafetyDetails: safetyDetailsBinding(for: approval.id) + ) + } + + private func safetyDetailsBinding(for approvalID: UUID) -> Binding { + Binding( + get: { expandedApprovalID == approvalID }, + set: { expandedApprovalID = $0 ? approvalID : nil } + ) + } + + private func hostName(_ id: UUID) -> String { + hosts.host(id)?.name ?? String(id.uuidString.prefix(8)) + } + private var questions: some View { VStack(alignment: .leading, spacing: 12) { if conversation.pendingUserInputs.count > 1 { diff --git a/TFITests/MobileAssistantTests.swift b/TFITests/MobileAssistantTests.swift index 6ba35140..8f14b31a 100644 --- a/TFITests/MobileAssistantTests.swift +++ b/TFITests/MobileAssistantTests.swift @@ -99,7 +99,7 @@ struct MobileAssistantTests { ) #expect(abs(height - availableHeight) < 0.5) - conversation.respondToApproval(allow: false) + conversation.respondToApproval(request.id, allow: false) if case .deny = await approval.value { } else { Issue.record("Expected the approval request to be denied during cleanup") diff --git a/TFM/Features/AI/ConversationTranscriptView.swift b/TFM/Features/AI/ConversationTranscriptView.swift index b706f061..ae6fa0b3 100644 --- a/TFM/Features/AI/ConversationTranscriptView.swift +++ b/TFM/Features/AI/ConversationTranscriptView.swift @@ -3,6 +3,7 @@ import SwiftUI import TermindApplication import TermindDomain import TermindPresentation +import TermindRemoteInfrastructure import struct TermindDomain.Host @@ -525,37 +526,76 @@ private struct AgentApprovalCard: View { let request: AgentApprovalRequest let conversation: ConversationModel + @Environment(AppModel.self) private var app + @Environment(\.locale) private var locale + @State private var showsSafetyDetails = false + var body: some View { - VStack(alignment: .leading, spacing: Metrics.Space.s) { + let presentation = AgentToolPresentation.approval( + for: request, + presenter: TermindAgentTools.presenter(for: request.descriptor), + hostName: request.invocation.targetHostID.map(hostName), + locale: locale + ) + return VStack(alignment: .leading, spacing: Metrics.Space.m) { HStack { Label("Approval required", systemImage: "hand.raised.fill").font(.headline) Spacer() QueueDepthLabel(remaining: conversation.pendingApprovals.count) } - Text(request.descriptor.qualifiedName).font(.callout.weight(.semibold)) - if let host = request.invocation.targetHostID { - Text("Host: \(host.uuidString)").font(.caption).foregroundStyle(.secondary) + + HStack(alignment: .firstTextBaseline, spacing: Metrics.Space.s) { + Image(systemName: presentation.symbol) + .foregroundStyle(.orange) + .accessibilityHidden(true) + Text(presentation.title) + .font(.callout.weight(.medium)) + .fixedSize(horizontal: false, vertical: true) } - Text(AgentSensitiveDataRedactor.redactedJSON(request.invocation.call.arguments).encodedString) - .font(.caption.monospaced()).textSelection(.enabled) - ForEach(request.localReasons, id: \.self) { - Text($0).font(.caption).foregroundStyle(.orange) + + ViewThatFits(in: .vertical) { + approvalDetails(presentation) + .fixedSize(horizontal: false, vertical: true) + ScrollView { + approvalDetails(presentation) + .frame(maxWidth: .infinity, alignment: .leading) + } + .scrollBounceBehavior(.basedOnSize) } + HStack { - Button("Deny", role: .destructive) { conversation.respondToApproval(allow: false) } - .keyboardShortcut(.cancelAction) + Button("Deny", role: .destructive) { + conversation.respondToApproval(request.id, allow: false) + } + .accessibilityIdentifier("AssistantApproval.Deny") Spacer() - Button("Allow Once") { conversation.respondToApproval(allow: true) } - .buttonStyle(.borderedProminent) - .keyboardShortcut(.defaultAction) + Button("Allow Once") { + conversation.respondToApproval(request.id, allow: true) + } + .buttonStyle(.borderedProminent) + .accessibilityIdentifier("AssistantApproval.AllowOnce") } } + .accessibilityElement(children: .contain) + .accessibilityIdentifier("AssistantApproval.Card") .activeFormKeyboardActions( isDefaultEnabled: true, - onDefault: { conversation.respondToApproval(allow: true) }, - onCancel: { conversation.respondToApproval(allow: false) } + onDefault: { conversation.respondToApproval(request.id, allow: true) }, + onCancel: { conversation.respondToApproval(request.id, allow: false) } ) } + + private func approvalDetails(_ presentation: AgentApprovalPresentation) -> some View { + AssistantApprovalDetails( + detail: presentation.detail, + reasons: request.localReasons, + showsSafetyDetails: $showsSafetyDetails + ) + } + + private func hostName(_ id: UUID) -> String { + app.hostLibrary.host(id)?.name ?? String(id.uuidString.prefix(8)) + } } /// How much else is waiting behind this card. diff --git a/TFM/Features/Assistant/AssistantView.swift b/TFM/Features/Assistant/AssistantView.swift index c5ee73c8..2a4c761a 100644 --- a/TFM/Features/Assistant/AssistantView.swift +++ b/TFM/Features/Assistant/AssistantView.swift @@ -3,6 +3,15 @@ import ListViewKit import SwiftUI import TermindApplication +nonisolated enum AssistantSidebarEscapePolicy { + static func shouldCloseConversation( + hasConversation: Bool, + hasBlockingInteraction: Bool + ) -> Bool { + hasConversation && !hasBlockingInteraction + } +} + /// Assistant's conversation navigator inside the window's shared sidebar: the /// chat list, over a bar holding the two things you do to the list itself. /// @@ -134,6 +143,13 @@ struct AssistantPanelView: View { let dropConversation = window.sidebarAssistantConversation ?? window.sidebar.assistantDraft + // Capture this render's state so an Escape which resolves the form + // cannot fall through to Back after the resolution synchronously + // removes the pending request. + let blocksSidebarEscape = + window.sidebarAssistantConversation.map { + $0.pendingApproval != nil || $0.pendingUserInteraction != nil + } ?? false return VStack(spacing: 0) { Group { @@ -170,7 +186,12 @@ struct AssistantPanelView: View { isEnabled: isFocusedOnWorkspace ) .onKeyPress(.escape, phases: .down) { _ in - guard window.sidebar.assistantConversationID != nil else { return .ignored } + guard + AssistantSidebarEscapePolicy.shouldCloseConversation( + hasConversation: window.sidebar.assistantConversationID != nil, + hasBlockingInteraction: blocksSidebarEscape + ) + else { return .ignored } window.closeAssistantInSidebar() return .handled } @@ -235,7 +256,6 @@ struct AssistantPanelView: View { .labelStyle(.iconOnly) } .buttonStyle(.borderless) - .keyboardShortcut(.cancelAction) .help(Text("Back to Chats")) Text(conversation.derivedTitle) diff --git a/TFMTests/AgentApprovalQueueTests.swift b/TFMTests/AgentApprovalQueueTests.swift index 226a1890..69a49a51 100644 --- a/TFMTests/AgentApprovalQueueTests.swift +++ b/TFMTests/AgentApprovalQueueTests.swift @@ -121,9 +121,19 @@ struct AgentApprovalQueueTests { #expect(conversation.pendingApproval?.id == conversation.pendingApprovals.first?.id) #expect(conversation.runState == .waitingForApproval) - conversation.respondToApproval(allow: true) + guard let firstID = conversation.pendingApproval?.id else { + Issue.record("Expected the first approval") + return + } + conversation.respondToApproval(firstID, allow: true) await wait { conversation.pendingApprovals.count == 1 } - conversation.respondToApproval(allow: true) + conversation.respondToApproval(firstID, allow: false) + #expect(conversation.pendingApprovals.count == 1) + guard let secondID = conversation.pendingApproval?.id else { + Issue.record("Expected the second approval") + return + } + conversation.respondToApproval(secondID, allow: true) await waitForTerminal(conversation) #expect(await log.finished.sorted() == ["first", "second"]) @@ -155,9 +165,17 @@ struct AgentApprovalQueueTests { )) await wait { conversation.pendingApprovals.count == 2 } - conversation.respondToApproval(allow: false) + guard let firstID = conversation.pendingApproval?.id else { + Issue.record("Expected the first approval") + return + } + conversation.respondToApproval(firstID, allow: false) await wait { conversation.pendingApprovals.count == 1 } - conversation.respondToApproval(allow: true) + guard let secondID = conversation.pendingApproval?.id else { + Issue.record("Expected the second approval") + return + } + conversation.respondToApproval(secondID, allow: true) await waitForTerminal(conversation) #expect(await log.finished == ["allowed"]) diff --git a/TFMTests/AgentToolPresentationTests.swift b/TFMTests/AgentToolPresentationTests.swift index 3ae1205f..300706e7 100644 --- a/TFMTests/AgentToolPresentationTests.swift +++ b/TFMTests/AgentToolPresentationTests.swift @@ -14,6 +14,87 @@ import struct TermindDomain.Host @Suite struct AgentToolPresentationTests { + @Test + func approvalUsesTheRegisteredToolArgumentProjectionWithoutRedactingIt() throws { + let hostID = UUID() + let command = "sudo deploy --token=visible-to-the-user" + let request = AgentApprovalRequest( + invocation: AgentToolInvocation( + runID: UUID(), + call: AgentToolCall( + id: "approval", + name: "ssh_exec", + arguments: .object([ + "host_id": .string(hostID.uuidString), + "command": .string(command), + "stdin": .string("password=hunter2"), + "secret_handle": .string("one-shot-handle"), + "stdin_purpose": .string("sudo password"), + ]) + ), + targetHostID: hostID + ), + descriptor: AgentToolDescriptor( + name: "ssh_exec", summary: "Execute over SSH", + inputSchema: .object([:]), target: .host, + safety: .requiresAuthorization, + presentation: .init( + symbol: "terminal", + activity: .semanticArgument(key: "command", fallback: .sshCommand), + actionKind: .run + ) + ), + userIntent: "Deploy" + ) + + let presentation = AgentToolPresentation.approval( + for: request, + presenter: SSHExecTool.presenter, + hostName: "Production", + locale: Locale(identifier: "en") + ) + + #expect(presentation.symbol == "terminal") + #expect(presentation.title.contains("Production")) + #expect(!presentation.title.contains(hostID.uuidString)) + #expect( + presentation.detail.items == [ + .text(.init(title: "Command", text: command, style: .monospaced)), + .text(.init(title: "Input", text: "password=hunter2", style: .monospaced)), + .field(.init(label: "Secret handle", value: "one-shot-handle", isMonospaced: true)), + .field(.init(label: "Purpose", value: "sudo password")), + ] + ) + } + + @Test + func approvalFallsBackToGenericFieldsForAnUnregisteredTool() { + let request = AgentApprovalRequest( + invocation: AgentToolInvocation( + runID: UUID(), + call: AgentToolCall( + id: "approval", name: "remote_tool", + arguments: .object(["password": .string("hunter2")]) + ) + ), + descriptor: AgentToolDescriptor( + name: "remote_tool", summary: "Remote tool", + inputSchema: .object([:]), target: .mcp, + safety: .requiresAuthorization + ), + userIntent: "Run it" + ) + + let presentation = AgentToolPresentation.approval( + for: request, locale: Locale(identifier: "en") + ) + #expect( + presentation.detail.items == [ + .field(.init(label: "Password", value: "hunter2")) + ] + ) + } + @Test func userInputDetailShowsOneQuestionAndAnswerFieldPerLine() throws { let arguments: [String: AgentJSONValue] = [ diff --git a/TFMTests/ChatComposerStateTests.swift b/TFMTests/ChatComposerStateTests.swift index b41f327e..4bc0cb32 100644 --- a/TFMTests/ChatComposerStateTests.swift +++ b/TFMTests/ChatComposerStateTests.swift @@ -243,6 +243,25 @@ struct ChatComposerStateTests { ) == .passThrough) } + @Test + func sidebarEscapeYieldsToAVisibleDecisionForm() { + #expect( + !AssistantSidebarEscapePolicy.shouldCloseConversation( + hasConversation: true, + hasBlockingInteraction: true + )) + #expect( + AssistantSidebarEscapePolicy.shouldCloseConversation( + hasConversation: true, + hasBlockingInteraction: false + )) + #expect( + !AssistantSidebarEscapePolicy.shouldCloseConversation( + hasConversation: false, + hasBlockingInteraction: false + )) + } + @Test func reasoningEffortDefaultsToMedium() { #expect(ConversationModel().reasoningEffort == .medium) diff --git a/Termind.xcodeproj/project.pbxproj b/Termind.xcodeproj/project.pbxproj index ea6ca215..62955f6a 100644 --- a/Termind.xcodeproj/project.pbxproj +++ b/Termind.xcodeproj/project.pbxproj @@ -2513,7 +2513,7 @@ repositoryURL = "https://github.com/AkinoKaede/AgentKit.git"; requirement = { kind = upToNextMajorVersion; - minimumVersion = 0.6.3; + minimumVersion = 0.6.4; }; }; /* End XCRemoteSwiftPackageReference section */ diff --git a/Termind.xcodeproj/project.xcworkspace/xcshareddata/swiftpm/Package.resolved b/Termind.xcodeproj/project.xcworkspace/xcshareddata/swiftpm/Package.resolved index 426e4af0..1475e836 100644 --- a/Termind.xcodeproj/project.xcworkspace/xcshareddata/swiftpm/Package.resolved +++ b/Termind.xcodeproj/project.xcworkspace/xcshareddata/swiftpm/Package.resolved @@ -6,8 +6,8 @@ "kind" : "remoteSourceControl", "location" : "https://github.com/AkinoKaede/AgentKit.git", "state" : { - "revision" : "248ea7e7f4bbeb9fdec1b45be95f573a764009bb", - "version" : "0.6.3" + "revision" : "8c1a72de2cc53675d34040aaa5f3ade61dfdefd1", + "version" : "0.6.4" } }, {