Conversation
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the Use the checkbox below for a quick retry:
WalkthroughThis pull request introduces a comprehensive Agent feature to the VultisigApp iOS application, including backend integration via REST and Server-Sent Events (SSE), MPC-based authentication, conversation management, and a complete UI layer. Additionally, the keysign flow is simplified by removing fastVaultPassword parameters throughout the codebase, and Chain model backward compatibility is enhanced. Changes
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
VultisigApp/VultisigApp/Views/Keysign/KeysignDiscoveryView.swift
Outdated
Show resolved
Hide resolved
VultisigApp/VultisigApp/View Models/KeysignDiscoveryViewModel.swift
Outdated
Show resolved
Hide resolved
…, and swap flows across multiple views and navigation routes.
- Add remove_chain and remove_coin execution logic using CoinService - Add list_vaults: returns all vaults with chains, type and date - Add get_addresses: returns all vault chain addresses (filterable) - Add search_token: searches local TokensStore by ticker/chain/contract - Add address_book_add / address_book_remove as action aliases - Add add_coin as alias for add_token (prompt uses both names) - Expand switch to cover all 30+ known action types with proper routing: - Server-side MCP actions return handled_server_side - sign_tx returns sign_requires_keysign_flow - Unknown actions return unknown_action_type - Add AgentRemoveChainParams, AgentRemoveChainResult, AgentRemoveTokenParams, AgentRemoveTokenResult to AgentModels.swift
- AgentBackendClient: SSE streaming improvements with error handling - AgentContextBuilder: vault context assembly for AI prompts - AgentChatViewModel: orchestrator with tool execution, stop generation - AgentConversationsViewModel: conversation list and management - AgentChatView: streaming UI with status indicators and stop button - AgentChatMessageView: message bubble with tool call inline display - AgentConversationsView: conversation list with starters - Endpoint.swift: agent backend URL endpoint definitions
- Keep agent chat files (AgentToolExecutor, AgentModels, AgentContextBuilder, etc.) - Merge push notifications feature from main (PushNotificationManager, NotificationService, etc.) - Merge ThorchainStagenet2 and VaultNotificationToggleRow from main - Resolve project.pbxproj conflicts by keeping all additions from both branches
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
Fix violations across 12 files: - trailing_whitespace: 108 in AgentToolExecutor, ~30 across other agent files - async_without_await: 11 in AgentToolExecutor - unused_parameter: 5 in AgentChatViewModel, AgentConversationsViewModel - statement_position: 1 in AgentAuthService - vertical_whitespace: 5 in AgentConversationsView, KeysignDiscoveryViewModel, KeysignViewModel - colon: 2 in AgentConversationsView
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 14
Note
Due to the large number of review comments, Critical, Major severity comments were prioritized as inline comments.
🟡 Minor comments (8)
VultisigApp/VultisigApp/Views/Keysign/KeysignDiscoveryView.swift-154-154 (1)
154-154:⚠️ Potential issue | 🟡 Minor
isFastis now hardcoded, which can hide fast-vault state.At Line 154, forcing
isFast: falseremoves dynamic behavior and may show incorrect discovery animation/state for fast vault flows. Please derive this from vault/view-model state instead of a constant.Suggested fix
- isFast: false, + isFast: vault.signers.count == 2,Based on learnings: In VultisigApp iOS codebase, vault classification depends on signer count (2 signers = fast vault, 3+ signers = active vault), even when
vault.isFastVaultis true.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@VultisigApp/VultisigApp/Views/Keysign/KeysignDiscoveryView.swift` at line 154, Replace the hardcoded "isFast: false" with a computed value derived from the vault/view-model state: inspect the view model or vault object used in KeysignDiscoveryView (e.g., viewModel.vault or vault) and set isFast based on signer count (treat 2 signers as a fast vault, 3+ as active) rather than a constant; update the call site where "isFast: false" is passed so it uses that computed boolean (for example, derive isFast = (vault.signers.count == 2) or equivalent property on the view model) to ensure the discovery animation/state reflects the actual vault classification.VultisigApp/VultisigApp/Views/Keysign/KeysignView.swift-20-20 (1)
20-20:⚠️ Potential issue | 🟡 MinorMake
viewModelprivate to clear SwiftLint.Line 20 exposes
@StateObjectwith non-private visibility; this triggersprivate_swiftui_stateand can break lint gating.🔧 Suggested fix
- `@StateObject` var viewModel = KeysignViewModel() + `@StateObject` private var viewModel = KeysignViewModel()🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@VultisigApp/VultisigApp/Views/Keysign/KeysignView.swift` at line 20, Make the `@StateObject` viewModel private to satisfy SwiftLint: change the visibility of the viewModel property in KeysignView (the `@StateObject` var viewModel = KeysignViewModel()) to private (e.g., `@StateObject` private var viewModel = KeysignViewModel()) and update any references inside KeysignView that access viewModel accordingly so nobody relies on it being internal/public.VultisigApp/VultisigApp/Views/Agent/AgentConversationsView.swift-201-205 (1)
201-205:⚠️ Potential issue | 🟡 MinorPersist starter before route push to avoid prefill race.
Line 202 navigates first and writes the pending starter afterward; the destination can read before the value exists.
Suggested patch
private func navigateToChat(with starter: String?) { - router.navigate(to:AgentRoute.chat(conversationId: nil)) if let starter { UserDefaults.standard.set(starter, forKey: "agent_pending_starter") } + router.navigate(to: AgentRoute.chat(conversationId: nil)) }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@VultisigApp/VultisigApp/Views/Agent/AgentConversationsView.swift` around lines 201 - 205, In navigateToChat(with:) move the UserDefaults write for the "agent_pending_starter" key to happen before calling router.navigate(to: AgentRoute.chat(conversationId: nil)) so the destination can read the persisted starter immediately; specifically, in the navigateToChat function set UserDefaults.standard.set(starter, forKey: "agent_pending_starter") (when starter is non-nil) first, then call router.navigate(to:AgentRoute.chat(...)).VultisigApp/VultisigApp/Services/Agent/AgentAuthService.swift-214-218 (1)
214-218:⚠️ Potential issue | 🟡 MinorFallback address derivation may be incorrect.
The fallback when WalletCore derivation fails uses the last 40 characters of the public key, which is not a valid Ethereum address derivation. This could lead to authentication failures or mismatched addresses.
Explanation
A proper fallback would need to:
- Decompress the public key if compressed
- Take the keccak256 hash of the uncompressed key (without prefix)
- Use the last 20 bytes as the address
The current implementation just takes the suffix of the hex-encoded public key, which is not the same as an Ethereum address.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@VultisigApp/VultisigApp/Services/Agent/AgentAuthService.swift` around lines 214 - 218, The fallback in AgentAuthService's address derivation (when WalletCore fails) currently returns the last 40 hex chars of vault.pubKeyECDSA; instead, detect and decompress a compressed EC public key (vault.pubKeyECDSA), convert to the uncompressed byte form without the 0x/0x04 prefix, compute keccak256 over those uncompressed bytes, take the last 20 bytes of the hash, and return "0x" + hex of those 20 bytes; update the fallback branch that constructs normalized and the return value to perform these steps and ensure you handle hex decoding/encoding and errors appropriately.VultisigApp/VultisigApp/View Models/Agent/AgentConversationsViewModel.swift-52-70 (1)
52-70:⚠️ Potential issue | 🟡 MinorRedundant
isConnected = trueassignment.
isConnectedis set totrueat line 56 before the network call, then set again at line 70 after success. The first assignment at line 56 is premature since the connection hasn't been verified yet.Suggested fix
func loadConversations(vault: Vault) async { let token = await getValidToken(vault: vault) print("[AgentConvos] 📝 loadConversations: token=\(token != nil ? "present" : "none")") - isConnected = true isLoading = true error = nil🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@VultisigApp/VultisigApp/View` Models/Agent/AgentConversationsViewModel.swift around lines 52 - 70, The loadConversations(vault: Vault) function sets isConnected = true prematurely before the network call; remove the early assignment and only set isConnected = true after a successful backendClient.listConversations response (the existing assignment after success can remain), ensuring any error path sets isConnected = false or leaves it unchanged; update references to isConnected within loadConversations so the state reflects the actual connection result from listConversations and not before getValidToken/ backendClient.listConversations completes.VultisigApp/VultisigApp/Services/Agent/AgentAuthService.swift-264-264 (1)
264-264:⚠️ Potential issue | 🟡 MinorAvoid force unwrapping URLs.
Force unwrapping
URL(string:)!can crash if the URL string is malformed. This pattern appears at lines 264, 297, 312, and 324.As per coding guidelines: "Avoid force unwrapping (
!); use optional binding, guard statements, or nil-coalescing operators instead."Suggested fix
-let url = URL(string: Endpoint.verifierAuth())! +guard let url = URL(string: Endpoint.verifierAuth()) else { + throw AgentAuthError.authFailed +}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@VultisigApp/VultisigApp/Services/Agent/AgentAuthService.swift` at line 264, The code in AgentAuthService uses force-unwrapped URL(string:) (e.g., the let url = URL(string: Endpoint.verifierAuth())! instance) which can crash on malformed strings; update each occurrence (those in AgentAuthService around the verifierAuth, other Endpoint.* calls) to safely unwrap the URL using guard let (or if let) and handle the nil case (returning/throwing an error or logging and exiting) instead of force-unwrapping; ensure all locations (the occurrences you saw at URL(string: Endpoint.verifierAuth()), and the other Endpoint.* invocations) follow the same pattern so no URL(string:) uses a trailing !.VultisigApp/VultisigApp/Services/Agent/AgentToolExecutor.swift-236-238 (1)
236-238:⚠️ Potential issue | 🟡 MinorSilent error suppression on
Storage.shared.save().Using
try?silently swallows save errors. This could hide data persistence failures. Consider logging the error at minimum.Suggested improvement
if anySuccess { - try? Storage.shared.save() + do { + try Storage.shared.save() + } catch { + print("[AgentToolExecutor] ⚠️ Failed to save: \(error.localizedDescription)") + } }This pattern repeats at lines 313, 370, 425, and 516.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@VultisigApp/VultisigApp/Services/Agent/AgentToolExecutor.swift` around lines 236 - 238, Replace the silent try? on Storage.shared.save() with proper error handling: wrap the call in a do/catch, call try Storage.shared.save() in the do block and log the caught error in the catch (e.g., logger.error or processLogger.error with "Failed to save storage: \(error)"), and apply the same change for the other occurrences mentioned (the save calls near anySuccess and the repeats at the other locations). This ensures Storage.shared.save() errors are not suppressed and provides actionable logs; modify the code around the anySuccess branch in AgentToolExecutor where Storage.shared.save() is invoked.VultisigApp/VultisigApp/Services/Agent/AgentModels.swift-586-592 (1)
586-592:⚠️ Potential issue | 🟡 MinorFragile
Hashableimplementation forAnyCodable.Using
String(describing: value)for equality and hashing can produce unexpected results since the string representation may vary or collide for different underlying values.Explanation
For example, two different arrays might produce the same string representation, or floating-point formatting might differ across platforms. If
AnyCodableis primarily used in dictionaries/sets where hashing matters, consider a more robust implementation or removingHashableconformance if not strictly needed.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@VultisigApp/VultisigApp/Services/Agent/AgentModels.swift` around lines 586 - 592, The current == operator and hash(into:) in AnyCodable use String(describing:) which is fragile; replace this by first attempting to normalize the wrapped value into a comparable/hashable form (e.g., try casting value to AnyHashable and compare/hash that), and if that fails fall back to a deterministic binary representation (e.g., encode value with JSONEncoder/PropertyListEncoder and compare/hash the resulting Data) or remove Hashable conformance entirely if sets/dictionary keys of AnyCodable are not needed. Update the implementations in the AnyCodable type (the static func == and func hash(into:)) to try AnyHashable casting first, then deterministic encoded-data fallback, and ensure identical logic is used for both equality and hashing so equal values produce the same hash.
🧹 Nitpick comments (21)
VultisigApp/VultisigApp/View Models/KeysignDiscoveryViewModel.swift (1)
69-78: TightenparticipantDiscoverylifecycle guaranteesLine 73 makes
participantDiscoveryrequired at setup, but later calls still optional-chain it. This can silently skip discovery ifstartDiscovery()is called beforesetData(...).Suggested hard-fail guard to avoid silent no-op
func startDiscovery() { self.logger.info("mediator server started") self.startKeysignSession() - self.participantDiscovery?.getParticipants( + guard let participantDiscovery = self.participantDiscovery else { + self.logger.error("participantDiscovery not configured before startDiscovery") + self.status = .FailToStart + return + } + participantDiscovery.getParticipants( serverAddr: self.serverAddr, sessionID: self.sessionID, localParty: self.localPartyID ) }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@VultisigApp/VultisigApp/View` Models/KeysignDiscoveryViewModel.swift around lines 69 - 78, The participantDiscovery is treated as required in setData(...) but optional elsewhere, which can cause startDiscovery() to silently no-op if called earlier; update the lifecycle so participantDiscovery cannot be nil when startDiscovery() runs by either making participantDiscovery non-optional (set its type to ParticipantDiscovery and assign it in setData) or adding an explicit guard/early-fail in startDiscovery() that throws or logs a fatal/preconditionFailure when participantDiscovery is nil; reference the setData(...) method and the startDiscovery() call site to implement the chosen fix so discovery always fails loudly instead of silently skipping.VultisigApp/VultisigApp/Views/Components/TabBar/VultiTabBar.swift (1)
231-233: Use theme color tokens in the new Agent preview tab.The new preview tab uses hardcoded colors; prefer
Theme.colorsto keep previews aligned with app tokens.As per coding guidelines: "Always use the
Themeenum for colors instead of hardcoding color values."🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@VultisigApp/VultisigApp/Views/Components/TabBar/VultiTabBar.swift` around lines 231 - 233, The preview tab in VultiTabBar is using hardcoded Color.purple and should use the app color tokens; replace the Color.purple usage in the Agent preview overlay (the view that sets .overlay(Text("Agent")...).tag(HomeTab.agent)) with the appropriate Theme.colors token (e.g., Theme.colors.primary or whichever Theme.colors.* token matches the intended purple) so the Agent preview follows the Theme enum colors rather than a literal Color.VultisigApp/VultisigApp/Utils/Endpoint.swift (1)
1133-1139: Encode dynamic conversation IDs in URL path builders.Use
encodePathComponent(_:)foridto avoid malformed path issues if IDs ever include reserved characters.🔧 Suggested fix
static func agentConversation(id: String) -> String { - "\(agentBackendUrl)/agent/conversations/\(id)" + "\(agentBackendUrl)/agent/conversations/\(encodePathComponent(id))" } static func agentConversationMessages(id: String) -> String { - "\(agentBackendUrl)/agent/conversations/\(id)/messages" + "\(agentBackendUrl)/agent/conversations/\(encodePathComponent(id))/messages" }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@VultisigApp/VultisigApp/Utils/Endpoint.swift` around lines 1133 - 1139, The URL builders agentConversation(id:) and agentConversationMessages(id:) currently interpolate raw id values into the path; update both to encode the id using encodePathComponent(_:) before interpolation (i.e., call encodePathComponent(id) and use that result in the path) so reserved characters in conversation IDs are safely escaped.VultisigApp/VultisigApp/Views/Agent/AgentPasswordPromptView.swift (1)
25-35: Replace hardcoded fonts withTheme.fontstokens.Typography here is hardcoded; switch to
Theme.fontsto keep UI consistency across screens.As per coding guidelines: "Always use the
Themeenum for fonts instead of hardcoding font values."Also applies to: 57-59
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@VultisigApp/VultisigApp/Views/Agent/AgentPasswordPromptView.swift` around lines 25 - 35, Replace hardcoded font calls with Theme.fonts tokens: change the Image(...).font(.system(size: 48)) to use the theme icon/font token (e.g. Theme.fonts.iconLarge), change Text("Enter Vault Password").font(.title3.bold()) to the theme title token (e.g. Theme.fonts.title3Bold), and change Text(...).font(.subheadline) to Theme.fonts.subheadline (and the other occurrences flagged at the later Text lines) so all typography uses Theme.fonts instead of hardcoded .font(...) calls.VultisigApp/VultisigApp/Services/Agent/AgentBackendClient.swift (1)
293-369: Consolidate duplicateddoRequestresponse handling.Both overloads repeat the same status/error/empty-body logic; extracting a shared executor will reduce drift and simplify maintenance.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@VultisigApp/VultisigApp/Services/Agent/AgentBackendClient.swift` around lines 293 - 369, Both doRequest overloads duplicate response parsing and error/empty-body handling; extract that logic into a single helper (e.g., a private func processResponse<T: Decodable>(data: Data, response: URLResponse) throws -> T) that encapsulates casting to HTTPURLResponse, 401/>=400 checks (using Self.parseErrorMessage), empty-body handling returning AgentEmptyResponse when T == AgentEmptyResponse.self, and JSON decoding; then simplify both doRequest(method:url:token:body: some Encodable) and doRequest(method:url:token:body: [String: Any]) to build the URLRequest, await URLSession.shared.data(for: request), and delegate the (data, response) pair to processResponse to return the decoded T.VultisigApp/VultisigApp/Views/Agent/AgentChatView.swift (4)
10-10: Screen naming convention: useScreensuffix.Per coding guidelines, views representing whole screens should use the
Screensuffix (e.g.,AgentChatScreeninstead ofAgentChatView).As per coding guidelines: "Always use the
Screencomponent for views that represent a whole screen, and use theScreensuffix in struct names."🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@VultisigApp/VultisigApp/Views/Agent/AgentChatView.swift` at line 10, Rename the top-level view struct AgentChatView to follow the screen naming convention (e.g., AgentChatScreen) and update all references/usages to the new name (struct declaration, initializers, previews, navigation destinations, and any files importing or instantiating AgentChatView) so the type is consistently renamed throughout the codebase.
258-261: Notification.Name extensions may conflict with other definitions.These notification names are defined in a file extension. Consider moving them to a centralized location (e.g., a dedicated
Notifications.swiftfile) or namespacing them to avoid potential conflicts.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@VultisigApp/VultisigApp/Views/Agent/AgentChatView.swift` around lines 258 - 261, The Notification.Name extension defining agentDidAcceptTx and agentDidRejectTx in AgentChatView.swift may collide with other names; move these definitions out of the view and into a centralized notifications module (e.g., create a Notifications.swift) or namespace them to avoid conflicts — for example, define them as uniquely prefixed/ scoped notification identifiers and expose them from a single place so callers use Notification.Name.<uniqueName> (referencing the existing symbols Notification.Name, agentDidAcceptTx and agentDidRejectTx when relocating).
116-116: Use.foregroundStyle()instead of deprecated.foregroundColor().Multiple occurrences throughout the file (lines 116, 200, 204, 233) use the deprecated modifier.
As per coding guidelines: "Use
.foregroundStyle()modifier instead of deprecated.foregroundColor()for text color styling."🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@VultisigApp/VultisigApp/Views/Agent/AgentChatView.swift` at line 116, In AgentChatView replace deprecated .foregroundColor(...) calls with .foregroundStyle(...) for each occurrence (e.g., change .foregroundColor(Theme.colors.textPrimary) to .foregroundStyle(Theme.colors.textPrimary)) so Text and other view modifiers use the modern API; update all instances referenced in AgentChatView (the ones around the existing .foregroundColor usages) to ensure consistent styling.
164-170: Consider usingTaskwithtry await Task.sleepinstead ofDispatchQueue.main.asyncAfter.The async/await pattern is preferred for delayed execution in Swift concurrency.
Example refactor
if let starter = UserDefaults.standard.string(forKey: "agent_pending_starter") { UserDefaults.standard.removeObject(forKey: "agent_pending_starter") inputText = starter - DispatchQueue.main.asyncAfter(deadline: .now() + 0.5) { - sendMessage() - } + Task { + try? await Task.sleep(for: .milliseconds(500)) + sendMessage() + } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@VultisigApp/VultisigApp/Views/Agent/AgentChatView.swift` around lines 164 - 170, Replace the DispatchQueue.main.asyncAfter delay with Swift concurrency: set the pending starter into inputText on the main actor, then create a Task that uses try await Task.sleep for 500ms and invokes sendMessage() on the main actor; reference the existing UserDefaults key "agent_pending_starter", the inputText variable and the sendMessage() function in AgentChatView so the delay uses Task + Task.sleep and all UI updates occur on MainActor.VultisigApp/VultisigApp/View Models/Agent/AgentConversationsViewModel.swift (2)
29-30: Unused timer variable.
startersRefreshTimeris declared but never scheduled. The refresh logic useslastStartersRefreshdate instead. Either remove the unused timer or implement the scheduled refresh if intended.Remove unused timer
- private var startersRefreshTimer: Timer? private var lastStartersRefresh: Date?And in deinit:
deinit { - startersRefreshTimer?.invalidate() }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@VultisigApp/VultisigApp/View` Models/Agent/AgentConversationsViewModel.swift around lines 29 - 30, The property startersRefreshTimer is declared but never scheduled; either remove it and any related references (including invalidation in deinit) or implement a scheduled Timer to drive the refresh logic that currently relies on lastStartersRefresh. If removing: delete the startersRefreshTimer property and any timer invalidation/cleanup calls. If implementing: create and schedule startersRefreshTimer (e.g., Timer.scheduledTimer...) to call your refresh method that updates lastStartersRefresh, and ensure you invalidate startersRefreshTimer in deinit. Use the identifiers startersRefreshTimer, lastStartersRefresh, and deinit to locate the relevant code to change.
91-133: Duplicate code withAgentChatViewModel.loadStarters().This method is nearly identical to the one in
AgentChatViewModel. Consider extracting the shared logic into a common service or helper to reduce duplication.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@VultisigApp/VultisigApp/View` Models/Agent/AgentConversationsViewModel.swift around lines 91 - 133, The loadStarters implementation in AgentConversationsViewModel duplicates AgentChatViewModel.loadStarters; extract the shared logic into a single helper/service (e.g., AgentStartersService.fetchStarters or AgentStarterProvider.fetch(for:vault:)) that accepts the Vault, backendClient, token-provider (getValidToken), and logger and returns a result object (starters array and lastRefresh / authorization state) or throws AgentBackendClient.AgentBackendError; move the network/request/response/fallback and error handling into that service and have both AgentConversationsViewModel.loadStarters and AgentChatViewModel.loadStarters call the new service and then map the returned data to their local properties (starters, lastStartersRefresh) and update isConnected/passwordRequired when an .unauthorized error is surfaced.VultisigApp/VultisigApp/Views/Agent/AgentChatMessageView.swift (3)
138-175: Consider using closures instead of NotificationCenter for button actions.Using
NotificationCenterto communicate button taps from the view to the ViewModel works but creates implicit coupling. A more explicit approach would be to pass action closures as parameters to the view.Alternative approach using closures
struct AgentChatMessageView: View { let message: AgentChatMessage var onAcceptTx: ((AgentTxReady) -> Void)? var onRejectTx: ((AgentTxReady) -> Void)? // Then in the button actions: Button { onRejectTx?(tx) } label: { // ... } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@VultisigApp/VultisigApp/Views/Agent/AgentChatMessageView.swift` around lines 138 - 175, The view currently posts NotificationCenter events (.agentDidAcceptTx / .agentDidRejectTx) from AgentChatMessageView button actions which creates implicit coupling; refactor AgentChatMessageView to accept optional action closures (e.g., onAcceptTx: ((AgentTxReady)->Void)? and onRejectTx: ((AgentTxReady)->Void)?), replace the NotificationCenter.post calls inside the "Yes"/"No"/"Sign Transaction" Button handlers with calls to those closures (onAcceptTx?(tx) / onRejectTx?(tx)), and update callers to provide the appropriate ViewModel-bound closures instead of relying on notifications.
123-126: Hardcoded estimated fee placeholder.The comment acknowledges this is a placeholder. Consider wiring up the actual estimated fee from the transaction proposal when available.
Would you like me to help implement proper fee extraction from the
AgentTxReadypayload, or should I open an issue to track this?🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@VultisigApp/VultisigApp/Views/Agent/AgentChatMessageView.swift` around lines 123 - 126, Replace the hardcoded fee display in AgentChatMessageView with the actual estimated fee from the transaction payload: read the fee value (or fee estimate) from the AgentTxReady/transaction proposal object (e.g., AgentTxReady.fee, .estimatedFee, or nested proposal fields) passed into the view (where Text("EST. FEE: 0.001 \(tx.fromSymbol)") is used), format it to the desired precision and currency symbol (fallback to a localized "N/A" or 0.0 if missing), and update the Text to interpolate that formatted fee with tx.fromSymbol; ensure nil-safe access and preserve the monospaced footnote styling and Theme.colors.textTertiary color.
34-34: Use.foregroundStyle()instead of deprecated.foregroundColor().The
.foregroundColor()modifier is deprecated. This applies throughout the file (lines 34, 64, 71, 86, 90, 94, 110, 115, 120, 125, 133, 145, 157, 169, 194, 204, 224, 229, 243, 246, 254).As per coding guidelines: "Use
.foregroundStyle()modifier instead of deprecated.foregroundColor()for text color styling."Example fix for line 34
Text(.init(message.content)) // Renders markdown .font(.body) - .foregroundColor(Theme.colors.textPrimary) + .foregroundStyle(Theme.colors.textPrimary) .textSelection(.enabled)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@VultisigApp/VultisigApp/Views/Agent/AgentChatMessageView.swift` at line 34, Replace all uses of the deprecated .foregroundColor(...) with .foregroundStyle(...) in AgentChatMessageView.swift; specifically update every modifier calls inside the AgentChatMessageView struct (e.g., in the body, message bubble, timestamp/metadata text, roleLabel, and any Text views rendered by functions like messageBubbleView or metadataView) to call .foregroundStyle(Theme.colors.textPrimary) or the equivalent Theme.colors value currently used, preserving the same color expression and any conditional logic so behavior is unchanged.VultisigApp/VultisigApp/View Models/Agent/AgentChatViewModel.swift (4)
124-124: Hardcoded model identifier.The model identifier
"anthropic/claude-sonnet-4.5"is hardcoded in multiple places (lines 124, 178, 500). Consider extracting this to a constant for easier maintenance.Suggested refactor
// Add to AgentChatViewModel or a config: private static let defaultModel = "anthropic/claude-sonnet-4.5" // Then use: model: Self.defaultModel🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@VultisigApp/VultisigApp/View` Models/Agent/AgentChatViewModel.swift at line 124, Extract the hardcoded model identifier into a single constant and replace all inline occurrences with that constant: add a private static let (e.g. private static let defaultModel = "anthropic/claude-sonnet-4.5") to AgentChatViewModel (or a shared config) and update every place using model: "anthropic/claude-sonnet-4.5" to use model: Self.defaultModel (or the config constant) so the identifier is defined once and easy to maintain.
323-325: AccessingAppViewModel.shareddirectly instead of via environment.The coding guidelines recommend using
@EnvironmentObjectfor app-level singletons. However, since this is within an SSE event handler where the vault might not be passed, consider refactoring to pass the vault through the event handling chain.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@VultisigApp/VultisigApp/View` Models/Agent/AgentChatViewModel.swift around lines 323 - 325, The code is directly accessing AppViewModel.shared inside the SSE handler instead of using an injected environment object; update AgentChatViewModel (and the SSE event handling chain) to accept the vault via dependency injection rather than referencing AppViewModel.shared: add an `@EnvironmentObject/AppViewModel` parameter to the view or pass the selected vault into the SSE callback so you call handleActions(actions, vault: vault) with the provided vault value; update the SSE subscription entry point that invokes this handler to thread the vault through (e.g., pass selectedVault from the view into the event handler) and remove direct references to AppViewModel.shared and selectedVault from inside the handler.
268-292: Duplicate code withAgentConversationsViewModel.loadStarters().This method is nearly identical to the one in
AgentConversationsViewModel. Consider extracting to a shared service.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@VultisigApp/VultisigApp/View` Models/Agent/AgentChatViewModel.swift around lines 268 - 292, The loadStarters implementation is duplicated between AgentChatViewModel.loadStarters() and AgentConversationsViewModel.loadStarters(); extract the common logic into a shared helper (e.g., AgentStarterService.fetchStarters(vault:backendClient:logger:) or a static helper on AgentStarter) that: obtains a token via getValidToken(vault:), builds context with AgentContextBuilder.buildContext(vault:), calls backendClient.getStarters(request:token:), returns either the fetched starters or the fallback (shuffled and prefixed to 4), and throws on unexpected errors; then replace both AgentChatViewModel.loadStarters and AgentConversationsViewModel.loadStarters with a simple call to that shared method and assign the returned value to starters.
405-418: RedundantMainActor.runinside@MainActorclass.Since
AgentChatViewModelis@MainActor, any code within it already runs on the main actor. Theawait MainActor.runon line 408 is unnecessary.Simplified version
Task { let result = await AgentToolExecutor.execute(action: action, vault: vault) - await MainActor.run { - if let idx = self.messages.firstIndex(where: { $0.id == toolCallId }) { - self.messages[idx].toolCall?.status = result.success ? .success : .error - self.messages[idx].toolCall?.resultData = result.data - self.messages[idx].toolCall?.error = result.error - } + if let idx = self.messages.firstIndex(where: { $0.id == toolCallId }) { + self.messages[idx].toolCall?.status = result.success ? .success : .error + self.messages[idx].toolCall?.resultData = result.data + self.messages[idx].toolCall?.error = result.error } // Stream the result back self.sendActionResult(result, vault: vault) }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@VultisigApp/VultisigApp/View` Models/Agent/AgentChatViewModel.swift around lines 405 - 418, The MainActor.run wrapper is redundant inside the `@MainActor` isolated AgentChatViewModel; remove the await MainActor.run { ... } block and perform the messages mutation directly in the Task after awaiting AgentToolExecutor.execute(action:vault:), i.e. update self.messages[idx].toolCall?.status/resultData/error inline (referencing AgentToolExecutor.execute, self.messages, toolCallId and sendActionResult) and then call sendActionResult(result, vault: vault).VultisigApp/VultisigApp/Services/Agent/AgentContextBuilder.swift (1)
49-49: Address book integration is pending.The TODO indicates that address book items need to be wired up. This would provide the agent with contact information for transaction destinations.
Would you like me to help implement the address book integration, or should I open an issue to track this task?
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@VultisigApp/VultisigApp/Services/Agent/AgentContextBuilder.swift` at line 49, AgentContextBuilder is currently passing addressBook: nil — wire up the real address book by injecting or retrieving the app's address book and supplying it to the AgentContext; update the AgentContextBuilder initializer or factory to accept an AddressBook or AddressBookService (e.g., AddressBook.shared or AddressBookService.fetchContacts()), map the stored contact model to the AgentContext's expected address book item type, and pass that value instead of nil (with a safe empty fallback if fetching fails). Ensure you modify AgentContextBuilder and any callers so the address book dependency is provided and handle errors gracefully.VultisigApp/VultisigApp/Services/Agent/AgentToolExecutor.swift (2)
240-243: Verbose JSON round-trip for result construction.The pattern of encoding to JSON then decoding back to
[String: Any]is repeated multiple times. Consider adding a helper method to simplify this.Helper method suggestion
private static func encodeToAnyCodableDict<T: Encodable>(_ value: T) -> [String: AnyCodable] { guard let data = try? JSONEncoder().encode(value), let dict = try? JSONSerialization.jsonObject(with: data) as? [String: Any] else { return [:] } return dict.mapValues { AnyCodable($0) } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@VultisigApp/VultisigApp/Services/Agent/AgentToolExecutor.swift` around lines 240 - 243, Replace the repeated JSON encode/decode round-trip with a shared helper: add a private static method (e.g., encodeToAnyCodableDict<T: Encodable>(_ value: T) -> [String: AnyCodable]) on the AgentToolExecutor (or adjacent utility scope) that uses JSONEncoder and JSONSerialization to produce [String: AnyCodable] (returning [:] on failure), then update the result construction (where AgentActionResult(action: action.type, actionId: action.id, success: true, data: ...)) to call encodeToAnyCodableDict(["results": results]) instead of the inline JSONEncoder/JSONSerialization code so all occurrences reuse the helper.
183-234: Batch multiple token additions efficiently.The loop processes tokens sequentially with individual
CoinService.addIfNeededcalls. For large batches, this could be slow. Also, discovered tokens are fetched individually for each native token added.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@VultisigApp/VultisigApp/Services/Agent/AgentToolExecutor.swift` around lines 183 - 234, The loop currently awaits CoinService.addIfNeeded sequentially and calls CoinService.addDiscoveredTokens per native token; refactor to process tokens concurrently using a TaskGroup (or map with async let) to call CoinService.addIfNeeded for each tokenParam so adds happen in parallel, collect successful newCoin results and which ones are native, then after the group completes call a single batched method to process discovered natives (either add a new CoinService.addDiscoveredTokens(nativeTokens: [Coin], to: Vault) or call the existing addDiscoveredTokens once in a loop but outside the per-token tasks); update the results assembly to use the grouped outcomes and still preserve chainAdded calculation and vault.defiChains updates.
ℹ️ Review info
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (41)
VultisigApp/VultisigApp.xcodeproj/project.pbxprojVultisigApp/VultisigApp/ContentView.swiftVultisigApp/VultisigApp/Features/Agent/Navigation/AgentRoute.swiftVultisigApp/VultisigApp/Features/Agent/Navigation/AgentRouter.swiftVultisigApp/VultisigApp/Features/Defi/Protocols/Circle/CircleWithdrawView.swiftVultisigApp/VultisigApp/Features/Home/HomeScreen.swiftVultisigApp/VultisigApp/Features/Home/Model/HomeTab.swiftVultisigApp/VultisigApp/Features/Wallet/ChainDetail/ChainDetailScreenContainer.swiftVultisigApp/VultisigApp/Model/Chain.swiftVultisigApp/VultisigApp/Navigation/VultisigRouter.swiftVultisigApp/VultisigApp/Services/Agent/AgentAuthService.swiftVultisigApp/VultisigApp/Services/Agent/AgentBackendClient.swiftVultisigApp/VultisigApp/Services/Agent/AgentContextBuilder.swiftVultisigApp/VultisigApp/Services/Agent/AgentModels.swiftVultisigApp/VultisigApp/Services/Agent/AgentToolExecutor.swiftVultisigApp/VultisigApp/Services/FastVault/FastVaultKeysignService.swiftVultisigApp/VultisigApp/Utils/Endpoint.swiftVultisigApp/VultisigApp/View Models/Agent/AgentChatViewModel.swiftVultisigApp/VultisigApp/View Models/Agent/AgentConversationsViewModel.swiftVultisigApp/VultisigApp/View Models/KeysignDiscoveryViewModel.swiftVultisigApp/VultisigApp/View Models/KeysignViewModel.swiftVultisigApp/VultisigApp/Views/Agent/AgentChatMessageView.swiftVultisigApp/VultisigApp/Views/Agent/AgentChatView.swiftVultisigApp/VultisigApp/Views/Agent/AgentConversationsView.swiftVultisigApp/VultisigApp/Views/Agent/AgentPasswordPromptView.swiftVultisigApp/VultisigApp/Views/Agent/AgentThinkingIndicator.swiftVultisigApp/VultisigApp/Views/Components/TabBar/VultiTabBar.swiftVultisigApp/VultisigApp/Views/FunctionCall/FunctionCallPairScreen.swiftVultisigApp/VultisigApp/Views/FunctionCall/FunctionCallRoute.swiftVultisigApp/VultisigApp/Views/FunctionCall/FunctionCallRouteBuilder.swiftVultisigApp/VultisigApp/Views/FunctionCall/FunctionCallRouter.swiftVultisigApp/VultisigApp/Views/FunctionCall/FunctionCallVerifyScreen.swiftVultisigApp/VultisigApp/Views/Keysign/KeysignDiscoveryView.swiftVultisigApp/VultisigApp/Views/Keysign/KeysignView.swiftVultisigApp/VultisigApp/Views/Send/Navigation/SendRoute.swiftVultisigApp/VultisigApp/Views/Send/Navigation/SendRouteBuilder.swiftVultisigApp/VultisigApp/Views/Send/Navigation/SendRouter.swiftVultisigApp/VultisigApp/Views/Send/Screens/SendPairScreen.swiftVultisigApp/VultisigApp/Views/Send/Screens/SendVerifyScreen.swiftVultisigApp/VultisigApp/Views/Settings/SettingsCustomMessageView.swiftVultisigApp/VultisigApp/Views/Swap/SwapCryptoView.swift
💤 Files with no reviewable changes (2)
- VultisigApp/VultisigApp/Views/Settings/SettingsCustomMessageView.swift
- VultisigApp/VultisigApp/Views/Swap/SwapCryptoView.swift
👮 Files not reviewed due to content moderation or server errors (1)
- VultisigApp/VultisigApp.xcodeproj/project.pbxproj
| init(from decoder: Decoder) throws { | ||
| let container = try decoder.singleValueContainer() | ||
| let rawValue = try container.decode(String.self) | ||
| if let chain = Chain(rawValue: rawValue) { | ||
| self = chain | ||
| } else if let migrated = Chain.removedChainMigrations[rawValue] { | ||
| self = migrated | ||
| } else { |
There was a problem hiding this comment.
Migration precedence bug: legacy values are decoded before remapping.
init(from:) checks Chain(rawValue:) first, so migrations for values that still exist as enum cases (e.g., "polygon", "thorChainChainnet") are never applied. This blocks the intended data migration path.
Proposed fix
init(from decoder: Decoder) throws {
let container = try decoder.singleValueContainer()
let rawValue = try container.decode(String.self)
- if let chain = Chain(rawValue: rawValue) {
- self = chain
- } else if let migrated = Chain.removedChainMigrations[rawValue] {
+ if let migrated = Chain.removedChainMigrations[rawValue] {
self = migrated
+ } else if let chain = Chain(rawValue: rawValue) {
+ self = chain
} else {
throw DecodingError.dataCorruptedError(
in: container,
debugDescription: "Cannot initialize Chain from invalid String value \(rawValue)"
)
}
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| init(from decoder: Decoder) throws { | |
| let container = try decoder.singleValueContainer() | |
| let rawValue = try container.decode(String.self) | |
| if let chain = Chain(rawValue: rawValue) { | |
| self = chain | |
| } else if let migrated = Chain.removedChainMigrations[rawValue] { | |
| self = migrated | |
| } else { | |
| init(from decoder: Decoder) throws { | |
| let container = try decoder.singleValueContainer() | |
| let rawValue = try container.decode(String.self) | |
| if let migrated = Chain.removedChainMigrations[rawValue] { | |
| self = migrated | |
| } else if let chain = Chain(rawValue: rawValue) { | |
| self = chain | |
| } else { | |
| throw DecodingError.dataCorruptedError( | |
| in: container, | |
| debugDescription: "Cannot initialize Chain from invalid String value \(rawValue)" | |
| ) | |
| } | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@VultisigApp/VultisigApp/Model/Chain.swift` around lines 59 - 66, The decoder
init currently tries Chain(rawValue:) before checking
Chain.removedChainMigrations, so legacy strings that still match current enum
cases (e.g., "polygon", "thorChainChainnet") never get remapped; update
init(from decoder: Decoder) to first consult
Chain.removedChainMigrations[rawValue] and, if present, assign self = migrated,
otherwise fall back to Chain(rawValue: rawValue) and then to the error path—this
ensures migrations take precedence over direct rawValue matching.
| private let keychainPrefix = "vultisig_agent_auth_" | ||
|
|
||
| /// In-memory token cache | ||
| private var tokens: [String: AgentAuthToken] = [:] |
There was a problem hiding this comment.
Thread-safety concern with tokens dictionary.
The tokens dictionary is accessed from multiple async contexts without synchronization. Since AgentAuthService is not @MainActor, concurrent access from different tasks could cause data races.
Suggested fix: Add actor isolation or use a lock
Option 1 - Make the class an actor:
-final class AgentAuthService {
+actor AgentAuthService {Option 2 - Make it @MainActor:
+@MainActor
final class AgentAuthService {Option 3 - Use a lock for the dictionary:
private let tokenLock = NSLock()
private var _tokens: [String: AgentAuthToken] = [:]
private var tokens: [String: AgentAuthToken] {
get { tokenLock.withLock { _tokens } }
set { tokenLock.withLock { _tokens = newValue } }
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@VultisigApp/VultisigApp/Services/Agent/AgentAuthService.swift` at line 20,
The tokens dictionary on AgentAuthService is not thread-safe (private var
tokens: [String: AgentAuthToken] = [:]) and can be accessed concurrently from
async tasks; fix by adding isolation: either convert AgentAuthService to an
actor (or mark it `@MainActor`) so tokens become actor-isolated, or keep it a
class and protect accesses with a lock (e.g., replace tokens with a locked
backing store and wrap all reads/writes to _tokens with the lock). Update all
methods that read/write tokens (references to tokens, _tokens, tokenLock, or
AgentAuthService methods manipulating tokens) to use the chosen isolation
mechanism consistently.
| AsyncThrowingStream { continuation in | ||
| let task = Task { | ||
| do { | ||
| let url = URL(string: Endpoint.agentConversationMessages(id: convId))! |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
wc -l VultisigApp/VultisigApp/Services/Agent/AgentBackendClient.swiftRepository: vultisig/vultisig-ios
Length of output: 132
🏁 Script executed:
sed -n '110,130p' VultisigApp/VultisigApp/Services/Agent/AgentBackendClient.swiftRepository: vultisig/vultisig-ios
Length of output: 1257
🏁 Script executed:
sed -n '320,370p' VultisigApp/VultisigApp/Services/Agent/AgentBackendClient.swiftRepository: vultisig/vultisig-ios
Length of output: 2058
Eliminate force unwraps and force casts in network operations.
Three instances where force unwraps/casts can crash the app:
- Line 117:
URL(...)! force unwrap - Lines 324, 363:
as! Tforce casts for empty response handling
Use guard statements with error throwing for URL construction, and conditional casting (as?) for type checks, as shown in your suggested patches. This aligns with the existing error handling pattern already used elsewhere in this file (e.g., line 340).
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@VultisigApp/VultisigApp/Services/Agent/AgentBackendClient.swift` at line 117,
Replace the force-unwrap of URL(string: Endpoint.agentConversationMessages(id:
convId)) and the force-casts `as! T` with safe checks: in the method that builds
the URL (where `URL(string: Endpoint.agentConversationMessages(id: convId))` is
used) change to a guard-let URL construction and throw a meaningful error if
nil; for the empty-response handling sites that use `as! T` (the two occurrences
in this file) replace with conditional casts (`as? T`) and guard-let the result,
throwing the same or a consistent network/parsing error when the cast fails so
it follows the existing error-handling pattern used elsewhere (see the pattern
around the code near the other guard/throw usages).
| print("[AgentBackend] 🌊 SSE response received") | ||
|
|
||
| guard let httpResponse = response as? HTTPURLResponse else { | ||
| print("[AgentBackend] ❌ Not an HTTP response") | ||
| throw AgentBackendError.noBody | ||
| } | ||
| print("[AgentBackend] 🌊 SSE HTTP status: \(httpResponse.statusCode)") | ||
| print("[AgentBackend] 🌊 Content-Type: \(httpResponse.value(forHTTPHeaderField: "Content-Type") ?? "nil")") |
There was a problem hiding this comment.
Remove raw SSE/body print statements from request/stream paths.
These logs include streamed payload content and error bodies, which can expose user prompts/responses and backend details in device logs.
Also applies to: 143-154, 182-188, 196-205, 210-210
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@VultisigApp/VultisigApp/Services/Agent/AgentBackendClient.swift` around lines
133 - 140, Remove all raw print(...) calls that emit SSE/body content in the
AgentBackendClient SSE/request/stream handling paths (the print lines that show
SSE response, full body, or error bodies); specifically eliminate prints in the
SSE response handler (the block that casts to HTTPURLResponse and references
AgentBackendError.noBody) and the other indicated ranges (143-154, 182-188,
196-205, 210) and replace them with non-sensitive logging: only record HTTP
statusCode, Content-Type, and/or the size or a redacted indicator (no payload
text) using the app's logger or OSLog with privacy flags. Ensure no streamed
payloads, prompts, responses, or full error bodies are written to device logs.
| func keysign(input: FastVaultKeysignInput) async throws -> FastVaultKeysignResult { | ||
| var lastError: Error? | ||
|
|
||
| for attempt in 1...input.maxAttempts { | ||
| do { | ||
| let result = try await keysignAttempt(input: input) | ||
| return result | ||
| } catch { | ||
| lastError = error | ||
| print("[FastVaultKeysign] ⚠️ Attempt \(attempt)/\(input.maxAttempts) failed: \(error.localizedDescription)") | ||
| if attempt < input.maxAttempts && isRetryable(error) { | ||
| try await Task.sleep(for: .seconds(2)) | ||
| continue | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
Fix retry control flow: invalid attempts can crash, and non-retryable errors still retry.
1...input.maxAttempts is unsafe for maxAttempts <= 0, and non-retryable failures currently continue to later attempts. In this flow, that can trigger unnecessary repeat keysign calls.
🛠️ Suggested fix
func keysign(input: FastVaultKeysignInput) async throws -> FastVaultKeysignResult {
+ guard input.maxAttempts > 0 else {
+ throw FastVaultKeysignError.keysignFailed("maxAttempts must be greater than 0")
+ }
var lastError: Error?
for attempt in 1...input.maxAttempts {
do {
let result = try await keysignAttempt(input: input)
return result
} catch {
lastError = error
print("[FastVaultKeysign] ⚠️ Attempt \(attempt)/\(input.maxAttempts) failed: \(error.localizedDescription)")
- if attempt < input.maxAttempts && isRetryable(error) {
- try await Task.sleep(for: .seconds(2))
- continue
- }
+ guard attempt < input.maxAttempts, isRetryable(error) else {
+ break
+ }
+ try await Task.sleep(for: .seconds(2))
+ continue
}
}
throw lastError ?? FastVaultKeysignError.keysignFailed("Unknown error")
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@VultisigApp/VultisigApp/Services/FastVault/FastVaultKeysignService.swift`
around lines 48 - 63, The loop in keysign(input:) is unsafe for
input.maxAttempts <= 0 and incorrectly allows non-retryable errors to proceed to
further attempts; guard input.maxAttempts > 0 at the start of keysign(input:),
and change the catch behaviour so that if isRetryable(error) is false you
rethrow immediately instead of continuing; otherwise only retry when attempt <
input.maxAttempts and isRetryable(error) (sleep then continue). Use the existing
symbols keysign(input:), keysignAttempt(input:), isRetryable(_:), lastError, and
input.maxAttempts, and ensure that if the loop exits without returning you throw
lastError (or a specific error) rather than letting execution fall through.
| Text("Loading conversations...") | ||
| .font(.subheadline) | ||
| .foregroundColor(Theme.colors.textTertiary) | ||
| .padding(.top, 8) |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# First, check if the file exists and find all .foregroundColor usages with line numbers
rg -n '\.foregroundColor\s*\(' VultisigApp/VultisigApp/Views/Agent/AgentConversationsView.swiftRepository: vultisig/vultisig-ios
Length of output: 871
Switch all foregroundColor usages to foregroundStyle.
This view contains 11 instances of the deprecated .foregroundColor() modifier that must be updated to .foregroundStyle() per the coding guidelines.
Affected lines and fix pattern
Lines: 61, 81, 85, 95, 98, 124, 127, 131, 154, 159, 164
-Text("...")
- .foregroundColor(Theme.colors.textTertiary)
+Text("...")
+ .foregroundStyle(Theme.colors.textTertiary)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| Text("Loading conversations...") | |
| .font(.subheadline) | |
| .foregroundColor(Theme.colors.textTertiary) | |
| .padding(.top, 8) | |
| Text("Loading conversations...") | |
| .font(.subheadline) | |
| .foregroundStyle(Theme.colors.textTertiary) | |
| .padding(.top, 8) |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@VultisigApp/VultisigApp/Views/Agent/AgentConversationsView.swift` around
lines 59 - 62, In AgentConversationsView, replace every usage of the deprecated
.foregroundColor(...) with .foregroundStyle(...) for the 11 occurrences inside
the view (e.g., the Text("Loading conversations...") and other Text/Image rows);
specifically change calls like .foregroundColor(Theme.colors.textTertiary) to
.foregroundStyle(Theme.colors.textTertiary) so the Theme.colors values are
passed to the new modifier; update all instances referenced in the review (the
ones within AgentConversationsView) to use .foregroundStyle to conform with the
coding guidelines.
| Button { | ||
| navigateToChat(with: nil) | ||
| } label: { | ||
| HStack { | ||
| Image(systemName: "plus.bubble.fill") | ||
| .foregroundColor(Theme.colors.bgPrimary) | ||
| Text("New Chat") | ||
| .font(.body.bold()) | ||
| .foregroundColor(Theme.colors.bgPrimary) | ||
| } | ||
| .padding() | ||
| .background(Theme.colors.turquoise) | ||
| .cornerRadius(12) | ||
| } |
There was a problem hiding this comment.
Replace custom “New Chat” CTA buttons with PrimaryButton.
These two CTA sections currently use custom button styling instead of the shared component.
As per coding guidelines: "Use PrimaryButton component for all buttons instead of creating custom button styles."
Also applies to: 121-139
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@VultisigApp/VultisigApp/Views/Agent/AgentConversationsView.swift` around
lines 90 - 103, Replace the custom-styled Button usages in
AgentConversationsView with the shared PrimaryButton component: where the view
currently calls Button { navigateToChat(with: nil) } label: { ... } (and the
similar CTA block later), swap them to use PrimaryButton so the CTA text, icon,
action (navigateToChat(with: nil) or appropriate argument) and the Theme styling
are provided by PrimaryButton; ensure you pass the action closure that calls
navigateToChat and supply the label/title and system image to PrimaryButton (or
adapt to its initializer) so no custom padding/background/cornerRadius styling
remains.
|
|
||
| private var connectionButton: some View { | ||
| Circle() | ||
| .fill(viewModel.isConnected ? Theme.colors.alertSuccess : Color.gray) |
There was a problem hiding this comment.
Avoid hardcoded disconnected color in the connection dot.
Use a semantic Theme.colors value instead of Color.gray so disconnected state follows theme design.
As per coding guidelines: "Always use the Theme enum for colors instead of hardcoding color values."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@VultisigApp/VultisigApp/Views/Agent/AgentConversationsView.swift` at line
186, The connection dot currently hardcodes the disconnected color (Color.gray);
update the ternary in the .fill modifier so the false branch uses a semantic
Theme color (e.g., replace Color.gray with Theme.colors.alertMuted or the
appropriate disconnected color) so it reads like .fill(viewModel.isConnected ?
Theme.colors.alertSuccess : Theme.colors.alertMuted) and ensure the chosen
Theme.colors property returns a SwiftUI Color to match the .fill usage.
| struct AgentPasswordPromptView: View { | ||
| let onSubmit: (String) -> Void | ||
|
|
||
| @State private var password = "" | ||
| @State private var isSubmitting = false | ||
| @Environment(\.dismiss) private var dismiss | ||
|
|
||
| var body: some View { | ||
| NavigationView { |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Align this flow to screen conventions (Screen + *Screen.swift).
This prompt is implemented as a full screen, but it is defined as AgentPasswordPromptView in *View.swift. Please move it to the screen pattern used in this repo.
As per coding guidelines: "Always use the Screen component for views that represent a whole screen, and use the Screen suffix in struct names..." and "Name screen files with the *Screen.swift pattern..."
Also applies to: 94-95
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@VultisigApp/VultisigApp/Views/Agent/AgentPasswordPromptView.swift` around
lines 10 - 18, This view is a full-screen prompt and must follow the repo's
Screen pattern: rename the struct AgentPasswordPromptView to
AgentPasswordPromptScreen (and move the file to
AgentPasswordPromptScreen.swift), update its type signature and initializer
usage (preserve onSubmit, password, isSubmitting, and dismiss bindings), and
replace any NavigationView usage inside with the project's Screen container
component so the screen integrates with the app's layout conventions; also
update all call sites that reference AgentPasswordPromptView to use
AgentPasswordPromptScreen.
| SecureField("Password", text: $password) | ||
| .textFieldStyle(.plain) | ||
| .padding() | ||
| .background(Theme.colors.bgSurface1) | ||
| .cornerRadius(12) | ||
| .foregroundColor(Theme.colors.textPrimary) | ||
| .padding(.horizontal) |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Use shared input/button components (CommonTextField, PrimaryButton).
The current raw SecureField + custom button styling diverges from the project’s standard components for consistency and maintainability.
As per coding guidelines: "Use CommonTextField for all text input fields" and "Use PrimaryButton component for all buttons instead of creating custom button styles."
Also applies to: 47-75
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@VultisigApp/VultisigApp/Views/Agent/AgentPasswordPromptView.swift` around
lines 39 - 45, Replace the raw SecureField and the custom-styled button in
AgentPasswordPromptView with the shared CommonTextField and PrimaryButton
components: swap the SecureField("Password", text: $password) usage to use
CommonTextField bound to the same password state (preserving .secure / isSecure
input variant or configuration on CommonTextField), remove the ad-hoc
padding/background/cornerRadius/foregroundColor styling and rely on Theme via
the shared component, and replace the custom Button (the block covering lines
47-75) with PrimaryButton wired to the same action/disabled logic so behavior
(submit/cancel) and state bindings remain identical. Ensure you keep the same
accessibility labels/placeholders and validation/disabled conditions when
mapping the handlers to CommonTextField and PrimaryButton.
…xt presentation in Agent views.
…esult to the backend to reduce API calls.
After auto-executing actions locally the first SSE stream is already done (isLoading=false). The few-second wait for the backend's follow-up response looked like a freeze. Re-enable isLoading before sendActionResult so the spinner stays visible until the next stream's first event arrives.
Plant an empty seed message (streamingMessageId set) before calling sendActionResult. The second SSE stream's text_delta events then append character-by-character to the seed, giving a natural typing effect. If the backend sends a message event instead, finalizeStreamingMessage replaces the seed cleanly — no duplicate messages, no flicker.
- AgentContextBuilder.buildContext: reads all AddressBookItem from
SwiftData and includes them in every request, so the AI knows
contacts immediately without calling get_address_book first.
(Fixes the TODO on addressBook: nil)
- AgentToolExecutor.executeGetAddressBook: chain filter now compares
against both chain.rawValue ('solana') and chain.name ('Solana'),
matching the pattern already used in executeGetAddresses. Prevents
empty results when the AI sends the display name as the filter.
When resolving default decimals for a chain's native coin, the previous code used .first(where: chain == X) which for Solana returned JUP (decimals:6) instead of native SOL (decimals:9). Fixed in 3 places: executeAddCoin, executeAddChain, executeAddAddressBook. Root cause of SOL balance showing 1000x too high (142 SOL vs actual 0.1422 SOL).
…upporting FastVault and handling broadcasted transactions.
|
@enriquesouza , this doesn't work on MacOS |
I will fix that |
|
Fixed the macOS compilation issue in |
|
Applied and pushed all actionable CodeRabbit review suggestions, including standardizing the Agent conversational views to |
… to fix provisioning profile issues
…rom multiple actions, improve error reporting, and add pre-keysign transaction validation.
…ne Agent conversation context building and navigation.
…and add search/filter to vault and address tools.
…omatic peer discovery for streamlined transaction signing.
2 participants
Vulti Agent Chat Integration
This PR integrates the Vulti AI Assistant (powered by the agent-backend) into the Vultisig iOS app as a full-featured chat experience.
Features
🤖 Agent Chat UI
🔐 Agent Authentication
🛠️ iOS-side Tool Executor
The
AgentToolExecutorhandles all actions that require local vault access. The AI calls these directly — no manual interaction needed.Chain & Token Management
add_chainadd_coin/add_tokenremove_chainremove_coinsearch_tokenVault Information
list_vaultsget_addressesget_balancesget_portfolioget_market_priceAddress Book
get_address_bookadd_address_book/address_book_adddelete_address_book/address_book_removeServer-side (MCP/agent-backend handled)
build_swap_tx,build_send_tx,build_custom_tx,plugin_install,create_policy,delete_policy,read_evm_contract,scan_tx,thorchain_query,get_eth_balance,get_token_balance,get_utxo_balanceand more — all forwarded to the MCP server.Pending
sign_tx— requires the full iOS Keysign flow (2-of-3 multi-party signing), not yet auto-executable from the agent.🧭 MCP Session Priming
set_vaultso MCP tools can resolve addresses server-side.🔧 Robustness Improvements
add_chain/add_coinis resilient to LLM shape variations (nested array or flat object both accepted)Files Changed
Services/Agent/AgentToolExecutor.swift— full tool executor with all iOS-side actionsServices/Agent/AgentModels.swift— all action parameter/result model typesServices/Agent/AgentBackendClient.swift— REST + SSE streaming clientServices/Agent/AgentAuthService.swift— vault-signed auth token managementServices/Agent/AgentContextBuilder.swift— vault context assembly for AI promptsViews/Agent/AgentChatView.swift— main chat UIViews/Agent/AgentConversationsView.swift— conversation listViewModels/Agent/AgentChatViewModel.swift— orchestratorViewModels/Agent/AgentConversationsViewModel.swift— conversation list stateRelated Repositories
prompt.goto add instructions forremove_chain,remove_coin, and the full Modifying Chains & Coins sectionSummary by CodeRabbit
Release Notes
New Features
Improvements