mirror of
https://github.com/we-promise/sure.git
synced 2026-09-03 13:51:29 +00:00
75aa16e4e2879003b4aee3268ea8a6782eed2e4a
1002
Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
 Sure Admin (bot)
|
75aa16e4e2 |
Log async rule run failures to debug log (#3045)
* Log async rule run failures to debug log * Propagate auto-categorize provider failures |
||
|
Andrew B
|
c9fbfd9f71 |
fix(chat): make the assistant response timeout configurable (#2910)
* fix(chat): make the assistant response timeout configurable (#2893)
Self-hosted users running a local model report the chat failing with
"assistant not available" after 90 seconds even though the model
generates a reply and tokens are billed.
Three timeouts are involved and only one was configurable:
- OPENAI_REQUEST_TIMEOUT (60s) — already settable, not the blocker.
- The browser watchdog in chat_controller.js (90s) — hardcoded, and
this is what actually fires.
- Chat::UNDELIVERED_RESPONSE_TIMEOUT (60s) — a constant, so raising
the client value alone would not have helped.
The watchdog cannot be avoided by streaming here: custom
OpenAI-compatible providers route through generic_chat_response, which
forces synchronous calls, so nothing renders until the whole generation
finishes. Time-to-last-token has to beat the deadline.
Adds AI_RESPONSE_TIMEOUT (ENV > Setting > 90s default, floored at 30s),
exposed on the Self-Hosting settings page and passed to the Stimulus
controller at all three mount points — show, index and the sidebar in
the application layout, each of which declares data-controller="chat"
independently.
The server floor is derived from the same value but kept 10s below it.
report_timeout answers 200 whether or not it acted and the client only
retries on a non-ok response, so a floor at or above the client value
would let clock skew strand a pending bubble permanently.
Also guards AssistantMessage#append_text!. The watchdog runs in the web
process while the job holds its own copy of the message, so a job
finishing after the bubble was destroyed or demoted would silently
resurrect it alongside the error the user was already shown.
* fix(chat): let the watchdog retry when report_timeout declines
`report_timeout` answered 200 whether or not `handle_undelivered_response!`
acted. The Stimulus watchdog only stops retrying a URL once it sees a 2xx, so
a declined report was treated as final.
That stranded the bubble whenever the client's clock ran more than
SERVER_TIMEOUT_GRACE ahead of the server's: the watchdog posts at its own
timeout, the server sees a message younger than its floor and no-ops, and
nothing ever retries. The bubble spins forever with no error and no Retry.
Answering 409 instead lets the next 5s tick try again, so any amount of skew
costs retries rather than a stuck chat. The grace window stays as an
optimisation to keep those retries rare, not as the correctness mechanism.
* docs(chat): correct the AI_RESPONSE_TIMEOUT ordering guidance
The docs, locale string and examples all said to keep OPENAI_REQUEST_TIMEOUT at
or above AI_RESPONSE_TIMEOUT. That is backwards.
The two limits span different things. OPENAI_REQUEST_TIMEOUT bounds each HTTP
call to the model on its own; AI_RESPONSE_TIMEOUT covers the whole turn and its
clock starts when the message is queued, so it also absorbs Sidekiq queue time
and, for a tool-using turn, two model calls plus the tool run between them.
Keeping the chat timeout the larger of the two means a slow model surfaces the
specific HTTP timeout error rather than a generic "no response", and the job
stops instead of running on after the chat has given up. The shipped 60/90
defaults already had this ordering; only the guidance was wrong.
compose.example.ai.yml gets 300/660 so the Ollama example can actually complete
a tool-using turn.
* fix(chat): claim the pending bubble atomically before appending
append_text! read the row's status and then saved, leaving a window in which
the watchdog could demote the row to `failed` between the two. The late
content would then land on a bubble the user had already been told failed,
flipping it back to `complete`.
Replaces the read with a conditional UPDATE that only succeeds while the row is
still pending, so the check and the state change cannot be separated.
Uses a conditional UPDATE rather than with_lock because append_text! is called
once per chunk on the streaming path; a row lock and transaction per chunk would
be far more expensive. The claim only runs on the first append, since later ones
are no longer pending.
* test(chat): isolate AI_RESPONSE_TIMEOUT from the environment
Chat.response_timeout reads ENV ahead of Setting, so stubbing only the Setting
left the assertions at the mercy of the environment they run in. With
AI_RESPONSE_TIMEOUT=45 exported, five of these tests failed — the default,
floor and grace assertions were all silently measuring the env value.
Adds a with_setting_timeout helper that stubs the Setting and clears the
variable together, and switches the controller tests to stub
Chat.undelivered_response_timeout directly, since what they care about is the
resolved floor rather than how it was configured.
Both files now pass with or without AI_RESPONSE_TIMEOUT set.
* docs(chat): size AI_RESPONSE_TIMEOUT for chained tool calls
The guidance assumed a tool-using turn costs two model calls. #2767 landed
after this branch was opened and made tool calls iterative: `Assistant::Responder`
now loops until `iteration > max_tool_call_iterations`, so a turn runs to
1 + ASSISTANT_MAX_TOOL_CALL_ITERATIONS calls — six by default — with tool
execution in between. At the default 60s per-call timeout that is up to 360s of
model time against a 90s watchdog.
Streaming does not rescue this either. `emit(:output_text)` only fires for a
response that carries text, and tool-call-only rounds carry none, so the bubble
stays on "Thinking…" through every round regardless of provider.
Documents ASSISTANT_MAX_TOOL_CALL_ITERATIONS as the cheaper lever: dropping it to
2 halves the worst case instead of demanding a half-hour timeout, at the cost of
failing long tool chains earlier with a clear limit error. compose.example.ai.yml
now shows that combination rather than a timeout sized for six calls it never had.
* docs(chat): state the whole-turn timeout as a sum, not a maximum
The guidance said to keep AI_RESPONSE_TIMEOUT "above" or "the larger of"
OPENAI_REQUEST_TIMEOUT. That understates it: the watchdog covers the entire turn,
so the bound is
(1 + ASSISTANT_MAX_TOOL_CALL_ITERATIONS) * OPENAI_REQUEST_TIMEOUT
+ tool execution + queue wait
Merely exceeding the per-call limit can still leave the chat reporting failure
while the worker keeps going.
One phrasing was outright wrong: "keep AI_RESPONSE_TIMEOUT the largest of the
three" compared a duration against ASSISTANT_MAX_TOOL_CALL_ITERATIONS, which is a
count, not seconds.
Resizes the examples against the formula — compose.example.ai.yml 1000 -> 1200 and
the Ollama doc example 600 -> 720, both now showing the arithmetic — and states
plainly that the 90s default is sized for typical cloud latency rather than the
worst-case bound, with the formula being what matters once per-call latency
approaches the timeout.
* docs(chat): list the AI settings fields and tag the formula fence
The Settings UI walkthrough listed three of the eight fields on the AI Provider
form. JSON Mode, the three Token Budget fields and the new Chat Response Timeout
were all missing, so the timeout was only discoverable from the troubleshooting
section. Rewrites the list to follow the form's own grouping and uses the labels
the form actually renders.
Also tags the whole-turn formula fence as `text` (markdownlint MD040).
* fix(compose): forward ASSISTANT_MAX_TOOL_CALL_ITERATIONS in standard compose
This file enumerates container environment explicitly — there is no env_file — so
a variable absent from the x-rails-env anchor never reaches web or worker.
The tool-call cap was only named in a comment here, while the docs added in
|
||
|
Sure Admin (bot)
|
73d43bc1d0 |
Fix SSO JIT new family creator role (#3024)
* Fix SSO JIT new family creator role * Preserve super admin SSO creator defaults * Update new family creator role test |
||
|
Brandon
|
1973c557e5 |
fix(ai): drop empty data-driven enums from assistant function schemas (#3016)
* fix(ai): drop empty data-driven enums from assistant function schemas
Enum values in tool schemas are built from family data (account names,
categories, merchants, tags, tickers). A family with none of these gets
enum: [], which is invalid JSON Schema. OpenAI tolerates it, but strict
OpenAI-compatible providers reject the entire request, breaking chat for
fresh families until they create a tag or merchant.
Prune empty enums in build_schema, falling back to a plain string. One
choke point covers every function and both consumers: chat tool
definitions for all providers, and the /mcp endpoint's tools/list.
* fix(ai): address review feedback on enum pruning
Stop recursion at populated enum values: enum members are literal
values, not subschemas, so a literal like enum: [{ enum: [] }] must be
preserved verbatim rather than rewritten.
Also cover PREVIEW_FUNCTION_CLASSES in the registry regression test by
enabling the preview preference on the test user, with a guard assertion
so the test fails if preview functions ever silently drop out.
|
||
|
GFRandClaude Sonnet 5
|
746d56c4bd |
fix: gracefully handle invalid family timezone instead of crashing (#2821)
* fix: gracefully handle invalid family timezone instead of crashing Family#timezone is a free-text IANA zone name with no validation on write. If it becomes stale (e.g. tzdata renames a zone, like the historical Europe/Kiev -> Europe/Kyiv switch) or a migration meant to remap legacy names never ran, Localize#switch_timezone passed the raw string straight to Time.use_zone, which raises ArgumentError for any unrecognized zone. Since switch_timezone runs as an around_action on every request, this crashed the entire app for the affected family, including the login page. Now validates the zone via ActiveSupport::TimeZone[] first and falls back to the app default (logging a DebugLogEntry) instead of raising. The log write is debounced per (family, bad value) via Rails.cache (once per day) so an affected family doesn't write one DebugLogEntry row per page view indefinitely. Fixes #390 Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * fix: address review feedback on timezone fallback - Make the invalid-timezone debounce lease atomic. Rails.cache.fetch is read-then-write, not atomic, so two concurrent requests could both observe a cache miss and both log before either write landed. Rails.cache.write(unless_exist: true) maps to Redis's atomic SET NX in production, so only one request ever wins the lease. (via CodeRabbit) - Stop using "Europe/Kiev" as the invalid-timezone value in tests. Whether ActiveSupport::TimeZone still resolves that legacy alias depends on the host's installed tzdata version (tzinfo-data is Windows/JRuby-only per Gemfile), so the test's pass/fail behavior wasn't deterministic across machines/CI. Use a deliberately nonexistent name instead. (via Codex) Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * fix: validate Family#timezone on write to address root cause of #390 The previous commit made the *crash* graceful, but left the actual defect in place: nothing stopped an unrecognized IANA zone name from being written to Family#timezone in the first place (direct DB/API access, an old dump predating a tzdata rename, or a future rename of a currently-valid zone). Add a Family-level validation using the same ActiveSupport::TimeZone[] lookup Localize#resolved_timezone uses at request time, so "valid at save" and "valid when rendering" can't drift apart. Deliberately not `inclusion: { in: ActiveSupport::TimeZone.all.map(&:name) }`, matching the neighboring locale/date_format validations: verified empirically that the settings form submits `tz.tzinfo.identifier` (e.g. "America/New_York"), not `tz.name` (e.g. "Eastern Time (US & Canada)"), and those differ for all 150 zones Rails ships. An inclusion check against `.name` would have rejected every legitimate value the form submits. The validation only runs when timezone is actually being changed (if: :timezone_changed?). A family with a pre-existing bad value (the exact #390 scenario) must still be able to save unrelated changes -- otherwise this would turn a previously-harmless bad value into a blocker for any other settings update or background job touching that family's record. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com> |
||
|
Guillem Arias Fauste
|
214a139a7e |
fix(goals): stop the days-left phrase splitting across lines (#2970)
* fix(goals): stop the days-left phrase splitting across lines The goal header read "Target 700 € by 08 de Febrer de 2027 · 184 days left" as one text node, so on a phone it wrapped wherever it ran out of room — "184 days" on the first line and "left" orphaned on the second. `header_summary_parts` now returns the dot-separated segments instead of a joined string, and the view renders each as its own span. Only the segments after the first are `whitespace-nowrap`: the first carries the target and a long-format date and has to stay free to wrap, or it would overflow a narrow screen. Checked at 320px, where the first segment takes two lines and "675 days left" still moves down whole. * test(goals): pin the clock for the days-left assertion The count is `target_date - Date.current`, and the target was set from `184.days.from_now` a moment earlier — a suite crossing midnight between the two would compute 183 and fail. Wrap it in travel_to, matching the idiom in budget_category_test and family_export_test. The neighbouring 'omits days left once reached' test asserts only the segment count, which 183 and 184 satisfy equally, so it is left alone. |
||
|
Guillem Arias Fauste
|
6245b9cbd0 |
fix(pagination): stop the pager wrapping on narrow screens (#2967)
* fix(pagination): stop the pager wrapping on narrow screens
The page-number strip put the last page on a second line on a phone. The
container was a plain block, so the inline-flex links flowed as inline
content and wrapped like words. Measured on a 760-page list: the strip needs
~198px, and a 360px viewport leaves ~158px once the chevrons and the
per-page select are taken out.
The failure was not even consistent — at 402px the row wrapped, at 360px the
same markup overflowed its parent instead, because a block box inside a flex
item resolves its width differently at each size.
Make the strip a nowrap flex row, and drop the pages that aren't useful on a
phone. Small screens keep the first page, the last page, the current page and
the gaps ("1 … 5 … 760", ~148px); the full series returns from `sm` up. Short
series are left alone — Pagy only emits those when the collection has that
few pages, so hiding there would render a 3-page pager as "1 3".
`sm:` is a viewport breakpoint and the pager also lives in narrow columns on
wide screens (the account activity feed with both sidebars open), so the row
keeps `overflow-x-auto` as a backstop. At 320px, where even the reduced set
is wider than the space, it scrolls rather than wrapping.
Shared by twelve call sites, so this covers transactions, imports, rules,
account activity, statements, merchants, exports and the debug log.
* fix(pagination): mark the pages the mobile pager drops
Hiding the middle page links without saying so made the survivors read as
adjacent. On a 760-page collection sitting on page 3, Pagy emits
[1, 2, "3", 4, 5, :gap, 760] and a phone rendered "1 3 … 760" — page 2 gone
with nothing to show for it. A gapless 6-page series was worse: "1 6".
Each collapsed run now renders its own mobile-only ellipsis, unless Pagy
already put a gap beside that run, which would otherwise read as "… …".
[1, 2, "3", 4, 5, :gap, 760] -> 1 … 3 … 760
["1", 2, 3, 4, 5, 6] -> 1 … 6
["1", 2, 3, 4, 5, :gap, 760] -> 1 … 760 (Pagy's gap already covers it)
[1, :gap, 3, 4, "5", 6, 7, …] -> 1 … 5 … 760
The helper now returns a per-slot plan rather than a class, since the view
needs to know where a run starts, not just whether a slot is hidden.
Tests carry a property check — no two page numbers may survive side by side
unless they are consecutive — with a guard that fails if every pair happens to
be separated by an ellipsis, which would make the property vacuous. It was, on
the first pass.
|
||
|
Blaž Dular
|
37301e15ae |
feat(mcp): add pagination to get_tags and get_categories tools (#2492)
* feat(mcp): add pagination to get_tags and get_categories tools Both tools now accept a `page` param and return `total_results`, `total_pages`, `page`, and `page_size` — matching the pattern used by get_transactions and get_holdings. Adds a migration for composite (family_id, name) indexes on tags and categories to support efficient paginated ordering within a family scope. * fix(mcp): make page param optional in get_tags/get_categories schema and fix migration version Page defaults to 1 in call() so marking it required in the JSON schema was incorrect. Also fixes migration to use ActiveRecord::Migration[7.2] instead of [8.0]. * fix: update schema.rb with family_id+name indexes for tags and categories * fix(mcp): stable pagination sort and unique family/name indexes on tags and categories - Make family_id+name indexes unique (matches existing AR uniqueness validations) - Add id tie-breaker to alphabetically/alphabetically_by_hierarchy scopes so paginated results are deterministic * refactor(mcp): replace Pagy::Backend mixin with Pagy.new in model layer Pagy::Backend is designed for controllers; using it in plain model classes is fragile. Switch all four assistant functions to call Pagy.new(count:, page:, limit:) directly and apply offset/limit on the scope, which is the correct approach for non-controller contexts. |
||
|
Sure Admin (bot)
|
23bdad74e5 | test(i18n): stop gating CI on German coverage (#3012) | ||
|
Guillem Arias Fauste
|
344cf091e1 |
feat(auth): sign in with a passkey, without a password (#2911)
* feat(auth): sign in with a passkey, without a password Passkeys could only ever replace the TOTP code: registration required 2FA to already be on, and the WebAuthn ceremony was reachable only after User.authenticate_by had succeeded. A registered passkey can now complete sign-in on its own, from the login page. The ceremony requests userVerification: "required", so the authenticator has to confirm the person as well as the device. That makes a lone passkey two independent factors, the same bar as the password plus TOTP flow it replaces, which is why this path deliberately skips the TOTP step. A credential that can only prove presence is rejected here and still works as a second factor. Sign-in is usernameless: no email is submitted, because the browser returns the account handle with the assertion. Nothing on this path can be probed to learn whether an account exists. Registration now asks for a discoverable credential with residentKey: "preferred" so the key is offered by the picker, while authenticators without a free resident-key slot still register as a second factor. Where conditional mediation is available, saved passkeys appear in the email field's autofill menu; everywhere else the button covers it. The automatic challenge request that conditional mediation makes on every page load gets its own looser Rack::Attack budget, so ordinary page views can no longer exhaust the limit that protects the MFA endpoints. Set AUTH_PASSKEY_LOGIN_ENABLED=false to keep passkeys as a second factor only. Passkey sign-in follows the same policy as local login, so it stays closed to regular users when AUTH_LOCAL_LOGIN_ENABLED is false. * refactor(auth): group the passkey button with the other sign-in methods It sat directly under the password fields, so the forgot-password link split it from the identical SSO buttons. It is an alternative to the credential form rather than part of it. * fix(auth): close the passkey challenge races and document the upgrade Three review passes converged on the conditional-mediation flow. The AbortController was created after `isConditionalMediationAvailable()` resolved, so a button click or a Turbo disconnect landing in that window found nothing to abort: the conditional task carried on, re-minted the challenge, and the assertion the user was about to produce verified against a challenge the server had already replaced. It is created before the first await now, and held in a local, because `abortConditionalMediation()` nulls the field. Checking that one signal after each await covers both triggers, so no separate connected flag is needed. The same symptom had a second cause nobody flagged: `authenticate()` was not re-entrant. A double-click minted a fresh challenge under an open authenticator prompt and rejected a perfectly valid passkey, with no race window at all — and it was live on the MFA step-up too, which shares the method. The conditional catch was silent for every failure, including a rejected assertion the user had deliberately chosen from the autofill menu. Splitting the try draws the line where it belongs: silence before the user has been asked anything, feedback once they have picked a passkey. Filtering on `error.name` cannot draw it, since `fetchOptions` and `verifyCredential` both raise a plain Error. Also documents the upgrade: passwordless is on by default and applies to already-registered credentials, so a passkey added purely as a second factor can now sign its owner in alone. Nothing in the schema marks a credential discoverable — the authenticator decides — and the opt-out is instance-wide. The invitation test is a guard, not coverage for this change. The pending token lives in the Rack session and `complete_sign_in` reads it right after creating the session, so a `reset_session` dropped in between strands the invitee in their own family, silently and with every existing test still green. * fix(auth): cancel the in-flight conditional options request Aborting the conditional flow did not cancel its options request, because `fetchOptions` never received the signal. A click landing while that POST was in flight left it to finish, and its response could apply last. The challenge rides in the session cookie, so "the server wrote it" only counts if the Set-Cookie reaches the browser. Threading the signal means an aborted request's response is discarded, which closes the window without needing the server to hold two challenges open. Also drops the absolute claim about which existing credentials gain passwordless sign-in. `residentKey: "preferred"` is a request an authenticator may decline, and nothing records what it decided, so the honest statement is that password managers and platform authenticators generally store discoverable credentials rather than always. |
||
|
Faldy Ikhwan Fadila
|
792047b82e |
feat(yahoo_finance): add Indonesia Stock Exchange (XIDX) support (#3000)
Add JKT → XIDX exchange MIC mapping, .JK symbol suffix normalization, IDR default currency, and ID country code for Jakarta exchange. Yahoo Finance returns Indonesian stocks (e.g. BBCA.JK) with exchange code 'JKT'. Without this mapping, the provider cannot resolve the exchange to the XIDX MIC already defined in config/exchanges.yml, and normalize_symbol cannot append the .JK suffix for price lookups. Tested manually: Yahoo Finance search and chart endpoints return valid results for IDX tickers (BBCA.JK, currency=IDR, timezone=WIB). |
||
|
Pedro Santos
|
7c56c8e2e8 |
Enable Banking: progressive date_from fallback on WRONG_TRANSACTIONS_PERIOD (#2992)
* Enable Banking: progressive date_from fallback on WRONG_TRANSACTIONS_PERIOD Some ASPSPs (e.g. Santander Totta and Activo Bank in PT) reject the transactions window with 422 WRONG_TRANSACTIONS_PERIOD but do NOT return a corrected date_from in the payload. The existing single-shot retry only fires when the API supplies detail.date_from, so for these banks the retry was skipped and the error surfaced as the generic "communication error"; transactions never synced even though the connection and session were valid. This adds a bounded, progressive fallback: when the period is rejected and no corrected date is available, retry with progressively shorter windows (89 -> 60 -> 30 days). The ASPSP-suggested date is still preferred on the first retry, so existing behaviour is preserved. The step-down only moves the window forward, guaranteeing progress and avoiding an infinite loop. Verified on a live instance: Activo Bank went from 0 to 82 transactions imported once the fallback kicked in. Refs #2989 Signed-off-by: Pedro Santos <pedro_santos@outlook.pt> * Address review: forward-only progress across all fallback windows - Accept the ASPSP-suggested corrected date only when it moves the window forward (current is nil or corrected > current), preserving the forward-only retry bound (CodeRabbit). - Skip fallback windows that are not newer than the current date_from and pick the first that advances, instead of bailing out on a stale first candidate. Fixes the case where an initial/user lookback (e.g. 45d) is newer than the first window (89d) but the bank caps at 30d (Codex). Signed-off-by: Pedro Santos <pedro_santos@outlook.pt> * Add tests for progressive transactions date_from fallback Covers the two cases the previous single-shot retry missed: - WRONG_TRANSACTIONS_PERIOD without a corrected date_from -> falls back to the first shorter window (89 days). - An initial lookback newer than the leading windows (45d) -> skips the 89/60 windows and retries with the first that advances (30 days). Signed-off-by: Pedro Santos <pedro_santos@outlook.pt> --------- Signed-off-by: Pedro Santos <pedro_santos@outlook.pt> |
||
|
|
176bb508e4 |
feat(i18n): complete the German locale (#2847)
* fix(i18n): replace hardcoded UI strings with translation keys
Fourteen views printed English text directly instead of going through
t(). Those strings could not be translated in any locale, so French and
Italian users saw English there as well, even though both locales are
otherwise complete.
The Mint import form was the worst of them with thirteen hardcoded
strings, including the intro text and the submit button. The valuation
confirmation dialog passed "set" and "update" into an interpolation as
bare English words, so the verb stayed English whatever the locale.
Adds 35 keys to en and de.
* feat(i18n): complete the German locale
German covered 55 percent of the English keys, against 97 for French and
90 for Italian. The gaps ran through settings, imports, goals, insights
and every provider integration, so the interface kept switching language
in the middle of a page.
This fills 3170 keys. Terminology follows what the existing German files
already used: Konto for account, Händler for merchant, Familie for
family. Provider names, ticker symbols and the IBKR flex query field
names stay in English, because that is what users see in those services
themselves.
Interpolation variables were checked against the English source for every
translated key.
* fix(i18n): use one form of address across the German locale
The German files mixed the informal du and the formal Sie, sometimes
within the same dialog. Most of the existing strings already used du, so
the remaining 39 now follow suit.
Several of those sentences also read like form letters. "Bitte versuchen
Sie es erneut" is now "Versuch es noch einmal".
* fix(i18n): correct German names for account types
Depository accounts were called Bargeld, meaning cash, in the views,
while the model called them Bankkonto. The icon is a bank building and
the type covers checking, savings, CDs and money market accounts, so
Bankkonto fits in both places.
Investment accounts were called Investition. In German that word means
the act of investing rather than the account that holds it, so the label
read as if the app were tracking transactions instead of a portfolio.
Investment is the term people use for the account itself.
* fix(i18n): pass the account name to the SimpleFIN dialog title
The key simplefin_items.select_existing_account.title already existed and
interpolates %{account_name}. Calling t(".title") without it raised
I18n::MissingInterpolationArgument before the dialog rendered.
The controller sets @account for this action, so the title can use it and
name the account being linked.
* fix(i18n): finish the German consistency pass
Follow-up to review feedback on this branch.
Quotation marks: 32 strings opened with the German „ but closed with the
straight ASCII quote, which renders as broken punctuation. They now close
with “.
Address form: 311 strings still used the formal Sie, mostly in the
provider integrations that the earlier commit left alone. Mixing both
registers inside one screen was worse than either choice on its own, so
they now use du like the rest.
Wording: the securities breadcrumb said Sicherheit, which means safety
rather than the financial instruments, and now reads Wertpapiere. The
Redbark setup failure had a typo in the imperative. An invitation message
broke off mid-sentence without naming the household. SimpleFin is spelled
SimpleFIN throughout, matching the provider.
* fix(i18n): repair verb forms left by the address change
The previous commit swapped Sie for du with a rule set, which changed the
pronoun but left the verb in its formal form. That produced sentences like
"die du importieren möchten" instead of "möchtest", and "bevor du Brex-Konten
verknüpfen können" instead of "kannst".
Also fixes lowercase deine at the start of a sentence, which came from
replacing Ihre without looking at position, and the last Sie forms in the
Sophtron block scalar and the chat demo banner.
41 strings in total.
* fix(i18n): keep straight quotes inside the HTML attribute
My quote sweep replaced the closing ASCII quote after an opening „ with “,
and in treat_as_html that ASCII quote was the delimiter of class="font-medium".
A curly quote cannot delimit an HTML attribute, so the span rendered with a
broken class attribute rather than merely looking odd.
The attribute now uses straight quotes and only the surrounding citation marks
are typographic. Checked every German value containing markup for the same
mistake; this was the only one.
* fix(i18n): address findings from an independent review pass
Three passes went over this branch: the two bots on the PR, plus a
separate audit of the German values, the t() calls and the YAML structure.
This closes what they found.
Two defects were invisible until then.
settings.providers.status carried the key "false" instead of "off". The
English file writes `off:` unquoted, which YAML reads as the boolean false,
and I had copied the parsed name rather than the intended one. The code
looks up :off, so the status pill for unconfigured providers found nothing
at all. Both files now quote the key, which repairs English too.
The valuation confirmation dialog built a sentence that only works in
English. There, Set and Update open the sentence as imperatives; my German
values were participles, and the partial capitalises them at the start, so
it read "Gesetzt Kontostand am ...". The values are now "Neu:" and
"Geändert:", which carry a sentence opening. The date runs through l()
instead of strftime with a US format, so the German dialog no longer says
"July 30, 2026".
Grammar left behind by the earlier rule-based Sie/du change: 33 strings had
lost the verb particle ("Richte X." instead of "Richte X ein."), six kept an
infinitive after "bevor du", and one turned a pronoun referring to merchants
into a form of address, which reversed the meaning.
Placeholders that had been hardcoded: seven %{moniker}, where users can
choose Gruppe but the German text said Familie regardless, one
%{product_name}, and %{message} in an authorization error that swallowed
the cause.
Structure: entries.selection_bar.edit gave way to the long-standing
transactions.selection_bar.edit, which exists in 15 languages and would
otherwise have fallen back to English for all of them. simplefin_items
gained the missing check_provider_health in en and de, and two buttons
there now use keys that already existed instead of English literals.
Two notes on scope and wording, since this branch has grown well past its
original shape.
The PR began as pure gap-filling and deliberately left existing
translations untouched. The address change and these grammar fixes reach
into them because a dialog that switches register mid-screen is worse than
one that stays formal throughout. Existing values are roughly nine percent
of the diff and sit in separate commits, so they can be dropped on their
own if you would rather keep them out.
German terms stay close to the length of the English source. The buttons
size to their content, but DS::Buttonish sets whitespace-nowrap, so a
longer word widens the button instead of wrapping and pushes its
neighbours around in button rows, table cells and the sidebar. That is why
this locale says Sync rather than Synchronisieren, Setup rather than
Einrichtung, API-Key rather than API-Schlüssel. Where surrounding files use
the longer form, the difference is deliberate.
* fix(i18n): keep flash messages within the notification clamp
The notification partial renders flash messages at md:max-w-80 with
line-clamp-3, so anything past roughly 130 characters is cut off and
survives only in the title attribute. Ten German messages had crossed that
line where the English stayed under it, the longest at 205 characters.
They now say the same thing with fewer detours. "Dieser Vorgang lässt sich
gerade nicht ändern, sein Job steht möglicherweise noch in der Warteschlange
oder läuft" became "Der Job läuft oder wartet noch".
Two of them carried the same text under different keys in the hosting
settings, the older wording at 175 characters. Both read the same now.
Also from review: the region message mixed grammatical cases ("Für diese
Region/Land"), one invitation string still used Sie, and four Sophtron
messages pointed users at an API key configuration when that provider uses
a user ID and an access key.
* test(i18n): cover German locale completeness
* feat(i18n): translate the keys that landed while this PR was open
Upstream added the Plan page, insight acknowledgements and the SnapTrade
account type picker after this branch was cut. The coverage test from
|
||
|
Brandon WolfandClaude Fable 5
|
00b7252fbf |
feat(mcp): Add MCP budget update tool (#2908)
* feat(mcp): Add MCP budget update tool Adds an update_budget assistant/MCP function so AI assistants can write monthly budgets: total budgeted spending, expected income, and per-category allocations in one transactional call. - Month resolution and slug format mirror get_budget (YYYY-MM or MMM-YYYY, custom month start respected); targeting a valid month with no budget row bootstraps it via Budget.find_or_bootstrap, same as the budgets UI. - Category allocations accept an exact (case-insensitive) name or id and go through BudgetCategory#update_budgeted_spending!, so subcategory writes keep the parent total in sync. - All writes in one call share a transaction: an invalid category rolls back a totals change from the same call. - Family-scoped like the budgets UI; amounts validated non-negative. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * refactor(mcp): harden update_budget per review feedback - Extract shared month resolution into Assistant::Function::MonthResolvable so get_budget and update_budget can't drift on custom month starts - Run budget bootstrap inside the update transaction so a failed entry no longer leaves a newly created budget behind - Apply explicit parent amounts after subcategory syncs so results don't depend on the caller's array order - Reject non-finite amounts (NaN/Infinity) - Explain the synthetic Uncategorized bucket instead of a generic category-not-found error Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com> |
||
|
Victor Dusart
|
c8252ed15e |
feat(icons): add search field for icon selection (#2862)
* feat(icons): add search field for icon selection * fix(icons): address review suggestions on the icon picker * fix(goals): prevent the icon picker popover from collapsing `updatePopupPosition` sets `bottom: 0px` when the popover would run past the fold, but never clears Tailwind's `top-full`. With both offsets set and `height: auto`, CSS derives the height from the offsets instead of the content, collapsing the popover to 26px. This PR is what makes it reachable: the search row (42px plus an 8px gap) and the grid going max-h-40 -> max-h-52 grow the popover ~98px, moving the overflow trigger point far enough to hit standalone /goals/new at 1280x800. `h-fit` makes the height non-auto, so the over-constraint resolves by ignoring `bottom`, which is why categories, already carrying it, was never affected. |
||
|
packetsnscripts
|
babb039ad1 |
Fix wise imports only 90 days history on initial setup (#2998)
* Wise connection get full transaction history and adjust for fees * undo devcontainers change * address comments in PR |
||
|
Guillem Arias Fauste
|
f1ddbcd1b5 |
fix(goals): recover goals left invalid by account deletion (#2964)
Deleting an account destroys its `goal_accounts` rows (Account has_many :goal_accounts, dependent: :destroy). Any goal funded only by that account survives with zero links and permanently fails `must_have_at_least_one_linked_account`. Two bugs made that state a dead end. `#update` saved the attributes before attaching the submitted accounts, so validation ran against the goal's old (empty) link set and raised before the new links were applied. Editing was the only route back to a valid goal, and it always returned 422. Assign, sync the links, then persist once — one save over the fully assembled goal validates what the user actually submitted. `perform_transition!` discarded the return value of AASM's bang event. AASM returns false rather than raising when the save that persists the new state fails validation, so an invalid goal flashed "Goal archived." while the state never moved. Check the result and surface the validation error instead. Neither fix changes behaviour for a valid goal: the bang events return true on success, and the update path persists the same attributes and links as before. This does not change what happens to a goal when its account is deleted — whether the goal should follow the account, block the deletion, or be surfaced as needing attention is a product decision left open. It only makes the resulting state recoverable and stops the UI reporting success when nothing happened. |
||
|
Guillem Arias Fauste
|
2bcef99441 |
fix(insights): drop the dismiss toast and fix both empty states (#2965)
* fix(insights): drop the dismiss toast and fix both empty states Three problems with acknowledging an insight, all in the turbo-stream path. Every dismissal appended an undo toast to the notification tray. Acknowledging already means "hidden until these numbers change" — GenerateInsightsJob resurfaces a row whose metadata moves materially, and 6 of 8 generators scope `dedup_key` to a month — so a toast interrupting the flow bought little. Removed, along with the now-orphaned `_undo_toast` partial and its two locale keys. Dismissing the last insight left the dashboard widget on screen: the stream re-rendered the well unconditionally, so the section shell stayed with its header above an empty box until a reload. A full render already drops it (PagesController#insights_feed_section sets `visible: @feed_insights.any?`); the stream now removes the whole section to match, targeted by `[data-section-key='insights_feed']` because the shared dashboard loop emits no id on the section element. Dismissing the last insight on /insights left a blank page. The card left via `turbo_stream.remove`, which emptied #insights-list without re-rendering the partial that owns the empty state, so "No insights yet" only appeared after a reload. The list is now replaced rather than the card removed — the same thing unacknowledge already did. InsightsController#unacknowledge, its route and Insight#unacknowledge! are kept and still work; only the toast that reached them is gone, so undo can be re-wired to a different surface without resurrecting them. * fix(insights): announce the dismissal now the toast is gone Removing the undo toast took the only `role=status` element with it, and the stream also replaces the list containing the "Got it" control the user just activated — so a screen-reader or keyboard user was left with no confirmation that anything happened. Add a shared, visually hidden live region to the notification tray and update it from the acknowledge stream. It sits outside every stream target and is rendered with the page, which matters: a live region that arrives together with its own content is not announced. Updated rather than appended, so messages replace instead of piling up, and it stays empty (and free) until something uses it. This is a general primitive, not an insights one — the tray already holds `#sync-toast` and `#cta` as stable stream targets, and any flow that changes the page without leaving something on screen to read can use it. Verified in a browser: after dismissing, the region reads "Insight dismissed" at 1x1px with `clip: rect(0,0,0,0)` — announced, invisible. |
||
|
FalloutmanandJustin
|
fc130a1957 |
fix: add Schwab to SimpleFIN total-basis cost_basis allowlist (#2986)
Charles Schwab's SimpleFIN feed reports `cost_basis` as the total position cost rather than per-share, violating the spec in the same way Vanguard (#1182) and Fidelity (#1718) already do. Schwab wasn't on the TOTAL_BASIS_INSTITUTIONS allowlist, so the raw total was stored directly into holdings.cost_basis and treated as per-share downstream. Holding#calculate_trend multiplies avg_cost by qty again when reconstructing original cost, so an unadjusted total gets squared by share count — a $46,950 position with a true +55.7% gain rendered as -99.8% / -$19.6M "return" on the dashboard and per-account holdings views. Picks up where #2626 left off: adds the allowlist entry, fixes the now-outdated compliant-institution test case it broke, and adds a dedicated regression test using real observed Schwab payload values. Fixes #2626 Co-authored-by: Justin <justin@local> |
||
|
Guillem Arias Fauste
|
440e04b942 |
fix(insights): blur amounts in insight prose under privacy mode (#2865)
Insight titles and bodies are stored as finished prose with the amounts already interpolated (by the i18n template or the LLM writer), so the dashboard feed and insight cards rendered raw figures even when the hide-numbers toggle was active — only the right-aligned key figure was tagged privacy-sensitive. Add InsightsHelper#insight_privacy_text, which wraps each numeric fragment (currency amounts, percentages, bare counts, including suffix-currency and no-break-space locale formats) in a privacy-sensitive span at render time, and use it for the title and body in both the dashboard insights feed and the insight card. The sentence stays readable while privacy mode blurs the numbers. The helper splits the raw text before escaping and reassembles it with safe_join, so HTML in stored prose is still escaped and digit-bearing entities like ' are never mangled by the number regex. |
||
|
Kenrick Tandrian
|
6148cd4639 |
fix(pages): set breadcrumbs for changelog and feedback pages (#2889)
* fix(app): set breadcrumbs for changelog and feedback pages * feat(test): add test to assert breadcrumbs * fix(test): remove changes * feat(app): update breadcrumbs to use semantic nav element * feat(test): add breadcrumb assertions to changelog and feedback pages * fix(app): replace breadcrumb nav element with div containing data-breadcrumbs attribute |
||
|
William Wei MingandCursor
|
62fd47def9 |
Add “Is not equal to” operator for transaction amount rules (#2922)
* Add not-equal operator for transaction amount rules Enable excluding a specific amount in rule conditions without needing paired greater/less than workarounds (#2882). Co-authored-by: Cursor <cursoragent@cursor.com> * Strengthen amount not-equal absolute-value coverage Include a -100 transaction so != 100 proves both signed amounts are excluded. Co-authored-by: Cursor <cursoragent@cursor.com> --------- Co-authored-by: Cursor <cursoragent@cursor.com> |
||
|
William Wei MingandCursor
|
35c0b08f11 |
Add tags support for transfer transactions (#2921)
* Add tags support for transfer transactions Expose TagSelect on transfer create/edit so users can classify fund movements; apply the same family-scoped tags to both sides. Co-authored-by: Cursor <cursoragent@cursor.com> * Require annotate permission on both transfer sides for tags Prevent tagging a read-only destination transaction when the user only has write access on the outflow account. Co-authored-by: Cursor <cursoragent@cursor.com> * Restore transfer tag selections on create form errors * Localize transfer create validation error messages --------- Co-authored-by: Cursor <cursoragent@cursor.com> |
||
|
sure-admin
|
f4ace12177 | Allow PWA assets without forgery checks | ||
|
|
521946b9d6 |
fix(messages): handle chat not found during message creation (#2896)
* fix(messages): handle chat not found during message creation * test(messages): cover missing chat race --------- Co-authored-by: sentry[bot] <39604003+sentry[bot]@users.noreply.github.com> Co-authored-by: sure-admin <sure-admin@splashblot.com> |
||
|
UnamedRusandClaude Opus 4.8
|
8c52e20906 |
fix(binance): correct base_url for futures api endpoint, start_time param (#2839)
* Update binance.rb Signed-off-by: UnamedRus <UnamedRus@users.noreply.github.com> * Update binance.rb Signed-off-by: UnamedRus <UnamedRus@users.noreply.github.com> * Update binance.rb Signed-off-by: UnamedRus <UnamedRus@users.noreply.github.com> * Fix parameter naming for get_spot_trades and get_futures_trades Signed-off-by: UnamedRus <UnamedRus@users.noreply.github.com> * Update processor_test.rb Signed-off-by: UnamedRus <UnamedRus@users.noreply.github.com> * reset startTime after fromId * Fix Binance trade sync param conflict with windowed initial fetch Binance rejects fromId combined with startTime/endTime, and an unbounded startTime (default sync ~1yr) exceeds the window cap (spot 24h, futures 7d), so the initial sync could fail or miss trades. Split fetch_new_trades into two non-mixing paths: - incremental (cached trades): fromId-only pagination - initial sync: walk forward in fixed windows (24h spot / 7d futures), clamped to the 6-month futures lookback Add endTime param to get_spot_trades/get_futures_trades. Add tests covering multi-window initial sync and multi-page fromId pagination. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> --------- Signed-off-by: UnamedRus <UnamedRus@users.noreply.github.com> Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com> |
||
|
|
efb7cc3935 |
Tooling for the wealth + tax agent harness (#2848)
* Expose the Statement Vault to external agents over MCP A user wants to manage patrimonial history — a document-backed record of a family's wealth where every figure traces back to the statement it came from — by pointing an external agent harness at Sure. That model belongs in the harness, not in Sure: it needs numbered build deltas, golden tests and closed periods that a mutable Postgres row cannot provide. What Sure was missing was the seam. The Statement Vault already does most of the work — original bytes retained, SHA-256 dedup, period detection, account matching with a confidence score, reconciliation against ledger balances, and a month-by-month coverage map — but it is reachable only from the web UI. An agent could not archive a document, cite one, or check for gaps. Adds five preview MCP tools over what already exists, plus a citation grammar for values the agent writes: - upload_account_statement, list_account_statements, get_account_statement, get_statement_coverage - record_valuation, whose source citation is parsed rather than trusted: ["estimated: "] citation [" (grade: A|B|C)"]. An uncited or free-styled value is rejected at the write boundary instead of landing in the ledger looking authoritative. link and reject are deliberately not exposed. Attaching a statement to an account is the human's decision, and the vault UI is where it is made; the agent reports the suggested match and stops there. Assistant.function_classes now takes a user so preview tools stay out of the default surface. They are hidden from tools/list and not callable by name without the preference enabled, and the vault tools re-check the manager role and per-account permissions, since MCP calls never pass through a controller. Docs: the blueprint this implements, and a guide covering which side owns which layer, the vocabulary map between the two, the monthly runbook, and the gaps (non-user holders, non-statement documents, one value per date). No migrations, no API endpoints, no UI. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01JFDp9HhXDeswadu4cxFojn * Address review feedback on the vault MCP tools Two non-blocking items from the review pass: Document why get_statement_coverage reads through accessible_by rather than writable_by. It reports which documents exist and writes nothing, so read access is the right bar — and tightening it would hide coverage gaps from people who can already see the figures those gaps sit behind. The comment exists so a future refactor doesn't "fix" it. Close the acknowledged verification gap with tests rather than a one-off manual check. The review noted that nothing proved a real vault payload serializes cleanly out through tools/call — vault responses are richer than the other tools' output, with nested account hashes, decimal balances, dates and a compacted hash. Two integration tests now drive the real /mcp endpoint end to end against a real AccountStatement: one listing it, one uploading bytes and reading back the SHA-256. Permanent regression coverage instead of a smoke test someone has to remember to repeat. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01JFDp9HhXDeswadu4cxFojn * docs(llm-guides): replace patrimonial blueprint with its final revision Swap the embedded early draft for the authoritative final revision of the wealth + tax modelling blueprint (MIT © 2026 diegomarino): - rename the domain vocabulary: patrimonial -> wealth, fiscal -> tax (tax_data/, the tax layer, tax_runner) - add §9.5 (the intel file: shape, generation, and the capture loop) - tighten worked examples down to placeholders - add the MIT header; keep the in-repo NOTE block (adapted to the new vocabulary) and the filename untouched so cross-links don't break * docs(llm-guides): align agent-harness guide with blueprint + fix reconcile semantics Follow the blueprint rename (patrimonial -> wealth, fiscal -> tax, fiscal_data/ -> tax_data/, "Phase 7 (fiscal layer)" -> "(the tax layer)") so the two docs stop disagreeing on vocabulary. Correct the reconciliation mapping, which conflated two different invariants: - blueprint reconcile-or-abort (§7 pass 3) is parse-integrity (parsed parts == the document's own printed total); Sure's reconciliation_checks is ledger agreement (statement balances vs the ledger). Sure has no parse-integrity check and never aborts. - opening_balance / closing_balance are user-entered, not auto-extracted, so over MCP reconciliation is "unavailable" until a human fills them. - tolerance differs: blueprint 1.00/account-period vs Sure's fixed 0.01. State in the ownership table, the invariants section, the vocabulary map and the monthly runbook that parse-integrity and the abort belong to the harness extractor. * Correct the vault tools' reconciliation claims and citation parsing Review findings from @diegomarino, all verified against the code before changing anything. The reconciliation claim was the serious one. get_account_statement told agents the checks were "the trustworthy part" and returned "the balances read off it" — but nothing reads balances off a document. MetadataDetector never touches them and create_from_prepared_upload! never sets them; they are user-editable fields in the Statement Vault UI. So a statement archived over MCP always came back with an empty check list, which an agent could easily read as "the document agrees with the ledger" when it means "nobody has entered the figures". The description now says so, and the payload carries a reconciliation_note spelling it out for anything reading only the JSON. Also noted that these checks are ledger agreement, not parse integrity: nothing here verifies a document's parts sum to its printed total. Provenance::Citation had two patterns disagreeing about spacing. GRADE_SUFFIX allowed "(grade:A)" but FORMAT required exactly one space, so that citation passed the pre-check and then parsed as ungraded with the grade swallowed into the text — silently discarding the reliability the caller supplied, which is the one thing this parser exists to prevent. list_account_statements downcases content_sha256 before querying. The column is constrained to lowercase hex, so uppercase input could never match, and an agent would read the empty result as "not archived" and upload a duplicate. Its period filters are renamed overlapping_from / overlapping_until, since they match on overlap and the old names claimed otherwise to anyone reading the schema without the descriptions. has_more now explains that there is no cursor and the way forward is a bigger limit or narrower filters. record_valuation no longer overwrites the entry's notes. Re-recording a date would destroy a note a person had written there. Nothing is removed now: an identical citation is a no-op, a changed one is appended, and the trail of what was cited when survives. Detecting "did this tool write that line?" is not possible — almost any prose parses as a valid ungraded citation — so the code does not guess. Minor: accept urlsafe base64 on upload, and explain in the code why record_valuation checks the account ACL rather than the vault manager role, so nobody "tightens" it into the wrong permission later. Tests cover each: the grade-spacing cases both ways, uppercase SHA lookup, overlap window boundaries, note preservation and no-stacking, the unavailable reconciliation note appearing and disappearing, and — per the review — that the download URL's signed id actually expires, rather than trusting the description's claim. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01JFDp9HhXDeswadu4cxFojn * Repair a bad merge in the MCP controller test The merge of main spliced the incoming `tools/call executes update_transaction` test into the middle of the upload round-trip test, before its closing `end`. That left the file one `end` short, so it did not parse — taking out both `ci / lint` (Lint/Syntax) and `ci / test_unit` (the whole file failed to load). Restores the missing `end`. Both tests are kept as their authors wrote them; nothing else changes. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01JFDp9HhXDeswadu4cxFojn * docs: use wealth history wording (#2885) * Stop the vault tools promising verification they don't perform Three findings from the automated review passes, all confirmed against the code before changing anything. The download URL was dead on arrival for the caller it was built for. Sure serves stored files through Active Storage controllers that config/initializers/active_storage_authorization.rb gates on `viewable_by?(Current.user)` — a signed-in browser session. An MCP client has a bearer token and no session, so following the URL would have redirected to sign-in. Removed it rather than leaving a link that cannot work, and the description now points at search_family_files or the vault UI. Coverage called a month `covered` when a document merely existed. An unreconciled statement is not mismatched, so it took the `covered` branch, and the payload carried nothing to correct the reading — the same "advertised verification that never happened" bug fixed last round in get_account_statement, in a second place. Months now carry their own reconciliation_status, and the description says covered means presence, not agreement. Listing filtered visibility after limiting. Beyond underfilling a page, with no cursor and a 100-row cap an accessible statement behind enough newer invisible ones was unreachable. Visibility now lives in the query, mirroring viewable_by? for a statement manager. Also: rescue unexpected upload failures into a tool error instead of a raw exception string, derive the documented size limit from MAX_FILE_SIZE, list every coverage status in mcp.md, and cover the failed-reconciliation and base64-normalisation branches. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01JFDp9HhXDeswadu4cxFojn * Keep storage exception detail out of the MCP response The upload_failed message interpolated the exception text, which crosses out to an external agent. A storage failure can carry bucket names, object keys, paths or request details, so the agent now gets a fixed message and the exception stays in the server log. The test asserts the absence of detail rather than pinning the leaked string into the contract. Also fixes a test that did not test what it claimed: the urlsafe-base64 case used a fixture encoding to plain base64, so it exercised the padding branch and never the "-_" translation. It now uses content whose encoding contains both characters and asserts that up front. Renames "rejects content that decodes to zero bytes" to "rejects blank content", which is what it actually covers — Base64.strict_encode64("") is "", which is blank and returns before the decoder runs, so invalid_content is correct and empty_file is not reachable from this path. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01JFDp9HhXDeswadu4cxFojn * docs: align wealth blueprint review feedback * Correct the harness runbook: parse before publishing The guide told an implementer to archive each document to Sure first and work from there. That strands them. Sure never returns a document's bytes over MCP — Active Storage serves stored files only to a signed-in browser session — and there is no text fallback either, because statements archived through upload_account_statement never enter the vector store, so search_family_files cannot see them. A statement in Sure is metadata to an agent and nothing more. That blocks exactly three blueprint steps, all of them operating on bank and broker statements: the extractors, the parts-vs-printed-total check, and the glyph decoder. Everything else it parses — tax returns, capital accounts, annual accounts — the harness already holds locally. So the order inverts: the harness ingests into its own vault, extracts there with the whole file in reach, and publishes to Sure afterwards. This restores principle 8 rather than bending it — the recurring pipeline reads from the canonical store, and treating Sure as canonical forced a re-fetch the architecture never sanctioned. Both sides hash the same bytes, so the SHA-256 verifies Sure holds the identical document without moving it. Writes down the two consequences: a statement uploaded straight into Sure's UI can be known but never parsed (reliability C or PENDING until a copy reaches the harness), and neither vault backs up the other. Also drops a stale tools-table row still advertising the 15-minute download URL removed earlier, corrects get_account_statement's description where it suggested search_family_files as a fallback it cannot be, and disambiguates "the vault" in the MCP tool table, which is what misled me in the first place. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01JFDp9HhXDeswadu4cxFojn --------- Signed-off-by: Juan José Mata <juanjo.mata@gmail.com> Co-authored-by: Claude <noreply@anthropic.com> Co-authored-by: diegomarino <diegomarino@users.noreply.github.com> Co-authored-by: Sure Admin (bot) <sure-admin@splashblot.com> |
||
|
William Wei MingandCursor
|
af2ddca4ce |
Preload transfer counterparty associations on transactions index (#2819)
* fix(ci): skip scheduled preview cleanup on forks Only run the hourly Cloudflare preview cleanup on we-promise/sure, where the required secrets exist. * Preload transfer counterparty associations on transactions index Transfer#categorizable? walks inflow_transaction.entry.account during list render, which N+1'd transactions, entries, and accounts per transfer row. Co-authored-by: Cursor <cursoragent@cursor.com> * Assert transfer rows render in transactions index N+1 test * Broaden transactions index N+1 SQL matchers for lazy loads * Drop unused outflow transfer preloads on transactions index * Treat only equality SQL lookups as N+1 in index test --------- Co-authored-by: Cursor <cursoragent@cursor.com> |
||
|
Maurício Pólvora
|
93ba2b8c08 |
Fix chained assistant tool calls (#2767)
* Fix chained assistant tool calls * Preserve chained tool context |
||
|
AtlasandJuan José Mata
|
53a87a7645 |
fix(mcp): assign OAuth clients read_write scope (#2884)
* fix(mcp): assign OAuth clients read_write scope * fix(mcp): default registered OAuth scope --------- Co-authored-by: Juan José Mata <juanjo.mata@gmail.com> |
||
|
Guillem Arias Fauste
|
23e7db4f4d |
feat(transactions): surface recently-used categories in the category picker (#2829)
* fix(transactions): don't crash the rule-prompt flash when clearing a category needs_rule_notification? only checked saved_change_to_category_id? and eligible_for_category_rule?, neither of which accounts for category_id being nil. Clearing a category (Clear category / entryable_attributes category_id: nil) satisfies both, so the caller went on to read transaction.category.name against a nil category and crashed. A rule prompt only makes sense when a category was assigned, not cleared, so bail out early when there's no category to build a rule around. * feat(transactions): surface recently-used categories in the category picker Reframes the recency-vs-muscle-memory question as additive, not either/or: a small "Recent" section pinned above the existing alphabetical list, which stays exactly where it always was below it. Precedent for reordering the primary list by frequency (Office's old adaptive menus, browser-history-style resorting) is a well-known anti-pattern — position drifts under the user's hand. Every picker that does recency well (VS Code's command palette, Spotify, Slack's emoji picker) adds a small separate recent cluster instead. - Category#last_used_at, touched only in TransactionCategoriesController#update — the one place a category is actually hand-picked by a person, as opposed to a rule or import auto-assigning one. - Category.recently_used_for(family:, excluding:, limit:) batches the family-scoped query; dropdowns_controller excludes the already- selected category from the Recent section since it's already pinned to the top of the main list. - "Recent" hides itself the moment a search query is typed — it's a pre-search shortcut, not a second copy of search results. Its rows are force-hidden (not just filtered) so keyboard nav can't land on a row that's invisible only because its ancestor section is hidden. * fix(categories): address review feedback on recent-categories picker - Track last_used_at from every manual assignment path (transaction edit form, categorization wizard bulk-update, create-and-assign), not just the category-picker endpoint. Centralized as Transaction#record_category_usage!, called explicitly from each manual controller action rather than wired to a blanket after_save callback, since rule/import auto-assignment must not count as a "recent" pick. - Give recent-section rows a distinct DOM id (recent_category_option_<id>) from their canonical-list counterpart so aria-activedescendant can't resolve to a hidden duplicate during keyboard nav. - Fix migration to ActiveRecord::Migration[7.2] to match the rest of the repo. - Materialize @recent_categories with .to_a to avoid a redundant query. |
||
|
|
5f0f5ec89d |
feat(mcp): Add MCP transaction update tool (#2719)
* Add MCP transaction update tool * Fix MCP transaction authorization * Ignore Pipelock false positive on SnapTrade token lookup Pipelock scan-diff flags `token = oauth_refresh_token...` as "Credential in URL" even though these are ActiveRecord attribute names, not embedded secrets. Add the established inline ignore. Co-authored-by: Martin Molcrette <Mart1M@users.noreply.github.com> --------- Co-authored-by: Cursor Agent <cursoragent@cursor.com> Co-authored-by: Martin Molcrette <Mart1M@users.noreply.github.com> |
||
|
Guillem Arias Fauste
|
9d3879a859 |
feat(plan): unify Budgets and Goals under a single Plan tab (#2687)
* feat(plan): unify Budgets and Goals under a single Plan tab
Preview users get one "Plan" nav entry (compass icon) in place of the
separate Budgets and preview-gated Goals items. It fronts a new /plan
hub with two summary cards — this month's budget (spent vs budgeted,
days left, top categories) and active goals (total saved vs targets,
behind/pending counts, per-goal rows) — each drilling into the existing
/budgets and /goals pages, whose breadcrumbs now start Home > Plan.
The two features share a home, not a model: no schema changes, no URL
changes. Users without preview features keep exactly the pre-Plan nav
(Budgets entry, Goals hidden), and /plan falls through to /budgets for
them.
Supporting changes:
- Goal.active_prepared_for: index-style sorted active goals with the
family-wide pooled-allocations + market-flows injection reused
- Goal::FUNDABLE_ACCOUNT_TYPES and Goal::ACTIVE_DISPLAY_STATUS_RANK
extracted from GoalsController
- Budget#days_remaining (same day math as suggested_daily_spending)
- Breadcrumbable#plan_breadcrumb_prefix for the conditional Plan crumb
- New BudgetsController web tests (previously untested) + Plans tests
* fix(plan): address review feedback on the Plan hub
- Keep the "All goals" footer link rendered when the family has only
completed/archived goals — the hub is a preview user's only route to
the goals index now that the Goals nav entry is gone (Codex P2)
- Replace the two hand-rolled footer button-links with DS::Link
(variant secondary, full_width, right icon) per DS Drift Patrol
- Fix DS::Link template comparing icon_position against the string
"right" — the initializer symbolizes it, so right-positioned icons
never rendered on links (DS::Button already compared symbols);
existing callers passing icon_position now get the layout they asked for
- Clamp progress-bar percentages to 0..100 instead of capping only the
upper bound (CodeRabbit)
* refactor(plan): single source of truth for goal loading, sorting, and counts
Addresses jjmata's draft review notes:
- GoalsController#index now builds on the shared Goal loaders instead
of hand-rolling its own copy: Goal.prepared_for (preloads + family-
wide backing-math injection, scope-able) and Goal.active_display_sort
carry the algorithm once; active_prepared_for composes them for the
hub. The controller-side constant alias is gone
- One definition of "behind pace": Goal#behind_pace? (excludes paused —
pausing stops the pace clock on purpose). Both the Plan hub summary
and GoalsController#kpi_payload's behind/needs-this-month figures use
it, so adjacent pages can't disagree. While there, the kpi on-track
numerator also excludes paused goals — it was counted against a
paused-excluding denominator, so the "X of Y" fraction could exceed
its own total
- BudgetCategory#suggested_daily_spending calls Budget#days_remaining
instead of keeping an inline copy of the day math
- Per the fat-model convention, the hub's aggregation moved off the
controller: Budget#top_spending_categories(limit:) and
Goal.summary_for(goals, currency:)
* fix(plan): move Edit budget/New goal into their own cards
Both actions lived in the hub's shared page header, unlinked to either
card and, on mobile, wrapping above all content before any real data
appeared. Each now lives in its own card's header instead: Edit budget
as a compact icon-only control next to the status pill (only when a
budget exists — the uninitialized state already has its own "Set up"
CTA), New goal as a small outline button next to the goals count (only
once there's a goal to sit beside; the empty state keeps its own CTA).
Also swaps the edit icon from "pencil" to "square-pen" — at the sizes
these header controls render, lucide's plain pencil is a thin diagonal
stroke that reads noticeably smaller than a neighboring bold glyph like
"plus", even in the same size box. square-pen carries more visual mass
and reads clearly at the same footprint.
* fix(plan): match established DS precedent for the card header actions
Edit budget was a bare icon-only button; verified against the app's
own precedent for this exact action (app/views/budgets/_budget_donut.html.erb,
the budget card already shipped on /budgets) and it's a labeled
secondary link with a trailing pencil, not icon-only and not a
three-dot menu. Matched that: DS::Link, variant secondary, size sm,
icon right. New goal gets the same treatment for consistency between
the two cards' header actions, rather than the full-page-scoped
"primary" weight goals/index.html.erb uses for its own create button —
that's calibrated for a whole page's sole CTA, not a compact card.
Adding a labeled button (wider than the bare icon this replaces)
crowded the header row on mobile enough to wrap "This month" onto two
lines and truncate the "· July 2026" meta away entirely. Header rows
now wrap as a whole (flex-wrap) with the title pinned (shrink-0) so
the action cluster drops to its own line instead of squeezing the
title and meta text.
Also drops the hub's footer note ("Budgets cap your spend; goals track
what you're saving toward...") — redundant with the subtitle right
above the cards.
* fix(plan): lead the budget card header with status, not the edit action
On Track/Over/Warning is what a glance at the card wants first; Edit
budget is the secondary action. Swapped their order so status leads
and the edit control trails, gap-2 unchanged.
* fix(plan): put the status pill on the left, next to the title
Meant the left side of the card, not just left of the edit button. On
Track/Over/Warning now sits beside "This month · July 2026" in normal
flow; ml-auto carries only the Edit budget link, alone on the right —
matching the goals card's own left-meta/right-action split ("· 7
active" left, "New goal" right).
* fix(plan): lead Edit budget with its icon, matching same-shape precedent
Wrong axis on the earlier match: _budget_donut's trailing pencil labels
the VALUE itself ("$12,850 ✎"), not a static action. Our button's label
is a static "Edit budget", and that shape takes a leading icon
everywhere else it appears — the categories "Edit" on budgets/show.html.erb
(icon: settings-2) and "Edit split" in transactions/show.html.erb both
lead with their icon. Drops icon_position: :right so it defaults to
left, matching New goal's shape in the sibling card.
* fix(plan): use the divider token for row separators, not border-primary
Traced against the dashboard outflows list (pages/dashboard/_outflows_donut.html.erb),
which renders its row separators via shared/_ruler → border-divider
(border-tertiary: black/8%, white/10%). Our category and goal rows used
border-b border-primary instead (black/15%, white/30%) — 2-3x heavier
than the established row-separator weight elsewhere in the app. Swapped
both to border-divider.
* fix(plan): lift the duplicated card shell into DS::Card
Codex P1: _budget_card.html.erb and _goals_card.html.erb hand-rolled
the identical "bg-container rounded-xl shadow-border-xs p-5 flex
flex-col" shell twice, with no DS:: card primitive to reach for
instead. Extracted a minimal wrapper — content-only, no header/footer
slots — matching what both cards actually need right now; the roadmap
cards (envelopes #2153, retirement #2044) can adopt it too instead of
copying the class string a third time.
Verified pixel-identical in a browser: same classes, same DOM shape,
just rendered through the component.
* fix(plan): batch pace queries before sorting goals
Codex P2: active_display_sort calls goal.status per goal to build the
sort key; Goal#status reaches Goal#pace for any goal with a
target_date, which fired its own Entry.sum(:amount) query per goal.
The /plan hub renders only the first 5 of active_prepared_for's list,
but paid the full O(N) query cost sorting all of them.
Adds Goal.pace_for(family) (account_id => 90-day net inflow), grouped
in one query and injected via inject_backing_math! alongside the
existing pooled_allocations/market_flows pattern. #pace now sums from
that shared map instead of firing its own query — same math, same
90-day window, same exclusions, just computed once per family instead
of once per goal.
|
||
|
Guillem Arias Fauste
|
59852ca0f3 |
fix(insights): respect recurring_transactions_disabled in subscription_audit (#2831)
* fix(insights): respect recurring_transactions_disabled in subscription_audit SubscriptionAuditGenerator queried family.recurring_transactions directly, so disabling recurring-transaction detection (Settings -> Recurring Transactions) never stopped already-identified rows from surfacing "recurring charge overdue" insights on the Insights feed — the family-wide flag was already checked at both call sites in IdentifyRecurringTransactionsJob, just not here. Returning [] early is enough for existing insights to self-clean up: produced_types is a class-level declaration, so GenerateInsightsJob still counts subscription_audit as a succeeded type and expires any insight whose dedup_key wasn't regenerated on the next nightly run. * docs(insights): document why cash_flow_warning skips the recurring-disabled guard Answers jjmata's open review question. Unlike SubscriptionAuditGenerator, recurring transactions here are one input into a broader cash-flow projection, not the insight's entire subject — so it intentionally keeps using the last-known identified set rather than gating on family.recurring_transactions_disabled?. No behavior change. |
||
|
kai392andClaude Opus 4.8
|
73aac31f89 |
fix(exports): include merchants.csv in family data export (#2758)
* fix: include merchants.csv in family data export The family export ZIP contained CSVs for accounts, transactions, trades, categories, and rules — but merchants were only present inside the all.ndjson bulk file, never as a standalone merchants.csv. Meanwhile merchants can already be imported via CSV (MerchantImport), so backups were lossy and the import/export cycle was asymmetric. Add generate_merchants_csv to Family::DataExporter, wired into the export ZIP, with headers (name,color,website_url) matching exactly what MerchantImport expects so the exported file round-trips. Fixes #2736 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * test: fix merchants CSV round-trip assertion for family fixtures The round-trip test asserted the target family's total merchant count grew by exactly 1, but the export legitimately includes every family merchant — including dylan_family's fixtures — so the import created 4, failing CI. Scope the count assertion to the merchant under test. Also assert the imported color now that merchant colors survive a save (the set_default_color callback only backfills when no valid color is present), giving the round-trip full name/color/website coverage. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com> |
||
|
Carlos Lindo
|
ef61466e87 |
fix(enable-banking): preserve manual card limit when provider reports zero (#2841)
* Preserve manual card limit when provider reports zero * Test non-positive provider credit limits * Address credit limit review feedback |
||
|
Guillem Arias Fauste
|
c40e7f4807 |
fix(insights): correct the budget card's figure, badge noise and toast a11y (#2799)
* fix(insights): correct the budget card's figure, badge noise and toast a11y
Four defects found while reviewing the insights surfaces for hierarchy.
**The budget_at_risk card's focal figure argued against its own headline.**
`insight_key_figure` returned `budget_spent_pct` for both budget cards, so
"2 categories need attention in your budget" displayed "14% / of budget" —
a reassuring number as the visual focus of a warning. It now leads with the
flagged count ("2 / need attention"); budget_on_track keeps the percentage,
where overall consumption genuinely is the subject.
**The "New" pill carried no information.** Visiting /insights marks every
insight read in one `update_all`, so at first paint the pill was on every
row. On the page it becomes a dot — same signal, without an uppercase
tracked chip stealing weight from the title beside it. In the dashboard
widget it goes entirely: the well's header already counts unread ("New · 3")
and, with three rows, the pill was usually on all of them.
**The undo toast was silent to screen readers.** A card leaves the page via
a Turbo `remove`, which announces nothing, and the toast that explains it
had no live region — unlike its neighbour `_sync_toast`, which sets
`role="status" aria-live="polite"`.
**The undo toast could only be closed with a mouse.** Its close affordance
was a bare `icon "x"` with a click action: not focusable, not named. Now a
real `DS::Button`, matching `_sync_toast`.
The controller test asserting a per-row badge is updated to assert the
header count that replaces it, and to lock in the pill's removal.
* feat(insights): acknowledge instead of dismiss, on both surfaces (#2800)
Two complaints about the insight feed: the close (×) control felt wrong,
and clearing an insight was only possible on /insights — not on the
dashboard widget, which is the surface people actually look at.
**The × was lying.** Dismissal has never been permanent. GenerateInsightsJob
resurfaces a row whose bucketed metadata changes materially "even if the user
had read or dismissed the stale version" (its own comment), and 6 of 8
generators scope dedup_key to a month token, so dismissing July's budget card
says nothing about August's. A destructive-looking control was performing a
non-destructive act. It is now "Got it", and the contract is statable:
acknowledgement covers the numbers you saw; new numbers are a new insight.
No migration. The DB value stays "dismissed" and dismissed_at keeps its name;
only the enum key and the vocabulary the code speaks change, so existing rows
stay hidden and become undoable under an honest label.
**The action pyramid was inverted.** The escape hatch was a chromed icon
button in the card's top-right — the strongest secondary scan position — while
the card's actual purpose ("View budget") was a borderless ghost link under
the body text. Both now sit in a footer strip: the subject action gets the
chrome, acknowledging is quiet labelled text beside it, and the key figure
gets the corner to itself instead of competing with a control.
**The widget can clear its own rows.** Each row gains an acknowledge control,
revealed on pointer hover, on keyboard focus, and shown unconditionally on
touch where there is no hover. No gesture, so the section's drag-to-reorder
handlers are untouched. The row becomes a stretched link plus a sibling
button, because button_to renders a <form> and a form cannot nest in an <a>.
The group is named (group/insight). The dashboard <section> is itself a
`.group` for its header controls, and a bare group-hover: matches any ancestor
group — hovering one row, or the section header, revealed every row's control.
Acknowledging re-renders the well rather than removing a row, so the next
insight is promoted into the freed slot; Insight::FEED_LIMIT is now shared
between the two controllers that render it so they cannot drift. Undo restores
the row on both surfaces, and carries autofocus so it is one keystroke away
after the acknowledged card leaves the DOM.
* fix(insights): guard unacknowledge! against non-acknowledged insights
CodeRabbit, Major: an arbitrary/stale PATCH /unacknowledge (e.g. an old
undo-toast link clicked after GenerateInsightsJob has since expired or
resurrected the insight) could force it back to :read regardless of
its actual current state — including pulling an :expired insight back
into visible view.
Guards the transition to only reverse an actual acknowledgement, per
CodeRabbit's suggested fix.
* test(insights): fix stale dismiss_insight_url route from main merge
main's preview-gate test used the pre-rename dismiss/undismiss route names;
this branch renamed those to acknowledge/unacknowledge earlier.
|
||
|
Gerald
|
9313ad4cba |
fix: tolerate Trade Republic Enable Banking pagination/PDNG errors (#392) (#2828)
* fix: tolerate Enable Banking pagination/PDNG errors for Trade Republic Trade Republic (available via Enable Banking since ~2026-07-22, see #392) fails to sync with two distinct errors on its own side: 1. The BOOK transaction fetch issues a continuation_key on page 1 that its own API then rejects on page 2 as mismatched with transaction_status (422 WRONG_REQUEST_PARAMETERS: "transactionStatus in request is not the same as in continuationKey"). This previously discarded every page already fetched. Once at least one page has succeeded, a validation error is now treated as pagination exhausted and the partial result is kept instead of raising. A validation error on the very first page still propagates as a real failure. 2. The PDNG (pending) fetch is rejected with a plain 400 (:bad_request) instead of the 422 (:validation_error) other ASPSPs use for the same "transaction status not supported" case. Both error types are now treated as "ASPSP doesn't support pending transactions". Verified against a live Trade Republic connection through Enable Banking. * fix: surface Enable Banking pagination truncation as a debug log entry Add a DebugLogEntry.capture call when a mid-pagination validation error truncates the transaction fetch (e.g. the Trade Republic continuation_key bug from the previous commit). This follows the project convention of using DebugLogEntry for support-relevant sync diagnostics rather than only Rails.logger, so a truncated sync is visible in /settings/debug instead of only in container logs. Currently harmless for narrow incremental sync windows (the account observed in production has under 100 transactions per window, fitting entirely on page 1), but a wider historical resync could otherwise lose data past page 1 with no visible indication. * fix: address CodeRabbit review feedback on PR #2828 - Add a DebugLogEntry when the PDNG fetch is skipped as unsupported, matching the pattern already used for pagination truncation — this was a partial-degradation case that was previously only visible via Rails.logger. - Replace the OpenStruct#define_singleton_method provider fakes in the three new pagination tests with sequenced Mocha stubs (expects(...).twice.returns(...).then.raises(...)), per the project's "use Mocha for stubs and mocks" guideline. * fix: don't swallow WRONG_TRANSACTIONS_PERIOD as pagination truncation The mid-pagination validation-error handling treated any 422 after page one as "ASPSP rejected the continuation key" and kept the partial result as a success. WRONG_TRANSACTIONS_PERIOD is a different, real failure (an invalid date range, already retried once with a corrected date_from at the provider level) and must still propagate instead of silently dropping the remaining pages. Addresses CodeRabbit review feedback on PR #2828. * fix: address jjmata's review feedback on partial-result asymmetry - fetch_paginated_transactions now tolerates :bad_request the same way it already tolerates :validation_error mid-pagination, matching the PDNG-unsupported rescue in fetch_and_store_transactions which already accepts both error types. Without this, a :bad_request on PDNG page 2+ would discard the already-fetched PDNG page 1 instead of keeping it like the BOOK path does. Trade Republic only 400s on PDNG page 1 today, so this was latent, not currently observed. - Bump the pagination-truncation log (Rails.logger + DebugLogEntry) from warn to error: this now discards data for any ASPSP/scenario matching the tolerated error types mid-pagination, not just the specific Trade Republic case it was written for, so it deserves higher visibility. - Add a regression test for :bad_request interrupting PDNG pagination on page 2+, pinning down the now-symmetric behavior with BOOK. |
||
|
|
af3100c0a5 |
fix(insights): keep rolling period labels on month boundaries (#2880)
* fix(ci): skip scheduled preview cleanup on forks Only run the hourly Cloudflare preview cleanup on we-promise/sure, where the required secrets exist. * fix(insights): pin meta-line forward-window test mid-month Date.current..Date.current+30 is a full calendar month on the 1st of 31-day months (e.g. Aug 1..31), so insight_period_label prefers the month name over "Next 30 days" and flakes CI. Co-authored-by: Cursor <cursoragent@cursor.com> * Fix rolling insight period labels on month boundaries --------- Co-authored-by: Cursor <cursoragent@cursor.com> Co-authored-by: sure-admin <sure-admin@splashblot.com> |
||
|
Sure Admin (bot)
|
1e800e2f93 | Include uncategorized spending in budget UI (#2877) | ||
|
Will WilsonandClaude Opus 4.8
|
b6cd8437a5 |
fix(oidc): honor http issuer scheme during OIDC discovery (#2853)
* fix(oidc): honor http issuer scheme during OIDC discovery Self-hosted IdPs served over plain HTTP (no SSL) failed OIDC login with "Failed to open TCP connection to <issuer>:443 (Connection refused)". The openid_connect/swd gems hardcode discovery to HTTPS: SWD.url_builder defaults to URI::HTTPS, and OpenIDConnect::Discovery::Provider::Config::Resource drops the issuer's scheme, rebuilding the .well-known URL from host/port only. So an http:// issuer is upgraded to https:443 and never connects. (This is why the in-app "Test connection" passes -- it uses Faraday against the raw issuer URL and never goes through the gem.) Patch Config::Resource to remember the issuer's scheme and build the discovery endpoint with URI::HTTP or URI::HTTPS accordingly. Per-request, no global mutable state, so mixed http/https providers coexist. Only discovery needs patching: the endpoints it returns are absolute and rack-oauth2 preserves an existing scheme, so the token/userinfo/jwks calls follow over http automatically. Verified against openid_connect 2.3.1 / swd 2.0.3. Fixes #2844 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(oidc): scheme-aware port and discovery cache key Address automated review feedback on the OIDC http-discovery patch: - Preserve explicitly configured non-default ports (http on 443, https on 80) by omitting only the scheme's own default port instead of both 80 and 443. - Override the discovery cache_key to include scheme/port/path (the gem keyed on host alone), so an http:// issuer can't reuse an https:// issuer's cached metadata on the same host. Latent today (default SWD cache is a no-op) but removed to keep the override self-consistent. Adds regression tests for both cases. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * test(oidc): cover port and path as cache-key components Extend the discovery cache-key test to assert scheme, port, and path each produce a distinct key on the same host, per review feedback. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com> |
||
|
Sure Admin (bot)
|
078c49b35b | Stabilize net worth breakdown series test dates (#2863) | ||
|
Maximus Barbare
|
4c1bc774e5 |
Fix SnapTrade account setup and reconnection (#2858)
* Infer SnapTrade account types from categories * Allow choosing SnapTrade account types * Reauthorize SnapTrade connections needing attention * Clear stale SnapTrade reconnect state * Always schedule SnapTrade reconnect syncs * Recognize SnapTrade credit card accounts * Recognize SnapTrade crypto account types * Schedule SnapTrade syncs after active imports * Fix SnapTrade account type CI tests * Normalize SnapTrade card account types |
||
|
|
8e6035c62c |
Wire @kraken_items into the accounts index (#2770)
app/views/accounts/index.html.erb never referenced @kraken_items. Kraken support was added upstream but left out of the accounts index in two places: the top-level empty-state condition that decides whether to render the "empty" partial, and the provider render section. AccountsController#index also did not assign @kraken_items at all. As a result, a family whose only connections are Kraken items saw the blank empty state instead of their Kraken accounts. Assign @kraken_items in the controller with the same eager-loading shape used for the other crypto providers, add @kraken_items.empty? to the empty-state condition, and render the items in the provider section in the correct order. Fixes #2577 Co-authored-by: agentloop <agentloop@localhost> Co-authored-by: sure-admin <sure-admin@splashblot.com> |
||
|
pro3958andagentloop
|
785f9a19c5 |
fix(accounts): skip digest on sidebar fragment cache (#2776)
The account sidebar fragment renders DS::* view components. Rails' ERB
dependency tracker parses `render DS::Foo.new` as a dynamic render
dependency named DS and inflects it into the nonexistent partial "Ds/D".
When the cache helper computes the template digest, ActionView::Digestor
fails to resolve that partial and logs "Couldn't find template for
digesting: Ds/D" on every sidebar cache miss.
The fragment's cache key is already manually versioned and invalidated
via account_sidebar_tabs_cache_key ("account_sidebar_tabs_v2",
invalidate_on_data_updates: true), so automatic template digesting adds
nothing. Pass skip_digest: true to the cache block so the digestor never
runs for this fragment. Rendered output is unchanged.
Fixes #2516
Co-authored-by: agentloop <agentloop@localhost>
|
||
|
pro3958andagentloop
|
150e659846 |
fix(tags): return 404 for missing or cross-family tag deletion (#2703)
Tag::DeletionsController looked up the tag and its replacement with find_by, so a missing id or a tag belonging to another family left @tag nil. create then called @tag.replace_and_destroy!, raising NoMethodError and returning a 500 instead of a 404. Use find in set_tag and set_replacement_tag so an out-of-scope id raises ActiveRecord::RecordNotFound, which is rendered as 404. The replacement lookup keeps a presence guard so deleting without a replacement still works. This mirrors Category::DeletionsController. Fixes #2469 Co-authored-by: agentloop <agentloop@localhost> |
||
|
ghost
|
a21ab1946e |
perf(accounts): preload transfer, category, and split-parent associations on show (#2441)
* perf(accounts): preload transfer, category, and split-parent associations on show AccountsController#show iterated over paginated entries and called transaction.transfer (two queries via transfer_as_inflow || transfer_as_outflow), transaction.category, and transaction.merchant individually per row, and fell back to entry.split_parent? (child_entries.exists? per entry) because @split_parent_entry_ids was never set. Fix by: - Batch-preloading transfer_as_inflow, transfer_as_outflow, category, and merchant on transaction entryables after pagination using Associations::Preloader (same API already used in accounts/index/_account_groups.erb). - Setting @split_parent_entry_ids with a single IN query after pagination, matching the identical pattern already in TransactionsController#index. Resolves Sentry issues SURE-APP-PN (60 users), SURE-APP-XE (32 users), SURE-APP-26 (51 users) and related slow-DB reports on AccountsController#show. * docs(accounts): note the show preload is intentionally page-scoped Address review feedback (jjmata): add a comment clarifying that the transfer/ category/merchant preload and the split-parent lookup operate on the current page (@entries) by design — only this page is rendered, so a child entry whose split parent is on another page deliberately won't resolve it. Comment-only; no behavior change. |
||
|
Guillem Arias Fauste
|
cedf28a5a9 |
fix(accounts): make the sync toolbar and toast agree, fix toast overlap (#2813)
* fix(accounts): make the sync toolbar and toast agree, fix toast overlap The Accounts page's own sync toolbar (refresh icon, "Cancel sync") and the global sync-complete toast were three inconsistently-styled, disconnected pieces of UI representing one action, and the toast overlapped the page's own header instead of sitting near it. - "Cancel sync" was hand-rolled markup instead of a DS::Button, unlike its sibling refresh icon right next to it — now both are DS::Button (:ghost). - The refresh icon just went `disabled` with no visible "working" state — now shows a spinning loader-circle while a family sync is in progress, matching the pattern already used in provider_sync_summary.html.erb. - The toolbar was plain server-rendered HTML with no way to know a sync finished, so it stayed stuck showing "still syncing" indefinitely next to a toast now saying otherwise. Family::SyncCompleteEvent now broadcasts a second replace target for the toolbar alongside the existing toast replace, so both resolve together. - The notification tray was a <body>-level fixed overlay centered on the full viewport, but every layout that renders it has a sidebar of some kind — so it never actually centered on the visible content pane, and landed on top of the settings-layout header. It now renders in-flow at the top of each layout's own content region (opt-in via notification_tray_inline, since the simpler single-column layouts don't have this mismatch and are unaffected). - Added the Catalan sync_toast/cancel_sync strings that were missing entirely, which is why the toast/toolbar showed English text on an otherwise-Catalan page. Verified live in a real browser via a new system test covering the idle, syncing, and cancel-flash states, plus a model test on the new broadcast target. * fix(accounts): keep the tray a floating overlay, sidebar-aware instead Codex on this PR: with the tray as first-child-of-scrollable-main, a notification delivered while scrolled down is inserted above the viewport and stays unseen — breaking the sync toast's manual-refresh path specifically, since sync_toast_controller.js suppresses auto-refresh while a form is focused and relies on the toast being visible to offer that manual refresh. Reverts the tray to a position: fixed overlay for every layout (so it can't be scrolled out of view), and fixes the actual bug that made it overlap the accounts toolbar in the first place — a ResizeObserver on <main> centers it on the real content pane instead of the viewport, for the two layouts with a sidebar (application, settings passed via sidebar_aware:). The five single-column layouts are untouched; for those, viewport-center already is content-pane-center. One trap worth flagging: this app renders turbo_refreshes_with method: :morph, and idiomorph resets any inline style a client script set that isn't in the freshly-fetched HTML — including the JS-set `left`. data-turbo-permanent looked like the fix but isn't: it invokes idiomorph's node-identity matching (same id preserved across ANY morphed page), which broke navigation once the id existed on structurally different layouts (app vs settings) — a real, reproduced bug, caught by the system test before it shipped. Went with the narrower turbo:before-morph-attribute event instead, which blocks only the `style` attribute on this one element, with no node-identity system involved. Rewrote the system test's positioning assertion to match: it now asserts the tray centers on <main> rather than sitting above the page header, since a fixed overlay was never going to satisfy the latter by construction. Verified: full bin/rails test (6023 runs, 0 failures), rubocop, erb_lint, brakeman (0 warnings) all clean. Live-verified in a browser across both sidebar-aware layouts and a simple layout, including the full cancel-sync -> morph -> re-render cycle. * fix(accounts): use declarative Stimulus action for morph-attribute guard Replace the manual addEventListener/removeEventListener pair for turbo:before-morph-attribute with a data-action, per the repo's declarative-actions convention. Same element, same listener — just no manual lifecycle management. |
||
|
Guillem Arias Fauste
|
07a3413250 |
fix(ds): keep DS::Menu/Popover panels anchored across Turbo morphs (#2812)
* fix(ds): keep DS::Menu/Popover panels anchored across Turbo morphs The app refreshes pages via Turbo morph (`turbo_refreshes_with method: :morph`), and same-page account actions (disable, exclude, set-default, etc.) trigger one. Two bugs in the shared floating-ui controllers surface as a result: - The panel's `position: fixed` only ever existed as a JS-applied inline style. Idiomorph resets every menu/popover's `style` attribute to match the server-rendered markup (which has none), silently stripping `position: fixed` from every panel on the page. The next dropdown/ popover opened before floating-ui's async recompute lands briefly renders in normal flex flow, shoving its own trigger sideways and making computePosition anchor to that phantom position instead of the real button. Fix: make `position: fixed` part of the static markup so it can never be stripped. - `this.show` was a plain instance property. Because the morph preserves the Stimulus controller in place (stable-id turbo frame), `this.show` doesn't reset when idiomorph re-closes the content element, so it can desync from the DOM and swallow the next click. Fix: derive `show` from the content element's own class instead of tracking it separately. Reproduced and verified against the actual Turbo/Stimulus/floating-ui pipeline in an isolated harness before and after the fix. * test(ds): add regression coverage for menu/popover reopen-after-morph Simulates what idiomorph does to an open panel on a same-page Turbo morph — resets the content element's class back to the always-hidden server-rendered markup and strips the JS-applied inline style, without going through toggle()/close(). Verified against the pre-fix controllers that this fails without the DOM-derived `show` getter. |
||
|
Otter
|
1c6d018b6a |
Fix transactions posting a day early when booked around midnight. (#2744)
* Fix 2668 * Fix CodeRabbit nitpick * Fix Akahu parsing * Anchor provider transaction date parsing to family timezone * Coderabbit suggestion for Date/DateTime order * Remove duplicate condition in wise * Safe unless column_exists + pass family to date parse to family components |