diff --git a/FrameworkTests/AssistantTranscriptPresentationTests.swift b/FrameworkTests/AssistantTranscriptPresentationTests.swift index a7eae2db..6397b3c9 100644 --- a/FrameworkTests/AssistantTranscriptPresentationTests.swift +++ b/FrameworkTests/AssistantTranscriptPresentationTests.swift @@ -148,6 +148,21 @@ struct AssistantTranscriptPresentationTests { #expect(conversation.transcript.count == 1) } + @Test func approvalPolicyDenialFinishesAsDeniedRatherThanFailed() { + let value = card(state: .proposed) + let conversation = ConversationModel() + conversation.handle(.toolProposed(value.invocation, value.descriptor)) + conversation.handle( + .toolFinished( + value.invocation, + AgentToolResult( + callID: value.invocation.call.id, content: "Denied", isError: true, + metadata: [AgentToolResult.approvalPolicyDeniedKey: .bool(true)] + ))) + + #expect(conversation.toolCards.first?.state == .denied) + } + @Test func resultProjectionUsesTheInjectedPresenterAndRetainsFullCopyOutput() throws { var value = card(state: .completed) value.result = AgentToolResult(callID: value.invocation.call.id, content: "{\"answer\":42}") @@ -193,7 +208,7 @@ struct AssistantTranscriptPresentationTests { runID: UUID(), call: .init(id: UUID().uuidString, name: "inspect", arguments: .object([:]))), descriptor: .init( name: "inspect", summary: "Inspect", inputSchema: .object([:]), target: .host, - safety: .locallyReadOnly, concurrency: .parallel), + approvalPolicy: .approve, concurrency: .parallel), state: state ) } diff --git a/Frameworks/Resources/Localizable.xcstrings b/Frameworks/Resources/Localizable.xcstrings index 209c776c..f3f4157a 100644 --- a/Frameworks/Resources/Localizable.xcstrings +++ b/Frameworks/Resources/Localizable.xcstrings @@ -17,150 +17,6 @@ } } }, - "`%@` changes system, file, or external state." : { - "localizations" : { - "zh-Hans" : { - "stringUnit" : { - "state" : "translated", - "value" : "`%@`会更改系统、文件或外部状态。" - } - }, - "zh-Hant" : { - "stringUnit" : { - "state" : "translated", - "value" : "`%@`會更改系統、檔案或外部狀態。" - } - } - } - }, - "Command analysis exceeded the supported complexity limit." : { - "localizations" : { - "zh-Hans" : { - "stringUnit" : { - "state" : "translated", - "value" : "命令分析超出了支持的复杂度限制。" - } - }, - "zh-Hant" : { - "stringUnit" : { - "state" : "translated", - "value" : "指令分析超出了支援的複雜度限制。" - } - } - } - }, - "Caution" : { - "localizations" : { - "zh-Hans" : { - "stringUnit" : { - "state" : "translated", - "value" : "谨慎" - } - }, - "zh-Hant" : { - "stringUnit" : { - "state" : "translated", - "value" : "謹慎" - } - } - } - }, - "Contains shell syntax that cannot be proven read-only." : { - "localizations" : { - "zh-Hans" : { - "stringUnit" : { - "state" : "translated", - "value" : "包含无法证明为只读的Shell语法。" - } - }, - "zh-Hant" : { - "stringUnit" : { - "state" : "translated", - "value" : "包含無法證明為唯讀的Shell語法。" - } - } - } - }, - "Force-overwrites `%@`." : { - "localizations" : { - "zh-Hans" : { - "stringUnit" : { - "state" : "translated", - "value" : "强制覆盖`%@`。" - } - }, - "zh-Hant" : { - "stringUnit" : { - "state" : "translated", - "value" : "強制覆寫`%@`。" - } - } - } - }, - "Runs a command on another host." : { - "localizations" : { - "zh-Hans" : { - "stringUnit" : { - "state" : "translated", - "value" : "在另一台主机上运行命令。" - } - }, - "zh-Hant" : { - "stringUnit" : { - "state" : "translated", - "value" : "在另一台主機上執行指令。" - } - } - } - }, - "Stops, disables, or removes service state." : { - "localizations" : { - "zh-Hans" : { - "stringUnit" : { - "state" : "translated", - "value" : "停止、禁用或移除服务状态。" - } - }, - "zh-Hant" : { - "stringUnit" : { - "state" : "translated", - "value" : "停止、停用或移除服務狀態。" - } - } - } - }, - "Writes directly to a device or kernel control path." : { - "localizations" : { - "zh-Hans" : { - "stringUnit" : { - "state" : "translated", - "value" : "直接写入设备或内核控制路径。" - } - }, - "zh-Hant" : { - "stringUnit" : { - "state" : "translated", - "value" : "直接寫入裝置或核心控制路徑。" - } - } - } - }, - "Writes to `%@`." : { - "localizations" : { - "zh-Hans" : { - "stringUnit" : { - "state" : "translated", - "value" : "写入`%@`。" - } - }, - "zh-Hant" : { - "stringUnit" : { - "state" : "translated", - "value" : "寫入`%@`。" - } - } - } - }, " Copy" : { "localizations" : { "zh-Hans" : { @@ -1182,6 +1038,7 @@ }, "%lld/%lld tools" : { "comment" : "A tool count for a MCP server. The first argument is the number of enabled tools. The second argument is the total number of tools.", + "extractionState" : "stale", "isCommentAutoGenerated" : true, "localizations" : { "en" : { @@ -1323,6 +1180,23 @@ } } }, + "`%@` changes system, file, or external state." : { + "extractionState" : "stale", + "localizations" : { + "zh-Hans" : { + "stringUnit" : { + "state" : "translated", + "value" : "`%@`会更改系统、文件或外部状态。" + } + }, + "zh-Hant" : { + "stringUnit" : { + "state" : "translated", + "value" : "`%@`會更改系統、檔案或外部狀態。" + } + } + } + }, "`%@` is not a recognised read-only command." : { "extractionState" : "stale", "localizations" : { @@ -2736,6 +2610,7 @@ } }, "Always Ask" : { + "extractionState" : "stale", "localizations" : { "zh-Hans" : { "stringUnit" : { @@ -2752,6 +2627,7 @@ } }, "Always Ask requires approval for every call. Follow Permissions uses the conversation's permission mode and safety checks." : { + "extractionState" : "stale", "localizations" : { "zh-Hans" : { "stringUnit" : { @@ -3146,6 +3022,22 @@ } } }, + "Approve" : { + "localizations" : { + "zh-Hans" : { + "stringUnit" : { + "state" : "translated", + "value" : "批准" + } + }, + "zh-Hant" : { + "stringUnit" : { + "state" : "translated", + "value" : "核准" + } + } + } + }, "Archive" : { "localizations" : { "zh-Hans" : { @@ -3263,6 +3155,22 @@ } } }, + "Ask" : { + "localizations" : { + "zh-Hans" : { + "stringUnit" : { + "state" : "translated", + "value" : "询问" + } + }, + "zh-Hant" : { + "stringUnit" : { + "state" : "translated", + "value" : "詢問" + } + } + } + }, "Ask a question or describe what you want to do." : { "localizations" : { "zh-Hans" : { @@ -4171,6 +4079,23 @@ } } }, + "Caution" : { + "extractionState" : "stale", + "localizations" : { + "zh-Hans" : { + "stringUnit" : { + "state" : "translated", + "value" : "谨慎" + } + }, + "zh-Hant" : { + "stringUnit" : { + "state" : "translated", + "value" : "謹慎" + } + } + } + }, "Certificate" : { "localizations" : { "zh-Hans" : { @@ -5064,6 +4989,23 @@ } } }, + "Command analysis exceeded the supported complexity limit." : { + "extractionState" : "stale", + "localizations" : { + "zh-Hans" : { + "stringUnit" : { + "state" : "translated", + "value" : "命令分析超出了支持的复杂度限制。" + } + }, + "zh-Hant" : { + "stringUnit" : { + "state" : "translated", + "value" : "指令分析超出了支援的複雜度限制。" + } + } + } + }, "Command Generator is already reading the current input." : { "extractionState" : "stale", "localizations" : { @@ -5860,6 +5802,23 @@ } } }, + "Contains shell syntax that cannot be proven read-only." : { + "extractionState" : "stale", + "localizations" : { + "zh-Hans" : { + "stringUnit" : { + "state" : "translated", + "value" : "包含无法证明为只读的Shell语法。" + } + }, + "zh-Hant" : { + "stringUnit" : { + "state" : "translated", + "value" : "包含無法證明為唯讀的Shell語法。" + } + } + } + }, "Content" : { "extractionState" : "stale", "localizations" : { @@ -7379,6 +7338,22 @@ } } }, + "Deny refuses the call. Ask uses the conversation's manual or automatic approval. Approve runs without approval and is unavailable for tools marked destructive." : { + "localizations" : { + "zh-Hans" : { + "stringUnit" : { + "state" : "translated", + "value" : "“拒绝”会拒绝调用;“询问”使用会话的手动或自动审批;“批准”无需审批即可运行,且不适用于标记为破坏性的工具。" + } + }, + "zh-Hant" : { + "stringUnit" : { + "state" : "translated", + "value" : "「拒絕」會拒絕呼叫;「詢問」使用工作階段的手動或自動核准;「核准」無需核准即可執行,且不適用於標示為破壞性的工具。" + } + } + } + }, "Derived from the private key" : { "localizations" : { "zh-Hans" : { @@ -9933,6 +9908,7 @@ } }, "Follow Permissions" : { + "extractionState" : "stale", "localizations" : { "zh-Hans" : { "stringUnit" : { @@ -10062,6 +10038,23 @@ } } }, + "Force-overwrites `%@`." : { + "extractionState" : "stale", + "localizations" : { + "zh-Hans" : { + "stringUnit" : { + "state" : "translated", + "value" : "强制覆盖`%@`。" + } + }, + "zh-Hant" : { + "stringUnit" : { + "state" : "translated", + "value" : "強制覆寫`%@`。" + } + } + } + }, "Fork" : { "localizations" : { "zh-Hans" : { @@ -19201,6 +19194,23 @@ } } }, + "Runs a command on another host." : { + "extractionState" : "stale", + "localizations" : { + "zh-Hans" : { + "stringUnit" : { + "state" : "translated", + "value" : "在另一台主机上运行命令。" + } + }, + "zh-Hant" : { + "stringUnit" : { + "state" : "translated", + "value" : "在另一台主機上執行指令。" + } + } + } + }, "Runs with elevated privileges." : { "extractionState" : "stale", "localizations" : { @@ -21683,6 +21693,23 @@ } } }, + "Stops, disables, or removes service state." : { + "extractionState" : "stale", + "localizations" : { + "zh-Hans" : { + "stringUnit" : { + "state" : "translated", + "value" : "停止、禁用或移除服务状态。" + } + }, + "zh-Hant" : { + "stringUnit" : { + "state" : "translated", + "value" : "停止、停用或移除服務狀態。" + } + } + } + }, "Store in" : { "localizations" : { "zh-Hans" : { @@ -26946,6 +26973,40 @@ } } }, + "Writes directly to a device or kernel control path." : { + "extractionState" : "stale", + "localizations" : { + "zh-Hans" : { + "stringUnit" : { + "state" : "translated", + "value" : "直接写入设备或内核控制路径。" + } + }, + "zh-Hant" : { + "stringUnit" : { + "state" : "translated", + "value" : "直接寫入裝置或核心控制路徑。" + } + } + } + }, + "Writes to `%@`." : { + "extractionState" : "stale", + "localizations" : { + "zh-Hans" : { + "stringUnit" : { + "state" : "translated", + "value" : "写入`%@`。" + } + }, + "zh-Hant" : { + "stringUnit" : { + "state" : "translated", + "value" : "寫入`%@`。" + } + } + } + }, "Written" : { "extractionState" : "stale", "localizations" : { @@ -27066,4 +27127,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 91c56006..5e261e60 100644 --- a/Frameworks/TermindApplication/Agent/ConversationModel.swift +++ b/Frameworks/TermindApplication/Agent/ConversationModel.swift @@ -2438,7 +2438,9 @@ public final class ConversationModel: Identifiable { // an error to the *model*, which is what makes the turn // replayable, but the card kept being relabelled `Failed` the // instant it was refused, so `denied` never reached the screen. - if card.state != .denied { + if result.isApprovalPolicyDenied { + card.state = .denied + } else if card.state != .denied { if result.isInterrupted { card.state = .interrupted } else { @@ -2691,7 +2693,7 @@ extension ConversationModel { let descriptor = AgentToolDescriptor( name: "ssh_exec", summary: text("Run a command on a remote host.", "在远程主机上运行命令。", "在遠端主機上執行指令。"), - inputSchema: .object([:]), target: .host, safety: .locallyReadOnly, + inputSchema: .object([:]), target: .host, approvalPolicy: .approve, concurrency: .parallel, presentation: AgentToolDescriptor.Presentation( symbol: "apple.terminal", activity: .argument(key: "command", fallback: "command"), diff --git a/Frameworks/TermindApplication/Agent/MCPToolApprovalPolicy.swift b/Frameworks/TermindApplication/Agent/MCPToolApprovalPolicy.swift new file mode 100644 index 00000000..74acc9db --- /dev/null +++ b/Frameworks/TermindApplication/Agent/MCPToolApprovalPolicy.swift @@ -0,0 +1,29 @@ +import AgentKit + +nonisolated extension MCPTool { + public var allowsTermindPreapproval: Bool { + annotations.destructiveHint != true + } + + /// Termind never treats a remotely flagged destructive tool as pre-approved. + /// The hint cannot authorize anything, but it may tighten a local choice. + public var termindApprovalPolicy: AgentToolDescriptor.ApprovalPolicy { + !allowsTermindPreapproval && accessPolicy == .approve ? .ask : accessPolicy + } + + public mutating func enforceTermindApprovalPolicy() { + accessPolicy = termindApprovalPolicy + } +} + +nonisolated extension MCPServer { + public func enforcingTermindApprovalPolicies() -> MCPServer { + var server = self + server.tools = tools.map { tool in + var tool = tool + tool.enforceTermindApprovalPolicy() + return tool + } + return server + } +} diff --git a/Frameworks/TermindApplication/Agent/TermindAgentPolicy.swift b/Frameworks/TermindApplication/Agent/TermindAgentPolicy.swift index 9970899a..957bc49d 100644 --- a/Frameworks/TermindApplication/Agent/TermindAgentPolicy.swift +++ b/Frameworks/TermindApplication/Agent/TermindAgentPolicy.swift @@ -42,9 +42,9 @@ nonisolated public enum TermindAgentPolicy { user's approval; one that destroys something is refused outright. SFTP reads, fetch and \ search all work. - What is refused is changing anything: writing or deleting remote files, pushing from \ - the scratch workspace, and every MCP tool. Do not attempt those, and never describe an \ - action as taken — you have taken none. + Direct remote file writes, deletes, and pushes from the scratch workspace are refused. \ + MCP tools follow their saved policy: Deny refuses the call, Ask sends it through the \ + conversation's approval mode, and Approve runs it without another approval step. Draft the plan in the scratch workspace with scratch_write, and revise it there with \ scratch_replace rather than rewriting it. That way a revision costs a diff instead of \ @@ -78,7 +78,7 @@ nonisolated public enum TermindAgentPolicy { version: Bundle.main.object(forInfoDictionaryKey: "CFBundleShortVersionString") as? String ?? "0") } - /// Plan mode with the one rule the runtime cannot write for us. + /// Plan mode with the host rules the runtime cannot write for us. /// /// `ssh_exec` is judged on what the command would *do*, not on who has to /// authorize it — and that difference is the point of the mode rather than a @@ -88,15 +88,18 @@ nonisolated public enum TermindAgentPolicy { /// Refusing those would leave the agent guessing at the state it is supposed /// to be planning against, which is worse than useless. /// - /// So the bar is *destructive*, not *provably safe*. A command the classifier + /// So the SSH bar is *destructive*, not *provably safe*. A command the classifier /// cannot vouch for still meets the approval gate exactly as it would outside /// plan mode — the user sees it under Ask for approval, Security Review sees /// it under Approve for me. Full access skips that, as it does everywhere - /// else; plan mode is a posture, not a sandbox, and the one thing it refuses - /// outright even there is a command that destroys something. + /// else; plan mode is a posture, not a sandbox, and the one SSH command it + /// refuses outright even there is one that destroys something. MCP calls + /// also proceed to the ordinary approval gate, where their saved Deny, Ask, + /// or Approve policy remains authoritative. public static func planModeHook(isPlanning: Bool) -> AgentPlanModeHook { var hook = AgentPlanModeHook(isPlanning: isPlanning) hook.hostDecision = { context in + if context.descriptor.target == .mcp { return .proceed } guard context.invocation.call.name == sshExecToolName else { return nil } let command = context.invocation.call.arguments diff --git a/Frameworks/TermindApplication/FeatureModels/AIConfigurationModel.swift b/Frameworks/TermindApplication/FeatureModels/AIConfigurationModel.swift index 0bafcbd0..57fa76e4 100644 --- a/Frameworks/TermindApplication/FeatureModels/AIConfigurationModel.swift +++ b/Frameworks/TermindApplication/FeatureModels/AIConfigurationModel.swift @@ -86,7 +86,7 @@ public final class AIConfigurationModel { self.webSearchPersistence = webSearchPersistence self.modelProviders = modelProviders self.endpoints = endpoints - self.mcpServers = mcpServers + self.mcpServers = mcpServers.map { $0.enforcingTermindApprovalPolicies() } self.roles = roles self.lastSelectedModel = lastSelectedModel self.lastSelectedPermissionMode = @@ -110,7 +110,7 @@ public final class AIConfigurationModel { self.endpoints = loaded.endpoints self.roles = loaded.roles self.lastSelectedModel = loaded.lastSelectedModel - self.mcpServers = loaded.mcpServers + self.mcpServers = loaded.mcpServers.map { $0.enforcingTermindApprovalPolicies() } } catch { storageError = error.localizedDescription } @@ -238,10 +238,11 @@ public final class AIConfigurationModel { } } - /// Every tool the model may be offered: enabled tools on enabled servers. + /// Every discovered tool on enabled servers. Denied tools stay visible to + /// the model so an attempted call receives an explicit policy refusal. public var availableTools: [(server: MCPServer, tool: MCPTool)] { usableMCPServers.flatMap { server in - server.enabledTools.map { (server, $0) } + server.tools.map { (server, $0) } } } @@ -277,7 +278,7 @@ public final class AIConfigurationModel { endpoints = snapshot.endpoints roles = snapshot.roles lastSelectedModel = localSelection.flatMap { lookup($0) == nil ? nil : $0 } - mcpServers = snapshot.mcpServers + mcpServers = snapshot.mcpServers.map { $0.enforcingTermindApprovalPolicies() } mcpStatuses = mcpStatuses.filter { id, _ in previousServers[id] == mcpServers.first { $0.id == id } } @@ -556,6 +557,7 @@ public final class AIConfigurationModel { // MARK: - MCP writes public func upsert(_ server: MCPServer) { + let server = server.enforcingTermindApprovalPolicies() mutatePersistedState { if let index = mcpServers.firstIndex(where: { $0.id == server.id }) { mcpServers[index] = server @@ -605,9 +607,9 @@ public final class AIConfigurationModel { ) mcpServers[index].tools = discovered.map { tool in - guard let old = previous[tool.id] else { return tool } var merged = tool - merged.accessPolicy = old.accessPolicy + if let old = previous[tool.id] { merged.accessPolicy = old.accessPolicy } + merged.enforceTermindApprovalPolicy() return merged } mcpServers[index].negotiatedVersion = version @@ -616,9 +618,12 @@ public final class AIConfigurationModel { } public func setToolAccessPolicy( - _ policy: MCPTool.AccessPolicy, tool toolID: String, on serverID: MCPServer.ID + _ policy: AgentToolDescriptor.ApprovalPolicy, tool toolID: String, on serverID: MCPServer.ID ) { - mutate(toolID, on: serverID) { $0.accessPolicy = policy } + mutate(toolID, on: serverID) { + $0.accessPolicy = policy + $0.enforceTermindApprovalPolicy() + } } private func mutate( diff --git a/Frameworks/TermindPersistence/SwiftDataAgentRunRepository.swift b/Frameworks/TermindPersistence/SwiftDataAgentRunRepository.swift index bfc87960..b37cbffa 100644 --- a/Frameworks/TermindPersistence/SwiftDataAgentRunRepository.swift +++ b/Frameworks/TermindPersistence/SwiftDataAgentRunRepository.swift @@ -621,7 +621,7 @@ public final class SwiftDataAgentRunRepository: AgentRunPersisting, Sendable { name: row.toolName, summary: row.toolName, inputSchema: .object(["type": .string("object")]), target: row.targetHostID == nil ? .local : .host, - safety: .requiresAuthorization, presentation: result?.toolPresentation + approvalPolicy: .ask, presentation: result?.toolPresentation ) let review = reviews.filter { $0.invocationID == row.id } .sorted { $0.createdAt < $1.createdAt } diff --git a/Frameworks/TermindRemoteInfrastructure/Agent/AgentBuiltInSFTPTools.swift b/Frameworks/TermindRemoteInfrastructure/Agent/AgentBuiltInSFTPTools.swift index 0d530672..8314f249 100644 --- a/Frameworks/TermindRemoteInfrastructure/Agent/AgentBuiltInSFTPTools.swift +++ b/Frameworks/TermindRemoteInfrastructure/Agent/AgentBuiltInSFTPTools.swift @@ -8,7 +8,7 @@ import struct TermindDomain.Host nonisolated public protocol SFTPToolDefinition: AgentToolDefinition, AgentToolSchemaBuilding { static var name: String { get } static var summary: String { get } - static var safety: AgentToolDescriptor.Safety { get } + static var approvalPolicy: AgentToolDescriptor.ApprovalPolicy { get } static var concurrency: AgentToolDescriptor.Concurrency { get } static var extraProperties: [String: AgentJSONValue] { get } static var extraRequired: [String] { get } @@ -23,7 +23,7 @@ nonisolated public protocol SFTPToolDefinition: AgentToolDefinition, AgentToolSc nonisolated extension SFTPToolDefinition { public static var concurrency: AgentToolDescriptor.Concurrency { - safety == .locallyReadOnly ? .parallel : .sequential + approvalPolicy == .approve ? .parallel : .sequential } public static var extraProperties: [String: AgentJSONValue] { [:] } public static var extraRequired: [String] { [] } @@ -36,7 +36,7 @@ nonisolated extension SFTPToolDefinition { return Self.descriptor( Self.name, Self.summary, properties: properties, required: ["host_id", "path"] + Self.extraRequired, - target: .host, safety: Self.safety, concurrency: Self.concurrency, + target: .host, approvalPolicy: Self.approvalPolicy, concurrency: Self.concurrency, presentation: Self.presentation ) } @@ -46,7 +46,7 @@ nonisolated extension SFTPToolDefinition { let arguments = try Arguments(invocation) checked.targetHostID = try arguments.uuid("host_id") _ = try AgentToolSupport.normalizeRemotePath(arguments.string("path")) - return AgentToolPreflight(invocation: checked, safety: Self.safety) + return AgentToolPreflight(invocation: checked, approvalPolicy: Self.approvalPolicy) } public func execute( @@ -69,7 +69,7 @@ nonisolated extension SFTPToolDefinition { nonisolated public struct SFTPListTool: SFTPToolDefinition { public static let name = "sftp_list" public static let summary = "List a remote directory." - public static let safety: AgentToolDescriptor.Safety = .locallyReadOnly + public static let approvalPolicy: AgentToolDescriptor.ApprovalPolicy = .approve public static let presenter = AgentToolDetailPresenter( id: "builtin.sftp_list", present: present ) @@ -96,7 +96,7 @@ nonisolated public struct SFTPListTool: SFTPToolDefinition { nonisolated public struct SFTPStatTool: SFTPToolDefinition { public static let name = "sftp_stat" public static let summary = "Read remote file metadata." - public static let safety: AgentToolDescriptor.Safety = .locallyReadOnly + public static let approvalPolicy: AgentToolDescriptor.ApprovalPolicy = .approve public static let presenter = AgentToolDetailPresenter( id: "builtin.sftp_stat", present: present ) @@ -123,7 +123,7 @@ nonisolated public struct SFTPStatTool: SFTPToolDefinition { nonisolated public struct SFTPReadTool: SFTPToolDefinition { public static let name = "sftp_read" public static let summary = "Read a bounded UTF-8 remote file inline." - public static let safety: AgentToolDescriptor.Safety = .locallyReadOnly + public static let approvalPolicy: AgentToolDescriptor.ApprovalPolicy = .approve public static let presenter = AgentToolDetailPresenter( id: "builtin.sftp_read", present: present ) @@ -157,7 +157,7 @@ nonisolated public struct SFTPReadTool: SFTPToolDefinition { nonisolated public struct SFTPWriteTool: SFTPToolDefinition { public static let name = "sftp_write" public static let summary = "Write a bounded UTF-8 remote file atomically." - public static let safety: AgentToolDescriptor.Safety = .requiresAuthorization + public static let approvalPolicy: AgentToolDescriptor.ApprovalPolicy = .ask public static let extraProperties = ["content": string(max: 131_072), "overwrite": boolean()] public static let extraRequired = ["content"] public static let presenter = AgentToolDetailPresenter( @@ -190,7 +190,7 @@ nonisolated public struct SFTPWriteTool: SFTPToolDefinition { nonisolated public struct SFTPMkdirTool: SFTPToolDefinition { public static let name = "sftp_mkdir" public static let summary = "Create a remote directory." - public static let safety: AgentToolDescriptor.Safety = .requiresAuthorization + public static let approvalPolicy: AgentToolDescriptor.ApprovalPolicy = .ask public static let presenter = AgentToolDetailPresenter( id: "builtin.sftp_mkdir", present: present, presentArguments: presentArguments @@ -219,7 +219,7 @@ nonisolated public struct SFTPMkdirTool: SFTPToolDefinition { nonisolated public struct SFTPMoveTool: SFTPToolDefinition { public static let name = "sftp_move" public static let summary = "Move or rename a remote path." - public static let safety: AgentToolDescriptor.Safety = .requiresAuthorization + public static let approvalPolicy: AgentToolDescriptor.ApprovalPolicy = .ask public static let extraProperties = ["destination": string(max: 4_096), "overwrite": boolean()] public static let extraRequired = ["destination"] public static let presenter = AgentToolDetailPresenter( @@ -256,7 +256,7 @@ nonisolated public struct SFTPMoveTool: SFTPToolDefinition { nonisolated public struct SFTPChmodTool: SFTPToolDefinition { public static let name = "sftp_chmod" public static let summary = "Change remote POSIX permissions." - public static let safety: AgentToolDescriptor.Safety = .requiresAuthorization + public static let approvalPolicy: AgentToolDescriptor.ApprovalPolicy = .ask public static let extraProperties = ["mode": string(max: 4)] public static let extraRequired = ["mode"] public static let presenter = AgentToolDetailPresenter( @@ -292,7 +292,7 @@ nonisolated public struct SFTPChmodTool: SFTPToolDefinition { nonisolated public struct SFTPDeleteTool: SFTPToolDefinition { public static let name = "sftp_delete" public static let summary = "Permanently delete one remote file or empty directory." - public static let safety: AgentToolDescriptor.Safety = .requiresAuthorization + public static let approvalPolicy: AgentToolDescriptor.ApprovalPolicy = .ask public static let extraProperties = ["kind": enumeration(["file", "directory"])] public static let extraRequired = ["kind"] public static let presenter = AgentToolDetailPresenter( diff --git a/Frameworks/TermindRemoteInfrastructure/Agent/AgentBuiltInSSHTools.swift b/Frameworks/TermindRemoteInfrastructure/Agent/AgentBuiltInSSHTools.swift index 29646e82..365443f5 100644 --- a/Frameworks/TermindRemoteInfrastructure/Agent/AgentBuiltInSSHTools.swift +++ b/Frameworks/TermindRemoteInfrastructure/Agent/AgentBuiltInSSHTools.swift @@ -12,7 +12,7 @@ nonisolated public struct ListHostsTool: AgentToolDefinition, AgentToolSchemaBui public var descriptor: AgentToolDescriptor { Self.descriptor( "list_hosts", "List Vault and temporary Termind SSH targets without credentials.", - properties: [:], required: [], target: .host, safety: .locallyReadOnly, + properties: [:], required: [], target: .host, approvalPolicy: .approve, concurrency: .parallel, presentation: .init( symbol: "server.rack", activity: .semanticLabel(.hosts), output: .json, @@ -65,7 +65,7 @@ nonisolated public struct SSHWaitTool: AgentToolDefinition, AgentToolSchemaBuild "handle": Self.string(max: 64), "timeout_seconds": Self.integer(min: 1, max: 900), ], - required: ["handle"], target: .host, safety: .locallyReadOnly, + required: ["handle"], target: .host, approvalPolicy: .approve, concurrency: .parallel, presentation: .init( symbol: "clock.arrow.circlepath", activity: .semanticLabel(.remoteCommand), @@ -119,7 +119,7 @@ nonisolated public struct TerminalSuggestTool: AgentToolDefinition, AgentToolSch "command": Self.string(max: 32_768), "explanation": Self.string(max: 4_096), ], - required: ["command"], target: .user, safety: .locallyReadOnly, + required: ["command"], target: .user, approvalPolicy: .approve, concurrency: .parallel, presentation: .init( symbol: "terminal", activity: .semanticArgument(key: "command", fallback: .command), @@ -247,7 +247,7 @@ nonisolated public struct SSHExecTool: AgentToolDefinition, AgentToolSchemaBuild "timeout_seconds": Self.integer(min: 1, max: 900), "stdin": Self.string(max: 131_072), "secret_handle": Self.string(), "stdin_purpose": Self.string(max: 256), - ], required: ["host_id", "command"], target: .host, safety: .requiresAuthorization, + ], required: ["host_id", "command"], target: .host, approvalPolicy: .ask, presentation: .init( symbol: "terminal", activity: .semanticArgument(key: "command", fallback: .sshCommand), @@ -291,11 +291,11 @@ nonisolated public struct SSHExecTool: AgentToolDefinition, AgentToolSchemaBuild let safe = assessment.risk == .safe return AgentToolPreflight( invocation: checked, - safety: safe ? .locallyReadOnly : .requiresAuthorization, + approvalPolicy: safe ? .approve : .ask, reasons: assessment.reasons, // Whether a command may run beside another is a property of // the command, not of `ssh_exec`, and it is decided by the - // same locally-proven evidence that just decided `safety`. + // same locally-proven evidence that just decided `approvalPolicy`. // A command the classifier cannot vouch for makes its whole // turn serial, so nothing observes the host while it is // being changed. diff --git a/Frameworks/TermindRemoteInfrastructure/Agent/AgentScratchTransferTools.swift b/Frameworks/TermindRemoteInfrastructure/Agent/AgentScratchTransferTools.swift index 63789837..f2a8601b 100644 --- a/Frameworks/TermindRemoteInfrastructure/Agent/AgentScratchTransferTools.swift +++ b/Frameworks/TermindRemoteInfrastructure/Agent/AgentScratchTransferTools.swift @@ -28,7 +28,7 @@ nonisolated public struct ScratchPullTool: AgentToolDefinition, AgentToolSchemaB "host_id": Self.string(), "remote_path": Self.string(max: 4_096), "scratch_path": Self.string(max: AgentScratchWorkspace.Limits.pathBytes), ], required: ["host_id", "remote_path", "scratch_path"], target: .host, - safety: .locallyContained, concurrency: .parallel, + approvalPolicy: .approve, concurrency: .parallel, presentation: .init( symbol: "arrow.down.doc", activity: .semanticArgument(key: "remote_path", fallback: .remoteFile), @@ -43,7 +43,7 @@ nonisolated public struct ScratchPullTool: AgentToolDefinition, AgentToolSchemaB checked.targetHostID = try arguments.uuid("host_id") _ = try AgentToolSupport.normalizeRemotePath(arguments.string("remote_path")) _ = try AgentScratchWorkspace.normalize(arguments.string("scratch_path")) - return AgentToolPreflight(invocation: checked, safety: .locallyContained) + return AgentToolPreflight(invocation: checked, approvalPolicy: .approve) } public func execute(_ invocation: AgentToolInvocation, context: AgentToolExecutionContext) async throws @@ -97,7 +97,7 @@ nonisolated public struct ScratchPushTool: AgentToolDefinition, AgentToolSchemaB "scratch_path": Self.string(max: AgentScratchWorkspace.Limits.pathBytes), "remote_path": Self.string(max: 4_096), "overwrite": Self.boolean(), ], required: ["host_id", "scratch_path", "remote_path"], target: .host, - safety: .requiresAuthorization, + approvalPolicy: .ask, presentation: .init( symbol: "arrow.up.doc", activity: .semanticArgument(key: "remote_path", fallback: .remoteFile), @@ -114,7 +114,7 @@ nonisolated public struct ScratchPushTool: AgentToolDefinition, AgentToolSchemaB let staged = try arguments.string("scratch_path") let file = try await workspace.data(at: staged) return AgentToolPreflight( - invocation: checked, safety: .requiresAuthorization, + invocation: checked, approvalPolicy: .ask, reasons: Self.evidence(path: staged, file: file), executionMetadata: ["scratch_sha256": .string(file.sha256)] ) diff --git a/Frameworks/TermindRemoteInfrastructure/Agent/GetMetricsTool.swift b/Frameworks/TermindRemoteInfrastructure/Agent/GetMetricsTool.swift index eef77ef9..717d2a6a 100644 --- a/Frameworks/TermindRemoteInfrastructure/Agent/GetMetricsTool.swift +++ b/Frameworks/TermindRemoteInfrastructure/Agent/GetMetricsTool.swift @@ -25,7 +25,7 @@ nonisolated public struct GetMetricsTool: AgentToolDefinition, AgentToolSchemaBu "type": .string("string"), "enum": .array(Self.sections.sorted().map(AgentJSONValue.string)), ]), max: 4), - ], required: ["host_id"], target: .host, safety: .locallyReadOnly, concurrency: .parallel, + ], required: ["host_id"], target: .host, approvalPolicy: .approve, concurrency: .parallel, presentation: .init( symbol: "gauge.with.dots.needle.67percent", activity: .semanticLabel(.metrics), output: .json, actionKind: .read @@ -38,7 +38,7 @@ nonisolated public struct GetMetricsTool: AgentToolDefinition, AgentToolSchemaBu let arguments = try Arguments(invocation) checked.targetHostID = try arguments.uuid("host_id") _ = try Self.includes(arguments) - return AgentToolPreflight(invocation: checked, safety: .locallyReadOnly) + return AgentToolPreflight(invocation: checked, approvalPolicy: .approve) } public func execute(_ invocation: AgentToolInvocation, context: AgentToolExecutionContext) async throws diff --git a/Memory/ARCHITECTURE.md b/Memory/ARCHITECTURE.md index 8c2bed44..5700c730 100644 --- a/Memory/ARCHITECTURE.md +++ b/Memory/ARCHITECTURE.md @@ -165,8 +165,8 @@ tool capability receive no tools; only recognized chat-purpose models appear in conversation pickers, and unknown purpose values safely default to chat. A batch is concurrent only if every call is parallel. Preflight may tighten this: -`ssh_exec` is parallel only when local command classification proves it safe; MCP -destructive hints or Ask First make a call sequential. Results always enter the +`ssh_exec` is parallel only when local command classification proves it safe; an MCP +destructive hint makes a call sequential. Results always enter the transcript in model order, and every call gets an answer even after cancellation or refusal. Approvals/questions are queued one at a time; state derives from outstanding work. Authorization is a structural executor stage and cannot be removed by hooks. @@ -176,14 +176,17 @@ commands, then applies argument- and subcommand-aware rules. Only explicitly pro read forms are locally read-only; unknown tools, options, embedded programs, excessive complexity, and unsupported shell syntax fail closed to a state-changing assessment. -Permission modes share all validation: +Every call has an approval policy before the run's permission mode is consulted: -- Ask for approval allows proven `locallyReadOnly`/`locallyContained` work and asks - for everything else. -- Approve for me sends other actions to Security Review; missing credentials, +- Deny refuses every permission mode while keeping the tool registered and visible + to the model; Approve runs without another approval step. +- Ask for approval sends Ask calls to the user. Approve for me sends them to Security + Review; missing credentials, timeout, transport failure, or malformed review fails closed and is recorded. -- Full access skips action approval/review but retains host-key, secret, schema, +- Full access pre-approves Ask calls but retains host-key, secret, schema, path, fetch, sandbox, and capability boundaries. +- Termind downgrades Approve to Ask for MCP tools carrying a destructive hint. MCP + tools otherwise follow Deny, Ask, or Approve in both acting and planning runs. Skills are trusted procedures only because users install them through Settings; tools, MCP servers, and fetched pages cannot write or enable them. `allowed-tools` diff --git a/TFI/Application/FeatureModels/MobileMCPEditorModel.swift b/TFI/Application/FeatureModels/MobileMCPEditorModel.swift index 0d2744f8..3c8ebefc 100644 --- a/TFI/Application/FeatureModels/MobileMCPEditorModel.swift +++ b/TFI/Application/FeatureModels/MobileMCPEditorModel.swift @@ -16,7 +16,7 @@ final class MobileMCPEditorModel { private let discover: Discover private var task: Task? private var generation = UUID() - private var policies: [String: MCPTool.AccessPolicy] = [:] + private var policies: [String: AgentToolDescriptor.ApprovalPolicy] = [:] init( ai: AIConfigurationModel, server: MCPServer, @@ -77,7 +77,8 @@ final class MobileMCPEditorModel { var seen = Set() draft.tools = outcome.tools.filter { seen.insert($0.id).inserted }.map { var tool = $0 - tool.accessPolicy = policies[tool.id] ?? .alwaysAsk + tool.accessPolicy = policies[tool.id] ?? .ask + tool.enforceTermindApprovalPolicy() return tool } draft.negotiatedVersion = outcome.protocolVersion diff --git a/TFI/Features/Settings/MobileMCPSettings.swift b/TFI/Features/Settings/MobileMCPSettings.swift index c98c3fdd..de1309d7 100644 --- a/TFI/Features/Settings/MobileMCPSettings.swift +++ b/TFI/Features/Settings/MobileMCPSettings.swift @@ -146,8 +146,8 @@ private struct MobileMCPStatusBadge: View { Text("Disabled") case .notConnected: Text("Not Connected") - case .tools(let enabled, let total): - Text("\(enabled)/\(total) tools") + case .tools(let count): + Text("\(count) tools") } } @@ -164,7 +164,7 @@ nonisolated enum MobileMCPServerStatus: Equatable { case conflict case disabled case notConnected - case tools(enabled: Int, total: Int) + case tools(count: Int) init(server: MCPServer, hasNamespaceConflict: Bool) { if hasNamespaceConflict { @@ -174,7 +174,7 @@ nonisolated enum MobileMCPServerStatus: Equatable { } else if server.lastConnectedAt == nil { self = .notConnected } else { - self = .tools(enabled: server.enabledTools.count, total: server.tools.count) + self = .tools(count: server.tools.count) } } } @@ -261,14 +261,16 @@ struct MobileMCPEditor: View { Text(tool.summary).font(.caption).foregroundStyle(.secondary).lineLimit(2) } Picker("Permission", selection: $tool.accessPolicy) { - Text("Disabled").tag(MCPTool.AccessPolicy.disabled) - Text("Always Ask").tag(MCPTool.AccessPolicy.alwaysAsk) - Text("Follow Permissions").tag(MCPTool.AccessPolicy.followPermissions) + Text("Deny").tag(AgentToolDescriptor.ApprovalPolicy.deny) + Text("Ask").tag(AgentToolDescriptor.ApprovalPolicy.ask) + Text("Approve") + .tag(AgentToolDescriptor.ApprovalPolicy.approve) + .disabled(!tool.allowsTermindPreapproval) } } } Text( - "Always Ask requires approval for every call. Follow Permissions uses the conversation's permission mode and safety checks." + "Deny refuses the call. Ask uses the conversation's manual or automatic approval. Approve runs without approval and is unavailable for tools marked destructive." ) .font(.caption).foregroundStyle(.secondary) } diff --git a/TFITests/MobileAISettingsTests.swift b/TFITests/MobileAISettingsTests.swift index 6ecb0487..4ac33835 100644 --- a/TFITests/MobileAISettingsTests.swift +++ b/TFITests/MobileAISettingsTests.swift @@ -186,7 +186,7 @@ struct MobileAISettingsTests { @Test func mcpDiscoveryUsesDraftAndPreservesPermissionsUntilSave() async throws { var tool = MCPTool(id: "existing") - tool.accessPolicy = .disabled + tool.accessPolicy = .deny let server = MCPServer(name: "Gateway", namespaceID: "", url: "https://original.invalid/mcp", tools: [tool]) let ai = AIConfigurationModel() let editor = MobileMCPEditorModel( @@ -203,10 +203,10 @@ struct MobileAISettingsTests { editor.connect() while editor.running { await Task.yield() } #expect(ai.mcpServers.isEmpty) - #expect(editor.draft.tools.map(\.accessPolicy) == [.disabled, .alwaysAsk]) + #expect(editor.draft.tools.map(\.accessPolicy) == [.deny, .ask]) try editor.save() #expect(ai.mcpServer(server.id)?.namespaceID == "gateway") - #expect(ai.mcpServer(server.id)?.tools.map(\.accessPolicy) == [.disabled, .alwaysAsk]) + #expect(ai.mcpServer(server.id)?.tools.map(\.accessPolicy) == [.deny, .ask]) } @Test func mcpCancellationAndRemoteDeletionCannotRestoreResults() async { @@ -345,12 +345,11 @@ struct MobileAISettingsTests { server.isEnabled = true server.lastConnectedAt = Date(timeIntervalSince1970: 1) - var disabled = MCPTool(id: "disabled") - disabled.accessPolicy = .disabled - server.tools = [MCPTool(id: "active"), disabled] + let denied = MCPTool(id: "denied", accessPolicy: .deny) + server.tools = [MCPTool(id: "active"), denied] #expect( MobileMCPServerStatus(server: server, hasNamespaceConflict: false) - == .tools(enabled: 1, total: 2)) + == .tools(count: 2)) } } diff --git a/TFITests/MobileAssistantRenderingTests.swift b/TFITests/MobileAssistantRenderingTests.swift index d0b92808..658cb71b 100644 --- a/TFITests/MobileAssistantRenderingTests.swift +++ b/TFITests/MobileAssistantRenderingTests.swift @@ -197,7 +197,7 @@ struct MobileAssistantRenderingTests { summary: "Write a file", inputSchema: .object([:]), target: .local, - safety: .requiresAuthorization + approvalPolicy: .ask ), userIntent: text( "Enable maintenance mode.", "启用维护模式。", "啟用維護模式。"), diff --git a/TFITests/MobileAssistantTests.swift b/TFITests/MobileAssistantTests.swift index 8f14b31a..0135f511 100644 --- a/TFITests/MobileAssistantTests.swift +++ b/TFITests/MobileAssistantTests.swift @@ -83,7 +83,7 @@ struct MobileAssistantTests { summary: "Write data", inputSchema: .object([:]), target: .local, - safety: .requiresAuthorization + approvalPolicy: .ask ), userIntent: "Update the requested data", localReasons: (0..<8).map { "Reason \($0) requires confirmation." } diff --git a/TFM/Application/FeatureModels/ToolAuditModel.swift b/TFM/Application/FeatureModels/ToolAuditModel.swift index 32650002..fc382545 100644 --- a/TFM/Application/FeatureModels/ToolAuditModel.swift +++ b/TFM/Application/FeatureModels/ToolAuditModel.swift @@ -111,7 +111,7 @@ final class ToolAuditModel { hostID: hostID, hostDisplayName: hostID.flatMap(hostName), summary: Self.summary(for: invocation.call), - risk: Self.risk(for: invocation.call, descriptor: descriptor), + risk: Self.risk(for: invocation.call), bytesWritten: Self.bytesWritten(for: invocation.call), startedAt: invocation.createdAt ) @@ -210,9 +210,7 @@ final class ToolAuditModel { return content.lengthOfBytes(using: .utf8) } - private static func risk( - for call: AgentToolCall, descriptor: AgentToolDescriptor - ) -> ExternalToolCall.Risk { + private static func risk(for call: AgentToolCall) -> ExternalToolCall.Risk { if call.name == "ssh_exec", let command = call.arguments.objectValue?["command"]?.stringValue { @@ -222,7 +220,12 @@ final class ToolAuditModel { case .destructive: return .destructive } } - if descriptor.safety.isAutoAllowed { return .readOnly } + if [ + "list_hosts", "get_metrics", "request_user_secret", "ssh_wait", + "sftp_list", "sftp_stat", "sftp_read", + ].contains(call.name) { + return .readOnly + } return call.name == "sftp_delete" ? .destructive : .modifiesState } } diff --git a/TFM/Features/Settings/MCPServerEditorSheet.swift b/TFM/Features/Settings/MCPServerEditorSheet.swift index d476c14b..eb3531de 100644 --- a/TFM/Features/Settings/MCPServerEditorSheet.swift +++ b/TFM/Features/Settings/MCPServerEditorSheet.swift @@ -245,7 +245,7 @@ struct MCPServerEditorPage: View { } footer: { if !server.tools.isEmpty { Text( - "Always Ask requires approval for every call. Follow Permissions uses the conversation's permission mode and safety checks." + "Deny refuses the call. Ask uses the conversation's manual or automatic approval. Approve runs without approval and is unavailable for tools marked destructive." ) .font(.caption) .foregroundStyle(.secondary) @@ -338,10 +338,7 @@ struct MCPServerEditorPage: View { // MARK: - Tool row -/// One discovered tool, with its two switches. -/// -/// "Ask First" rather than "Requires Approval": the label describes what will -/// happen to you, not the internal state. +/// One discovered tool and the local policy applied when the model calls it. private struct ToolRow: View { let tool: MCPTool let serverID: MCPServer.ID @@ -375,8 +372,10 @@ private struct ToolRow: View { .frame(minWidth: 0, maxWidth: .infinity, alignment: .leading) Picker("Permission", selection: permission) { - ForEach(MCPTool.AccessPolicy.allCases) { option in - Text(option.displayName).tag(option) + ForEach(AgentToolDescriptor.ApprovalPolicy.allCases) { option in + Text(option.displayName) + .tag(option) + .disabled(option == .approve && !tool.allowsTermindPreapproval) } } .labelsHidden() @@ -389,7 +388,7 @@ private struct ToolRow: View { .help(Text(tool.summary.isEmpty ? qualifiedToolName : tool.summary)) } - private var permission: Binding { + private var permission: Binding { Binding( get: { tool.accessPolicy }, set: { policy in @@ -399,12 +398,12 @@ private struct ToolRow: View { } } -extension MCPTool.AccessPolicy { +extension AgentToolDescriptor.ApprovalPolicy { fileprivate var displayName: LocalizedStringKey { switch self { - case .disabled: "Disabled" - case .alwaysAsk: "Always Ask" - case .followPermissions: "Follow Permissions" + case .deny: "Deny" + case .ask: "Ask" + case .approve: "Approve" } } } diff --git a/TFM/Features/Settings/MCPSettings.swift b/TFM/Features/Settings/MCPSettings.swift index c40e3500..92e7c087 100644 --- a/TFM/Features/Settings/MCPSettings.swift +++ b/TFM/Features/Settings/MCPSettings.swift @@ -87,7 +87,7 @@ private struct MCPServerRow: View { if server.tools.isEmpty { CardChip(text: String(localized: "Not Connected"), symbol: "questionmark.circle") } else { - CardChip(text: "\(server.enabledTools.count)/\(server.tools.count)", symbol: "wrench.and.screwdriver") + CardChip(text: "\(server.tools.count)", symbol: "wrench.and.screwdriver") } Toggle("", isOn: enabled) diff --git a/TFM/Infrastructure/MCP/MCPServerProtocol.swift b/TFM/Infrastructure/MCP/MCPServerProtocol.swift index b35c96a1..e95e09ae 100644 --- a/TFM/Infrastructure/MCP/MCPServerProtocol.swift +++ b/TFM/Infrastructure/MCP/MCPServerProtocol.swift @@ -365,7 +365,10 @@ actor MCPServerProtocol { } private static func toolJSON(_ descriptor: AgentToolDescriptor) -> AgentJSONValue { - let readOnly = descriptor.safety.isAutoAllowed + let readOnly = [ + "list_hosts", "get_metrics", "request_user_secret", "ssh_wait", + "sftp_list", "sftp_stat", "sftp_read", + ].contains(descriptor.name) let destructive = ["ssh_exec", "sftp_write", "sftp_move", "sftp_chmod", "sftp_delete"] .contains(descriptor.name) let openWorld = !["list_hosts", "ssh_wait", "request_user_secret"] diff --git a/TFMTests/AIProviderPersistenceTests.swift b/TFMTests/AIProviderPersistenceTests.swift index 16533894..4f70f3b1 100644 --- a/TFMTests/AIProviderPersistenceTests.swift +++ b/TFMTests/AIProviderPersistenceTests.swift @@ -33,7 +33,7 @@ struct AIProviderPersistenceTests { title: "Search", readOnlyHint: true, destructiveHint: false, idempotentHint: true, openWorldHint: false ), - accessPolicy: .followPermissions + accessPolicy: .approve ) let server = MCPServer( name: "Work Gateway", @@ -73,7 +73,7 @@ struct AIProviderPersistenceTests { let container = try makeContainer("MCPRediscovery") var server = MCPServer(name: "Gateway") server.tools = [ - MCPTool(id: "existing", accessPolicy: .followPermissions) + MCPTool(id: "existing", accessPolicy: .approve) ] let store = AIConfigurationModel( modelContext: container.mainContext, persistsSecrets: false @@ -90,8 +90,8 @@ struct AIProviderPersistenceTests { modelContext: container.mainContext, persistsSecrets: false ) let tools = try #require(reloaded.mcpServer(server.id)?.tools) - #expect(tools.first { $0.id == "existing" }?.accessPolicy == .followPermissions) - #expect(tools.first { $0.id == "new" }?.accessPolicy == .alwaysAsk) + #expect(tools.first { $0.id == "existing" }?.accessPolicy == .approve) + #expect(tools.first { $0.id == "new" }?.accessPolicy == .ask) #expect(reloaded.mcpServer(server.id)?.negotiatedVersion == "2025-06-18") #expect(reloaded.mcpServer(server.id)?.lastConnectedAt != nil) } diff --git a/TFMTests/AgentApprovalQueueTests.swift b/TFMTests/AgentApprovalQueueTests.swift index 69a49a51..36ca2687 100644 --- a/TFMTests/AgentApprovalQueueTests.swift +++ b/TFMTests/AgentApprovalQueueTests.swift @@ -63,7 +63,7 @@ private func gatedTool(_ name: String, log: CallLog) -> AnyAgentTool { AnyAgentTool( descriptor: AgentToolDescriptor( name: name, summary: name, inputSchema: queueSchema, - target: .host, safety: .requiresAuthorization, concurrency: .parallel + target: .host, approvalPolicy: .ask, concurrency: .parallel ) ) { invocation, _ in await log.start(name) @@ -77,7 +77,7 @@ private func hangingTool(_ name: String, log: CallLog) -> AnyAgentTool { AnyAgentTool( descriptor: AgentToolDescriptor( name: name, summary: name, inputSchema: queueSchema, - target: .local, safety: .locallyReadOnly, concurrency: .parallel + target: .local, approvalPolicy: .approve, concurrency: .parallel ) ) { invocation, _ in await log.start(name) @@ -203,7 +203,7 @@ struct AgentApprovalQueueTests { invocation: invocation, descriptor: AgentToolDescriptor( name: "review", summary: "review", inputSchema: queueSchema, - target: .host, safety: .requiresAuthorization + target: .host, approvalPolicy: .ask ), userIntent: "go" ) diff --git a/TFMTests/AgentBackgroundCommandTests.swift b/TFMTests/AgentBackgroundCommandTests.swift index 99df8e54..792be2d3 100644 --- a/TFMTests/AgentBackgroundCommandTests.swift +++ b/TFMTests/AgentBackgroundCommandTests.swift @@ -308,7 +308,7 @@ struct AgentBackgroundCommandTests { #expect(wait != nil) // It reads a buffer and a task. Nothing it does reaches the host, so // stopping to ask permission for it would be asking about nothing. - #expect(wait?.descriptor.safety == .locallyReadOnly) + #expect(wait?.descriptor.approvalPolicy == .approve) #expect(wait?.descriptor.concurrency == .parallel) // No kill: closing a channel does not stop a remote process, so a tool // by that name would promise what it cannot do. diff --git a/TFMTests/AgentInterruptedRunRecoveryTests.swift b/TFMTests/AgentInterruptedRunRecoveryTests.swift index 3ea44fe6..2aa915b1 100644 --- a/TFMTests/AgentInterruptedRunRecoveryTests.swift +++ b/TFMTests/AgentInterruptedRunRecoveryTests.swift @@ -154,7 +154,7 @@ struct AgentInterruptedRunRecoveryTests { invocation: invocation, descriptor: AgentToolDescriptor( name: "ssh_exec", summary: "exec", inputSchema: .object([:]), - target: .host, safety: .requiresAuthorization + target: .host, approvalPolicy: .ask ), state: "started", result: nil ) diff --git a/TFMTests/AgentPersistenceTests.swift b/TFMTests/AgentPersistenceTests.swift index e532c55a..b9396a75 100644 --- a/TFMTests/AgentPersistenceTests.swift +++ b/TFMTests/AgentPersistenceTests.swift @@ -43,7 +43,7 @@ struct AgentPersistenceTests { ) let descriptor = AgentToolDescriptor( name: call.name, summary: "Search", inputSchema: .object([:]), - target: .network, safety: .locallyReadOnly, presentation: presentation + target: .network, approvalPolicy: .approve, presentation: presentation ) var result = AgentToolResult( callID: call.id, @@ -307,7 +307,7 @@ struct AgentPersistenceTests { ) let descriptor = AgentToolDescriptor( name: "web_search", summary: "Search", inputSchema: .object([:]), - target: .network, safety: .locallyReadOnly + target: .network, approvalPolicy: .approve ) try await repository.record(.toolProposed(invocation, descriptor), runID: snapshot.id) try await repository.record( @@ -357,7 +357,7 @@ struct AgentPersistenceTests { invocation, AgentToolDescriptor( name: "ssh_exec", summary: "exec", inputSchema: .object([:]), - target: .host, safety: .requiresAuthorization + target: .host, approvalPolicy: .ask )), runID: snapshot.id ) @@ -423,7 +423,7 @@ struct AgentPersistenceTests { invocation, AgentToolDescriptor( name: "fetch", summary: "fetch", inputSchema: .object([:]), - target: .network, safety: .locallyReadOnly + target: .network, approvalPolicy: .approve )), runID: snapshot.id ) @@ -610,7 +610,7 @@ struct AgentPersistenceTests { invocation, AgentToolDescriptor( name: "ssh_exec", summary: "exec", inputSchema: .object([:]), - target: .host, safety: .requiresAuthorization + target: .host, approvalPolicy: .ask )), runID: invocation.runID ) @@ -651,7 +651,7 @@ struct AgentPersistenceTests { invocation: invocation, descriptor: AgentToolDescriptor( name: "list_hosts", summary: "List", inputSchema: .object([:]), - target: .local, safety: .locallyReadOnly + target: .local, approvalPolicy: .approve ), userIntent: "List hosts" ) diff --git a/TFMTests/AgentPlanModeTests.swift b/TFMTests/AgentPlanModeTests.swift index 1eb28448..40e19d0c 100644 --- a/TFMTests/AgentPlanModeTests.swift +++ b/TFMTests/AgentPlanModeTests.swift @@ -45,13 +45,14 @@ private let emptySchema = AgentJSONValue.object([ private func spyTool( _ name: String, - safety: AgentToolDescriptor.Safety, + approvalPolicy: AgentToolDescriptor.ApprovalPolicy, + target: AgentToolDescriptor.Target = .local, ran: RanFlag ) -> AnyAgentTool { AnyAgentTool( descriptor: AgentToolDescriptor( name: name, summary: name, inputSchema: emptySchema, - target: .local, safety: safety, concurrency: .parallel + target: target, approvalPolicy: approvalPolicy, concurrency: .parallel ), execute: { invocation, _ in await ran.mark() @@ -61,7 +62,7 @@ private func spyTool( } /// An `ssh_exec` stand-in whose preflight is the real one: the classifier -/// decides its safety, exactly as `AgentBuiltInSSHTools` does. +/// decides its approval policy, exactly as `AgentBuiltInSSHTools` does. private func execTool(ran: RanFlag) -> AnyAgentTool { let schema = AgentJSONValue.object([ "type": .string("object"), @@ -72,14 +73,14 @@ private func execTool(ran: RanFlag) -> AnyAgentTool { return AnyAgentTool( descriptor: AgentToolDescriptor( name: TermindAgentPolicy.sshExecToolName, summary: "exec", - inputSchema: schema, target: .host, safety: .requiresAuthorization + inputSchema: schema, target: .host, approvalPolicy: .ask ), preflight: { invocation in let command = invocation.call.arguments.objectValue?["command"]?.stringValue ?? "" let safe = CommandRiskClassifier.assess(command).risk == .safe return AgentToolPreflight( invocation: invocation, - safety: safe ? .locallyReadOnly : .requiresAuthorization + approvalPolicy: safe ? .approve : .ask ) }, execute: { invocation, _ in @@ -148,7 +149,7 @@ struct AgentPlanModeTests { func planModeRefusesAToolThatChangesSomethingOutsideTermind() async { let ran = RanFlag() let events = await drain( - tools: [spyTool("sftp_write", safety: .requiresAuthorization, ran: ran)], + tools: [spyTool("sftp_write", approvalPolicy: .ask, ran: ran)], turns: callingTurns(["sftp_write"]), isPlanning: true ) @@ -163,7 +164,7 @@ struct AgentPlanModeTests { func theSameToolRunsOnceThePostureIsOff() async { let ran = RanFlag() _ = await drain( - tools: [spyTool("sftp_write", safety: .requiresAuthorization, ran: ran)], + tools: [spyTool("sftp_write", approvalPolicy: .ask, ran: ran)], turns: callingTurns(["sftp_write"]), isPlanning: false ) @@ -174,7 +175,7 @@ struct AgentPlanModeTests { func readsAreUntouchedWhilePlanning() async { let ran = RanFlag() let events = await drain( - tools: [spyTool("sftp_read", safety: .locallyReadOnly, ran: ran)], + tools: [spyTool("sftp_read", approvalPolicy: .approve, ran: ran)], turns: callingTurns(["sftp_read"]), isPlanning: true ) @@ -188,7 +189,7 @@ struct AgentPlanModeTests { func containedWritesAreAllowedSoThePlanCanBeDrafted() async { let ran = RanFlag() _ = await drain( - tools: [spyTool("scratch_edit", safety: .locallyContained, ran: ran)], + tools: [spyTool("scratch_edit", approvalPolicy: .approve, ran: ran)], turns: callingTurns(["scratch_edit"]), isPlanning: true ) @@ -235,6 +236,28 @@ struct AgentPlanModeTests { #expect(await ran.didRun) } + @Test + func mcpPoliciesRemainAuthoritativeWhilePlanning() async { + let asked = RanFlag() + let approved = RanFlag() + let denied = RanFlag() + let events = await drain( + tools: [ + spyTool("mcp_ask", approvalPolicy: .ask, target: .mcp, ran: asked), + spyTool("mcp_approve", approvalPolicy: .approve, target: .mcp, ran: approved), + spyTool("mcp_deny", approvalPolicy: .deny, target: .mcp, ran: denied), + ], + turns: callingTurns(["mcp_ask", "mcp_approve", "mcp_deny"]), + isPlanning: true + ) + + #expect(await asked.didRun) + #expect(await approved.didRun) + #expect(await denied.didRun == false) + #expect(results(events).count == 3) + #expect(results(events).contains { $0.isApprovalPolicyDenied }) + } + @Test @MainActor func sshExecPreflightUsesClassifierForApprovalAndConcurrency() async throws { @@ -268,12 +291,12 @@ struct AgentPlanModeTests { } let read = try await tool.preflight(invocation("systemctl status nginx")) - #expect(read.safety == .locallyReadOnly) + #expect(read.approvalPolicy == .approve) #expect(read.concurrency == .parallel) #expect(read.reasons.isEmpty) let write = try await tool.preflight(invocation("systemctl restart nginx")) - #expect(write.safety == .requiresAuthorization) + #expect(write.approvalPolicy == .ask) #expect(write.concurrency == .sequential) #expect(!write.reasons.isEmpty) } @@ -318,21 +341,21 @@ struct AgentPlanModeTests { func askingTheUserIsNotBlockedWhilePlanning() async { let ran = RanFlag() _ = await drain( - tools: [spyTool("request_user_input", safety: .locallyReadOnly, ran: ran)], + tools: [spyTool("request_user_input", approvalPolicy: .approve, ran: ran)], turns: callingTurns(["request_user_input"]), isPlanning: true ) #expect(await ran.didRun) } - /// Blocked by name rather than by safety: `manage_tasks` changes nothing, so - /// the safety gate has no reason to stop it, and it still has no business + /// Blocked by name rather than by approval policy: `manage_tasks` changes + /// nothing, so the approval gate has no reason to stop it, and it still has no business /// running before anything has been agreed to. @Test func theTaskListIsRefusedWhilePlanningEvenThoughItIsReadOnly() async { let ran = RanFlag() let events = await drain( - tools: [spyTool("manage_tasks", safety: .locallyReadOnly, ran: ran)], + tools: [spyTool("manage_tasks", approvalPolicy: .approve, ran: ran)], turns: callingTurns(["manage_tasks"]), isPlanning: true ) @@ -344,7 +367,7 @@ struct AgentPlanModeTests { func theTaskListRunsOutsidePlanMode() async { let ran = RanFlag() _ = await drain( - tools: [spyTool("manage_tasks", safety: .locallyReadOnly, ran: ran)], + tools: [spyTool("manage_tasks", approvalPolicy: .approve, ran: ran)], turns: callingTurns(["manage_tasks"]), isPlanning: false ) @@ -359,8 +382,8 @@ struct AgentPlanModeTests { let ran = RanFlag() let events = await drain( tools: [ - spyTool("sftp_read", safety: .locallyReadOnly, ran: ran), - spyTool("sftp_write", safety: .requiresAuthorization, ran: ran), + spyTool("sftp_read", approvalPolicy: .approve, ran: ran), + spyTool("sftp_write", approvalPolicy: .ask, ran: ran), ], turns: callingTurns(["sftp_read", "sftp_write"]), isPlanning: true ) @@ -377,7 +400,7 @@ struct AgentPlanModeTests { tools: [ spyTool( AgentPlanModeHook.presentPlanToolName, - safety: .locallyReadOnly, ran: ran + approvalPolicy: .approve, ran: ran ) ], turns: callingTurns([AgentPlanModeHook.presentPlanToolName]), @@ -401,7 +424,7 @@ struct AgentPlanModeTests { tools: [ spyTool( AgentPlanModeHook.presentPlanToolName, - safety: .locallyReadOnly, ran: ran + approvalPolicy: .approve, ran: ran ) ], turns: callingTurns([AgentPlanModeHook.presentPlanToolName]), diff --git a/TFMTests/AgentRuntimeTests.swift b/TFMTests/AgentRuntimeTests.swift index b232550c..c605ae55 100644 --- a/TFMTests/AgentRuntimeTests.swift +++ b/TFMTests/AgentRuntimeTests.swift @@ -231,7 +231,7 @@ struct AgentRuntimeTests { "type": .string("object"), "properties": .object([:]), "required": .array([]), "additionalProperties": .bool(false), ]), - target: .local, safety: .locallyReadOnly + target: .local, approvalPolicy: .approve ) ) { invocation, _ in AgentToolResult(callID: invocation.call.id, content: "ok") @@ -313,7 +313,7 @@ struct AgentRuntimeTests { descriptor: AgentToolDescriptor( name: name, summary: name, inputSchema: .object(["type": .string("object")]), - target: .local, safety: .locallyReadOnly + target: .local, approvalPolicy: .approve ) ) { invocation, _ in await recorder.append(name) @@ -341,7 +341,7 @@ struct AgentRuntimeTests { descriptor: AgentToolDescriptor( name: "wait", summary: "wait", inputSchema: .object(["type": .string("object")]), - target: .local, safety: .locallyReadOnly + target: .local, approvalPolicy: .approve ) ) { invocation, _ in await gate.beginAndWait() @@ -501,7 +501,7 @@ struct AgentRuntimeTests { "required": .array([.string("path")]), "additionalProperties": .bool(false), ]), - target: .host, safety: .requiresAuthorization + target: .host, approvalPolicy: .ask ) ) { invocation, _ in await executions.increment() @@ -542,7 +542,7 @@ struct AgentRuntimeTests { "type": .string("object"), "properties": .object([:]), "required": .array([]), "additionalProperties": .bool(false), ]), - target: .host, safety: .locallyReadOnly + target: .host, approvalPolicy: .approve ) ) { invocation, _ in #expect(invocation.call.arguments == .object([:])) @@ -581,7 +581,7 @@ struct AgentRuntimeTests { "type": .string("object"), "properties": .object([:]), "required": .array([]), "additionalProperties": .bool(false), ]), - target: .host, safety: .locallyReadOnly + target: .host, approvalPolicy: .approve ) ) { invocation, _ in #expect(invocation.call.arguments == .object([:])) @@ -604,7 +604,7 @@ struct AgentRuntimeTests { func approvalModesRouteOnlyApproveForMeToReviewer() async { let descriptor = AgentToolDescriptor( name: "write", summary: "write", inputSchema: .object([:]), - target: .host, safety: .requiresAuthorization + target: .host, approvalPolicy: .ask ) let invocation = AgentToolInvocation( runID: UUID(), call: AgentToolCall(id: "1", name: "write", arguments: .object([:])) @@ -654,20 +654,20 @@ struct AgentRuntimeTests { "type": .string("object"), "properties": .object([:]), "required": .array([]), "additionalProperties": .bool(false), ]), - target: .network, safety: .locallyReadOnly + target: .network, approvalPolicy: .approve ) let tool = AnyAgentTool( descriptor: descriptor, preflight: { invocation in AgentToolPreflight( - invocation: invocation, safety: .locallyReadOnly, + invocation: invocation, approvalPolicy: .approve, executionMetadata: ["scope": .string("public")] ) }, execute: { invocation, context in #expect(context.preflightMetadata["scope"] == .string("public")) var escalation = descriptor - escalation.safety = .requiresAuthorization + escalation.approvalPolicy = .ask let decision = await context.authorize( AgentApprovalRequest( invocation: invocation, descriptor: escalation, @@ -709,7 +709,7 @@ struct AgentRuntimeTests { func reviewerFailureAndCriticalRiskFailClosed() async { let descriptor = AgentToolDescriptor( name: "delete", summary: "delete", inputSchema: .object([:]), - target: .host, safety: .requiresAuthorization + target: .host, approvalPolicy: .ask ) let request = AgentApprovalRequest( invocation: AgentToolInvocation( @@ -750,7 +750,7 @@ struct AgentRuntimeTests { ), descriptor: AgentToolDescriptor( name: "write", summary: "write", inputSchema: .object([:]), - target: .host, safety: .requiresAuthorization + target: .host, approvalPolicy: .ask ), userIntent: "change it" ) @@ -826,7 +826,7 @@ struct AgentRuntimeTests { "type": .string("object"), "properties": .object([:]), "required": .array([]), "additionalProperties": .bool(false), ]), - target: .host, safety: .locallyReadOnly + target: .host, approvalPolicy: .approve ) ) { invocation, _ in await executed.append(origin) @@ -873,7 +873,7 @@ struct AgentRuntimeTests { "type": .string("object"), "properties": .object([:]), "required": .array([]), "additionalProperties": .bool(false), ]), - target: .local, safety: .locallyReadOnly + target: .local, approvalPolicy: .approve ) ) { invocation, _ in AgentToolResult(callID: invocation.call.id, content: "ok") @@ -917,30 +917,24 @@ struct AgentRuntimeTests { } @Test - func mcpToolAccessPoliciesControlExposureAndApproval() throws { + func mcpToolAccessPoliciesControlApprovalWithoutHidingTools() throws { var server = MCPServer(name: "Gateway") + var destructive = MCPTool(id: "destructive", accessPolicy: .ask) + destructive.annotations.destructiveHint = true server.tools = [ - MCPTool(id: "disabled", accessPolicy: .disabled), - MCPTool(id: "always-ask", accessPolicy: .alwaysAsk), - MCPTool(id: "follow-permissions", accessPolicy: .followPermissions), + MCPTool(id: "denied", accessPolicy: .deny), + MCPTool(id: "asked", accessPolicy: .ask), + MCPTool(id: "approved", accessPolicy: .approve), + destructive, ] let descriptors = AgentMCPTools.make(servers: [(server: server, bearerToken: nil)]) .map(\.descriptor) - #expect(descriptors.count == 2) - #expect(!descriptors.contains { $0.name == "disabled" }) - - let alwaysAsk = try #require(descriptors.first { $0.name == "always-ask" }) - #expect(alwaysAsk.safety == .requiresAuthorization) - #expect(alwaysAsk.alwaysAskUser) - #expect(alwaysAsk.concurrency == .sequential) - - let followPermissions = try #require( - descriptors.first { $0.name == "follow-permissions" } - ) - #expect(followPermissions.safety == .requiresAuthorization) - #expect(!followPermissions.alwaysAskUser) - #expect(followPermissions.concurrency == .parallel) + #expect(descriptors.count == 4) + #expect(descriptors.first { $0.name == "denied" }?.approvalPolicy == .deny) + #expect(descriptors.first { $0.name == "asked" }?.approvalPolicy == .ask) + #expect(descriptors.first { $0.name == "approved" }?.approvalPolicy == .approve) + #expect(descriptors.first { $0.name == "destructive" }?.concurrency == .sequential) } @Test @@ -984,7 +978,7 @@ private func hangingTool(_ gate: AsyncGate, onCancel: Recorder? = nil) -> AnyAge "type": .string("object"), "properties": .object([:]), "required": .array([]), "additionalProperties": .bool(false), ]), - target: .local, safety: .locallyReadOnly, concurrency: .parallel + target: .local, approvalPolicy: .approve, concurrency: .parallel ) ) { invocation, _ in await gate.begin() diff --git a/TFMTests/AgentScratchTests.swift b/TFMTests/AgentScratchTests.swift index f260b6c6..55ea6ae3 100644 --- a/TFMTests/AgentScratchTests.swift +++ b/TFMTests/AgentScratchTests.swift @@ -472,12 +472,10 @@ struct AgentScratchTests { } } - // MARK: - Safety + // MARK: - Approval policy - /// `.locallyContained` has to mean the same thing in every mode, or it is just - /// `.requiresAuthorization` with extra steps. @Test - func containedActionsSkipTheGateInEveryPermissionMode() async { + func approvedActionsSkipTheGateInEveryPermissionMode() async { for mode in AgentPermissionMode.allCases { let reviewer = CountingSecurityReviewer() let broker = AgentApprovalBroker( @@ -485,24 +483,22 @@ struct AgentScratchTests { manualApproval: { _ in .deny("a person was asked") } ) let allowed = await broker.authorize( - Fixture.request(safety: .locallyContained), mode: mode + Fixture.request(approvalPolicy: .approve), mode: mode ) - #expect(allowed == .allow, "\(mode) gated a contained action") - #expect(await reviewer.count == 0, "\(mode) sent a contained action to review") + #expect(allowed == .allow, "\(mode) gated an approved action") + #expect(await reviewer.count == 0, "\(mode) sent an approved action to review") } } - /// The other half of the claim: adding a third case widened nothing. An action - /// that needs authorization still reaches whichever gate the mode nominates. @Test - func remoteWritesStillReachTheGate() async { + func askedRemoteWritesReachTheSelectedGate() async { let asked = CountingSecurityReviewer() let manual = AgentApprovalBroker( reviewer: asked, manualApproval: { _ in .deny("denied by the user") } ) #expect( await manual.authorize( - Fixture.request(safety: .requiresAuthorization), mode: .askForApproval + Fixture.request(approvalPolicy: .ask), mode: .askForApproval ) == .deny("denied by the user") ) // Ask for approval asks the person, not the reviewer. @@ -515,19 +511,12 @@ struct AgentScratchTests { ) #expect( await automatic.authorize( - Fixture.request(safety: .requiresAuthorization), mode: .approveForMe + Fixture.request(approvalPolicy: .ask), mode: .approveForMe ) == .allow ) #expect(await reviewed.count == 1) } - @Test - func autoAllowIsAPropertyOfTheCaseAndNotOfTheCallSite() { - #expect(AgentToolDescriptor.Safety.locallyReadOnly.isAutoAllowed) - #expect(AgentToolDescriptor.Safety.locallyContained.isAutoAllowed) - #expect(!AgentToolDescriptor.Safety.requiresAuthorization.isAutoAllowed) - } - // MARK: - Registry /// A card records the presenter ID it was written with, and older cards record @@ -573,19 +562,10 @@ struct AgentScratchTests { "scratch_copy", "scratch_delete", "scratch_diff", "scratch_list", "scratch_move", "scratch_read", "scratch_replace", "scratch_search", "scratch_write", ]) - // Every purely local tool is parallel, and only the ones that change - // something claim to. + // Every purely local scratch tool is parallel and pre-approved. #expect(local.allSatisfy { $0.descriptor.concurrency == .parallel }) #expect(local.allSatisfy { $0.descriptor.target == .local }) - #expect( - local.filter { $0.descriptor.safety == .locallyContained } - .map(\.descriptor.name).sorted() - == [ - "scratch_copy", "scratch_delete", "scratch_move", "scratch_replace", - "scratch_write", - ] - ) - #expect(!local.contains { $0.descriptor.safety == .requiresAuthorization }) + #expect(local.allSatisfy { $0.descriptor.approvalPolicy == .approve }) let search = try #require(local.first { $0.descriptor.name == "scratch_search" }) let properties = search.descriptor.inputSchema.objectValue?["properties"]?.objectValue #expect( @@ -604,12 +584,12 @@ struct AgentScratchTests { #expect(transfer.count == 2) #expect(transfer.allSatisfy { $0.descriptor.target == .host }) #expect( - transfer.first { $0.descriptor.name == "scratch_push" }?.descriptor.safety - == .requiresAuthorization + transfer.first { $0.descriptor.name == "scratch_push" }?.descriptor.approvalPolicy + == .ask ) #expect( - transfer.first { $0.descriptor.name == "scratch_pull" }?.descriptor.safety - == .locallyContained + transfer.first { $0.descriptor.name == "scratch_pull" }?.descriptor.approvalPolicy + == .approve ) // A pull is a remote read landing in contained storage and may overlap // anything; a push changes a host and takes its turn serial. @@ -628,7 +608,7 @@ struct AgentScratchTests { let withWeb = Fixture.scratchTools(workspace: workspace, web: Fixture.web()) let fetch = try #require(withWeb.first { $0.descriptor.name == "scratch_fetch" }) #expect(fetch.descriptor.target == .network) - #expect(fetch.descriptor.safety == .locallyContained) + #expect(fetch.descriptor.approvalPolicy == .approve) #expect(fetch.descriptor.concurrency == .parallel) } @@ -644,7 +624,7 @@ struct AgentScratchTests { let properties = fetch?.inputSchema.objectValue?["properties"]?.objectValue ?? [:] #expect(properties["scratch_path"] == nil) #expect(properties.keys.sorted() == ["format", "url"]) - #expect(fetch?.safety == .locallyReadOnly) + #expect(fetch?.approvalPolicy == .approve) #expect(!tools.contains { $0.descriptor.name.hasPrefix("scratch_") }) } @@ -780,7 +760,7 @@ struct AgentScratchTests { let preflight = try await tool.preflight(invocation) let reasons = preflight.reasons.joined(separator: "\n") - #expect(preflight.safety == .requiresAuthorization) + #expect(preflight.approvalPolicy == .ask) #expect(preflight.invocation.targetHostID == Fixture.hostID) #expect(reasons.contains("app.conf")) #expect(reasons.contains("\(body.utf8.count) bytes")) @@ -1126,13 +1106,15 @@ private enum Fixture { ) } - static func request(safety: AgentToolDescriptor.Safety) -> AgentApprovalRequest { + static func request( + approvalPolicy: AgentToolDescriptor.ApprovalPolicy + ) -> AgentApprovalRequest { AgentApprovalRequest( invocation: invocation("t", [:]), descriptor: AgentToolDescriptor( name: "t", summary: "t", inputSchema: .object(["type": .string("object")]), - target: .local, safety: safety + target: .local, approvalPolicy: approvalPolicy ), userIntent: "do the thing" ) diff --git a/TFMTests/AgentSkillTests.swift b/TFMTests/AgentSkillTests.swift index bee00111..5830ebc6 100644 --- a/TFMTests/AgentSkillTests.swift +++ b/TFMTests/AgentSkillTests.swift @@ -328,9 +328,7 @@ struct LoadSkillToolTests { @Test func readsNothingAndSoNeedsNoApproval() { let descriptor = LoadSkillTool(skills: catalog).descriptor - #expect(descriptor.safety == .locallyReadOnly) - #expect(descriptor.safety.isAutoAllowed) - #expect(descriptor.safety.isAllowedWhilePlanning) + #expect(descriptor.approvalPolicy == .approve) #expect(descriptor.concurrency == .parallel) #expect(descriptor.target == .local) } diff --git a/TFMTests/AgentToolPresentationTests.swift b/TFMTests/AgentToolPresentationTests.swift index 2f40ed49..47bda8d7 100644 --- a/TFMTests/AgentToolPresentationTests.swift +++ b/TFMTests/AgentToolPresentationTests.swift @@ -37,7 +37,7 @@ struct AgentToolPresentationTests { descriptor: AgentToolDescriptor( name: "ssh_exec", summary: "Execute over SSH", inputSchema: .object([:]), target: .host, - safety: .requiresAuthorization, + approvalPolicy: .ask, presentation: .init( symbol: "terminal", activity: .semanticArgument(key: "command", fallback: .sshCommand), @@ -89,7 +89,7 @@ struct AgentToolPresentationTests { descriptor: AgentToolDescriptor( name: "remote_tool", summary: "Remote tool", inputSchema: .object([:]), target: .mcp, - safety: .requiresAuthorization + approvalPolicy: .ask ), userIntent: "Run it" ) @@ -862,7 +862,7 @@ struct AgentToolPresentationTests { descriptor: AgentToolDescriptor( name: name, summary: name, inputSchema: .object(["type": .string("object")]), - target: target, safety: .locallyReadOnly, presentation: presentation + target: target, approvalPolicy: .approve, presentation: presentation ) ) } diff --git a/TFMTests/AgentToolProgressTests.swift b/TFMTests/AgentToolProgressTests.swift index 3f2d4e69..c1124ef5 100644 --- a/TFMTests/AgentToolProgressTests.swift +++ b/TFMTests/AgentToolProgressTests.swift @@ -65,7 +65,7 @@ struct AgentToolProgressTests { let tool = AnyAgentTool( descriptor: AgentToolDescriptor( name: "slow", summary: "slow", inputSchema: emptySchema, - target: .local, safety: .locallyReadOnly, concurrency: .parallel + target: .local, approvalPolicy: .approve, concurrency: .parallel ) ) { invocation, context in context.report(AgentToolResult(callID: invocation.call.id, content: "10 of 20")) @@ -105,7 +105,7 @@ struct AgentToolProgressTests { ) let descriptor = AgentToolDescriptor( name: "slow", summary: "slow", inputSchema: emptySchema, - target: .local, safety: .locallyReadOnly + target: .local, approvalPolicy: .approve ) conversation.handle(.toolProposed(invocation, descriptor)) conversation.handle(.toolStarted(invocation)) diff --git a/TFMTests/ChatComposerStateTests.swift b/TFMTests/ChatComposerStateTests.swift index 4bc0cb32..f5105c2d 100644 --- a/TFMTests/ChatComposerStateTests.swift +++ b/TFMTests/ChatComposerStateTests.swift @@ -385,7 +385,7 @@ struct ChatComposerStateTests { invocation, AgentToolDescriptor( name: "lookup", summary: "lookup", inputSchema: .object([:]), - target: .local, safety: .locallyReadOnly + target: .local, approvalPolicy: .approve ))) conversation.handle(.toolStarted(invocation)) @@ -425,7 +425,7 @@ struct ChatComposerStateTests { invocation, AgentToolDescriptor( name: "lookup", summary: "lookup", inputSchema: .object([:]), - target: .local, safety: .locallyReadOnly + target: .local, approvalPolicy: .approve ))) let after = try #require(conversation.workGroups.first) @@ -466,7 +466,7 @@ struct ChatComposerStateTests { invocation, AgentToolDescriptor( name: "lookup", summary: "lookup", inputSchema: .object([:]), - target: .local, safety: .locallyReadOnly + target: .local, approvalPolicy: .approve ))) let next = AgentTranscriptMessage(role: .assistant) @@ -568,7 +568,7 @@ struct ChatComposerStateTests { invocation, AgentToolDescriptor( name: "lookup", summary: "lookup", inputSchema: .object([:]), - target: .local, safety: .locallyReadOnly + target: .local, approvalPolicy: .approve ))) conversation.handle( .toolFinished( @@ -755,7 +755,7 @@ struct ChatComposerStateTests { "required": .array([.string("query")]), "additionalProperties": .bool(false), ]), - target: .local, safety: .locallyReadOnly + target: .local, approvalPolicy: .approve ) ) { invocation, _ in #expect(invocation.call.arguments == .object(["query": .string("hosts")])) @@ -809,7 +809,7 @@ struct ChatComposerStateTests { "required": .array([.string("query")]), "additionalProperties": .bool(false), ]), - target: .local, safety: .locallyReadOnly + target: .local, approvalPolicy: .approve ) ) { invocation, _ in AgentToolResult(callID: invocation.call.id, content: "host-a") @@ -857,7 +857,7 @@ struct ChatComposerStateTests { "type": .string("object"), "properties": .object([:]), "required": .array([]), "additionalProperties": .bool(false), ]), - target: .local, safety: .locallyReadOnly + target: .local, approvalPolicy: .approve ) ) { invocation, _ in await probe.markStarted() @@ -925,7 +925,7 @@ struct ChatComposerStateTests { "type": .string("object"), "properties": .object([:]), "required": .array([]), "additionalProperties": .bool(false), ]), - target: .local, safety: .locallyReadOnly + target: .local, approvalPolicy: .approve ) ) { invocation, _ in AgentToolResult(callID: invocation.call.id, content: "host-a") @@ -970,7 +970,7 @@ struct ChatComposerStateTests { ))) let descriptor = AgentToolDescriptor( name: "lookup", summary: "lookup", inputSchema: .object([:]), - target: .local, safety: .locallyReadOnly + target: .local, approvalPolicy: .approve ) let source = AgentTranscriptMessage( role: .assistant, @@ -1042,7 +1042,7 @@ struct ChatComposerStateTests { conversation.handle(.messageStarted(source)) let descriptor = AgentToolDescriptor( name: "web_search", summary: "search", inputSchema: .object([:]), - target: .network, safety: .locallyReadOnly + target: .network, approvalPolicy: .approve ) let invocations = ["first", "second"].map { id in AgentToolInvocation( @@ -1106,7 +1106,7 @@ struct ChatComposerStateTests { let descriptor = AgentToolDescriptor( name: "lookup", summary: "lookup", inputSchema: .object([:]), - target: .local, safety: .locallyReadOnly + target: .local, approvalPolicy: .approve ) let invocations = ["first", "second"].enumerated().map { offset, callID in AgentToolInvocation( @@ -1179,7 +1179,7 @@ struct ChatComposerStateTests { let descriptor = AgentToolDescriptor( name: "lookup", summary: "lookup", inputSchema: .object([:]), - target: .local, safety: .locallyReadOnly + target: .local, approvalPolicy: .approve ) for (offset, call) in source.toolCalls.enumerated() { conversation.handle( @@ -1399,7 +1399,7 @@ struct ChatComposerStateTests { "type": .string("object"), "properties": .object([:]), "required": .array([]), "additionalProperties": .bool(false), ]), - target: .local, safety: .locallyReadOnly + target: .local, approvalPolicy: .approve ) ) { invocation, _ in AgentToolResult(callID: invocation.call.id, content: "ok") @@ -1460,7 +1460,7 @@ struct ChatComposerStateTests { "type": .string("object"), "properties": .object([:]), "required": .array([]), "additionalProperties": .bool(false), ]), - target: .local, safety: .locallyReadOnly + target: .local, approvalPolicy: .approve ) ) { invocation, _ in AgentToolResult(callID: invocation.call.id, content: "ok") @@ -1513,7 +1513,7 @@ struct ChatComposerStateTests { ))) let lookup = AgentToolDescriptor( name: "lookup", summary: "lookup", inputSchema: .object([:]), - target: .local, safety: .locallyReadOnly + target: .local, approvalPolicy: .approve ) let suggestion = AnyAgentTool(TerminalSuggestTool()).descriptor #expect(suggestion.presentation?.groupsWithAdjacentTools == false) @@ -1701,7 +1701,7 @@ struct ChatComposerStateTests { invocation, AgentToolDescriptor( name: "web_search", summary: "Provider search", - inputSchema: .object([:]), target: .network, safety: .locallyReadOnly + inputSchema: .object([:]), target: .network, approvalPolicy: .approve ))) conversation.handle( .toolFinished( @@ -2144,7 +2144,7 @@ struct ChatComposerStateTests { invocation, AgentToolDescriptor( name: "lookup", summary: "Lookup", inputSchema: .object([:]), - target: .local, safety: .locallyReadOnly + target: .local, approvalPolicy: .approve ))) conversation.handle( .toolFinished( @@ -2197,7 +2197,7 @@ struct ChatComposerStateTests { invocation, AgentToolDescriptor( name: "lookup", summary: "Lookup", inputSchema: .object([:]), - target: .local, safety: .locallyReadOnly + target: .local, approvalPolicy: .approve ))) #expect(conversation.workGroups.count == 1) diff --git a/TFMTests/ExternalRequestTests.swift b/TFMTests/ExternalRequestTests.swift index bb67688d..37dc361d 100644 --- a/TFMTests/ExternalRequestTests.swift +++ b/TFMTests/ExternalRequestTests.swift @@ -516,7 +516,7 @@ private func descriptor(name: String, readOnly: Bool) -> AgentToolDescriptor { AgentToolDescriptor( name: name, summary: name, inputSchema: .object(["type": .string("object")]), target: .host, - safety: readOnly ? .locallyReadOnly : .requiresAuthorization + approvalPolicy: readOnly ? .approve : .ask ) } diff --git a/TFMTests/GetMetricsToolTests.swift b/TFMTests/GetMetricsToolTests.swift index f180f073..7896070c 100644 --- a/TFMTests/GetMetricsToolTests.swift +++ b/TFMTests/GetMetricsToolTests.swift @@ -173,7 +173,7 @@ struct GetMetricsToolTests { let invocation = AgentToolInvocation(runID: UUID(), call: call) let checked = try await tool.preflight(invocation) #expect(checked.invocation.targetHostID == host.id) - #expect(tool.descriptor.safety == .locallyReadOnly) + #expect(tool.descriptor.approvalPolicy == .approve) #expect(tool.descriptor.concurrency == .parallel) let decision = await AgentPlanModeHook(isPlanning: true).willExecute( AgentToolCallContext( diff --git a/TFMTests/MCPServerSessionTests.swift b/TFMTests/MCPServerSessionTests.swift index a763dc59..7120bff6 100644 --- a/TFMTests/MCPServerSessionTests.swift +++ b/TFMTests/MCPServerSessionTests.swift @@ -57,6 +57,15 @@ struct MCPServerSessionTests { try responseObject(listed)["result"]?.objectValue?["tools"]?.arrayValue ) #expect(tools.count == 13) + let readOnly = Set([ + "list_hosts", "get_metrics", "request_user_secret", "ssh_wait", + "sftp_list", "sftp_stat", "sftp_read", + ]) + for tool in tools { + let name = try #require(tool.objectValue?["name"]?.stringValue) + let hint = tool.objectValue?["annotations"]?.objectValue?["readOnlyHint"] + #expect(hint == .bool(readOnly.contains(name)), "\(name)") + } } @Test diff --git a/TFMTests/ScrubberWebClientTests.swift b/TFMTests/ScrubberWebClientTests.swift index 2bc52e6a..927d880b 100644 --- a/TFMTests/ScrubberWebClientTests.swift +++ b/TFMTests/ScrubberWebClientTests.swift @@ -111,7 +111,7 @@ struct ScrubberWebClientTests { "url": .string("http://192.168.1.1/admin") ]) let preflight = try await FetchTool(web: fixtureClient()).preflight(invocation) - #expect(preflight.safety == .locallyReadOnly) + #expect(preflight.approvalPolicy == .approve) #expect(preflight.reasons.isEmpty) } diff --git a/TFMTests/SettingsNavigationTests.swift b/TFMTests/SettingsNavigationTests.swift index eaa9de41..af7d849c 100644 --- a/TFMTests/SettingsNavigationTests.swift +++ b/TFMTests/SettingsNavigationTests.swift @@ -193,7 +193,7 @@ struct SettingsNavigationTests { @Test func mcpToolAccessPolicyIsAtomicAndSurvivesRediscovery() throws { var existingTool = MCPTool(id: "search") - existingTool.accessPolicy = .disabled + existingTool.accessPolicy = .deny var server = MCPServer(name: "Gateway") server.tools = [existingTool] let store = AIConfigurationModel(mcpServers: [server], persistsSecrets: false) @@ -204,13 +204,36 @@ struct SettingsNavigationTests { ) let rediscovered = try #require(store.mcpServer(server.id)) - #expect(rediscovered.tools.first { $0.id == "search" }?.accessPolicy == .disabled) - #expect(rediscovered.tools.first { $0.id == "new-tool" }?.accessPolicy == .alwaysAsk) + #expect(rediscovered.tools.first { $0.id == "search" }?.accessPolicy == .deny) + #expect(rediscovered.tools.first { $0.id == "new-tool" }?.accessPolicy == .ask) - store.setToolAccessPolicy(.followPermissions, tool: "search", on: server.id) + store.setToolAccessPolicy(.approve, tool: "search", on: server.id) #expect( store.mcpServer(server.id)?.tools.first { $0.id == "search" }?.accessPolicy - == .followPermissions + == .approve ) } + + @Test + func destructiveMCPToolsCannotRemainApprovedAndDeniedToolsStayAvailable() throws { + var destructive = MCPTool(id: "destroy", accessPolicy: .approve) + destructive.annotations.destructiveHint = true + let denied = MCPTool(id: "denied", accessPolicy: .deny) + let server = MCPServer(name: "Gateway", tools: [destructive, denied]) + let store = AIConfigurationModel(mcpServers: [server], persistsSecrets: false) + + let normalized = try #require(store.mcpServer(server.id)) + #expect(!normalized.tools[0].allowsTermindPreapproval) + #expect(normalized.tools[0].accessPolicy == .ask) + #expect(store.availableTools.map(\.tool.id).contains("denied")) + + store.setToolAccessPolicy(.approve, tool: "destroy", on: server.id) + #expect(store.mcpServer(server.id)?.tools[0].accessPolicy == .ask) + + var newlyDestructive = MCPTool(id: "denied", accessPolicy: .ask) + newlyDestructive.annotations.destructiveHint = true + store.setToolAccessPolicy(.approve, tool: "denied", on: server.id) + store.setTools([newlyDestructive], version: "2025-06-18", for: server.id) + #expect(store.mcpServer(server.id)?.tools[0].accessPolicy == .ask) + } } diff --git a/TFMTests/TranscriptListViewTests.swift b/TFMTests/TranscriptListViewTests.swift index 715c1eef..a7f98cbe 100644 --- a/TFMTests/TranscriptListViewTests.swift +++ b/TFMTests/TranscriptListViewTests.swift @@ -964,7 +964,7 @@ struct TranscriptListViewBridgeTests { invocation: invocation, descriptor: AgentToolDescriptor( name: "lookup", summary: "lookup", inputSchema: .object([:]), - target: .local, safety: .locallyReadOnly + target: .local, approvalPolicy: .approve ) ) card.state = .running diff --git a/TFMTests/WebSearchTests.swift b/TFMTests/WebSearchTests.swift index 217874db..8a3a5e11 100644 --- a/TFMTests/WebSearchTests.swift +++ b/TFMTests/WebSearchTests.swift @@ -18,8 +18,8 @@ struct WebSearchTests { let tool = WebSearchTool(web: stubClient()) #expect(tool.descriptor.name == "web_search") #expect(tool.descriptor.target == .network) - #expect(tool.descriptor.safety == .locallyReadOnly) - #expect(try await tool.preflight(invocation(query: "swift")).safety == .locallyReadOnly) + #expect(tool.descriptor.approvalPolicy == .approve) + #expect(try await tool.preflight(invocation(query: "swift")).approvalPolicy == .approve) await #expect(throws: AgentToolError.self) { try await tool.preflight( @@ -73,7 +73,7 @@ struct WebSearchTests { let descriptor = AgentToolDescriptor( name: "web_search", summary: "Web search run by the model provider.", inputSchema: .object(["type": .string("object")]), - target: .network, safety: .locallyReadOnly + target: .network, approvalPolicy: .approve ) conversation.handle(.toolProposed(invocation, descriptor)) conversation.handle( @@ -96,7 +96,7 @@ struct WebSearchTests { let descriptor = AgentToolDescriptor( name: "web_search", summary: "Search the public web.", inputSchema: .object(["type": .string("object")]), - target: .network, safety: .locallyReadOnly + target: .network, approvalPolicy: .approve ) conversation.handle(.toolProposed(invocation, descriptor)) conversation.handle( diff --git a/Termind.xcodeproj/project.pbxproj b/Termind.xcodeproj/project.pbxproj index 62955f6a..3e39880d 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.4; + minimumVersion = 0.7.0; }; }; /* End XCRemoteSwiftPackageReference section */ diff --git a/Termind.xcodeproj/project.xcworkspace/xcshareddata/swiftpm/Package.resolved b/Termind.xcodeproj/project.xcworkspace/xcshareddata/swiftpm/Package.resolved index 1475e836..0f7497a8 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" : "8c1a72de2cc53675d34040aaa5f3ade61dfdefd1", - "version" : "0.6.4" + "revision" : "131f0bdf61a74245383331a15d871c1cfb6f5896", + "version" : "0.7.0" } }, {