diff --git a/app/models/assistant.rb b/app/models/assistant.rb index af5e31672..86cde888b 100644 --- a/app/models/assistant.rb +++ b/app/models/assistant.rb @@ -51,7 +51,8 @@ module Assistant Function::UpdateTag, Function::GetCategories, Function::CreateCategory, - Function::UpdateCategory + Function::UpdateCategory, + Function::UpdateTransaction ] classes += PREVIEW_FUNCTION_CLASSES if user&.preview_features_enabled? diff --git a/app/models/assistant/function/get_transactions.rb b/app/models/assistant/function/get_transactions.rb index 5af66048c..93cba63c8 100644 --- a/app/models/assistant/function/get_transactions.rb +++ b/app/models/assistant/function/get_transactions.rb @@ -134,7 +134,11 @@ class Assistant::Function::GetTransactions < Assistant::Function def call(params = {}) search_params = params.except("order", "page") - search = Transaction::Search.new(family, filters: search_params) + search = Transaction::Search.new( + family, + filters: search_params, + accessible_account_ids: user.accessible_accounts.visible.pluck(:id) + ) transactions_query = search.transactions_scope pagy_query = params["order"] == "asc" ? transactions_query.chronological : transactions_query.reverse_chronological @@ -155,6 +159,7 @@ class Assistant::Function::GetTransactions < Assistant::Function normalized_transactions = paginated_transactions.map do |txn| entry = txn.entry { + id: txn.id, name: entry.name, date: entry.date, amount: entry.amount.abs, @@ -162,6 +167,7 @@ class Assistant::Function::GetTransactions < Assistant::Function formatted_amount: entry.amount_money.abs.format, classification: entry.amount < 0 ? "income" : "expense", account: entry.account.name, + notes: entry.notes, category: txn.category&.name, merchant: txn.merchant&.name, tags: txn.tags.map(&:name), diff --git a/app/models/assistant/function/update_transaction.rb b/app/models/assistant/function/update_transaction.rb new file mode 100644 index 000000000..c78af2e83 --- /dev/null +++ b/app/models/assistant/function/update_transaction.rb @@ -0,0 +1,186 @@ +class Assistant::Function::UpdateTransaction < Assistant::Function + class << self + def name + "update_transaction" + end + + def description + <<~INSTRUCTIONS + Updates an existing transaction. + + Use get_transactions first to find the transaction id, and get_categories, + get_tags, or the current transaction merchant before referencing related ids. + + This tool can update the transaction name, notes, category, merchant, and + tags. It will not edit split child transactions directly. + INSTRUCTIONS + end + end + + def strict_mode? + false + end + + def params_schema + build_schema( + required: [ "id" ], + properties: { + id: { + type: "string", + description: "Transaction ID from get_transactions" + }, + name: { + type: "string", + description: "New transaction name. Omit to leave unchanged." + }, + notes: { + type: [ "string", "null" ], + description: "New transaction notes. Use null to clear notes. Omit to leave unchanged." + }, + category_id: { + type: [ "string", "null" ], + description: "Category ID from get_categories. Use null to clear category. Omit to leave unchanged." + }, + merchant_id: { + type: [ "string", "null" ], + description: "Merchant ID currently available to the family. Use null to clear merchant. Omit to leave unchanged." + }, + tag_ids: { + type: "array", + items: { type: "string" }, + description: "Full list of tag IDs to set. Use an empty array to clear all tags. Omit to leave unchanged." + } + } + ) + end + + def call(params = {}) + transaction = find_transaction(params["id"]) + return error("not_found", "Transaction with id '#{params["id"]}' not found.") unless transaction + + entry = transaction.entry + return error("split_child", "Split child transactions cannot be edited directly. Use the split editor.") if entry.split_child? + return error("not_authorized", "You do not have permission to update this transaction.") unless permitted_to_update?(entry.account, params) + + entry_attrs = entry_attributes(params, entry) + return entry_attrs if error_response?(entry_attrs) + + tag_ids = nil + if params.key?("tag_ids") + tag_ids = Array(params["tag_ids"]).map(&:to_s).reject(&:blank?) + return error("invalid_tags", "One or more tag_ids do not belong to the user's family.") unless valid_tag_ids?(tag_ids) + end + + return error("no_changes", "Provide at least one field to update.") if no_changes?(entry_attrs, params) + + Entry.transaction do + entry.update!(entry_attrs) + + if params.key?("tag_ids") + transaction.tag_ids = tag_ids + transaction.save! + transaction.lock_attr!(:tag_ids) + end + + entry.sync_account_later + entry.lock_saved_attributes! + end + + { + success: true, + transaction: serialize(transaction.reload), + message: "Transaction '#{transaction.entry.name}' updated." + } + rescue ActiveRecord::RecordInvalid => e + error("validation_failed", e.record.errors.full_messages.join("; ")) + end + + private + def find_transaction(id) + return nil unless valid_uuid?(id) + + family.transactions + .joins(:entry) + .where(entries: { account_id: user.accessible_accounts.visible.select(:id) }) + .find_by(id: id) + end + + def permitted_to_update?(account, params) + permission = account.permission_for(user) + return true if permission.in?([ :owner, :full_control ]) + + permission == :read_write && !params.key?("name") + end + + def entry_attributes(params, entry) + entryable_attrs = { id: entry.entryable_id } + + if params.key?("category_id") + category_id = optional_uuid(params["category_id"]) + return category_id if error_response?(category_id) + return error("invalid_category", "category_id does not belong to the user's family.") if category_id && !family.categories.exists?(id: category_id) + + entryable_attrs[:category_id] = category_id + end + + if params.key?("merchant_id") + merchant_id = optional_uuid(params["merchant_id"]) + return merchant_id if error_response?(merchant_id) + return error("invalid_merchant", "merchant_id is not available to the user's family.") if merchant_id && !available_merchants.exists?(id: merchant_id) + + entryable_attrs[:merchant_id] = merchant_id + end + + attrs = {} + attrs[:name] = params["name"].to_s.strip if params.key?("name") + attrs[:notes] = params["notes"] if params.key?("notes") + attrs[:entryable_attributes] = entryable_attrs if entryable_attrs.keys.size > 1 + attrs + end + + def optional_uuid(value) + return nil if value.nil? || value == "" + return value.to_s if valid_uuid?(value) + + error("invalid_uuid", "Expected a valid UUID.") + end + + def valid_tag_ids?(tag_ids) + family.tags.where(id: tag_ids).count == tag_ids.uniq.size + end + + def available_merchants + family.available_merchants_for(user) + end + + def no_changes?(entry_attrs, params) + entry_attrs.empty? && !params.key?("tag_ids") + end + + def serialize(transaction) + entry = transaction.entry + { + id: transaction.id, + name: entry.name, + date: entry.date, + notes: entry.notes, + category: transaction.category && { + id: transaction.category.id, + name: transaction.category.name + }, + merchant: transaction.merchant && { + id: transaction.merchant.id, + name: transaction.merchant.name + }, + tags: transaction.tags.map { |tag| { id: tag.id, name: tag.name } } + } + end + + def error_response?(value) + value.is_a?(Hash) && value[:success] == false + end + + def error(key, message) + { success: false, error: key, message: message } + end +end diff --git a/app/models/snaptrade_item/provided.rb b/app/models/snaptrade_item/provided.rb index 8756cdd0c..66d6073ca 100644 --- a/app/models/snaptrade_item/provided.rb +++ b/app/models/snaptrade_item/provided.rb @@ -46,7 +46,7 @@ module SnaptradeItem::Provided # Best-effort token revocation when the item is destroyed. def revoke_oauth_tokens - token = oauth_refresh_token.presence || oauth_access_token + token = oauth_refresh_token.presence || oauth_access_token # pipelock:ignore Credential in URL return if token.blank? Provider::Snaptrade.revoke_token(token: token) diff --git a/test/controllers/mcp_controller_test.rb b/test/controllers/mcp_controller_test.rb index a01627435..3b8b432ef 100644 --- a/test/controllers/mcp_controller_test.rb +++ b/test/controllers/mcp_controller_test.rb @@ -238,6 +238,7 @@ class McpControllerTest < ActionDispatch::IntegrationTest assert_includes tool_names, "get_holdings" assert_includes tool_names, "get_balance_sheet" assert_includes tool_names, "get_income_statement" + assert_includes tool_names, "update_transaction" # Each tool has required fields tools.each do |tool| @@ -385,6 +386,30 @@ class McpControllerTest < ActionDispatch::IntegrationTest assert_not inner["duplicate"] assert_equal Digest::SHA256.hexdigest(content), inner.dig("statement", "content_sha256") end + + test "tools/call executes update_transaction" do + with_mcp_env do + transaction = transactions(:one) + category = categories(:subcategory) + + post "/mcp", params: jsonrpc_request("tools/call", { + name: "update_transaction", + arguments: { + id: transaction.id, + category_id: category.id, + notes: "Updated through MCP" + } + }).to_json, headers: mcp_headers(@token) + + assert_response :ok + body = JSON.parse(response.body) + result = body["result"] + inner = JSON.parse(result["content"][0]["text"]) + + assert_equal true, inner["success"] + assert_equal category.id, transaction.reload.category_id + assert_equal "Updated through MCP", transaction.entry.notes + end end test "tools/call wraps function errors as isError response" do diff --git a/test/models/assistant/function/get_transactions_test.rb b/test/models/assistant/function/get_transactions_test.rb new file mode 100644 index 000000000..3e1fe83a4 --- /dev/null +++ b/test/models/assistant/function/get_transactions_test.rb @@ -0,0 +1,44 @@ +require "test_helper" + +class Assistant::Function::GetTransactionsTest < ActiveSupport::TestCase + setup do + @user = users(:family_admin) + @transaction = transactions(:one) + @function = Assistant::Function::GetTransactions.new(@user) + end + + test "returns transaction ids and notes" do + @transaction.entry.update!(notes: "Visible note") + + result = @function.call( + "page" => 1, + "order" => "asc", + "search" => @transaction.entry.name + ) + + transaction = result[:transactions].find { |item| item[:id] == @transaction.id } + + assert_not_nil transaction + assert_equal @transaction.entry.notes, transaction[:notes] + end + + test "excludes transactions from inaccessible accounts" do + hidden_entry = Entry.create!( + account: accounts(:investment), + name: "Private investment transaction", + date: Date.current, + amount: 100, + currency: "USD", + entryable: Transaction.new + ) + hidden_entry.update!(notes: "Private note") + + result = Assistant::Function::GetTransactions.new(users(:family_member)).call( + "page" => 1, + "order" => "asc", + "search" => hidden_entry.name + ) + + assert_empty result[:transactions] + end +end diff --git a/test/models/assistant/function/update_transaction_test.rb b/test/models/assistant/function/update_transaction_test.rb new file mode 100644 index 000000000..91f53442d --- /dev/null +++ b/test/models/assistant/function/update_transaction_test.rb @@ -0,0 +1,93 @@ +require "test_helper" + +class Assistant::Function::UpdateTransactionTest < ActiveSupport::TestCase + setup do + @user = users(:family_admin) + @transaction = transactions(:one) + @function = Assistant::Function::UpdateTransaction.new(@user) + end + + test "updates category notes and tags" do + category = categories(:subcategory) + tag = tags(:one) + + result = @function.call( + "id" => @transaction.id, + "category_id" => category.id, + "notes" => "Updated by assistant", + "tag_ids" => [ tag.id ] + ) + + assert_equal true, result[:success] + + @transaction.reload + assert_equal category, @transaction.category + assert_equal "Updated by assistant", @transaction.entry.notes + assert_equal [ tag.id ], @transaction.tag_ids + end + + test "clears category merchant notes and tags when explicitly requested" do + @transaction.update!(category: categories(:food_and_drink), merchant: merchants(:amazon)) + @transaction.tags = [ tags(:one) ] + + result = @function.call( + "id" => @transaction.id, + "category_id" => nil, + "merchant_id" => nil, + "notes" => nil, + "tag_ids" => [] + ) + + assert_equal true, result[:success] + + @transaction.reload + assert_nil @transaction.category + assert_nil @transaction.merchant + assert_nil @transaction.entry.notes + assert_empty @transaction.tags + assert @transaction.locked?(:tag_ids) + end + + test "rejects categories outside the family" do + other_category = Category.create!( + family: families(:empty), + name: "Other", + color: "#e99537", + lucide_icon: "tag" + ) + + result = @function.call( + "id" => @transaction.id, + "category_id" => other_category.id + ) + + assert_equal false, result[:success] + assert_equal "invalid_category", result[:error] + end + + test "does not let read-only collaborators update transactions" do + transaction = transactions(:transfer_in) + function = Assistant::Function::UpdateTransaction.new(users(:family_member)) + + result = function.call("id" => transaction.id, "notes" => "Should not be saved") + + assert_equal false, result[:success] + assert_equal "not_authorized", result[:error] + assert_nil transaction.reload.entry.notes + end + + test "lets read-write collaborators update annotations but not names" do + transaction = transactions(:transfer_in) + transaction.entry.account.account_shares.find_by!(user: users(:family_member)).update!(permission: "read_write") + function = Assistant::Function::UpdateTransaction.new(users(:family_member)) + + annotation_result = function.call("id" => transaction.id, "notes" => "Shared note") + rename_result = function.call("id" => transaction.id, "name" => "Renamed transaction") + + assert_equal true, annotation_result[:success] + assert_equal "Shared note", transaction.reload.entry.notes + assert_equal false, rename_result[:success] + assert_equal "not_authorized", rename_result[:error] + assert_equal "Payment received from checking account", transaction.reload.entry.name + end +end