feat(simplefin): discover new bridge accounts on sync - #1877
Merged
Conversation
…ounts discovered on sync SimpleFIN is a bridge: one access URL spans every bank linked at the bridge. The prior integration modeled it like a per-bank provider — it prompted for a setup token in the Add-connection flow and never re-discovered accounts after connect, so banks added at the bridge later never appeared. This reworks it to reflect what SimpleFIN actually is. Sync-time account discovery (the real backend gap): - Provider.SyncResult gains an Accounts field. SimpleFIN populates the full deduped account set each sync; fixed-set providers (Plaid, CSV) leave it nil. - The sync engine upserts discovered accounts (metadata only, never balances) via a new UpsertAccountMetadata query before processing transactions, refreshing its account caches. Banks linked at the bridge after connect now flow in automatically. Settings-managed singleton token: - The token is pasted in Settings → Providers → SimpleFIN (side drawer), not the per-bank flow. At most one active connection per household: GetActiveConnectionByProvider resolves the singleton, so the handler creates on first claim or rotates the credential in place — re-pasting can no longer orphan accounts. - Add-connection shows SimpleFIN as a non-selectable informational row linking out to the bridge and to Settings, reinforcing the one-token mental model. Tests: unit coverage for deduped account discovery across sync windows; integration coverage for mid-sync account creation + transaction resolution onto the new account. Docs and provider rules updated. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…ect path + collapse dedup Addresses two review findings on the bridge-model PR: 1. Singleton enforced in one place. ProvidersSaveSimpleFINHandler creates or rotates the household's single SimpleFIN connection, but the generic /-/exchange-token handler still accepted provider=simplefin and called RegisterNewConnection unconditionally — a second creation path that bypassed the GetActiveConnectionByProvider singleton and left duplicate connections the Settings drawer (LIMIT 1) couldn't see or manage. Reject simplefin there (Settings is the one connect surface; relink rotates in place via reauthSimplefin). This also lets us delete the now-unreachable SimpleFIN-specific branches in ExchangeTokenHandler (institution-name override + daily-schedule assignment). 2. One dedup authority. The provider re-collected the full account set on every sync window and deduped it with a map, duplicating the engine's generic cross-result dedup. The bridge returns the full set on every window (the date range only bounds nested transactions), so capture it once and let the engine remain the single dedup point. Adds a unit test asserting /-/exchange-token rejects SimpleFIN with 400. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…er-connection + sync-discovery Reverses the singleton bridge model from this PR's earlier commits and returns SimpleFIN to the normal per-connection flow (like Plaid/Teller): each pasted setup token creates its own connection, attributed to a household member, and a household can have multiple SimpleFIN bridges. The singleton bought nothing that per-connection didn't already give — one token already covers every bank at a bridge — while blocking multi-bridge households and fighting the connections model. Kept (the genuinely valuable half): sync-time account discovery. Each SimpleFIN connection re-reads its full account set every sync and the engine upserts new accounts (metadata only, never balances) before processing transactions, so banks added at the bridge after connect flow in automatically. This is model-agnostic and works the same per connection. Reverted to pre-PR state: the Settings token drawer + singleton create-or-rotate handler, the GetActiveConnectionByProvider singleton query, the /-/exchange-token SimpleFIN reject, and the connect-flow UI (restores the inline token-paste step). Docs updated to describe per-connection + sync discovery. Per-account owner editing (for a single bridge shared across members) is tracked as a separate, provider-agnostic follow-up. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
canalesb93
changed the title
Address code-review findings on the discovery path: - engine: reset pendingAccounts + seen-set on ErrSyncRetryable so a retried sync rebuilds the discovery buffers from scratch - simplefin: restore union-across-windows account dedup (a single window can omit accounts; capture-once dropped them) - integration test: assert a pre-existing account stays exactly one row after discovery, guarding against duplication Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01EtFunz2hhqiMwRnWk6gdvE
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
A SimpleFIN bridge spans every bank you link there with one token — and that set grows as you add banks at the bridge later. Breadbox synced transactions for those banks but never re-discovered the accounts, so a bank added after connect never showed up. This fixes that, on the normal per-connection model.
What it does
provider.SyncResultgains anAccountsfield. A SimpleFIN connection returns its full current account set on every sync (captured once — the bridge lists the full set on every window). Fixed-account-set providers (Plaid, CSV) leave itnil.UpsertAccountMetadataquery — metadata only, never balances (those are owned by the balance-refresh path),connection_idset only on INSERT — and refreshes its account caches. So a bank linked at the bridge after connect appears automatically on the next sync, and its transactions resolve in that same sync instead of being dropped.Model: per-connection (not a singleton)
An earlier revision of this PR reworked SimpleFIN into a settings-managed singleton (one token, managed in Settings). That was rolled back: SimpleFIN stays a normal per-connection provider like Plaid/Teller — each token creates its own connection, attributed to a household member, and a household can run multiple SimpleFIN bridges. One token already covers every bank at a bridge, so the singleton added a constraint that blocked multi-bridge households for no benefit. The account-discovery fix above is model-agnostic and is the keeper.
Per-account owner editing (to split a single shared bridge across household members) is a separate, provider-agnostic follow-up.
Tests
TestSyncTransactions_ReturnsDiscoveredAccounts).TestSync_DiscoversNewAccountsMidSync).go build/vet ./...+ unit + integration all green on a tree merged with latestmain.Docs:
docs/simplefin-integration.md+.claude/rules/providers.mddocument the on-sync account discovery.🤖 Generated with Claude Code