-
Notifications
You must be signed in to change notification settings - Fork 7
Approval Flow #1
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
41cf5fe
bec6a04
a40d088
3434e40
c01050b
5fcc668
42a9be3
53bfcbe
1ebc4ba
9502956
2fb5933
65edc08
06e157c
4e3b330
2c1636e
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -11,7 +11,7 @@ sync_dependencies_command() { | |
| } | ||
|
|
||
| close_xcode() { | ||
| if pgrep -x "Xcode" > /dev/null; then | ||
|
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. cc @gsabran changes made by linter |
||
| if pgrep -x "Xcode" >/dev/null; then | ||
| # Kill Xcode. | ||
| pkill -x "Xcode" | ||
| fi | ||
|
|
@@ -40,7 +40,7 @@ clean_command() { | |
| while read file; do rm -rf "$file"; done | ||
| # Reset xcode state | ||
| cd "$(git rev-parse --show-toplevel)/app" && | ||
| find . -path '*.xcuserstate' 2>/dev/null | git check-ignore --stdin | xargs -I{} rm {} | ||
| find . -path '*.xcuserstate' 2>/dev/null | git check-ignore --stdin | xargs -I{} rm {} | ||
| } | ||
|
|
||
| test_swift_command() { | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,66 @@ | ||
| // Copyright command. All rights reserved. | ||
| // Licensed under the XXX License. See License.txt in the project root for license information. | ||
|
|
||
| import Foundation | ||
| import SwiftUI | ||
|
|
||
| // MARK: - IconsLabelButton | ||
|
|
||
| public struct IconsLabelButton: View { | ||
|
|
||
| public init( | ||
| action: @escaping () -> Void, | ||
| systemNames: [String], | ||
| label: String, | ||
| onHoverColor: Color = Color.primary.opacity(0.1), | ||
| padding: CGFloat = 4, | ||
| cornerRadius: CGFloat = 4) | ||
| { | ||
| self.action = action | ||
| self.systemNames = systemNames | ||
| self.label = label | ||
| self.onHoverColor = onHoverColor | ||
| self.padding = padding | ||
| self.cornerRadius = cornerRadius | ||
| } | ||
|
|
||
| public var body: some View { | ||
| HoveredButton( | ||
| action: action, | ||
| onHoverColor: onHoverColor, | ||
| padding: padding, | ||
| cornerRadius: cornerRadius) | ||
| { | ||
| HStack(spacing: 2) { | ||
| Text(label) | ||
| ForEach(Array(systemNames.enumerated()), id: \.offset) { _, systemName in | ||
| Image(systemName: systemName) | ||
| } | ||
| } | ||
| } | ||
| } | ||
|
|
||
| let action: () -> Void | ||
| let systemNames: [String] | ||
| let label: String | ||
| let onHoverColor: Color | ||
| let padding: CGFloat | ||
| let cornerRadius: CGFloat | ||
|
Comment on lines
+43
to
+48
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. nit: private? 🤷♂️ |
||
| } | ||
|
|
||
| #Preview { | ||
| VStack { | ||
| IconsLabelButton( | ||
| action: { }, | ||
| systemNames: ["command", "return"], | ||
| label: "Accept") | ||
| IconsLabelButton( | ||
| action: { }, | ||
| systemNames: ["return"], | ||
| label: "Send") | ||
| IconsLabelButton( | ||
| action: { }, | ||
| systemNames: ["shift", "command", "delete.left"], | ||
| label: "Reject") | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -94,6 +94,7 @@ final class ChatTabViewModel: Identifiable, Equatable { | |
| func cancelCurrentMessage() { | ||
| streamingTask?.cancel() | ||
| streamingTask = nil | ||
| input.cancelAllPendingToolApprovalRequests() | ||
| } | ||
|
|
||
| /// Are we queing too much on the main thread? | ||
|
|
@@ -105,6 +106,10 @@ final class ChatTabViewModel: Identifiable, Equatable { | |
| defaultLogger.error("not sending as already streaming") | ||
| return | ||
| } | ||
|
|
||
| // Cancel any pending tool approvals from previous messages | ||
| input.cancelAllPendingToolApprovalRequests() | ||
|
|
||
| guard let selectedModel = input.selectedModel else { | ||
| defaultLogger.error("not sending as no model selected") | ||
| return | ||
|
|
@@ -139,6 +144,9 @@ final class ChatTabViewModel: Identifiable, Equatable { | |
| project: projectInfo?.path, | ||
| projectRoot: projectInfo?.dirPath, | ||
| prepareForWriteToolUse: { [weak self] in await self?.handlePrepareForWriteToolUse() }, | ||
| requestToolApproval: { [weak self] toolUse in | ||
| try await self?.handleToolApproval(for: toolUse) | ||
| }, | ||
| chatMode: input.mode), | ||
| handleUpdateStream: { newMessages in | ||
| Task { @MainActor [weak self] in | ||
|
|
@@ -189,6 +197,8 @@ final class ChatTabViewModel: Identifiable, Equatable { | |
| } | ||
| } | ||
|
|
||
| private static let userDefaultsAlwaysApproveKey = "alwaysApprove_" | ||
|
|
||
| @ObservationIgnored private var workspaceRootObservation: AnyCancellable? | ||
|
|
||
| @ObservationIgnored | ||
|
|
@@ -214,6 +224,28 @@ final class ChatTabViewModel: Identifiable, Equatable { | |
| } | ||
| } | ||
|
|
||
| private func handleToolApproval(for toolUse: any ToolUse) async throws { | ||
| // Check if user has already approved this tool type | ||
| if shouldAlwaysApprove(toolName: toolUse.toolName) { | ||
| return // Skip approval for this tool | ||
| } | ||
|
|
||
| let approvalResult = await input.requestApproval( | ||
| for: toolUse) | ||
|
|
||
| switch approvalResult { | ||
| case .denied: | ||
| throw LLMServiceError.toolUsageDenied | ||
| case .approved: | ||
| break // Continue execution | ||
| case .alwaysApprove(let toolName): | ||
| // Store preference and continue | ||
| storeAlwaysApprovePreference(for: toolName) | ||
| case .cancelled: | ||
| throw CancellationError() | ||
| } | ||
| } | ||
|
|
||
| private func handlePrepareForWriteToolUse() async { | ||
| guard let projectInfo = updateProjectInfo() else { | ||
| return | ||
|
|
@@ -242,6 +274,14 @@ final class ChatTabViewModel: Identifiable, Equatable { | |
| } | ||
| } | ||
|
|
||
| private func storeAlwaysApprovePreference(for toolName: String) { | ||
| userDefaults.set(true, forKey: "\(Self.userDefaultsAlwaysApproveKey)\(toolName)") | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 👍 using user default is ok for now. I think eventually we'll want to move this to the Setting, so that we can display this in the settings and users can also update them from there.
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. yeah I can se this moving there eventually. |
||
| } | ||
|
|
||
| private func shouldAlwaysApprove(toolName: String) -> Bool { | ||
| userDefaults.bool(forKey: "\(Self.userDefaultsAlwaysApproveKey)\(toolName)") | ||
| } | ||
|
|
||
| private func updateProjectInfo() -> SelectedProjectInfo? { | ||
| if let projectInfo { | ||
| return projectInfo | ||
|
|
@@ -274,17 +314,20 @@ struct DefaultChatContext: ChatContext { | |
| project: URL?, | ||
| projectRoot: URL?, | ||
| prepareForWriteToolUse: @escaping @Sendable () async -> Void, | ||
| requestToolApproval: @escaping @Sendable (any ToolUse) async throws -> Void, | ||
| chatMode: ChatMode) | ||
| { | ||
| self.project = project | ||
| self.projectRoot = projectRoot | ||
| self.prepareForWriteToolUse = prepareForWriteToolUse | ||
| self.requestToolApproval = requestToolApproval | ||
| self.chatMode = chatMode | ||
| } | ||
|
|
||
| let project: URL? | ||
| let projectRoot: URL? | ||
| let prepareForWriteToolUse: @Sendable () async -> Void | ||
| let requestToolApproval: @Sendable (any ToolUse) async throws -> Void | ||
| let chatMode: ChatMode | ||
| } | ||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -15,9 +15,34 @@ import LoggingServiceInterface | |
| import PDFKit | ||
| import SettingsServiceInterface | ||
| import SwiftUI | ||
| import ToolFoundation | ||
| import UniformTypeIdentifiers | ||
| import XcodeObserverServiceInterface | ||
|
|
||
| // MARK: - ToolApprovalRequest | ||
|
|
||
| struct ToolApprovalRequest: Identifiable { | ||
| let id = UUID() | ||
| let displayName: String | ||
| } | ||
|
|
||
| // MARK: - ToolApprovalResult | ||
|
|
||
| enum ToolApprovalResult { | ||
| case approved | ||
| case denied | ||
| case alwaysApprove(toolName: String) | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. maybe also "toolUseCancel" ?
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. instead of denied?
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. in addition |
||
| case cancelled | ||
| } | ||
|
|
||
| // MARK: - PendingToolApproval | ||
|
|
||
| /// The current tool approvals request pending user response. | ||
| private struct PendingToolApproval { | ||
| let request: ToolApprovalRequest | ||
| let continuation: CheckedContinuation<ToolApprovalResult, Never> | ||
| } | ||
|
|
||
| // MARK: - ChatInputViewModel | ||
|
|
||
| @Observable @MainActor | ||
|
|
@@ -109,6 +134,9 @@ final class ChatInputViewModel { | |
| /// When searching for references, the index of the selected search result (at this point the selection has not yet been confirmed). | ||
| var selectedSearchResultIndex = 0 | ||
|
|
||
| /// The current tool approval request pending user response. | ||
| var pendingApproval: ToolApprovalRequest? { toolCallsPendingApproval.first?.request } | ||
|
|
||
| /// Which LLM model is selected to respond to the next message. | ||
| var selectedModel: LLMModel? { | ||
| didSet { | ||
|
|
@@ -277,9 +305,36 @@ final class ChatInputViewModel { | |
| textInput = TextInput(str) | ||
| } | ||
|
|
||
| /// Request approval for a tool use operation. | ||
| func requestApproval(for toolUse: any ToolUse) async -> ToolApprovalResult { | ||
| await withCheckedContinuation { continuation in | ||
| let request = ToolApprovalRequest( | ||
| displayName: toolUse.toolDisplayName) | ||
| self.toolCallsPendingApproval.append(PendingToolApproval(request: request, continuation: continuation)) | ||
| } | ||
| } | ||
|
|
||
| func cancelAllPendingToolApprovalRequests() { | ||
| for item in toolCallsPendingApproval { item.continuation.resume(returning: .cancelled) } | ||
| } | ||
|
|
||
| /// Handle the user's approval response. | ||
| func handleApproval(of request: ToolApprovalRequest, result: ToolApprovalResult) { | ||
| guard let index = toolCallsPendingApproval.firstIndex(where: { $0.request.id == request.id }) else { | ||
| defaultLogger.error("Could not find pending tool approval request with ID: \(request.id)") | ||
| return | ||
| } | ||
| let pendingApproval = toolCallsPendingApproval.remove(at: index) | ||
| pendingApproval.continuation.resume(returning: result) | ||
| } | ||
|
|
||
| private static let userDefaultsSelectLLMModelKey = "selectedLLMModel" | ||
| private static let userDefaultsChatModeKey = "chatMode" | ||
|
|
||
| /// Queue of tool approval requests waiting for user response. | ||
| /// Each entry contains both the request details and the continuation that will receive the user's decision. | ||
| private var toolCallsPendingApproval: [PendingToolApproval] = [] | ||
|
|
||
| /// References to attachments within the text input. | ||
| @ObservationIgnored private var inlineReferences = [String: Attachment]() | ||
|
|
||
|
|
@@ -356,7 +411,6 @@ final class ChatInputViewModel { | |
| } | ||
| selectedModel = activeModels.first | ||
| } | ||
|
|
||
| } | ||
|
|
||
| // MARK: - TextInput | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,54 @@ | ||
| // Copyright command. All rights reserved. | ||
| // Licensed under the XXX License. See License.txt in the project root for license information. | ||
|
|
||
| import DLS | ||
| import SwiftUI | ||
| import ToolFoundation | ||
|
|
||
| // MARK: - ToolApprovalView | ||
|
|
||
| struct ToolApprovalView: View { | ||
|
|
||
| let request: ToolApprovalRequest | ||
| let onApprove: () -> Void | ||
| let onDeny: () -> Void | ||
| let onAlwaysApprove: () -> Void | ||
|
|
||
| var body: some View { | ||
| VStack(alignment: .leading) { | ||
| Text("**cmd** wants to use the *\(request.displayName)* tool") | ||
| .frame(maxWidth: .infinity, alignment: .leading) | ||
| HStack(spacing: 12) { | ||
| IconsLabelButton( | ||
| action: onAlwaysApprove, | ||
| systemNames: ["command", "shift", "return"], | ||
| label: "Always Allow") | ||
| .keyboardShortcut(.return, modifiers: [.shift, .command]) | ||
|
|
||
| IconsLabelButton( | ||
| action: onApprove, | ||
| systemNames: ["command", "return"], | ||
| label: "Allow Once") | ||
| .keyboardShortcut(.return, modifiers: .command) | ||
|
|
||
| IconsLabelButton( | ||
| action: onDeny, | ||
| systemNames: ["command", "shift", "delete.left"], | ||
| label: "Reject") | ||
| .keyboardShortcut(.delete, modifiers: [.shift, .command]) | ||
| } | ||
| } | ||
| .padding(12) | ||
| } | ||
| } | ||
|
|
||
| // MARK: - Previews | ||
|
|
||
| #Preview { | ||
| ToolApprovalView( | ||
| request: ToolApprovalRequest( | ||
| displayName: "get_workspace_info"), | ||
| onApprove: { }, | ||
| onDeny: { }, | ||
| onAlwaysApprove: { }) | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
cc @gsabran like this?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yes perfect