fix(goals): pledge lifecycle + connected-account detection

Behavioural fixes touching Goal, GoalPledge, the reconciler and the
goals controller. No schema change.

B5 — connected-account detection covered only Plaid. SimpleFIN, Brex,
Enable Banking, IBKR, Kraken, SnapTrade and Lunchflow users got
"manual_save" pledges by default; their auto-synced Transactions then
failed to match (reconciler matches Transactions to "transfer" pledges
only). Pledges sat in the yellow banner until expiry. Switch the
detection to !Account#manual?, which mirrors the existing
`Account.manual` scope (no account_providers, no plaid_account_id, no
simplefin_account_id). Add `Account#manual?` so the per-instance and
per-query checks can't drift.

B7 — `extend!` widens `expires_at` but `matches?` was anchored on
`created_at ± 5d`, so an extension that pushed the expiry past day 5
didn't actually buy any match runway. Widen the upper bound to
`max(created_at + 5d, expires_at)`. The lower bound stays at
`created_at − 5d`.

B8 — `Goal#open_pledges` returned `status: open` regardless of expiry.
Between a pledge timing out (day 7) and the 15-min sweep job marking
it `expired`, the show page rendered a ghost yellow banner with
"0 days left" that the reconciler would no longer touch. Add
`expires_at >= NOW` to the scope so the visible state matches the
match-eligible state.

B9 — Double-click on Record pledge produced two identical open
pledges, which then stacked as two yellow banners. Add a create-time
validation rejecting duplicates against (goal_id, account_id, amount,
status=open, expires_at >= NOW).

B10 — The reconciler used `transaction.with_lock` but didn't lock the
pledge. Two concurrent reconcile attempts on different transactions
could both target the same pledge; one would lose to the partial
unique index on `transactions.extra->'goal'->>'pledge_id'` and the
RecordNotUnique was caught by the outer StandardError rescue, which
silently dropped the other transaction's match attempt entirely.
Lock the pledge first, re-check `status_open?` inside the lock, and
catch RecordNotUnique alongside RecordInvalid/NotOpenError in the
reconciler — so on a lost race we fall through to the next candidate
pledge instead of exiting the loop. Extract the Valuation-match path
to `GoalPledge#resolve_with_valuation!` so it goes through the same
locked status-recheck.

B12 — When a goal is destroyed, `dependent: :destroy` reaped pledges
but left `transactions.extra["goal"]["pledge_id"]` pointing at the
now-deleted UUIDs. The partial unique index on that JSON path then
indexed stale references. Add a `before_destroy` on GoalPledge that
clears the matching transaction's `extra` if it still points back to
the pledge.

B6 — `last_matched_pledge_at` used `goal_pledges.maximum(:updated_at)`
on matched rows. Any backfill or sync-resync that touches a matched
pledge bumped `updated_at`, so a single resync set every goal's "Last
saved N days ago" header back to "today". Switch to the entry's
`date` via a join through `matched_transaction_id`, which reflects the
date the money actually moved.

B22 — `scope :chronological` ordered DESC, the opposite of what the
name promises. Rename to `:reverse_chronological` and update the one
caller in `goals#show`. (Other models' `chronological` scopes are
unrelated and ordered correctly.)

Also: preload `account_providers` on `linked_accounts` in the index
and show controllers so `Account#manual?` walks the in-memory
collection instead of triggering N queries.

Tests: add fixture-backed coverage for extend-widens-match-window,
post-extend rejection beyond expiry, and the duplicate-pledge
validation. Existing assertions still hold against the new
`matches?` window math.

app/controllers/goals_controller.rb

@@ -11,7 +11,10 @@ class GoalsController < ApplicationController
       h[state] = state == "all" ? state_counts.values.sum : (state_counts[state] || 0)
     end
 
-    all_goals = Current.family.goals.alphabetically.includes(:linked_accounts, :open_pledges).to_a
+    all_goals = Current.family.goals
+                       .alphabetically
+                       .includes(:open_pledges, linked_accounts: :account_providers)
+                       .to_a
     @active_goals = all_goals.reject { |g| %w[completed archived].include?(g.state) }
                              .sort_by { |g| [ g.paused? ? 3 : ACTIVE_STATUS_RANK.fetch(g.status, 4), g.name.downcase ] }
     @completed_goals = all_goals.select { |g| g.state == "completed" }
@@ -28,7 +31,7 @@ class GoalsController < ApplicationController
   end
 
   def show
-    @open_pledges = @goal.open_pledges.chronological.to_a
+    @open_pledges = @goal.open_pledges.reverse_chronological.to_a
     @breadcrumbs = [
       [ t("breadcrumbs.home"), root_path ],
       [ t("goals.index.title"), goals_path ],
@@ -140,7 +143,7 @@ class GoalsController < ApplicationController
   private
     def set_goal
       @goal = Current.family.goals
-                             .includes(:linked_accounts, :open_pledges)
+                             .includes(:open_pledges, linked_accounts: :account_providers)
                              .find(params[:id])
     end
 

app/models/account.rb

@@ -344,7 +344,13 @@ class Account < ApplicationRecord
   def manual_crypto_exchange?
     accountable_type == "Crypto" &&
       accountable&.subtype == "exchange" &&
-      account_providers.none? &&
+      manual?
+  end
+
+  # True when the account has no live sync provider attached. Mirrors the
+  # `Account.manual` scope so per-instance checks don't drift from the query.
+  def manual?
+    account_providers.none? &&
       plaid_account_id.blank? &&
       simplefin_account_id.blank?
   end

app/models/goal.rb

@@ -10,7 +10,9 @@ class Goal < ApplicationRecord
   has_many :goal_accounts, dependent: :destroy
   has_many :linked_accounts, through: :goal_accounts, source: :account
   has_many :goal_pledges, dependent: :destroy
-  has_many :open_pledges, -> { where(status: "open") }, class_name: "GoalPledge"
+  has_many :open_pledges,
+           -> { where(status: "open").where("expires_at >= ?", Time.current) },
+           class_name: "GoalPledge"
 
   validates :name, presence: true, length: { maximum: 255 }
   validates :target_amount, presence: true, numericality: { greater_than: 0 }
@@ -208,21 +210,34 @@ class Goal < ApplicationRecord
     end
   end
 
-  # Days since the most recently matched pledge. Used by the show header to
-  # show "Last saved N days ago". Returns nil if no pledge has resolved yet.
+  # Date of the most-recently-matched pledge's underlying entry. Used by the
+  # show header to display "Last saved N days ago". Anchoring on the entry's
+  # date keeps the readout stable under sync re-runs (which would bump
+  # pledge#updated_at). Returns nil if no pledge has resolved yet.
   def last_matched_pledge_at
-    @last_matched_pledge_at ||= goal_pledges.where(status: "matched").maximum(:updated_at)
+    return @last_matched_pledge_at if defined?(@last_matched_pledge_at)
+
+    @last_matched_pledge_at = Entry
+      .where(entryable_type: "Transaction")
+      .joins("INNER JOIN goal_pledges ON goal_pledges.matched_transaction_id = entries.entryable_id")
+      .where(goal_pledges: { goal_id: id, status: "matched" })
+      .maximum(:date)
   end
 
   def last_matched_pledge_days_ago
     last = last_matched_pledge_at
     return nil if last.nil?
 
-    (Date.current - last.to_date).to_i
+    (Date.current - last).to_i
   end
 
+  # True when any linked account is wired to a live sync provider (Plaid,
+  # SimpleFIN, or any AccountProvider — Brex, Enable Banking, IBKR, Kraken,
+  # SnapTrade, Lunchflow). Drives the pledge-create copy: connected accounts
+  # get the "I just transferred…" path; manual-only accounts get "I just
+  # saved…" so users aren't told to wait for a sync that won't happen.
   def any_connected_account?
-    linked_accounts.any? { |a| a.respond_to?(:plaid_account) && a.plaid_account.present? }
+    linked_accounts.any? { |a| !a.manual? }
   end
 
   # "I just transferred" for bank-connected accounts, "I just saved" for manual-only.

app/models/goal_pledge.rb

@@ -22,23 +22,29 @@ class GoalPledge < ApplicationRecord
   validates :expires_at, presence: true
   validate :account_must_be_linked_to_goal
   validate :currency_matches_goal
+  validate :no_duplicate_open_pledge, on: :create
 
   monetize :amount
 
-  scope :chronological, -> { order(created_at: :desc) }
+  # Newest first. Used by the show page to render pending-pledge banners in
+  # "most-recent on top" order. Not actually chronological — kept for clarity.
+  scope :reverse_chronological, -> { order(created_at: :desc) }
   scope :open_and_expired_now, -> {
     where(status: "open").where("expires_at < ?", Time.current)
   }
 
   before_validation :assign_defaults, on: :create
+  before_destroy :clear_matched_transaction_extra
 
-  # Tolerance check: ±5 days from pledge date, amount within ±$0.50 OR ±1%.
+  # Tolerance check: entry date within [created_at − 5d, expires_at] (so
+  # extend! widens the upper bound) and amount within ±$0.50 OR ±1%.
   def matches?(entry)
     return false unless status_open?
     return false unless entry.account_id == account_id
 
-    date_diff = (entry.date - created_at.to_date).abs.to_i
-    return false if date_diff > MATCH_DATE_TOLERANCE_DAYS
+    earliest = created_at.to_date - MATCH_DATE_TOLERANCE_DAYS.days
+    latest = [ created_at.to_date + MATCH_DATE_TOLERANCE_DAYS.days, expires_at.to_date ].max
+    return false unless entry.date >= earliest && entry.date <= latest
 
     txn_amount = entry.amount.to_d.abs
     pledge_amount = amount.to_d
@@ -51,15 +57,28 @@ class GoalPledge < ApplicationRecord
   end
 
   def resolve_with!(transaction)
-    transaction.with_lock do
-      pledge_id_in_extra = transaction.extra.dig("goal", "pledge_id")
-      raise ActiveRecord::RecordInvalid if pledge_id_in_extra.present? && pledge_id_in_extra != id
+    with_lock do
+      raise NotOpenError, "Pledge no longer open" unless status_open?
 
-      extra = transaction.extra || {}
-      extra["goal"] = (extra["goal"] || {}).merge("pledge_id" => id)
-      transaction.update!(extra: extra)
+      transaction.with_lock do
+        pledge_id_in_extra = transaction.extra.dig("goal", "pledge_id")
+        raise ActiveRecord::RecordInvalid if pledge_id_in_extra.present? && pledge_id_in_extra != id
 
-      update!(status: "matched", matched_transaction_id: transaction.id)
+        extra = transaction.extra || {}
+        extra["goal"] = (extra["goal"] || {}).merge("pledge_id" => id)
+        transaction.update!(extra: extra)
+
+        update!(status: "matched", matched_transaction_id: transaction.id)
+      end
+    end
+  end
+
+  # Valuation-backed match: no transaction to stamp, just flip the pledge.
+  def resolve_with_valuation!
+    with_lock do
+      raise NotOpenError, "Pledge no longer open" unless status_open?
+
+      update!(status: "matched")
     end
   end
 
@@ -111,4 +130,30 @@ class GoalPledge < ApplicationRecord
 
       errors.add(:currency, :must_match_goal)
     end
+
+    # Guards against a double-click that creates two identical open pledges,
+    # which would render two yellow banners and leave one orphaned to expiry.
+    def no_duplicate_open_pledge
+      return unless goal_id && account_id && amount && status_open?
+
+      exists = GoalPledge
+        .where(goal_id: goal_id, account_id: account_id, amount: amount, status: "open")
+        .where("expires_at >= ?", Time.current)
+        .exists?
+
+      errors.add(:base, :duplicate_open_pledge) if exists
+    end
+
+    def clear_matched_transaction_extra
+      return if matched_transaction_id.blank?
+
+      txn = Transaction.find_by(id: matched_transaction_id)
+      return if txn.nil?
+      return unless txn.extra.dig("goal", "pledge_id") == id
+
+      new_extra = txn.extra.deep_dup
+      new_extra["goal"]&.delete("pledge_id")
+      new_extra.delete("goal") if new_extra["goal"]&.empty?
+      txn.update!(extra: new_extra)
+    end
 end

app/models/goal_pledge/reconciler.rb

@@ -16,12 +16,19 @@ class GoalPledge::Reconciler
       next unless pledge.matches?(entry)
 
       begin
-        pledge.resolve_with!(entry.transaction) if entry.entryable.is_a?(Transaction)
-        pledge.update!(status: "matched") if entry.entryable.is_a?(Valuation)
+        if entry.entryable.is_a?(Transaction)
+          pledge.resolve_with!(entry.transaction)
+        elsif entry.entryable.is_a?(Valuation)
+          pledge.resolve_with_valuation!
+        end
         Rails.logger.info("GoalPledge ##{pledge.id} matched entry ##{entry.id}")
         return
-      rescue ActiveRecord::RecordInvalid => e
-        Rails.logger.warn("GoalPledge ##{pledge.id} match failed: #{e.message}")
+      rescue GoalPledge::NotOpenError,
+             ActiveRecord::RecordInvalid,
+             ActiveRecord::RecordNotUnique => e
+        # Race vs another worker (this pledge got claimed, or this txn got
+        # stamped by another pledge). Fall through and try the next pledge.
+        Rails.logger.warn("GoalPledge ##{pledge.id} match failed: #{e.class}: #{e.message}")
       end
     end
   rescue StandardError => e

config/locales/models/goal_pledge/en.yml

@@ -17,3 +17,4 @@ en:
               must_be_linked_to_goal: Pick one of the goal's linked accounts.
             currency:
               must_match_goal: Pledge currency must match the goal currency.
+          duplicate_open_pledge: You already have an open pledge for this amount on this account. Cancel or extend the existing one before recording another.

test/models/goal_pledge_test.rb

@@ -77,6 +77,35 @@ class GoalPledgeTest < ActiveSupport::TestCase
     assert @pledge.expires_at > before + 6.days
   end
 
+  test "matches? widens upper bound to expires_at after extend!" do
+    # Day 8 — past the default 5-day creation-anchored window but inside the
+    # extended expiry window. Without the widening this would be a regression
+    # of B7 (extend doesn't actually buy match runway).
+    @pledge.extend!
+    far_date = @pledge.created_at.to_date + 8.days
+    assert far_date <= @pledge.expires_at.to_date
+    entry = build_entry(account: @account, amount: -200, date: far_date)
+    assert @pledge.matches?(entry)
+  end
+
+  test "matches? rejects entries past extended expires_at" do
+    @pledge.extend!
+    far_date = @pledge.expires_at.to_date + 1.day
+    entry = build_entry(account: @account, amount: -200, date: far_date)
+    assert_not @pledge.matches?(entry)
+  end
+
+  test "duplicate open pledge for same goal+account+amount is rejected on create" do
+    dup = @goal.goal_pledges.new(account: @account, amount: @pledge.amount, currency: @goal.currency)
+    assert_not dup.valid?
+    assert dup.errors[:base].any? { |m| m.include?("open pledge") }
+  end
+
+  test "duplicate validation does not block different amounts" do
+    dup = @goal.goal_pledges.new(account: @account, amount: @pledge.amount.to_d + 1, currency: @goal.currency)
+    assert dup.valid?, dup.errors.full_messages.to_sentence
+  end
+
   test "extend! raises for non-open pledge" do
     pledge = goal_pledges(:matched_transfer)
     assert_raises(GoalPledge::NotOpenError) { pledge.extend! }