## What
The admin Discord channel is Spanish, but the four scheduled `stats:*`
reports posted English. They now post Spanish, and the three
cohort/experiment reports open with a short AI-written summary so a
reader understands the situation without decoding the table.
### 1. Spanish reports
`stats:daily-report`, `stats:subscription-funnel`,
`stats:experiment-funnel` and `stats:ai-cohort-report` now post Spanish
embed titles, field names, ASCII table headers, legends and disclaimers,
with Spanish dates (`sáb., 13 jun. 2026`). Table headers and cells stay
ASCII (`Semana`, `Variante`, `UMad`, `pdte`, …) so `sprintf`'s byte
padding keeps the columns aligned inside the code block.
Hardcoded, not `__()`: this is an internal channel, not user-facing UI,
so it never needs a second language and doesn't belong in
`lang/es.json`. Everything else — code, comments, PHPDoc, command
descriptions, `$this->info()` — stays English.
### 2. AI summary (best-effort)
New `ReportSummarizer` + `ReportSummaryAgent` (laravel/ai, same
Gemini-Flash pattern and `AI_PROVIDER` switch as the other AI features,
config in `config/ai_reports.php`). It prepends a few sentences to the
embed description that compare the current period against the previous
one (week over week for the two weekly reports, month over month for the
monthly cohort one), say what got better or worse, and call out
explicitly when a figure isn't conclusive — small sample, immature
cohort, signup surge week, or no previous period.
- **Data**: only what each collector already computes. The two cohort
reports pass their weekly series with the per-metric maturity flags; the
experiment report passes the per-variant figures plus the
already-rendered significance verdict (one source of truth), with money
figures nulled exactly where the table renders `—`, so the summary can't
report a zero where the reader sees no data.
- **Previous period**: the experiment report has no time series, so each
run caches its figures (with a capture timestamp) as the next run's
baseline. A same-day manual re-run doesn't overwrite it, and the model
is told to flag a gap that isn't roughly one period.
- **Degrades safely**: no API key, a provider outage, a bad provider
name or a slow response (30s timeout) → the report is posted unchanged,
without the summary. Transient provider errors are logged; anything else
is reported, matching `CategorizeTransactions` /
`LaravelAiRuleSuggestionGenerator`.
### 3. Fix found while reviewing: Discord's embed limits
The translated "Cómo leerlo" field came out at 1152 characters, past
Discord's 1024-character cap on a field value — Discord rejects the
**whole** payload, so the experiment funnel would have silently stopped
appearing in the channel every Monday (`DiscordWebhook` only logs the
400). Both long fields are now tighter (745 and 898 chars),
`DiscordWebhook` trims anything still over the limit instead of losing
the report, and a test measures every scheduled report's embed so
growing copy fails in CI rather than in production.
## Testing
- `tests/Feature/Ai/ReportSummarizerTest.php`: baseline in/out, same-day
re-run, dry run, backtick stripping, truncation, empty answer,
transient-vs-reported failures.
- `tests/Feature/DiscordReportEmbedLimitsTest.php`: all four embeds
inside Discord's limits, plus the trimming fallback.
- The four command tests: Spanish assertions, summary is the first thing
in the description, and the report is still posted when the AI throws.
- 64 tests / 338 assertions pass locally, plus PHPStan and Pint.
## QA
Ran all four commands against the local DB with `--no-discord` (and
dumped the real webhook payloads with the Discord call faked). Real
Gemini output, e.g. the experiment funnel on the second run:
> No se ha producido ningún cambio en las métricas del experimento
respecto a la ejecución previa del 10 de agosto de 2026. La tasa de
conversión sobre usuarios maduros se mantiene en el 4,7 % para la
variante reducida, el 4,0 % para pay_now y el 3,7 % para el control. Las
cifras no son conclusivas ya que las diferencias entre variantes no son
estadísticamente significativas, con un p-valor de 0,593 frente al
umbral α de 0,017.
Also verified with an unreachable provider: the report renders in full,
without the summary.
## Not included
- `stats:stuck-cohort-report` is still English. It posts to the same
webhook but isn't scheduled in `routes/console.php`, so it was out of
the four active reports; worth its own decision.
- The `—` and `⚡` cells are still a couple of bytes off inside the code
block (multibyte in `sprintf`). Pre-existing on `main`, unchanged here.
## Why
Publishing the MCP server to the ChatGPT app directory requires proving
we own the host that serves it. The submission portal issues a token and
fetches `https://whisper.money/.well-known/openai-apps-challenge`,
expecting the bare token back. That path currently 404s.
## What
- `GET /.well-known/openai-apps-challenge` returns
`OPENAI_APPS_CHALLENGE` as `text/plain`, nothing else — no JSON
envelope, no extra tokens.
- Aborts with 404 when the token is unset, so a host without the
variable configured cannot answer with an empty body that the verifier
would read as a mismatched token.
- Token lives in config/env rather than the repo: it is per-plugin and
rotates independently of the code.
## Testing
`tests/Feature/OpenAiAppsChallengeTest.php` covers both branches: the
token is served verbatim when configured, and the route 404s when it is
not.
The production env var is already set, so the endpoint answers once this
deploys.
## Why
You could not add a transaction by hand to a bank-connected account.
There was no good reason for it: bank sync **only inserts** rows it has
not seen before (dedup runs on `dedup_fingerprint` /
`external_transaction_id`, both `null` on manual rows) and **never
deletes or updates**, so a hand-entered transaction survives every later
sync untouched.
The one thing that genuinely does not make sense is letting a user set a
**balance** on a connected account, because the next sync overwrites it.
That restriction stays.
## What was actually blocking it
Less than it looked. The HTTP endpoint already allowed it,
`ManualBalanceAdjuster` already skipped connected accounts, and the
transaction dialog already handled them (it forces `updateBalance:
false`). Only two surfaces blocked it:
- **MCP** — `WriteTool::writableAccount()` rejected every connected
account for *all* writes. Split into `accountInSpace()` (no connection
check — used by `create_transaction` / `update_transaction`) and
`balanceWritableAccount()` (still rejects connected — used by
`create_balance`).
- **The account detail page** — hid its "Add transaction" button for
connected accounts, even though the same dialog on the transactions page
already offered them in its account picker.
## Also in here
Two problems the change surfaced, both fixed:
- **`EnableBankingSyncer` linked-account watermark** took the newest
transaction of *any* source. With manual rows now able to land on a
connected account, one dated later than the bank's last posting would
shrink the fetch window and skip the bank history in between —
permanently, since the watermark only moves forward. On the QA data this
would have skipped **36 days**. Now restricted to bank-sourced rows.
- **`calculateHistoricalBalances`** derives history by walking back from
a bank-provided reference balance, summing transactions unfiltered.
Counting a hand-entered row subtracts money the bank never had. Now
walks bank-sourced rows only. (Safe before only because manual rows
could not reach a connected account.)
Plus the honesty/copy work:
- `ManualBalanceAdjuster` returns whether it shifted anything, so
`create` / `update` / `delete_transaction` all report `balance_updated`
instead of silently no-opping and letting the agent claim a balance
moved.
- The **OAuth consent screen** and the AI Connector settings page said
"bank-connected accounts stay read-only". That was a trust statement,
and it is no longer true — both now say bank-*synced transactions*
cannot be edited or deleted and connected balances stay untouched.
- `create_balance`'s description no longer contradicts the server
instructions shipped alongside it.
- The transaction dialog now *explains* why the "Update account balance"
checkbox is absent on a connected account instead of just hiding it (it
defaults to on and is localStorage-persisted, so it used to vanish
mid-form with no reason given).
## What stays blocked
- Balances on connected accounts — MCP `create_balance` rejects them,
the adjuster no-ops, the UI shows the explanation instead of the
checkbox.
- Editing or deleting bank/imported transactions — still gated on
`source === manually_created`, unchanged.
## Testing
- `create_transaction` on a connected account leaves its balances alone;
moving a transaction onto a connected account unwinds only the manual
side it came from.
- A manual row survives a sync and does not block the bank's own rows —
the invariant the whole change rests on, previously untested.
- A manual transaction does not move the linked-account sync window.
- The account page offers "Add transaction" on connected accounts, and
hides it on non-transactional ones.
- Full suite: 2047 passing, phpstan clean.
QA'd in the browser end to end (create from a connected account, edit it
afterwards, contrast with a manual account, switch accounts mid-form)
and over real MCP calls against the running server (`create_transaction`
returned `balance_updated: false` on connected / `true` on manual;
`create_balance` refused the connected account).
## Demo
https://github.com/user-attachments/assets/7a7f1cbb-8f68-402d-ba35-3288dd7ea77f
<!-- PLACEHOLDER: drag the video here -->
## Problem
A paying user reported that "Save & Sync" on the account mapping screen
did nothing — no error, no navigation, nothing.
EnableBanking returned their Société Générale card (`CB Visa`) with
`uid: null`:
```json
{"uid": null, "name": "CB Visa", "cash_account_type": "CARD", "account_id": {"iban": null, "other": {"scheme_name": "CPAN", "identification": "************8210"}}}
```
The page renders one mapping row per pending account, so the form posted
`bank_account_uid: null`. `MapAccountsRequest` requires it, the request
422'd, and since the page never rendered validation errors the button
looked dead. The user retried the connection six times.
A second problem kept them from doing what they actually wanted: both
accounts reported `currency: "XXX"` — ISO 4217 for "no currency".
`getCompatibleAccounts` filtered their existing EUR accounts against
`"XXX"`, matched nothing, and the "Link to existing account" option was
never rendered. The backend already knew `XXX` isn't a currency
(`AccountUserCurrencyService::resolveImportedCurrency`); the frontend
didn't.
## Fix
- Accounts without a uid are filtered out of the mapping screen. They
can never be synced anyway — every sync service early-returns on an
empty `external_account_id` — and `CreatesAccountsFromPending` already
skipped them during onboarding. That rule now lives in one place,
`BankingConnection::mappablePendingAccounts()`.
- The skipped accounts are named on the page, so users don't go hunting
for an account they can see in their bank app.
- A connection where *no* account has a uid is closed out instead of
parking on a mapping page that could only ever 422.
- Validation errors surface as a toast, with app-authored messages in
`MapAccountsRequest::messages()` instead of raw field paths.
- `XXX` (and a blank/lowercase variant) is treated as "unknown
currency": the code isn't displayed, and it no longer filters out every
linkable account.
## Scope
One user affected in production (the reporter). Their connection has one
valid account alongside the card, so this fix unblocks them on deploy —
no data change needed.
## Testing
- `tests/Feature/OpenBanking` — 315 passed, including two new cases:
uid-less accounts are hidden and named, and an all-uid-less connection
is closed rather than left awaiting mapping.
- Browser QA against a local reproduction of the exact production
payload: only `Compte Bancaire` is offered, `CB Visa` is named as
skipped, "Link to existing account" appears despite the `XXX` currency,
submitting without picking an account toasts the error, and picking
`Dany SG` links it (`external_account_id` set, connection `active`).
## Demo
https://github.com/user-attachments/assets/6275834a-a518-47b9-9015-a698e5926d71
<!-- PLACEHOLDER: drag account-mapping-fix-qa.mp4 here -->
## Not in this PR
`AuthorizationController::refreshAccountIds()` consumes the raw pending
list on reconnect; its positional fallback can pair a legacy account
with a uid-less entry. Not reachable for any account in production
today, it belongs to a different flow, and it deserves its own test —
filed separately rather than smuggled in here.
Reported by a user who spent over an hour and a half on the onboarding
syncing step, on two devices, without ever getting into the app.
## What was happening
When Enable Banking rate limits a connection, `SyncBankingConnectionJob`
records the error and backs off, but leaves the connection **Active with
`last_synced_at` still NULL** — the exact shape `syncStatus` treated as
"still syncing". The step polled that endpoint forever, and there was no
deadline or escape on the client.
Confirmed in production for the reporter: a Trade Republic connection
returned `429` on every scheduled sync for two days straight. They
unblocked themselves by deleting the connection — `onboarded_at` was set
four seconds later. Three other users are in the same state right now,
one of them from today.
## What changed
- **`syncStatus` no longer waits on a connection that already failed**,
and reports it as `failed` separately from `pending`.
- **The step says so instead of pretending it worked.** Advancing
silently dropped the user on a "You are all set!" screen with an empty
dashboard and no way to find out why. Now they get a short explanation
and a Continue button.
- **Client deadline raised to 5 minutes** — the previous 2 was under the
worst case of a healthy first sync (3 attempts of a 120s job, 30s apart)
— and the status request now has a timeout, so a hanging poll can't
stall the step either. A failing status check keeps polling until the
deadline rather than skipping the user ahead.
- **`AccountMappingController` clears the stale error** when it
reactivates a connection, so a genuine sync in flight isn't reported as
failed.
## Testing
- Feature tests for the rate-limited case, for a healthy connection
syncing alongside a failed one, and for the failed flag; a
`rateLimited()` factory state replaces the hand-rolled attributes.
- Vitest coverage for the three client branches: failed, deadline
reached, and the normal advance.
- Browser QA against the real app, reproducing the production state: the
honest screen appears instead of the spinner, Continue moves on, the
user completes onboarding and reaches the app; and a genuinely pending
sync still spins and then advances by itself once the connection syncs.
No console errors.
## Demo
https://github.com/user-attachments/assets/e54e029d-db7a-47b0-8b93-0da5569aa363
<!-- PLACEHOLDER: drag the QA video here -->
## Not in this PR
`settings/connections.tsx` has the same `active && !last_synced_at` =
"syncing" heuristic, so a rate-limited connection shows a permanent
green "Syncing" badge there and its error is never rendered (the error
block is gated on `status === 'error'`). Same bug class, separate
surface — worth a follow-up.
## Why
A support ticket: Bankinter transactions arrive with the raw ISO 20022
remittance tag in front of the text, so AI categorization reads the tag
instead of the merchant.
```
/TXT/D|SumUp *GELATERIA SALV
/TXT/H|TRANSF NOMI /AIGUA DE RIGAT, S ← the payroll that got categorized as Fuel
/TXT/CONDIS SANT JUST DESV07/07/26|20260714
```
`/TXT/` is the unstructured remittance tag, `D|`/`H|` is the debe/haber
marker (which only repeats the sign of the amount), and card payments
append the purchase and settlement dates. None of it describes the
transaction.
In production this hits **7297 transactions across 20 users** —
Bankinter (6569) and Unicaja Banco (728), which ships the same shape.
## What
**`RemittanceTagFormatter`** strips the tag, the credit/debit marker,
the card dates and the `#` marker on card charges. The tag identifies
itself, so the formatter is keyed on the **description** rather than on
a bank name — it works for any bank shipping the same shape instead of
needing a new class per bank. `BankFormatter::matches()` now takes the
description as well; `BbvaFormatter` keeps matching on the bank name.
**`banking:backfill-descriptions`** fixes the rows that are already
imported. It also rewrites the automation rules that match on the raw
text: **48 user-authored rules across 4 users** contain literals like
`/TXT/D|RECIBO VISA CLASICA`, and rewriting descriptions without
rewriting those rules would silently stop them from ever matching again.
A test pins that a rule still matches its transaction after both are
rewritten.
```
banking:backfill-descriptions [--user=email] [--dry-run] [-v]
```
## QA
Ran the formatter over **all 2755 distinct tagged descriptions in
production**: 0 no-ops, 0 leftover tags/dates/markers, 0 degenerate
output.
Ran the command against a database seeded with production-shaped rows
and rules:
```
===== DRY RUN (-v) =====
DRY RUN — no changes will be saved.
/TXT/D|SumUp *GELATERIA SALV → SumUp *GELATERIA SALV
/TXT/H|TRANSF NOMI /AIGUA DE RIGAT, S → TRANSF NOMI /AIGUA DE RIGAT, S
/TXT/EL CLANDESTI 27/07/26|20260803 → EL CLANDESTI
/TXT/CONDIS SANT JUST DESV07/07/26|20260714 → CONDIS SANT JUST DESV
/TXT/RECIBO MES TARJETA|20260806 → RECIBO MES TARJETA
/TXT/D|#RECOBRO RECIBO VISA → RECOBRO RECIBO VISA
/TXT/OPENAI *CHATGPT SUBSCR MP → OPENAI *CHATGPT SUBSCR MP
rule …080: {"in":["RECIBO VISA CLASICA",{"var":"description"}]}
8 transaction(s) and 2 automation rule(s) would be reformatted.
===== SECOND RUN =====
0 transaction(s) and 0 automation rule(s) reformatted. ← idempotent
```
The raw text is kept in `original_description`; an already-stored
original is never overwritten; untagged descriptions and rules are left
alone; transactions the server cannot read (`description_iv`) are
skipped.
## Not in this PR
**Already-categorized transactions keep their category.** The backfill
fixes the text, not the past AI decisions — 4005 of the affected rows
already have a category (1024 of them AI-sourced), and
`ai:categorize-backfill` only picks up uncategorized ones.
Rule-categorized rows are unaffected because the rules are migrated. If
we want the AI ones re-run, that's a separate deliberate step: clear
`category_source = 'ai'` on the affected rows, then run the existing
backfill command.
## Rollout
```bash
php artisan banking:backfill-descriptions --dry-run -v # inspect
php artisan banking:backfill-descriptions # apply
```
## The bug
Sentry
[PHP-LARAVEL-53](https://whisper-money.sentry.io/issues/PHP-LARAVEL-53)
— `SQLSTATE[22001]: Data too long for column 'title'` on `PATCH
/transactions/{transaction}`. 6 events, 1 user.
Changing a transaction's category learns a forward-looking automation
rule, and `AiRuleLearner::title()` rebuilds the rule's title from the
merchant names it matches. This user's bank puts the **entire statement
line** in `creditor_name`:
```
15/06 12/06 Pago Con Tarjeta En Discos, Libros, Fotos Y Pc's -59,00 189,27 | 4940197152806468 Classpass* Monthly
```
Two corrections into the same category produced a 279-char title against
a `varchar(255)` column, so `$rule->save()` threw and took the whole
request with it. Since `creditor_name` is itself capped at 255 on
ingest, a single correction can overflow it too.
## The fix
1. **`AiRuleLearner::title()`** caps each merchant token (40 chars) and
the category name (60), so the title stays readable instead of merely
short — the `→ Category` tail is the part that carries the meaning.
2. **A `title` mutator on `AutomationRule`** truncates to the column
width. Both reviews flagged that fixing the learner alone leaves the
sibling generator, `ApplyRuleSuggestions::title()`, unguarded: it joins
three AI match tokens that are each validated at `max:255`, so it can
overflow the same column and 500 the accept-suggestions step **during
onboarding**. The mutator covers every generator, present and future, in
one place. User-authored titles never reach it —
`StoreAutomationRuleRequest` already rejects longer ones with a 422.
3. **Learning can no longer abort the correction.**
`CategoryOverrideHandler::record()` runs before the transaction is saved
and outside any DB transaction, so the incident left the correction
logged and the ai rules stripped while the category the user asked for
was never written. `bulkUpdate` is worse: it records every transaction
before the mass update, so one throw mutated rules for the earlier ones
and categorized none of them. A learning failure is now reported and
swallowed — both callers already handle "nothing learned".
## Tests
- `AiRuleLearnerTest` — replays the exact production payload; reproduces
the identical `SQLSTATE[22001]` on the pre-fix code and asserts the
title stays inside the column with the category still visible.
- `AutomationRuleTest` — the mutator truncates rather than failing the
write.
- `CategoryOverrideHandlerTest` — a throwing learner is reported, not
propagated.
191 tests green locally (`tests/Feature/Ai` + the three automation-rule
suites) against the MySQL testcontainer; `pint --test` clean.
## Deliberately out of scope
- **Dead-weight rules.** With such a merchant name the learned clause is
`creditor_name == "…-59,00 189,27 | …"` — it contains amounts and a
running balance, so it can never match again, yet it accumulates one
clause per correction and the UI toasts *"Learned: similar transactions
will be categorized automatically"*. The review's prod data says length
is the wrong guard (62 long merchant names DO repeat, across 490
transactions); the right one is rejecting the statement-line *signature*
(an embedded `dd/mm` or `nn,nn`) in `merchantKey()` so it falls through
to the already-guarded description-token path. Worth its own PR.
- **Rules list UI**: a long title sits in a `whitespace-nowrap` cell and
pushes the actions column off-screen (`settings/automation-rules.tsx`).
- **Unbounded writes of the same class elsewhere**, found while
reviewing: `transactions.currency_code` `varchar(3)` written unvalidated
from the Enable Banking payload (`TransactionSyncService.php:179`), and
`banking_connections.aspsp_name`/`aspsp_logo` with no `max:` rule in
`StartAuthorizationRequest`.
Fixes PHP-LARAVEL-53
> **Draft on purpose.** The migration can put connections into
`Expired`, which sends `BankingConnectionExpiredEmail` to real users —
up to ~18 of them shortly after deploy. That is the right outcome (they
have been broken for weeks without being told), but outbound email to
real users should be a human's call, not an autonomous one. Everything
else here is ready.
## The bug
Sentry
[PHP-LARAVEL-3J](https://whisper-money.sentry.io/issues/PHP-LARAVEL-3J)
— `RequestException: HTTP 403` in
`EnableBankingProvider::getTransactions`. 45 events, 10 users,
regressed.
`EnableBankingProvider` classifies known upstream failures into domain
exceptions and rethrows the rest as a bare `RequestException`. Two
responses were unclassified:
- `401 {"error":"CLOSED_SESSION"}`
- `403 {"detail":{"error_name":"PsuActionRequiredException"}}`
`SyncBankingConnectionJob` reads any bare 401/403 as an auth failure:
the connection goes to `Error` with `consecutive_sync_failures =
MAX_SCHEDULED_RETRIES + 1`. Both the scheduler and `banking:sync` filter
that out, so **the connection is never dispatched again**. Enable
Banking's syncer has `notifiesOnAuthFailure() === false`, and the
"reconnect your bank" notice only counts `Expired` connections — so
nothing told the user. Their bank sync was dead, silently.
Prod confirms the damage: **18 connections across 17 users** stranded
that way, 14 with this bug's exact error message, the oldest not synced
since **2026-05-06**.
## What each response now does
**`401 CLOSED_SESSION` → expired session.** The session is genuinely
gone (revoked at the bank, or superseded by a newer authorization) and
only the user can fix it. It now joins `EXPIRED_SESSION`: connection
marked `Expired`, reconnect email sent, notice shown, not reported as an
application error.
**`403 PsuActionRequiredException` → transient.** My first pass expired
these too; the product review pulled the prod data and it says
otherwise. The nine connections that hit this on 2026-08-05 had all
synced cleanly at 18:00 the evening before, failed inside the same
**nine-minute** window, all had months of consent left, and none has
recurred in the four days since. Nine users at nine different banks do
not revoke consent inside nine minutes — that is the provider faulting
behind a generic `"Internal error."` envelope.
Expiring is a one-way door: expired connections are never dispatched,
manual sync is refused, and the only way out is a full reauthorization
with SCA. Expiring on first sight would have pushed nine users through a
re-consent they did not need. So it is classified transient: the
connection stays schedulable and self-heals next cycle, and the error
logs as a warning instead of being reported. If a PSU action really is
required, the consent lapses on its own and the 401 path takes over.
A 403 or 401 the bank sends for any other reason still surfaces exactly
as before — tested, because widening either check to "any 401/403" would
silently expire every connection during a credentials bug of our own.
## Rescuing the stranded connections
The classification fix cannot reach the 18 already-parked rows: they are
excluded from dispatch before any of this code runs. The migration
clears their failure counter so the next scheduled cycle re-evaluates
them through the fixed path — they resume, or they expire properly, with
the email and the notice the user should have had months ago.
Also: the reconnect email's five keys were missing from `lang/es.json`,
so Spanish users got it in English. Unenforced because the localization
test only scans `resources/js`.
## Tests
`tests/Feature/OpenBanking` — 319 green, including 6 new provider cases
(both new codes on `getTransactions` and `getBalances`, plus negative
cases for an unrecognised 401 code, a bare 403, and a non-object
`detail`). The job-level behaviour — `Expired` + email + not reported —
is already covered end to end in `SyncRetryAndLoggingTest`, through the
real syncer.
## Follow-ups (not in this PR)
- **`Expired` badge next to a future "Expires:" date.** `markExpired()`
doesn't touch `valid_until`, and this fix expires connections whose
window hasn't lapsed, so `settings/connections.tsx:411` will show both.
Hide the line for expired connections or backdate the field.
- **Forensics gap.** `logSyncAttempt` records no
`error_class`/`error_message` on the expired path, so a provider-wide
wave becomes indistinguishable from ordinary consent churn in
`banking_sync_logs` — that is exactly the data that disproved my first
reading here.
- **Reconnect loop.** `AuthorizationController` sets the connection back
to `Active` and syncs immediately; if the bank refuses again the user
gets a second "reconnect" email seconds later.
- **`banking:sync --connection=<id>` obeys the health filter**, so ops
has no escape hatch for a stranded connection.
- **Duplicated catch ladders** in `getTransactions`/`getBalances` — the
shape that let a code get classified in one and not the other.
Fixes PHP-LARAVEL-3J
## Why
The analysis drawer was built for **bounded** filters — a trip, a
project — where "total spent" and "avg / day" are the answer.
Filter by something open-ended instead, like a single category, and it
surfaces a year or two of expenses. There both numbers stop meaning
anything: the cumulative line only ever climbs, `€12.32 / day` is not a
figure anyone decides on, and the largest-expenses table fills with the
same monthly transfer repeated five times.
## What
A fourth analysis view that answers the open-ended question instead:
**the monthly rate, and where the recent months sit against it**.
Automatic picks it past a 120-day span; the existing per-filter override
now carries four options, each named after the number it produces.
`Automatic` · `Total spent` · `Income & expenses` · **`Monthly trend`**
| Panel | Bounded view | Monthly trend |
|---|---|---|
| Rate | Total spent + avg / day | **Monthly average** and **last 3
months** with the change between them |
| Chart | Bars + cumulative line | **Monthly bars**, dashed line at the
average, no cumulative |
| Largest expenses | top 10 | **not rendered** — across an open-ended
span the biggest single transaction is trivia, and the payee and
category breakdowns already answer where the money went |
| Category · payee · account · label | unchanged | unchanged |
Footer names the months covered rather than counting days. When income
is a meaningful share the cards report net figures and the chart adds
income bars.
### Only whole months count
Every monthly figure covers calendar months the series spans end to end.
Both edges would otherwise drag the average down, in the same direction:
- The **trailing** month is still in progress — on the 2nd it holds two
days of spending.
- The **leading** month is partial whenever the series does not start on
its 1st, whether a filter clipped it or the first transaction simply
landed on the 18th.
Skipping only the trailing one manufactured a rise: flat €300/month over
a 120-day span starting 18 March reported **+16%** in amber on spending
that never changed. Both are excluded now, which is what the one new
backend field (`summary.first_date`) is for. The part-month bars are
faded rather than dropped, so the solid bars are exactly the months
behind the average.
The average card states how many whole months it spans and the
percentage says it is against the average — two figures that otherwise
silently covered less than they appeared to.
### Edge cases
- Forcing the view onto a span with no whole month falls back to the
bounded shape, toggle label included, and the popover says why.
- The recent card is hidden when whole months do not outnumber its own
window, rather than repeating the overall average.
- A **day override** is the user stating the real duration, so it
decides the automatic view too: a trip whose transactions posted over
eight months stays bounded instead of jumping to trend and leaving that
override inert.
## Cost
**Backend: one enum case and one summary field.** No migration —
`analysis_mode` is already a nullable string behind `Rule::enum`. Every
other figure is derived in the browser from the `over_time` series the
endpoint already returned.
## Tests
`resolveAnalysisView`, `monthlyRates` and `isAdverseChange` are pure and
unit-tested directly — the part-month cutoff, the change against a
negative average, the zero-average guard, and the adverse-direction rule
that inverts for net. 32 component tests, plus feature coverage for the
new field and the persisted mode.
## QA
Browser QA against a year of grocery spending on the demo account (352
transactions, July 2025 – July 2026): the trend view resolves
automatically, all four options switch correctly, and a 30-day date
filter still lands on the bounded view. 22 assertions, no console
errors.
## Demo
https://github.com/user-attachments/assets/f4a14349-3d58-4da4-b85c-9a4ca076551f
Budget email notifications shipped opt-in, then #733 turned all three on
by default. The per-transaction notice is the wrong one to force on: it
emails on **every single transaction** assigned to a budget, and a
catch-all budget matches all of them. This puts it back to opt-in and
leaves the limit alerts (close to limit, over limit) on by default.
## Why a second migration instead of editing the first one
`2026_07_24_100000_enable_budget_notifications_by_default` **has already
run in production** (batch 76). Laravel never re-runs an applied
migration, so editing it in place would have been inert: 382 budgets and
174 settings rows would keep the flag on, and the `user_settings` column
default would stay `true`.
That default matters beyond new signups: every settings controller
writes its own column through `setting()->updateOrCreate(...)`, so a
user who merely changed their chart colour scheme would get a settings
row with `budget_notify_on_new_transaction = 1` filled in by MySQL —
silently opted in, with the checkbox rendering as checked.
So the original migration is restored to its merged form and a new one
resets the flag: column default back to `false`, plus a one-off reset of
existing `user_settings` and `budgets` rows. It uses the query builder,
so `updated_at` is untouched — same as the force-enable did. 381 of the
382 affected budgets had not been touched by their owner since the
deploy, so the reset undoes a force-enable rather than a user's choice.
## Changes
- New migration resetting
`user_settings.budget_notify_on_new_transaction` (default + rows) and
`budgets.notify_on_new_transaction`.
- `BudgetController::store` and
`NotificationPreferenceController::index` fall back to `false` for the
per-transaction flag when a user has no settings row.
- `UserSettingFactory` matches the new default.
Users who want the per-transaction notice can still turn it on per
budget, or as their default for new budgets, in Settings →
Notifications.
## QA
Ran the migration against a real database and checked the end state:
`budget_notify_on_new_transaction` default `0` with all 151 settings
rows and 333 budgets reset, both limit flags default `1` and left on.
Then drove real transactions through the listener and queue on a live
budget (limit €300) and read the resulting mail off Mailhog:
| Event | Spent / limit | Email |
| --- | --- | --- |
| New transaction, default settings | €50 / €300 | none |
| Reaches 90% | €270 / €300 | «Presupuesto de Ocio» está cerca de su
límite |
| Exceeds the limit | €320 / €300 | «Presupuesto de Ocio» ha superado su
límite |
| New transaction, after opting in | €10 / €5000 | Nueva transacción en
«Presupuesto de Ocio» |
Assignment still happens in every case — only the email is suppressed.
265 tests green across the Budget/Setting/Notification suites, including
new assertions that the rendered defaults have the per-transaction flag
off and that a settings row created without the budget columns does not
opt the user in.
## What
Follow-up to #731 (per-budget email notifications). Two changes:
1. **Enable budget email notifications by default.** They shipped opt-in
with every toggle defaulting to `false`, so no one received them. They
are now on by default.
2. **Coalesce budget alerts into one email per transaction.** A single
transaction that crossed a budget's limit could send two emails for the
same budget (the "new transaction" notice *and* the over/close-limit
alert). Now at most one email is sent per budget per assignment.
## Enable by default
- `user_settings.budget_notify_on_*` column defaults flipped to `true`,
and **all existing `user_settings` and `budgets` rows are backfilled to
`true`** so current users and budgets start notifying — not just new
ones.
- `BudgetController::store` and
`NotificationPreferenceController::index` fall back to `true` when a
user has no settings row yet.
- `UserSettingFactory` reflects the new default.
- The `budgets` columns keep their own `false` DB default on purpose:
new budgets are seeded from the user's `user_settings` defaults at
creation, so only the one-off backfill is needed there.
## Coalescing
`BudgetNotificationService::processPeriod` now sends **at most one email
per budget per transaction assignment**, picking the most severe
applicable event with priority **over-limit > close-to-limit >
new-transaction**, and passes the triggering transaction through to
whichever fires. So a transaction that pushes a budget over its limit
produces a single over-limit email that still names the transaction and
shows how far over the budget is.
Preserved from #731: the atomic per-period `close_to_limit_notified` /
`over_limit_notified` claim (one email per crossing, safe under
concurrent workers), the reset-and-re-notify path when spending drops
back below the threshold, and the no-emails-on-historical-backfill
guarantee. Coalescing is **per budget** — a transaction matching two
different budgets still sends one email to each.
## Trade-offs (deliberate)
- **`notify_on_new_transaction` is on by default**, which means one
email per transaction assigned to any budget (no batching yet — flagged
in #731). A catch-all budget matches every expense, so this can be
high-volume; the whole-user-base backfill also produces a send spike on
first sync after deploy. Batching the new-transaction type into a daily
digest (mirroring the bank-sync digest) is the natural follow-up if
volume proves noisy.
- The migration backfill is intentionally unconditional and one-way for
data: `down()` restores the column defaults but not the backfilled row
values.
## Every email links to the settings page
Each budget email already carries a "Manage notifications in
notification settings" link (`route('notifications.index')`) so users
can turn any of them off. A render-test assertion now locks this in.
## Tests
- Coalescing: a single limit-crossing transaction sends exactly one
over-limit email that references the triggering transaction.
- On-by-default: a new budget created without custom preferences enables
all three types.
- All existing #731 sending/preference tests still pass (dedup,
re-notify after drop, no-limit budgets, historical backfill sends
nothing, enabling while already over).
QA (backend): rendered the coalesced over-limit email and confirmed it
names the transaction, shows "Over by", and links to the settings page;
verified the migration backfill flips pre-existing `false` rows to
`true` against a real DB.
## Problem
The production image runs the queue workers via supervisor but **nothing
runs the Laravel scheduler** (there is no `schedule:work` program and no
cron running `schedule:run`). As a result, none of the commands in
`routes/console.php` ever fire automatically — most notably
`banking:sync` (every 6h), so self-hosted users have to trigger bank
synchronization by hand. The manual "sync" button works because it
dispatches the same job to the queue, which the existing workers pick
up; only the scheduled trigger was missing.
## Changes
- **`docker/supervisor/supervisord.conf`**: add a `[program:scheduler]`
running `php artisan schedule:work`, mirroring the existing worker
programs (autostart/autorestart, dedicated log). This is what makes
every scheduled command fire on its own cadence.
- **Gate the operator-only stats reports** (`stats:daily-report`,
`stats:subscription-funnel`, `stats:ai-cohort-report`,
`stats:experiment-funnel`) behind `config('subscriptions.enabled')` with
an early return. These are subscription-conversion telemetry destined
for the admin Discord; on a self-hosted instance
(`subscriptions.enabled=false`) `stats:daily-report` would otherwise
make an unnecessary live Stripe API call, and the others would build
reports for an unconfigured Discord channel. On the SaaS (subscriptions
enabled) behavior is unchanged.
The guard lives **inside each command** (not in `routes/console.php`) so
it stays unit-testable. `banking:cancel-free-enablebanking` was left
alone (already a safe no-op via `hasProPlan()` when subscriptions are
off), as were the drip-email commands (already gated by
`mail.drip_emails_enabled`).
## Testing
- `vendor/bin/pint --test` passes.
- New Pest test per gated command asserting it skips and makes no
external call when subscriptions are disabled; existing tests updated to
enable subscriptions so they still exercise the real path.
- Full suite green except 5 unrelated pre-existing environment failures
(timezone alias, dashboard 409, Passport OAuth key) that fail
identically on `main`.
- Verified end-to-end with a real supervisor run: the `scheduler`
program reaches `RUNNING`, `schedule:work` boots, and autorestart
resurrects it; `php artisan schedule:test --name=banking:sync`
dispatches successfully.
> Takes effect after rebuilding the production image.
## What
Adds opt-in **email notifications per budget** for three events, plus a
dedicated **Notifications** settings page.
Each budget can independently enable:
- **New transaction** — a transaction was assigned to the budget
- **Close to limit** — spending crossed 90% of the limit
- **Over limit** — spending reached/passed the limit
A **Default (new budgets)** row lets you set the toggles that newly
created budgets inherit.
The existing *bank-transactions-synced* email toggle was **moved** from
the account page into this new Notifications page, so all email
preferences live in one place.
## UI
`Settings → Notifications`: one "Email notifications" section containing
the bank-sync toggle and a "Budgets" table (rows = Default + each
budget, columns = the three events).
<img width="1265" height="638" alt="budget-notifications-settings"
src="https://github.com/user-attachments/assets/b8386410-98dd-4a5e-9dee-6e5bf7effa31"
/>
## How it works
- Notifications fire from the **live transaction-assignment path**
(`BudgetTransactionService::assignTransaction`, already a queued
listener), never from historical backfill when a budget is created.
- Only the budget's **current period** is considered (the emails
describe the live "available before the limit" state).
- **New transaction** fires for a genuinely newly-assigned transaction;
the email includes the transaction.
- **Close/over** use two atomic per-period flags
(`close_to_limit_notified` / `over_limit_notified`) flipped with a
compare-and-set `UPDATE`, so exactly one email is sent per crossing even
under concurrent queue workers. The flags reset once spending drops back
below the close threshold, so a later crossing can notify again.
- Budgets with no limit (`allocated_amount <= 0`) never send close/over
emails; the "new transaction" email for such a budget omits the
limit/available rows.
- Every email shows the budget's current status (period, spent, limit,
available or over-by). Subcopy links back to the Notifications page.
- New budgets inherit the user's default toggles at creation time.
## Data model
- `budgets`: `notify_on_new_transaction`, `notify_on_close_to_limit`,
`notify_on_over_limit` (default `false`)
- `user_settings`: `budget_notify_on_*` defaults for new budgets
(default `false`)
- `budget_periods`: `close_to_limit_notified`, `over_limit_notified`
(dedup flags)
## Deliberate decisions
- **Threshold is a fixed 90%** for "close to limit" (not configurable) —
kept simple; easy to make per-budget later if requested.
- **"New transaction" sends one email per assigned transaction.** It is
opt-in and defaults to off. A bank sync importing many matching
transactions into an opted-in budget will therefore send several emails;
if this proves noisy we can batch per sync run (mirroring the daily
bank-sync digest). Flagged here rather than pre-building batching.
- **Limit basis is `allocated_amount`** (matches the budget
cards/spending chart), carry-over excluded.
## Tests
- Preference endpoints: page renders budgets + defaults, per-budget
toggle update, cross-user update forbidden, user-settings defaults
update, new budget inherits defaults.
- Sending: new-transaction opt-in/out, over-limit send, close-limit at
90%, no-resend dedup, re-notify after dropping below and crossing again,
no-limit budget skips close/over, historical backfill sends nothing,
notify after enabling the preference while already over. Plus a render
test covering all three email variants.
## Notes
- CI-enforced `es.json` keys for the new page and emails are included.
- All new PHP follows existing mail/queue conventions (`ShouldQueue`
mailable, `emails` queue, `RateLimited` middleware).
## What
Adds an Artisan command to delete a specific user's AI consent by email:
```bash
php artisan ai:delete-consent user@example.com
```
It looks up the user by email and **hard-deletes** all their AI consent
records — as if they had never granted it (no `revoked_at` trail left
behind). Since `AiCategorizationGate` requires `hasActiveAiConsent()`,
AI stops being used for that account immediately (queued categorization
jobs re-check the gate at execution time and early-return).
The command is idempotent: it reports "nothing to do" when there's no
consent on record and fails cleanly when the email doesn't exist.
## Why
Two goals:
1. **Stop using AI for a given user** on request, with no lingering
consent record.
2. **Let users who enabled AI by mistake fall back to a free plan.**
Non-Pro users with active AI consent are pushed toward the paid plan by
`EnsureUserIsSubscribed`. Deleting consent moves them back into the
free-plan-eligible bucket. This billing-gating change is **intended**,
not a side effect.
## Notes / scope decisions
- **Hard delete, not revoke.** Removes every consent row for the user
(all scopes/versions) so there's no trace they ever consented.
- **Prompt flag untouched.** `ai_consent_prompt_dismissed_at` is left
as-is, so the in-app consent banner does not re-appear to nudge them
back on.
- **Live users only.** Deleted (soft-deleted) accounts are out of scope
— they can't use the app, so their consent is moot.
- **No subscription changes.** The command only touches AI consent; it
doesn't cancel Stripe subscriptions (`user:delete` already handles that
separately).
## Tests
New Pest feature test covering all three paths (delete all consent
records incl. a revoked one / no consent on record / user not found).
Green locally.
## What
Adds an artisan command that deletes all transactions and balances of a
specific user's **non-connected** (manual) accounts, identified by
email.
```bash
php artisan user:delete-manual-account-data user@example.com
```
## Behavior
- Looks up the user by email (`withTrashed`, matching `user:delete`).
- Selects accounts with `banking_connection_id IS NULL` (manual
accounts).
- Shows a confirmation prompt with the transaction/balance counts and
the number of affected accounts.
- On confirm, deletes inside a `DB::transaction`:
- Transactions via `forceDelete()` (they use `SoftDeletes`, so this also
purges already soft-deleted rows).
- Balances via `delete()` (`AccountBalance` has no `SoftDeletes`, so
it's a hard delete).
- The accounts themselves are **not** deleted — only their transactions
and balances.
Related `label_transaction`, `budget_transactions` and
`category_corrections` rows are cleaned up by existing
`cascadeOnDelete()` foreign keys.
## Tests
Pest feature test covering: manual-only deletion (connected accounts
untouched), cross-user isolation, soft-deleted transactions counted and
purged, connected-only user reports zero, cancellation, and
user-not-found.
QA skipped per request (backend maintenance command, covered by feature
tests).
## What
Adds label filtering to the `search_transactions` MCP tool. It's the one
filter the web transactions view has that the MCP surface was missing.
- New optional `label_ids` array param. Returns transactions carrying
**any** of the given labels (OR-semantics), matching the web app's label
filter (`Transaction::scopeApplyFilters`).
- Combines with the other filters (account, category, date, amount) with
AND, consistent with how the tool already treats them — so `category_id`
+ `label_ids` means "in that category **and** carrying one of these
labels".
- Each returned transaction now also exposes its `labels` (`id`,
`name`), so the agent can see why a row matched, consistent with what
the write tools already return.
## Validation & consistency
- `label_ids` are validated against the space via `labelsInSpace`,
promoted from `WriteTool` to the shared `McpTool` base so read and write
tools use one implementation. An unknown or stale label id now returns
the same actionable *"call list_labels"* error the rest of the MCP
surface gives, instead of a silent empty result.
- The tool `#[Description]` now mentions `label`, so agents discover the
capability without inspecting each param.
## Tests
- `search_transactions` filters by label id (matching row in, unlabelled
row out).
- A foreign/unknown label id is rejected with an error.
- Full MCP read + write suites stay green (26 tests) — confirms moving
`labelsInSpace` to the base didn't regress the write tools.
Pint, Larastan and the affected Pest suites pass locally.
## Problem
Sentry **PHP-LARAVEL-4Q** fires on POST `/mcp/oauth`: a client presents
a bearer token with a bad/expired/wrong JWT signature ("Token signature
mismatch"), Passport's `TokenGuard` → league/oauth2-server throws
`OAuthServerException::accessDenied` (HTTP **401**). Laravel returns the
correct 401 to the client (`handled: yes`) **but also reports it to
Sentry as an error** — noise, not a server fault. It's recurring (fresh
events after the initial ones).
These OAuth exceptions are, by design, client-facing 4xx protocol
responses (expired/invalid tokens, denied consent, bad grants). Any
public OAuth endpoint — and the MCP endpoint accepts
dynamically-registered Claude/ChatGPT clients — sees a steady stream of
them. They should not page as errors.
`OAuthServerException` does not implement `HttpExceptionInterface`, so
Laravel's built-in `internalDontReport` does not already cover it.
## Fix
Add a `dontReportWhen` rule in `bootstrap/app.php` for
`OAuthServerException` whose HTTP status is **< 500**, mirroring the
existing `MaxAttemptsExceededException` guard right above it.
`serverError` (500) — the only 5xx variant, the genuine "unexpected
condition" case — still reports, so real OAuth-flow faults stay visible.
**Reporting-only change — no user-facing behavior changes.** Clients
still receive the same 4xx OAuth error responses (status/body/headers
unchanged).
## Tests
`tests/Feature/Mcp/McpOAuthTest.php`: assert
`ExceptionHandler::shouldReport()` is `false` for `accessDenied()` (401)
and `invalidCredentials()` (400), and `true` for `serverError()` (500).
## Notes
- Two independent reviews (architecture + product/bug): no must-fix,
both "ship as-is".
- Follow-up (not code): a mass signing-key mismatch would fail as
`access_denied` (401) and thus be suppressed here — the right way to
catch that is an MCP success-rate / 401-rate alert, not error-count.
Worth adding as an alert later.
## Problem
`bun run build` failed whenever `REGISTRATION_ENABLED=false`.
`config/fortify.php` only added `Features::registration()` when the env
flag
was truthy, so with the flag off Fortify never registered the
`/register`
routes. Wayfinder only generates helpers for registered routes, so the
`register` / `register.store` helpers imported by
`resources/js/pages/auth/login.tsx`
and `resources/js/pages/auth/register.tsx` no longer resolved and the
build
broke. The build outcome depended on an env flag — not deterministic.
## Fix
Decouple *route registration* from *whether sign-ups are accepted*:
- **Always register** `Features::registration()` in
`config/fortify.php`, so the
`/register` routes (and their Wayfinder helpers) always exist and the
build is
deterministic regardless of the flag.
- Gate acceptance at **runtime** via a new
`config('auth.registration_enabled')`
value (mapped from `env('REGISTRATION_ENABLED', true)`), the single
source of truth.
- When registration is disabled:
- `GET /register` (Fortify register view) and `POST /register`
(`CreateNewUser`)
return **403**.
- Registration CTAs stay hidden via `canRegister` (login view + landing
page).
- Guests are redirected to `/login`
(`AuthEntryPointService::guestRedirectRoute`).
- With the flag enabled (the default), registration behaves exactly as
before.
## Tests
- Rewrote `tests/Feature/Auth/RegistrationDisabledTest.php` to the
runtime
mechanism: registration enabled by default, route helpers always
registered
regardless of the flag, `GET`/`POST /register` return 403 when disabled
(and no
user is created), and the landing/login CTAs hide.
- Updated the `DashboardTest` disabled-registration case to toggle
`config('auth.registration_enabled')` instead of filtering the Fortify
feature.
- Verified `bun run build` succeeds with `REGISTRATION_ENABLED=false`
and that the
`register` / `register.store` Wayfinder helpers are generated.
## Docs
- Updated `README.md` and `.env.example` to describe the 403 behavior
(routes stay
registered rather than being removed).
## Summary
Closes#716.
Both AI execution paths hard-coded `provider: Lab::Gemini`, so even
though `laravel/ai` already understands `OLLAMA_URL` and the model was
env-overridable, no other provider could ever be reached by the actual
UI features. This makes the **AI provider configurable**, keeping
**Gemini as the default** so existing deployments are unaffected.
Although issue #716 asked specifically for **Ollama**, the fix is
generic: because the provider is resolved through the
`Laravel\Ai\Enums\Lab` enum, this unlocks **any text provider
`laravel/ai` supports** — `gemini`, `openai`, `anthropic`, `azure`,
`groq`, `xai`, `deepseek`, `mistral`, and self-hosted `ollama`. Ollama
is the headline case (fully local, private processing), but nothing in
the code is Ollama-specific.
> Note: each provider still needs its own credentials configured for
`laravel/ai` (e.g. `GEMINI_API_KEY`, `OPENAI_API_KEY`, `OLLAMA_URL`),
and only text-capable providers apply — a non-text or unknown provider
fails fast.
## Changes
- **`config/ai_suggestions.php` / `config/ai_categorization.php`** — add
a `provider` key. Each reads its own `AI_SUGGESTIONS_PROVIDER` /
`AI_CATEGORIZATION_PROVIDER`, both falling back to a shared
`AI_PROVIDER` and finally `gemini`. So `AI_PROVIDER=<provider>` flips
every AI feature at once, and either feature can still be overridden
individually.
- **`app/Services/Ai/CategorizeTransactions.php` /
`app/Services/Ai/LaravelAiRuleSuggestionGenerator.php`** — resolve the
configured provider to the `Laravel\Ai\Enums\Lab` enum via
`Lab::from((string) config('...provider'))` and pass it to `prompt()`
(the SDK recommends referencing providers by the `Lab` enum rather than
a plain string). `Lab::from()` also **validates** the value: an unknown
provider fails fast with a clear `ValueError` instead of erroring deep
in the provider stack.
- **`.env.example`** — document the provider vars plus an Ollama block
(`OLLAMA_URL`, `OLLAMA_API_KEY`, `AI_CATEGORIZATION_MODEL`), kept
commented so defaults stay Gemini.
- **`README.md`** — new *AI Provider* section: the generic provider
switch, the list of supported text providers, and a local-Ollama
example.
- **Tests** — cover the `gemini` default and a non-Gemini (`ollama`)
override for both the categorization and rule-suggestion paths, plus the
fail-fast `ValueError` on an unknown provider.
## Usage
Any supported provider follows the same pattern — set `AI_PROVIDER`,
that provider's credentials, and the `*_MODEL` vars. Example, fully
local/private with Ollama:
```dotenv
AI_PROVIDER=ollama
OLLAMA_URL=http://ollama.example.local:11434
AI_SUGGESTIONS_MODEL=gemma3:12b
AI_CATEGORIZATION_MODEL=gemma3:12b
```
## Validation
- `./vendor/bin/pest tests/Feature/Ai` → **128 passed**.
- `vendor/bin/pint` and `vendor/bin/phpstan` (level 5) → clean.
- **End-to-end against a real Ollama server** (`gemma3:12b`), through
the actual application code (not faked):
- Categorization: `MERCADONA COMPRA` → *Groceries*, confidence 0.95.
- Rule suggestion: `netflix` → *Subscriptions*, structured output
intact.
## Backward compatibility
Default provider is unchanged (`gemini`); no env changes are required
for existing installs.
## What & why
The `HIDE_AUTH_BUTTONS` flag and its signed-link auth-override existed
only to gate registration/login behind a waitlist during the launch
period. That period is long over, so this removes the flag and the whole
waitlist apparatus. Registration and login are now always open, still
governed by `REGISTRATION_ENABLED` (Fortify's registration feature),
which is untouched.
## Removed
- **`HIDE_AUTH_BUTTONS`** config/env and
**`LandingAuthOverrideService`** — the signed-link override, override
cookie, and `?force=` bypass — plus its
`GenerateLandingAuthLinkCommand`.
- **Waitlist lead-capture + invitation apparatus**: `UserLeadController`
+ `waitlist.*`/`user-leads.*` routes, `StoreUserLeadRequest`, the
landing `WaitlistForm`, `waitlist/*` pages, waitlist/invitation mails,
invitation & re-invitation commands, the lead verification notification,
`LeadCohort`/`LeadCohortResolver`/`LeadPromoCodeAllocator`, and the
lead→Resend segment sync (scheduled tasks removed too).
- Frontend `hideAuthButtons`/`forcedRegistration` threading and the
`?force=` query on auth links (header, welcome, register, login).
- All tests for the removed features; `landing.hide_auth_buttons` config
stubs stripped from surviving tests.
## Kept on purpose (data preservation)
The `user_leads` **table, its migrations, and a lean `UserLead` model
are retained** — no destructive migration. Pre-launch contacts are not
lost, and new signups whose email matches a preserved lead still receive
that lead's promo code at Stripe checkout
(`SubscriptionController::resolveLeadPromotionCodeId`, unchanged).
## Testing
- Full non-Browser Pest suite green locally (**1948 passed**), plus
Pint, Prettier, ESLint.
- Browser QA (Playwright) on the running app: landing no longer shows
the waitlist form and renders the register/login CTAs; `/register`
renders the form (previously returned null when hidden); `/login` shows
the "Sign up" link pointing to `/register` with no `?force`. 16/16
functional checks passed; only pre-existing external-resource 404s in
console (gravatar `d=404`, `via.placeholder.com` seed images).
## Demo
https://github.com/user-attachments/assets/18a70f6d-b82f-4d92-b9de-da2610dc017a
## Summary
Adds a dedicated `REGISTRATION_ENABLED` env var (default `true`) so
self-hosters on a publicly reachable instance can **close public
sign-ups while keeping login open** — the exact "sign-ups closed, login
open, permanently" case from #711 that `HIDE_AUTH_BUTTONS` can't cover.
Closes#711.
## What changed
- **`config/fortify.php`** — the Fortify `registration()` feature is now
conditional on `env('REGISTRATION_ENABLED', true)`. When disabled:
- Fortify **never registers the `/register` routes** (GET form + POST
submit → `404`), i.e. a real server-side block, not just a hidden
button.
- The existing `canRegister =
Features::enabled(Features::registration())` flag flips to `false`
automatically everywhere it's used (`routes/web.php`,
`FortifyServiceProvider`, login page).
- **`resources/js/pages/welcome.tsx`** — the hardcoded `/register` CTAs
(hero button, Free/paid pricing cards, final "Ready to take control"
CTA) now respect `canRegister`, falling back to a `Log in` → `/login`
button when sign-ups are closed. The header already gated its `Register`
button on `canRegister`.
- **`.env.example` / `README.md`** — document the new variable.
**Backward compatible / optional:** `env('REGISTRATION_ENABLED', true)`
defaults to `true`, so existing installs that upgrade without setting
the var keep public sign-ups exactly as today. It's shown as a
commented, optional example in `.env.example`.
`/login` and the "Iniciar sesión / Log in" button stay fully available
in all cases. `AuthEntryPointService::guestRedirectRoute()` already
redirects guests to `/login` when registration is disabled, so no dead
`route('register')` calls remain.
## Tests
New `tests/Feature/Auth/RegistrationDisabledTest.php`:
- registration feature present by default / removed when
`REGISTRATION_ENABLED=false` (config-level).
- landing page exposes `canRegister=true` by default and `false` when
the feature is disabled.
- login page hides its sign-up link when registration is disabled.
- **`/register` GET + POST return 404 and the named routes are gone**
when disabled (via `refreshApplication()` with the env set), while
`/login` still returns 200.
All new tests pass; the wider `tests/Feature/Auth` + landing-override
suites stay green (the one unrelated `Asia/Calcutta` legacy-timezone
failure is a pre-existing tzdata quirk of the CI-less local box, not
touched here). `pint`, `prettier --check`, and `eslint` are clean.
## Manual QA
Verified against a running instance (headless Chromium) in both states:
| `REGISTRATION_ENABLED=true` (default) | `REGISTRATION_ENABLED=false` |
| --- | --- |
| Header: `Log in` + `Register`; hero CTA `Get Started` → `/register` |
Header: `Register` gone, only `Log in`; hero CTA `Log in` → `/login` |
| `GET /register` → `200` | `GET /register` → `404`, `GET /login` →
`200`, `Route::has('register')` → `false` |
## Problem
Connecting the MCP connector from **ChatGPT on Android** fails. The
installed Whisper Money PWA is a Chrome **WebAPK** that auto-verifies as
an Android **App Link handler** for the whole app origin (manifest
`scope: "/"`), so `https://whisper.money/oauth/authorize` gets routed
**into the app**. Once inside the standalone app, the redirect back to
the OAuth client can't complete → the connection fails. (Claude works
because it opens OAuth in a Custom Tab.)
Confirmed on an Android emulator: the WebAPK shows `AutoVerify=true`,
`whisper.money: verified`. DB evidence: ChatGPT registers + reaches
consent (auth codes issued) but never exchanges a token.
## Why not `handle_links`
`handle_links: "not-preferred"` (tried in #707, reverted in #708) is
**origin-wide** — it would push *every* `whisper.money` link (bank-auth
callback, email verification, shared deep links) to the browser, not
just `/oauth`. We want links to keep opening the installed app.
## Fix (surgical)
Move the OAuth **authorization server** to a dedicated host outside the
PWA scope. `config('mcp.authorization_server')` becomes env-driven
(`MCP_AUTHORIZATION_SERVER`); in prod → `https://oauth.whisper.money`
(DNS already points at the same app).
Every endpoint derives from the request host (no forced root URL), so
pointing the auth server at the subdomain makes `issuer` +
`authorize`/`token`/`register` all resolve to `oauth.whisper.money` —
**same origin as each other**, no cross-origin metadata mismatch. The
protected resource (`/mcp/oauth`) and **all other app links stay on
`whisper.money`**, so deep-linking into the app is fully preserved. Only
the OAuth flow leaves the app — into the browser, where the round-trip
completes.
## Activation (after merge + deploy)
1. Set `MCP_AUTHORIZATION_SERVER=https://oauth.whisper.money` in prod
env, redeploy.
2. I'll curl the discovery chain to confirm it resolves to the
subdomain.
3. Test the ChatGPT connect on a real phone.
Safe until step 1: env unset → `authorization_server` stays `null` →
current behavior. No effect on local/dev.
## Tests
Added a Pest test: with `mcp.authorization_server` configured,
protected-resource metadata advertises the dedicated host and
auth-server metadata (fetched from that host) keeps `issuer` + all
endpoints on it. App has no `TrustHosts` restriction (already serves the
subdomain) and `SESSION_DOMAIN=null` (host-only cookies — subdomain gets
its own session, no security downgrade).
## Problem
A user reported being stuck forever on the onboarding "Looking for
patterns" step (the "Polishing your suggestions…" spinner). Confirmed in
prod: their `suggestion_runs` row has been stuck in `processing` for a
week, and `failed_jobs` shows `TimeoutExceededException:
GenerateRuleSuggestionsJob has timed out`.
### Root cause
- The user has 4,163 transactions — the largest onboarding dataset (max
with a completed run was 3,481). The Gemini generation exceeded the
job's `$timeout = 120`.
- The worker timeout is raised **outside** the `try/catch` in
`GenerateRuleSuggestions::run()`, so the code that sets `status =
Failed` never ran.
- `GenerateRuleSuggestionsJob` had **no `failed()` handler** (5 other
jobs do), so nothing flipped the run to `Failed`. It stayed
`processing`, `show()` kept returning `processing`, and the client
polled every 3s forever.
- The timeout never reached Sentry, so it was invisible. Only 1 user
affected, but it's a latent bug that recurs on any large account.
## Fix
**Backend** — add `failed()` to `GenerateRuleSuggestionsJob`: mark the
run `Failed` (only if not already terminal) and `report()` the exception
so these surface in Sentry.
**Frontend** (`step-ai-suggestions.tsx`) — belt-and-suspenders so the
client never spins forever even if the worker dies before it can mark
the run:
- `poll()` now guards against transient errors (network blip / expired
session / 5xx) instead of silently dying.
- A client-side deadline (3 min, just beyond the "up to two minutes"
copy) surfaces the existing "couldn't generate" screen with **Try again
/ Skip**.
Not included: raising the timeout / capping the number of groups sent to
Gemini so large accounts *complete* instead of just being able to skip.
Follow-up if we want big accounts to get suggestions.
## Tests
- `tests/Feature/Ai/GenerateRuleSuggestionsJobTest.php` — `failed()`
marks the run `Failed` + reports; doesn't overwrite an already-terminal
run.
-
`resources/js/components/onboarding/step-ai-suggestions-timeout.test.tsx`
— after the deadline the spinner gives way to the failed screen.
Green locally: pint, prettier, eslint, backend + frontend tests.
## What & why
Import configuration (the CSV/Excel column mapping and date format a
user
sets up per account) was stored in the browser's `localStorage`, so it
never
followed the user to another device. Importing the same account's
statement on
mobile — or on a new machine — meant reconfiguring the mapping from
scratch.
This moves that configuration to the backend, keyed per account and per
import
type, so it's preconfigured and loaded automatically wherever the user
imports.
## Demo
Cross-device QA: import transactions on "device 1" with a custom column
mapping, then clear **all** cookies + local/session storage (simulating
a
fresh device), log back in, and start a new import — the mapping
(Description → *Movement*, Amount → *How Much*, date format
*DD-MM-YYYY*)
is auto-loaded from the backend with no manual setup.
https://github.com/user-attachments/assets/e449dfe9-3970-48d4-97ca-9e415c73835b
## How
- New `account_import_configs` table: one row per `(account_id, type)`
where
`type` is `transaction` or `balance`. The mapping + date format are
stored as
an opaque `config` JSON blob (exactly what the client sends).
- `GET/PUT /api/accounts/{account}/import-config`
(`AccountImportConfigController`),
authorized via the existing `AccountPolicy` (`view`/`update`) — mirrors
`AccountBalanceController`. `PUT` upserts on `(account_id, type)`.
- Frontend: the two import-config storage helpers now read/write the
endpoint
via `axios` instead of `localStorage` (merged into a single module — the
transaction and balance variants shared the same logic).
- The saved config is fetched **off the file-parse critical path**: the
parsed
file shows immediately with auto-detected columns, and the saved mapping
is
applied when it arrives (guarded so a slow load can't clobber a file
picked
afterwards). A slow or hanging request never blocks the preview or Next
button. When no config exists or the request fails, it falls back to
auto-detection exactly as before.
## Notes / decisions
- **No localStorage migration.** Existing per-device configs aren't
migrated;
on the next import the mapping is auto-detected and re-saved to the
backend,
so it self-heals after one import. The old `import_config_account_*`
keys are
simply no longer read.
- The mapping-validity check against the actual file headers stays
client-side
(it depends on the just-parsed file).
## Testing
- `tests/Feature/AccountImportConfigTest.php` (9 tests): auth required,
cross-user 403 on read and write, save→persist→load round-trip, upsert
de-duplication, transaction/balance independence, and validation
(unknown type, missing column mapping).
- Sibling suites green (`AccountBalanceControllerTest`,
`SavedFilterTest`) —
no regression from the new `Account::importConfigs()` relation or
routes.
> **Stacked on #696.** That PR introduced the AI-categorization upgrade
dialog this one generalizes. It targets `main` so CI runs, so until #696
merges this PR's diff also contains #696's commit — review/merge #696
first, then this diff resolves to just its own three commits.
## What & why
The contextual "this is a paid feature → pick a plan → checkout" modal
built for AI categorization is now a **shared component** reused at two
more Pro-feature entry points, and every checkout it starts is
**attributed to the upsell point** so we can measure revenue per point.
### 1. Reusable upgrade modal
Extracted `AiUpgradeDialog` (+ `PlanCard`) out of `settings/billing.tsx`
into a shared `UpgradeDialog`
(`resources/js/components/subscription/upgrade-dialog.tsx`). It reads
`pricing`/`locale` from `usePage`, takes `title` / `description` /
`source`, renders the plan picker, and links to Stripe checkout. Used
at:
| Point | Trigger | Copy |
|---|---|---|
| AI categorization | Manage Plan toggle (unchanged) | "AI
categorization is a paid feature" |
| **Connections** | "Connect Bank" | "Bank connections are a paid
feature" |
| **Connected accounts** | Create Account → "Connected" | "Connected
accounts are a paid feature" |
Both new points already gated on `isFreePlan`, so the modal only shows
to free users. The old plain `UpgradeConnectionDialog` (which just
routed to billing) is deleted.
### 2. Revenue attribution
The upsell `source` is captured two ways:
- **Intent** — a PostHog `upgrade_checkout_started` event (`{ source,
plan }`) fires on click, matching the repo's existing `{ source }` event
convention.
- **Revenue** — `source` rides the checkout URL (`?plan=&source=`), and
`SubscriptionController::checkout` validates it against the new
`App\Enums\UpsellSource` enum and attaches it as Stripe **subscription
metadata** (`withMetadata`). When the subscription webhook lands,
`PersistUpsellSourceFromStripe` (on Cashier's `WebhookHandled`, so the
row already exists) copies it onto a new `subscriptions.upsell_source`
column — **write-once** (`whereNull`), so a later `subscription.updated`
never overwrites the original attribution.
Measuring revenue per point is then a group-by on
`subscriptions.upsell_source` joined to invoices, using existing local
tooling.
### Scoping notes (from review)
- **Paywall & the billing on-page upgrade button are intentionally left
untagged** (`upsell_source = NULL`). "Upsell source" means a
*feature-gate nudge*; the paywall/billing pages are the baseline upgrade
surface, not a nudge — so they form the null/baseline bucket rather than
getting their own enum value.
- The listener is **synchronous** (not `ShouldQueue` like the sibling
Discord listener) on purpose: a single indexed, idempotent `UPDATE`
doesn't warrant a queue-worker dependency for attribution.
- The PostHog event fires just before a full-page navigation; posthog-js
flushes via beacon but the event (analytics only, not the revenue source
of truth) could occasionally be lost. Cheap to harden later if the
funnel proves lossy.
## Tests
- `upgrade-dialog.test.tsx` — renders per-feature copy, checkout link
carries `plan` + `source`, click fires the PostHog event.
- `SubscriptionTest` — checkout tags valid sources as Stripe metadata
and ignores unknown ones.
- `PersistUpsellSourceFromStripeTest` — persists from webhook metadata,
doesn't overwrite an existing attribution, ignores unknown/absent
sources.
## Demo
Free user hitting both new upsell points (Connections → "Connect Bank",
Accounts → "Connected"):
<!-- 📎 PLACEHOLDER: drag in
~/Downloads/upsell-points-connections-accounts.mp4 -->
https://github.com/user-attachments/assets/a27435d9-2296-4dc9-ae7f-753b6f7950f0
> Note: the clip ends at the modal (doesn't cross into Stripe) because
this local dev env has a pre-existing Stripe tax-rate 500 on
`/subscribe/checkout`, unrelated to this change. The `source` reaching
the checkout URL is verified in the browser and by the tests.
## What
Several related tweaks to the subscription paywall shown before a user
subscribes:
1. **Remove the Balance stat** from the top stat card.
2. **Mobile dismiss (X)** replacing the bottom "Continue for free"
button on small screens (when the free plan is available).
3. **Support button** when the paywall *can't* be skipped.
4. **Copy updates** to the social-proof slider.
## 1. Remove the Balance stat
The Balance column showed summed account balances per currency (e.g.
`58.031 MXN 65 US$`). As a variable-length, multi-currency string it
overflowed the stat card on mobile and broke the four-column layout. The
remaining stats (Accounts, Transactions, Categories) are short integer
counts, so it now reads as a clean three-column row.
- `SubscriptionController@getUserStats`: also drops the
`balancesByCurrency` computation — which ran an **N+1 query** (one
`AccountBalance` lookup per account) — and the never-rendered
`automationRulesCount`.
## 2. Mobile dismiss (X)
The "Continue for free" escape (shown only when `canUseFreePlan`) now
renders on **mobile** as a dismiss **X** fixed to the top-right corner
instead of a full-width bottom button, matching the common
mobile-paywall pattern. Same 5s delayed fade-in, same action (continue
on the free plan). On **desktop (md+)** the bottom button is unchanged.
Reuses the app's `MobileBackButton` treatment (44px target, rounded
pill, border/shadow/backdrop-blur) for legibility.
## 3. Support escape hatch (`!canUseFreePlan`)
When the user has **no** free-plan escape, the paywall previously
offered no way out. It now shows a subtle **help/support** affordance in
the same slots the free-plan escape uses — bottom button on desktop,
top-right corner on mobile. It fades in slightly later than the
free-plan escape (**7s vs 5s**) and is deliberately low-key (muted
ghost, no pill) so it doesn't compete with the subscribe CTA. It opens
the existing `SupportDialog` (join the community / email support) — the
same one behind the user-menu "Support" entry.
The two escapes are mutually exclusive per page load, so the free-button
timer collapses into one `escapeVisible` timer whose delay depends on
`canUseFreePlan`.
## 4. Copy updates
Shortened the social-proof lines (`taking control of their finances` →
`trusting us`, etc.), bumped the user count to `2,500+ users`, slightly
reduced the proof icon. `lang/es.json` updated to match.
## QA
Real browser QA across all four states (see Demo), each ending by
exercising the actual action:
- **With free plan** → the X (mobile) / "Continue for free" (desktop)
navigates to `/dashboard`.
- **Without free plan** → the "Need help?" button opens the support
modal (Join the community / Email support).
No console errors from the paywall.
## Demo
**Desktop — with "Continue for free":**
<!-- PLACEHOLDER: drag paywall-qa-desktop-with-free.mp4 here -->
https://github.com/user-attachments/assets/f3c943d8-5dfd-4b08-8f94-1683d38b11f7
**Desktop — without "Continue for free" (support button):**
<!-- PLACEHOLDER: drag paywall-qa-desktop-no-free.mp4 here -->
https://github.com/user-attachments/assets/f7542b41-a0d1-491e-ab8b-4734d1c77af2
**Mobile — with "Continue for free":**
<!-- PLACEHOLDER: drag paywall-qa-mobile-with-free.mp4 here -->
https://github.com/user-attachments/assets/6265ab86-cea2-4ade-9720-17ea8b003b1c
**Mobile — without "Continue for free" (support button):**
<!-- PLACEHOLDER: drag paywall-qa-mobile-no-free.mp4 here -->
https://github.com/user-attachments/assets/e53d68ee-3ff9-4091-bba8-028489dd0b8d
## MCP Phase 3 — OAuth 2.1 for Claude Desktop/web & ChatGPT connectors
Phase 1 shipped a read-only MCP server (#689); Phase 2 added write tools
+ the read/read_write token scope (#690). This phase adds **OAuth 2.1
(Authorization Code + PKCE)** so Anthropic's Claude Desktop/web custom
connectors and OpenAI's ChatGPT connectors can authenticate — those
clients sign in with OAuth rather than pasting a static bearer token, so
until now they only saw a "coming soon" note.
It reuses `laravel/mcp`'s built-in OAuth support (inert until Passport
is installed) wired to `laravel/passport ^13`. We do not hand-write the
authorization server, discovery endpoints, DCR endpoint, or the
`WWW-Authenticate` challenge — the package provides all of it.
### What's in it
- **`laravel/passport ^13`** + an `api` (passport) guard alongside the
existing session `web` guard; Passport migrations (UUID user columns),
config, and signing keys.
- **`Mcp::oauthRoutes()`** — RFC 8414/9728 discovery, RFC 7591 DCR
(`oauth/register`), and the `mcp:use` scope.
- **A second MCP endpoint `POST /mcp/oauth`** guarded by `auth:api`. The
existing Sanctum `/mcp` endpoint (Claude Code static PAT) is left 100%
unchanged.
- **On-brand OAuth consent screen** (Blade, light + dark, localized)
naming the connecting client and its redirect host, and stating plainly
what the connection can do (read/analyse + make changes, bank-connected
data excepted).
- **Settings UI**: the "Claude Desktop & ChatGPT" block now shows real
connect instructions (the `/mcp/oauth` URL to add as a custom connector,
no token needed) instead of "coming soon".
## Decision #1 — OAuth connections have read + write access
`laravel/mcp` advertises and uses a single `mcp:use` scope; it has no
read/write granularity, so there is no per-connection scope choice over
OAuth. **OAuth connections get full read + write access**, gated by the
user explicitly approving the connection on the Whisper Money consent
screen. (An earlier revision made them read-only; that restriction has
been lifted per request.)
`WriteTool` (`app/Mcp/Tools/WriteTool.php`) grants writes when the
request resolves through the `api` (Passport) guard **or** carries a
Sanctum `mcp:write` ability; a read-only Sanctum PAT is still rejected.
Bank-connected accounts and their transactions remain read-only for
every caller (only manual data can be created/edited/deleted; any
transaction can still be categorised/labelled). The consent screen and
settings copy state the read + write capability and the bank-connected
exception.
Possible follow-up: a consent-time read-only/read-write toggle, if
per-connection granularity is wanted (not offered by the standard MCP
OAuth flow's single scope).
## Other locked decisions
- **Route topology**: a separate `/mcp/oauth` endpoint rather than
multi-guarding `/mcp`. Keeps the Claude Code path unchanged (its
`abilities:mcp:read` gate would 403 an OAuth `mcp:use` token) and gives
each client type a clean documented URL. The package's nested discovery
`/.well-known/oauth-protected-resource/mcp/oauth` returns `resource =
url('/mcp/oauth')`.
- **Registration**: ship DCR (`oauth/register`). Redirect allowlist
tightened to `https://claude.ai` and `https://chatgpt.com` only — no
wildcard. CIMD is a possible later enhancement; both clients accept DCR.
## Deviation from the original plan — the User model is untouched
The plan proposed aliasing Passport's `HasApiTokens` trait alongside
Sanctum's (with `insteadof`/`as`) and implementing `OAuthenticatable`.
**Both are impossible here and, it turns out, unnecessary:**
- The two `HasApiTokens` traits declare an **incompatible `$accessToken`
property** (Sanctum untyped vs Passport `?ScopeAuthorizable`), which is
a hard PHP fatal that `insteadof` cannot resolve (it only resolves
methods).
- `OAuthenticatable::tokens(): HasMany` is incompatible with Sanctum's
canonical `tokens(): MorphMany`, and the Claude Code PAT suite depends
on Sanctum's `tokens()`. The interface is never enforced at runtime by
Passport (docblock-only).
- Passport's resource guard only calls `$user->withAccessToken()`, which
Sanctum already provides (untyped, so it accepts the Passport
`AccessToken`); and Passport's `AccessToken::can()` makes
`tokenCan('mcp:write')` behave correctly for OAuth tokens. So Sanctum
stays canonical and the Claude Code PAT path is genuinely unchanged.
## Signing keys (deploy note)
Passport signs OAuth tokens with a key pair. This PR provisions it
everywhere it's needed: CI (`passport:keys` before tests), the
production Docker entrypoint (generates into the persisted `storage/`
volume unless provided via `PASSPORT_PRIVATE_KEY`/`PASSPORT_PUBLIC_KEY`
env), `worktree.sh`, and a documented `.env.example` entry. **For a
multi-instance deployment, set `PASSPORT_*` env** so every instance
validates tokens with the same key.
## Tests (`tests/Feature/Mcp/McpOAuthTest.php`)
Discovery metadata (RFC 9728/8414), the mandatory **401 bootstrap**
challenge + `WWW-Authenticate` header, DCR (allowed + rejected redirect
URIs), the full **Authorization Code + PKCE** flow reaching a read tool,
and **write access over OAuth** (an OAuth connection calling
`create_label` succeeds and the row is created). The existing
`McpTokenTest` / `Mcp/*` suites (incl. the read-only Sanctum PAT
guardrail) and `LocalizationTest` still pass unchanged.
## QA
- **Protocol** (curl, over HTTPS): both discovery endpoints return the
exact required JSON; unauthenticated `POST /mcp/oauth` returns `401` +
`WWW-Authenticate: Bearer …
resource_metadata="…/.well-known/oauth-protected-resource/mcp/oauth"`;
DCR accepts `claude.ai`/`chatgpt.com` callbacks and rejects others with
`400 invalid_redirect_uri`.
- **Browser**: consent screen verified in light and dark mode (client
name, signed-in email, redirect host, read + write capability +
bank-connected read-only note, Cancel/Connect); updated settings page
verified. No JS errors.
- Full PKCE token exchange + a write tool call is covered by the green
Pest e2e test.
## Fast-follows (not in this PR)
- **"Connected apps" revoke UI** — `McpTokenController` manages only
Sanctum PATs today, so there's no in-app revoke for OAuth grants yet.
The consent copy says "disconnect from the connected app" for now; a
Passport-grant list + revoke is the top follow-up (more important now
that OAuth grants can write).
- CIMD registration; optional consent-time read-only/read-write toggle.
## Stacking
Was developed stacked on `mcp-write-tools` (#690), itself on #689.
**Both have since merged to `main`**, so this branch was rebased onto
`main` (`git rebase --onto origin/main mcp-write-tools`) and targets
`main` directly.
## MCP Phase 2 — write tools
> **Stacked on #689** (`mcp-functionality`). Base this PR on
`mcp-functionality`, not `main`, and merge it **after** #689.
Phase 1 shipped a read-only MCP server for Pro accounts. This adds the
**write** surface and re-enables the read/read-write token scope the UI
dropped in PR1.
### Write tools
A new `WriteTool` base extends `McpTool`: on top of the Pro-plan gate it
requires the calling token to carry `mcp:write`, returning a clear error
for read-only tokens. Each concrete tool is annotated `#[IsDestructive]`
(PHP attributes aren't inherited, so the annotation lives on each tool,
not the base — a docblock on `WriteTool` notes this).
- `create_transaction` — manual (non-connected) accounts only; forces
`source = manually_created`.
- `update_transaction` / `delete_transaction` — manually-created
transactions only; bank/imported ones stay locked.
- `categorize_transaction` — sets/clears the category on **any**
transaction (imported included), marking it `category_source = manual`.
- `label_transaction` — add/remove labels on **any** transaction.
- `create_balance` — balance snapshot on manual accounts only.
- `create_category` / `update_category` / `delete_category` — mirrors
the settings controller (parent/depth/cycle rules, cashflow derivation,
child strategies).
- `create_label` / `update_label` / `delete_label`.
- `create_automation_rule` / `update_automation_rule` /
`delete_automation_rule` — JsonLogic conditions + category/label
actions, at least one action required.
- `list_labels` — a small **read** tool added so label ids are
discoverable (label/automation tools are unusable without it).
### Guardrails
Write tools never touch bank-sourced data: the existing
`TransactionSource` enum and `Account::isConnected()` are the barriers,
reused not reinvented. There is no server-side write confirmation
(client-controlled, accepted decision) — hence `#[IsDestructive]`.
### Token scope
`StoreMcpTokenRequest` re-adds `scope` (`read` | `read_write`); the
controller grants `['mcp:read']` or `['mcp:read', 'mcp:write']`. The
settings page gets its scope selector back with honest copy (new strings
added to `lang/es.json`). The `/mcp` route stays gated on
`abilities:mcp:read` — any MCP token can connect and read; the per-tool
`mcp:write` check is what blocks writes.
### Tests
Happy path + guardrail failures for every write tool, the
read-only-token rejection (via a real read-only PAT so the `tokenCan`
gate runs exactly as over HTTP), cross-user isolation, the inherited Pro
gate, and read/read_write scope validation.
### Notes
- `AutomationRule::labels()` gained a generic return annotation (needed
for larastan level 5 on the new label mapping).
### Verification
- `vendor/bin/pint --test` ✅
- `vendor/bin/phpstan analyse` (larastan level 5) — 0 errors ✅
- `php artisan test tests/Feature/Mcp
tests/Feature/Settings/McpTokenTest.php
tests/Feature/LocalizationTest.php` ✅
- `prettier --check` / `eslint` on `settings/mcp.tsx` ✅
## What & why
Adds a **read-only MCP server** so a paid ("Pro") user can connect
Whisper Money to their own AI assistant (Claude web/desktop, Claude
Code, ChatGPT) and analyse their own finances — spending, cashflow, net
worth, transactions.
This is **Phase 1 (PR1): read-only**. Write tools (create/edit/delete
transactions, categories, labels, rules, balances) are a deliberate
follow-up (PR2); the token plumbing already reserves an `mcp:write`
ability for them.
## How it works
- **Transport:** remote streamable HTTP server via `laravel/mcp`,
mounted at `/mcp` (`routes/ai.php`).
- **Auth:** Sanctum personal access tokens with **MCP-only abilities**
(`mcp:read`). The route is gated by `auth:sanctum` +
`abilities:mcp:read` + `throttle:60,1`, so a future public-API token
(different ability) can't reach it and vice versa.
- **Pro gating** is enforced **per request inside the tools**
(`User::canUseFeature(PlanFeature::McpAccess)`), so a lapsed
subscription stops working on its own without the user revoking the
token. Free users can still create tokens (marked **PRO** in the UI) but
every call returns a "paid plan required" error with an upgrade URL.
- **Consent:** connecting is the consent — a clearly-weighted
data-egress disclaimer + per-client connection instructions on the
settings page. No separate checkbox (by design).
## Tools (all read-only)
| Tool | Scope |
|------|-------|
| `search_transactions` | space-scoped (optional `space`, defaults to
personal) |
| `list_accounts`, `list_categories`, `list_spaces` | space-scoped |
| `spending_by_category`, `get_cashflow`, `get_net_worth` | user's whole
account (reuse existing analytics services/controllers) |
Recurring-charge detection is left to the agent over
`search_transactions` results (no dedicated tool).
## Settings → MCP access
New page to create / rotate / revoke tokens (name + one-time secret
reveal), with `last_used_at`, a PRO badge, the egress disclaimer, and
copy-paste connection instructions for Claude (web/desktop), Claude Code
and ChatGPT.
## Tests
- Tool behaviour + Pro gating + **cross-user / cross-space isolation**
(`tests/Feature/Mcp/McpToolsTest.php`).
- HTTP auth boundary: 401 without a token, 403 without `mcp:read`, 200
with it (`tests/Feature/Mcp/McpEndpointAuthTest.php`).
- Token CRUD + ownership + free-tier creation
(`tests/Feature/Settings/McpTokenTest.php`).
## Reviewed & adjusted
Ran technical + product reviews and applied the fixes: kept PR1 strictly
read-only (dropped a UI scope selector that promised non-existent
write), routed gating through the `PlanFeature` convention, put token
rotation behind a confirmation, removed a silent on-load clipboard copy,
weighted the egress disclaimer, and fixed a `list_spaces` N+1.
### Known, deliberate tradeoffs
- `get_cashflow` / `get_net_worth` / `spending_by_category` reuse the
existing **user-scoped** analytics controllers/services, so they cover
the whole account rather than a single space (documented in the server
instructions). Per-space analytics is a follow-up.
- Space tools scope by `space_id` gated by membership
(`accessibleSpaces`) — the intended shared-tenant model — rather than a
per-row `user_id` filter.
## Not runnable in this environment
Browser QA of the settings page wasn't run here (no local
`node_modules`); that surface relies on CI build/typecheck/lint and
follows existing settings-page conventions.
---
## Updates since opening
- **Behind a feature flag.** New `App\Features\Mcp` (default off) hides
the whole settings screen — the nav item and every `settings/mcp*` route
(404 when off). Pro-plan gating still happens per request. Enable it
with `php artisan feature:enable "App\Features\Mcp" <email|all|25%>`.
- **Renamed** the user-facing page from "MCP access" to **"AI
Connector"** (nav, title, breadcrumb) so non-technical users understand
it. Route names, files and the feature stay internal.
- **Shared `ProBadge`** component (amber), now used on both the AI
Connector and billing pages instead of an inline badge.
- **Softer data-egress notice** (amber shield icon instead of a red
alert) and plainer copy throughout.
- **Accurate connection instructions (important).** Verified against the
official docs: a personal access token works with **Claude Code** today.
**Claude Desktop** and **ChatGPT** custom connectors authenticate over
**OAuth** and do not accept a static token, so they're now marked
**"coming soon"**. OAuth is the real unlock for those clients and is the
recommended follow-up (it also maps to the deferred write-tools work).
Sources: [Claude custom
connectors](https://support.claude.com/en/articles/11175166-get-started-with-custom-connectors-using-remote-mcp),
[ChatGPT developer
mode](https://developers.openai.com/api/docs/guides/developer-mode).
- UI reviewed in a real browser; layout/alignment checked across states
(empty, new-token reveal, token list).
## What
`encryption:notify-removal` now prints, on every run, the total number
of **non-deleted** users who still hold legacy browser-encrypted data:
```
4 non-deleted user(s) still have encrypted data.
```
The count is taken before the billing exclusion, so it reflects the full
remaining scope of browser-encrypted data. Soft-deleted users are
excluded (model default scope), as are users whose data is already
plaintext.
## Why
It's the signal for deciding when the browser-side encryption code can
finally be removed: once this reaches zero, nothing in the app depends
on client-side encrypted columns anymore.
## Not changed
Email-sending logic is untouched. Subscribed users are still never
warned — the recipient set continues to go through
`excludeBilledUsers()` exactly as before. This PR only adds a reporting
line.
## Testing
- New test asserts the count spans everyone with encrypted data
(subscribed included), while excluding soft-deleted and plaintext users,
and that sending still skips the subscribed user.
- Full `NotifyEncryptedDataRemovalCommandTest` suite green.
## What & why
Until now only the **category, notes, labels and (for manual
transactions) the description** could be changed after a transaction was
created — the **amount and date were immutable and the account was
create-only**. Users creating transactions by hand had no way to fix a
wrong amount or date.
This lets **manually created transactions edit every field at any time
after creation**: account, date, description and amount, on top of
category/labels/notes. **Imported / bank-synced transactions keep
amount, date, account, currency and description locked** to their source
data.
## Changes
**Backend**
- `UpdateTransactionRequest` now validates `amount`, `transaction_date`,
`account_id` and `currency_code` **only when the transaction's `source`
is `manually_created`**. For imported transactions those keys are not
validated, so `validated()` drops them and they can't be changed even
via a crafted request.
- `TransactionController::update()` moves the manual account balance to
match an edited amount/date/account, **opt-in via the same
`update_balance` flag used by create/delete**. It snapshots the pre-edit
state, and only rebalances when one of amount/date/account actually
changed. Connected accounts are skipped inside the adjuster.
- `ManualBalanceAdjuster` was refactored to a shared private `adjust()`
primitive (removing duplication between the existing create/delete
paths) and gains `reverseCreatedTransaction()` so an edit can reverse
the old contribution and apply the new one.
**Frontend**
- `edit-transaction-dialog.tsx` unifies the create/edit editability
decision behind a single `canEditAllFields` flag and renders the
account, date and amount inputs (plus the "update account balance"
checkbox) when editing a manual transaction.
- `transaction-sync.ts` `update()` gains an `{ updateBalance }` option,
mirroring `create()`.
## Known limitation (by design)
Like create/delete, the balance update trusts the opt-in flag and nudges
a **single dated snapshot** — it doesn't cascade to later snapshots and
keeps no record of whether creation adjusted the balance. So it's exact
for the common case (recent transaction, flag used consistently) and can
drift otherwise. This matches the app's existing snapshot-based balance
model; a transaction-derived balance would be a separate, larger change.
Documented with a `ponytail:` comment at the call site.
## Tests
- **Feature (`tests/Feature/TransactionTest.php`)**: manual transaction
edits amount/date/account/currency; imported transaction cannot; balance
moves by the delta on amount change; no change when not requested;
moving between accounts reverses the old and credits the new;
currency-only edit doesn't rebalance; connected accounts never change.
All green locally (55 passed).
- **Component (`edit-transaction-dialog.test.tsx`)**: manual transaction
shows editable amount/date/description; imported keeps them read-only.
## QA
Real browser QA (Playwright) against the running app with the
`demo@whisper.money` data:
- Opened a manual transaction → edited date, description and amount,
kept "update account balance" checked, saved. Verified in the DB:
`amount -4200 → -7550`, `date 2026-07-10 → 2026-07-12`, description
updated, and the account balance snapshots moved accordingly.
- Opened an imported transaction → amount and description render
read-only (locked).
## Demo
https://github.com/user-attachments/assets/1c8790e8-31f5-4283-b260-353650ad007c
## Problem
When a transaction is created (or deleted) with the **update balance**
option on
a **past date**, only that day's balance snapshot was adjusted. Balances
are
stored as sparse per-date snapshots read with carry-forward semantics
(`BalanceLookup::getBalanceAt` returns the most recent snapshot
on/before a
date), so later snapshots — most importantly *today's current balance* —
kept
their stale value.
Reproduction: yesterday = 10, today = 10. Add a −5 expense dated
yesterday with
"update balance" on. Before this fix, yesterday became 5 but today
stayed 10.
Expected: both become 5.
## Fix
`ManualBalanceAdjuster` now shifts the transaction's own day **and every
later
snapshot** by the same delta, on both create and delete:
- `applyCreatedTransaction` seeds a snapshot on the transaction's date
from the
carried-forward balance (when none exists yet), then increments that
date and
all later snapshots by `+amount`.
- `reverseDeletedTransaction` now applies the exact inverse from the
transaction's own date forward (previously it only ever adjusted
*today*,
which left the past day stale and would corrupt a create→delete
round-trip of
a back-dated transaction).
The shared forward-shift is extracted into `shiftBalancesFrom(delta)`,
used by
both paths.
## Also in this PR (small UI fix)
The balance history modal (`BalancesModal`) rendered each row's
date/balance
text top-aligned while the action icons were centered, because the base
`TableCell` defaults to `align-top`. Center the data-row cells so every
column
lines up. Scoped to this modal — the shared `TableCell` is untouched.
## Notes / intended semantics
- **Later user-set balances shift too.** Snapshots are treated as points
on one
running balance, so backfilling/removing a past transaction moves
today's
balance as well. That is exactly the reported expectation ("today should
also
change"). We deliberately do **not** try to distinguish "anchor"
snapshots
(a value the user reconciled by hand) from transaction-derived ones —
there is
no such distinction in the schema, and adding one is a separate product
decision.
- **Pre-existing, out of scope:** editing a transaction's amount/date
never
adjusts balances (`amount`/`transaction_date` aren't editable via
`UpdateTransactionRequest`), and the create vs. delete "update balance"
toggles are independent — creating with the box off but deleting with it
on
can over-correct. Neither is introduced here.
## Tests
- New: creating a past-dated transaction updates that date **and** every
later
balance (the exact reported bug).
- Rewrote the delete test to assert the reverse propagates from the
transaction's date forward (the old test encoded the now-removed "always
write
today" behavior).
- Full `TransactionTest` suite green (49 passed).
## QA
Verified end-to-end in the running app on a manual account (yesterday =
10.00,
today = 10.00): created a −5.00 expense dated yesterday with "update
balance"
on. The balance history modal shows **both** records — yesterday and
today —
drop from 10.00 to 5.00, and the DB confirms both snapshots = 500.
## Demo
https://github.com/user-attachments/assets/18aa043d-37aa-4073-990c-a269c982a818
## Summary
Audited `stats:experiment-funnel` end-to-end (data acquisition, queries,
per-row counting, financial math, and statistics) and fixed the issues
that made it misleading for the win/no-win decision. The report used to
rank variants by an **absolute** contribution-margin total that
mechanically favoured whichever variant matured fastest, and it declared
statistical significance with a normal approximation that is invalid at
the small conversion counts this experiment has.
## What changed
**Presentation & comparability**
- Replace `A2P%` (numerator not a subset of the denominator → could
exceed 100%) with `Conv%` = conversions ÷ matured-assigned — always
≤100% and comparable across variants.
- Print `MatU` (matured cohort size) so the rate denominators are
visible and the table reconciles; revive `ARPU`.
- Reframe the guidance: compare on per-user `Conv%`/`ARPU`, not the
absolute `MRR`/`Cost`/`Burn`/`CM` totals (which scale with `MatU`).
**Data correctness**
- Count soft-deleted users (`withTrashed`) — they were assigned a
variant and their connections incurred real cost.
- Attribute by the deterministic `SubscriptionExperiment::bucket()`
instead of the resolved Pennant flag, so setting `force_variant` (the
winner rollout switch) no longer collapses the whole report onto one
variant. Also stops writing Pennant rows as a side effect.
- Resolve `MRR` for subscriptions on rotated/archived Stripe price ids
(fetch prices by product), warn on any net-active sub whose price is
unmapped, round yearly ÷ 12, and skip foreign-currency prices.
- `Burn` counts only users who never earned net revenue (no
subscription, or paid-then-refunded) — a paid-then-churned user is no
longer booked as connect-and-leave leak.
**Statistics**
- Measure conversion as "ever charged, net of refund" (time-invariant)
instead of a live active-now snapshot that biases older cohorts (which
have had longer to churn).
- Decide significance with **Fisher's exact test** (exact at any sample
size), Bonferroni-corrected over the three arms, instead of the
normal-approx z that overstates evidence when expected cell counts fall
below 5. Add a Newcombe difference-of-proportions CI and a small-sample
caveat.
- Extract the inference into `App\Services\Stats\ProportionSignificance`
(+ a `BinomialProportion` value object), with unit tests that pin the
exact interval and p-value numbers.
## Validation
Reconstructed every column in raw SQL against a production dump — all
reconcile **to the cent / to the row**. Independent recomputation of the
statistics (Wilson, Fisher exact, Newcombe, z) matches the command's
output.
## Testing
26 tests (20 feature + 6 unit, 94 assertions). `pint` and
`phpstan`/larastan green.
## Note
This branch also carries two small pre-existing commits unrelated to the
funnel (`fix(categories): fall back to gray…`, `chore(schedule): stop
scheduling the stuck cohort report`). Happy to split them into their own
PR if preferred.
## Problem
`EnableBankingProvider::getTransactions` and `getBalances` classify a
set of expected `RequestException`s (401 expired session, 400
inaccessible account, 422 wrong period, 400 ASPSP_ERROR) but let
**everything else** fall through to a raw `throw $e`. An upstream **500
`Internal server error`** — from EnableBanking itself or the ASPSP
behind it — therefore propagates as a plain `RequestException` and gets
`report()`ed as an app error.
**Sentry:** `PHP-LARAVEL-3J` — `Illuminate\Http\Client\RequestException:
HTTP request returned status code 500` in
`EnableBankingProvider::getTransactions`, **36 events / 3 users** since
2026-06-16 (regressed 2026-07-15). Nothing per-event our code can do
about the bank's server erroring.
## Fix
A 5xx is a transient server-side failure — the same class as a
`ConnectionException` (no response), which is **already** wrapped as
`TransientBankingProviderException` (logged at `warning`, retried,
self-healing, `ShouldntReport`). Classify any upstream 5xx the same way
in both `getTransactions` and `getBalances`, via a small
`isTransientServerError()` helper placed alongside the other
classifiers:
```php
private function isTransientServerError(RequestException $e): bool
{
return $e->response->status() >= 500;
}
```
So provider outages retry/self-heal instead of paging Sentry.
## Not changed
All non-5xx paths are untouched and stay reportable — 401/400/422/ASPSP
keep their dedicated exceptions, and genuine 4xx client errors (e.g. the
existing "keeps non-ASPSP client errors reportable" / "keeps unrelated
422 validation errors reportable" tests) still surface.
## Test
Added two cases to `EnableBankingProviderTest.php` (an upstream 500 on
`getTransactions` and on `getBalances` →
`TransientBankingProviderException` / `ShouldntReport`). Full provider
suite: **14/14 green** locally.
🤖 Found and fixed autonomously via the Sentry monitoring loop.
## Summary
A credit card is a spending account, not wealth. This change:
- **Excludes credit cards from net worth entirely** — they no longer add
to *or* subtract from the net worth total, the evolution chart, or its
trends. They are filtered out at the source, so every net-worth
aggregation (backend summary + frontend chart/trend/MoM) is consistent.
- **Shows the credit card balance as a positive figure** on the
per-account cards, on both the dashboard and the Accounts page (like any
other account).
- **Fixes a loan sign inconsistency**: a loan used to show *positive* on
the Accounts page but *negative* on the dashboard. The Accounts list now
applies the liability sign too, so loans render negative in both places.
### Why
A user reported that a credit card showed as a positive balance on the
Accounts screen but dragged their net worth down on the dashboard — the
same account rendered with opposite signs on the two screens. The root
cause was that credit cards were modelled as liabilities (like loans)
and each screen computed the balance through a different path. Product
decision: a credit card is spendable credit, so its balance is shown
as-is and simply doesn't participate in net worth.
## Behavior
| Account (e.g. 90k / 50k) | Per-account cards (dashboard + Accounts) |
Net worth chart & total |
| --- | --- | --- |
| **Credit card** | `+90,000` | **excluded** (not counted) |
| **Loan** | `−50,000` (was `+50,000` on Accounts) | subtracts
(unchanged) |
| Checking / savings / … | unchanged | unchanged (asset) |
## ⚠️ Existing-data impact
Previously credit cards were forced to **reduce** net worth
(`-abs(balance)`). With this change they are excluded, so **users with
existing credit-card accounts will see their net worth rise** by
whatever their cards used to subtract. This is a deliberate semantic
change, not a migration — no stored balances are altered.
## Implementation
- `AccountType::countsInNetWorth()` — new predicate, `false` only for
credit cards; `calculateNetWorthAt()` skips excluded types.
- `AccountType::reducesNetWorth()` / `LIABILITY_TYPES` — now only
`loan`, so `netWorthContribution()` renders credit cards positive on the
per-account cards.
- `net-worth-chart.tsx` — credit cards filtered out of
`includedAccounts` (one point that cascades to segments, totals, trends
and `useChartViews`).
- `Accounts/Index.tsx` — applies `netWorthContribution()` so loans
render negative, matching the dashboard.
## Testing
- Backend: `AccountTypeTest` (32) and `DashboardAnalyticsTest` (credit
card excluded, loan still subtracts) — green.
- Frontend: `chart-calculations.test.ts` (37) — green.
- `pint`, `prettier`, `eslint` — clean.
Browser QA skipped per request (the logged-in flow is also currently
blocked locally by pending Spaces migrations). Logic is fully covered by
the tests above.
## What
Reworks `stats:experiment-funnel` from a pure conversion funnel into a
**contribution-margin** one, so the 3-way trial/pricing experiment can
be judged on margin — not just conversion. Signups aren't free: each
bank connection costs money per month, so trials that connect banks and
never pay are burned cash, which the old report was blind to.
## Funnel
Per variant: **assigned → activated → carded → net-paying**
- **activated** = connected ≥1 bank connection **OR** gave AI consent
(triggered paid infrastructure), whether or not they paid.
- **carded** = completed Stripe Checkout (card on file).
- **net-paying** = live, non-refunded, mature subscription.
The **activated → carded** gap is where a user connects a bank and walks
away without paying — the exact leak the report now puts a number on.
## Metrics
| Column | Meaning |
|---|---|
| `A2P%` | net-paying ÷ activated (mature) |
| `Cost` | mature-cohort connections × cost/connection |
| `Burn` | connection cost of mature non-payers (money lost) |
| `CM` | MRR − Cost (the decision metric) |
- New `--cost-per-connection` option, default **0.40** per connection.
- Connection count includes soft-deleted/revoked connections (they still
cost money).
- All money/rate columns are gated on each variant's maturity window;
`Cost`/`Burn`/`CM` print `—` until a variant has mature volume. Legend
notes that raw `Assg`/`Actd`/`Card` are lifetime counts while
rates/money are mature-cohort only.
- Existing MRR/ARPU/Net% fields stay on the collector; dropped from the
printed table.
## Tests
Added coverage for activation (bank OR AI),
cost/burn/contribution-margin math, and the cost-per-connection
argument. Full file green (12 passed).
## Why
In the experiment funnel report, variants with a trial (control,
reduced) show `0` under Actv/Cncl/Rfnd while their signups are still
mid-trial — so the table looked empty even though many of those trials
have already been canceled by the user (Cashier keeps serving the trial
until it ends, then simply doesn't charge). That "already lost, just not
settled yet" cohort was invisible.
On prod right now: 28 subscriptions in `trialing`, of which **10 are
already scheduled to cancel** — a leading churn signal the report was
hiding.
## What
- `ExperimentFunnelCollector`: new per-variant `trialingCanceling`
counter = `stripe_status === 'trialing'` **and** `ends_at !== null`.
- Report table: two new columns — `Trl` (currently in trial, previously
collected but never printed) and `TrlX` (of those, already scheduled to
cancel and won't convert). Discord legend updated.
- Test covering the new counter.
`Trl`/`TrlX` are orthogonal leading indicators; the mature Net%/MRR/ARPU
metrics are unchanged.
## Testing
`php artisan test
tests/Feature/SendExperimentFunnelReportCommandTest.php` — 9 passed.
## Why
A balance of exactly **0** could not be saved. Zero is a perfectly valid
balance for any account (an emptied wallet, a paid-off loan, a closed
position), so the app was rejecting legitimate input.
## Root cause
The shared `AmountInput` renders a numeric value of `0` as an **empty
string** (so the field shows the `0.00` placeholder instead of a literal
"0.00"). Several balance forms combined that with the native HTML
`required` attribute — which then rejected the *only* value that renders
empty: `0`. `required` on this input never guarded against a missing
value (the state is always numeric; empty == 0), it only ever blocked
zero.
The backend already accepted `0` (`required|integer`, and Laravel's
`required` treats integer `0` as present), so this was purely a frontend
constraint.
## Changes
- Drop `required` from the balance `AmountInput` in the **Update
balance** dialog and the **Balance history** edit modal.
- Remove the explicit `balanceInCents === 0` guard (and `required`) in
the onboarding **Set balance** step.
- Extend the same fix to the CSV-import **reference balance** field
(identical root cause; the compute path already gates on
`referenceBalance !== null`, so an explicit `0` is a valid anchor).
- Remove the now-orphaned `"Please enter a balance"` translation from
`es.json` / `fr.json`.
- Add feature tests asserting a zero balance can be stored and set as
the current balance.
Transaction and budget amount inputs keep `required` — `0` is not a
meaningful value there, so they are intentionally untouched.
## QA
- **Onboarding balance step**: saving with an empty/zero field now
advances instead of showing "Please enter a balance". ✅
- **Update balance dialog**: set €1,000.00, then overwrote it with €0.00
→ dialog closed and the record persisted. Verified in the DB
(`account_balances.balance = 0`). ✅
- Backend contract locked by new tests in
`AccountBalanceControllerTest`.
## Demo
<!-- PLACEHOLDER: drag the video here -->
_Video to attach: `~/Downloads/allow-zero-balances-demo.mp4`_
## Spaces / Business plan — Phase 0: invisible foundation
First of **three stacked PRs** introducing multi-tenant **Spaces** (the
basis for the Business plan). This one is a **pure, behaviour-preserving
foundation**: it can ship to production on its own with zero
user-visible change.
- **Stacked PRs:** this → `enterprise-spaces-ui` (Phase 1+2) →
`enterprise-spaces-invitations` (Phase 3).
### What a Space is
A Space groups its own accounts, connections, transactions, categories,
labels, budgets and rules. **Every user gets one invisible "Personal"
space**, provisioned automatically on creation — so the architecture is
identical for free, Standard and Business accounts, even though only
Business will ever see more than one.
### What this PR does (no behaviour change)
- `spaces`, `space_user`, `space_invitations` tables;
`users.current_space_id`; a nullable, indexed `space_id` on the 8 owned
tables (plain column, **no FK** — avoids a validating table-scan/lock on
`transactions` during a phased rollout).
- `Space` model + `BelongsToSpace` trait that stamps `space_id` on
create (from the row's user's current space; a transaction inherits its
account's space, so bank-sync lands rows correctly).
- Idempotent, chunked `spaces:backfill` command (run from a migration)
that gives every existing user a personal space and stamps their rows —
so the read switch in the next PR is safe.
- **Reads are untouched here** (still user-scoped); every user has
exactly one space, so behaviour is identical.
### Testing
- New `tests/Feature/Spaces/SpaceFoundationTest.php` (provisioning,
default-space stamping, account-anchored transaction space,
stale-pointer self-heal, backfill).
- Full suite green.
### Notes / deliberate simplifications
- `space_id` stays **nullable** (populated by backfill + on every
write); the NOT NULL constraint is deferred until prod is confirmed
fully backfilled.
- For very large `transactions` tables, `spaces:backfill` can be run
out-of-band before deploy so the migration's call is a no-op.
## What & why
A queued historical-balance generator job exhausted the worker's 128MB
PHP memory limit and **died on every retry** — Sentry `PHP-LARAVEL-49` +
`PHP-LARAVEL-4A` (two fingerprints of the *same* poison message:
identical `trace_id`/`message.id`, `retry.count` 1 then 2, OOM at
slightly different `Arr.php` lines).
### Root cause (trace-confirmed)
The Sentry trace localized the OOMing message to
`App\Jobs\GenerateHistoricalRealEstateBalancesJob`, dispatched from
`Settings\AccountController::store` for the "older than 12 months" slice
of history. The fatal frame is `Arr::map` inside Eloquent's
`AccountBalance::upsert()` value-prep (`addUniqueIds`/`addTimestamps`
per-row `array_merge`, `Query\Builder::upsert` `Arr::flatten` bindings,
and a compiled multi-row SQL string — all **O(n)** over the rows).
`purchase_date` (`before_or_equal:today`) and `loan_start_date` (no
constraint) had **no lower bound**, so a mistyped/ancient year (e.g.
`0201`, `1080`) made the generator build a **multi-century monthly
series** and hand it all to a single `upsert`, blowing past 128MB.
## The fix (two commits)
1. **`fix(balances): upsert historical balances in batches`** — build +
upsert in batches of 500 (`UPSERT_CHUNK_SIZE`) in both
`RealEstateBalanceGeneratorService` and `LoanBalanceGeneratorService`,
instead of one giant operation. Same rows, same `(account_id,
balance_date)` conflict target — peak memory is now bounded regardless
of series length. Defense-in-depth.
2. **`fix(accounts): floor purchase/loan start dates at 1900`** — the
actual root cause: add `after_or_equal:1900-01-01` to `purchase_date`
and `loan_start_date`. Rejects the data-entry typos that drive the
runaway series (and the resulting DB bloat / misleading multi-century
net-worth ramp) while accepting any legitimately old asset in a
personal-finance app.
## Review (two agents, before this PR)
Both reviewed against the live trace and the code.
- **Root cause & fix confirmed** — the batching caps exactly the
`Arr::map`-over-N-rows frame from the trace; no independent OOM source
exists (no per-row model events; `upsert` bypasses events).
- **Chunking is correct** — the unique index `(account_id,
balance_date)` exists, so the conflict target is real; `buildDateList`
yields a strictly-increasing de-duplicated list, so batches are disjoint
(no dropped/duplicated dates, no off-by-one). The fresh per-row `id`
UUID is discarded on conflict → idempotent retries.
- **No material user-facing regression.** Balance consumption
(dashboard/net-worth) is date-range-clamped. Batching introduces a
theoretical partial-write-on-mid-job-failure window, but it's idempotent
and self-heals on retry — agents advised **against** wrapping it in a
transaction (holds locks, doesn't help memory).
Applied recommendation #2 (the date floor) as its own commit. Deferred
(both agents agree): extracting the two near-identical generators into a
shared base/trait — a larger refactor that shouldn't ride a hotfix.
## Tests
- Regression test in each service: generate a ~46-year (>500-point)
series and assert the batched writes produce no dropped/duplicated dates
and keep endpoints anchored (crosses multiple batches).
- Validation test: a pre-1900 `purchase_date` is rejected at
`accounts.store`.
Note: the PHP feature suite boots a MySQL testcontainer (Docker) that
isn't available in my environment, so these were validated by
static/`pint`/`php -l` locally and rely on CI for execution — auto-merge
is gated on green CI.
Fixes PHP-LARAVEL-49
Fixes PHP-LARAVEL-4A
## What
The `encryption:delete-accounts` and `encryption:notify-removal`
commands target users who still hold legacy client-side encrypted data.
They should **never** warn or delete an account that is still being
billed.
## Changes
- Add `excludeBilledUsers()` to the shared
`FindsUsersWithLegacyEncryption` trait. It drops any user where
`hasActiveSubscriptionOrTrial()` is true — a valid subscription outside
its grace period, or an active generic trial. This reuses the existing
domain method (whose docblock already states such users must cancel
before deletion) instead of reimplementing Cashier's subscription-status
logic in SQL.
- Both commands apply the filter right after fetching their candidates.
- Eager-load `subscriptions` in the base query to avoid an N+1 when the
filter runs.
## Tests
- `it never deletes a subscribed user` (delete command)
- `it never warns a subscribed user` (notify command)
Both new tests plus the existing suite pass (8/8). Pint clean.
## Issue (Sentry PHP-LARAVEL-44 — low volume: 0 users, 1 event)
`Laravel\Ai\Exceptions\ProviderOverloadedException: AI provider [gemini]
is overloaded` (underlying Gemini HTTP 503 "high demand"), thrown from
the AI rule-suggestion generator. This is an expected, transient,
self-healing provider overload — but
`LaravelAiRuleSuggestionGenerator::generate()` called `report()` on
every failed batch, so a transient hiccup surfaced in Sentry as an
error.
## Fix
Split the per-batch catch so a `FailoverableException` (overload /
rate-limit / insufficient-credits) is logged at `warning` **without**
reporting, while every other `Throwable` is still `report()`ed. This
mirrors the existing, test-enforced handling in the sibling
`CategorizeTransactions` service.
Unchanged (deliberately preserved):
- **Partial tolerance** — suggestions from batches that succeeded are
still kept.
- **Rethrow-when-all-fail** — a genuine total failure still rethrows, so
the run is marked `Failed` (the onboarding "Try again / Skip" UI) and is
reported **once** at the run level. Non-transient batch errors are still
reported immediately.
## What I deliberately did NOT do
Two review agents converged that adding a **retry backoff** (the other
candidate fix) is not worth it here and carries real risk:
- The run **already self-heals**: failed/empty runs don't count toward
the throttle, so the user's "Try again" button re-runs immediately at
zero cost.
- A synchronous per-batch `sleep` on the single-worker `database` queue,
across up to ~10 batches, could push the run past its **120s job timeout
(failing more often, not less)** and starve other queued jobs during a
provider brownout.
So this PR is scoped to the safe, high-signal change: stop reporting an
expected transient as an error. Genuine misconfigurations (bad
model/key/quota) throw non-`FailoverableException` types and remain
fully reported.
## Testing
- New: an expected transient overload returns the successful batches'
suggestions and is **not** reported
(`Exceptions::assertNothingReported()`); an unexpected batch failure
**is** reported (`assertReported(RuntimeException::class)`).
- Full `tests/Feature/Ai` suite green (121 tests); Pint and Larastan
clean.
Fixes PHP-LARAVEL-44
---
🤖 Opened by the autonomous Sentry-triage loop. Auto-merge enabled:
low-risk, mirrors an existing tested pattern, keeps genuine failures
visible.
## Problem (Sentry PHP-LARAVEL-42)
EnableBanking's `GET /accounts/{id}/transactions` returns **HTTP 422
"Wrong transactions period requested"** when the requested date range is
wider than the bank is willing to serve. The catch-ladder in
`EnableBankingProvider::getTransactions()` only matched
401/EXPIRED_SESSION, 400/AccountNotAccessible and 400/ASPSP_ERROR, so
the 422 rethrew a **raw `RequestException`**. That:
1. escaped the per-account loop in `EnableBankingSyncer::sync` (which
only skips `InaccessibleBankAccountException`), so **every remaining
account in the connection stopped syncing too**;
2. hit the job's generic `catch (\Throwable)` → retried 3×
(deterministic, always the same 422) → connection marked **Error** and
**reported to Sentry**;
3. after the scheduled-retry budget (`consecutive_sync_failures >= 3`)
the whole connection was **dropped from scheduled sync** until a manual
reconnect.
Real user impacted (active connection). The failing request was a
~92-day window on the incremental/linked path.
## Fix (3 commits)
1. **Classify the 422** — new `WrongTransactionsPeriodException`
(`ShouldntReport`) thrown from `getTransactions()` when status is 422
and the message names the period. Also stop logging handled 422s at
`error` level in the HTTP client callback.
2. **Clamp + retry** — on that exception, `TransactionSyncService::sync`
restarts the account from page 1 with a progressively narrower window
(`90 → 30 → 7` days before `date_to`), so the user keeps as much history
as the bank will serve. `strategy='longest'` is dropped on the narrowed
retry so the explicit `date_from` is honoured; re-fetched pages are
idempotent (fingerprint dedup + date-keyed daily balances).
3. **Graceful skip** — if even the narrowest window is refused, the
syncer skips just that account (like an inaccessible account) and keeps
the connection Active, instead of failing the whole sync.
## Why draft (needs a human call, per two review agents)
The crash fix itself is well-covered and safe. What needs sign-off is
the **product tradeoff** the clamp introduces:
- **First-sync history truncation.** First sync requests
`now()->subYear()`. If the bank refuses a year, we narrow to ≤90 days
and there is **no back-fill path** (incremental syncs only move
`date_from` forward), so the skipped history is lost. This is bounded
and logged, but it is a deliberate behaviour change.
- **Incremental catch-up gap.** If the watermark is older than the
bank's servable window, the days between the watermark and the clamp are
never fetched (silent, but logged).
- **Heuristics worth a human eye:** the ladder values `[90, 30, 7]`,
dropping `strategy='longest'` on retry, and detecting the error by
`status 422 + message contains "period"` (ideally confirmed against
EnableBanking docs / a stable error code).
Low-risk per review: duplicate imports (fingerprint + `(account_id,
dedup_fingerprint)` unique index are robust to overlapping windows) and
balances (truncated, not corrupted).
## Testing
- Provider: 422 wrong-period → `WrongTransactionsPeriodException`;
unrelated 422 stays a raw `RequestException`.
- Service: clamps + retries once (asserts the clamped date and the
`strategy` drop); gives up after the ladder is exhausted; does not retry
an already-narrow window.
- Syncer: a refused account is skipped, connection stays
Active/unreported, siblings still sync.
- Full `tests/Feature/OpenBanking` + `tests/Feature/Sync` green (315
tests); Pint and Larastan clean.
Fixes PHP-LARAVEL-42
---
🤖 Opened by the autonomous Sentry-triage loop. Draft on purpose — the
data-truncation tradeoff above is a product decision for a human.
## What & why
Fixes **PHP-LARAVEL-2D** — `MaxAttemptsExceededException:
CategorizeUncategorizedTransactionsJob has been attempted too many
times.` (recurring, still firing as of yesterday, ~18 events).
### Root cause
The `database` queue connection used `retry_after = 90s`, but several
jobs declare a `$timeout` far above it (up to **600s**). `retry_after`
is **per-connection**. When a job outlives its 90s reservation, the DB
queue driver hands it to a second worker. For a `tries = 1` job that
immediately raises `MaxAttemptsExceededException` — and, worse, the body
can execute twice. `CategorizeUncategorizedTransactionsJob` runs on the
`ai` queue, which the deploy runs with **two concurrent workers**
(`docker/supervisor/supervisord.conf`), so the double-run re-bills
Gemini and resets progress — the exact harm its `tries = 1` was meant to
prevent.
At `retry_after = 90` **8 jobs** violated the invariant (600s rule jobs,
300s AI jobs, 120s sync jobs).
## Changes (one concern per commit)
1. **fix(queue): raise `retry_after` above the longest job timeout** —
default `90 → 900s` (env-overridable via `DB_QUEUE_RETRY_AFTER`),
clearing the 600s longest job with margin. Adds `QueueConfigTest`
asserting every `app/Jobs` `$timeout` stays `< retry_after` so a new
long job can't silently re-break it.
2. **fix(ai): de-duplicate the backfill per user** — `retry_after` stops
one dispatch being re-reserved, but not a *duplicate* dispatch (double
"Enable AI" click / re-enable mid-run) on the 2-worker `ai` queue.
Implement `ShouldBeUnique` keyed on user id, mirroring the sibling
`RetryTransientAiCategorizationJob`.
3. **test(queue): widen the guard** to also scan queued Mailables
(`app/Mail`) and Notifications (`app/Notifications`), which share the
connection and honor `$timeout`.
4. **docs(queue): correct the `retry_after` comment** — per-job
`$timeout` takes precedence over the worker `--timeout` (which is
currently unset), so the earlier "must sit between" phrasing was
inaccurate.
## Verification
- `QueueConfigTest` fails at the old `retry_after=90` (8 offenders) and
passes at `900`.
- `vendor/bin/pint --test` clean; 131 tests across `tests/Feature/Ai`,
AI-consent flow, and queue config pass.
- Confirmed against the real deploy: 2 workers on the `ai` queue,
`pcntl` installed (SIGALRM enforced), workers set no `--timeout` so each
job's own `$timeout` bounds execution below 900s.
## Reviewed by two independent agents (architecture + product)
Both concluded **ship it**. The double-dispatch gap they flagged is
closed by change #2. Neither found a correctness regression.
## Operational notes (no code change needed, flagged for awareness)
- **Reclaim window:** `retry_after` is per-connection, so
`emails`/`default` also inherit the 90→900s window. A job orphaned by a
*hard* worker crash (OOM/SIGKILL) is now reclaimed after up to ~15 min
instead of ~90s. Graceful deploys (SIGTERM + `stopwaitsecs=3600`) don't
orphan jobs, so this only bites on hard crashes — an acceptable trade. A
dedicated connection for the long AI queue would remove it; left as a
possible follow-up.
- **Worker `--timeout`:** the fix relies on `pcntl` staying installed
and long jobs keeping a `$timeout`. Setting an explicit `--timeout` (<
900) on the supervisord `queue:work` commands would make it
self-enforcing; not done here to avoid changing short-job hang
detection.
- If prod sets `DB_QUEUE_RETRY_AFTER` explicitly, ensure it's above the
longest job timeout (≥ 600, ideally 900).
## Summary
Wave 2 structural refactor: the same financial math was copy-pasted
across the
dashboard and the analytics API endpoints, so it could silently diverge
between
screens — a real risk in a finance app. This consolidates the duplicated
calculations into single sources of truth, kills a net-worth N+1, and
aligns
the PHP/TS rule engines and the `TransactionSource` enum. Every change
is
**behavior-preserving**; the numeric outputs of every endpoint are
unchanged and
are locked down with characterization/parity tests.
Builds on merged #640 (Wave 1); no file overlap. No dependencies
changed.
## Changes (per commit)
- **Consolidate savings-rate math into `CashflowSummaryService`** —
`savings_rate`
and `net` were byte-identical inline in `DashboardController` and
`Api/CashflowAnalyticsController`. Extracted to
`CashflowSummaryService::summarize(income, expense)`;
both controllers now spread its result (same keys, same values).
- **Move income/expense-side classification onto the `Transaction`
model** —
the income/expense side test was reimplemented in three places
(`Api/TransactionAnalysisController`, `Api/CashflowAnalyticsController`,
`DashboardController`). Now `Transaction::isIncomeSide()` /
`isExpenseSide()`.
- **Extract duplicated `getCategorySpending` into
`CategorySpendingService`** —
the tree-rollup expense-spending query was duplicated verbatim between
`DashboardController` and `Api/DashboardAnalyticsController`. Moved to
`CategorySpendingService::forPeriod()` (drill-parent parameterized).
- **Batch net-worth balance lookups to kill the per-account N+1** —
`Api/DashboardAnalyticsController::calculateNetWorthAt` ran one
`AccountBalance` query per account per compared period. Now uses the
existing
`BalanceLookup::forAccounts()` (fixed 3 queries via carry-forward seed +
in-range records), reproducing the exact "latest balance <= date, else
0"
semantics.
- **Align server rule normalization with the client and lock it with
parity
fixtures** — `AutomationRuleService::normalizeRuleJson` protected only
`['description','notes']` while `rule-engine.ts` also protected
`creditor_name`/`debtor_name`. Aligned to the superset (structurally a
no-op
since those var names are already lowercase, so no matching change) and
added
shared PHP+TS parity fixtures so the two engines can never drift
unnoticed.
- **Add missing `TransactionSource` cases to the TS type** —
`transaction.ts`
was missing `enablebanking`/`wise`; now mirrors
`App\Enums\TransactionSource`.
- **Guard `sumTransactions` against unsupported category types**
(reviewer fix) —
replaced the income/expense ternary that silently treated any non-Income
type
as expense with a `match` that throws on Savings/Investment/Transfer.
- **Document category eager-load expectation on `Transaction` side
methods**
(reviewer fix) — doc-only note to prevent a future N+1.
## Test plan
New tests:
- `tests/Unit/Services/CashflowSummaryServiceTest.php` —
net/savings-rate/rounding/div-by-zero.
- `tests/Feature/TransactionSideClassificationTest.php` — income/expense
side across signs, uncategorized, and transfer/savings/investment =
neither side.
- `tests/Feature/DashboardAnalyticsTest.php` — new net-worth test
asserts both the values (600000 / 540000) and a flat balance-query count
(<= 3) regardless of account count.
- `tests/Feature/RuleEngineParityTest.php` +
`resources/js/lib/rule-engine-parity.test.ts` +
`tests/Fixtures/rule-engine-parity.json` — one shared fixture set
driving both the PHP and TS rule engines.
Results (targeted, local):
- `--filter=Cashflow` (exclude Browser): 57/57 passed
- `--filter=DashboardAnalytics`: 41/41 passed
- `--filter=AutomationRule`: 60/60 passed
-
`--filter=TransactionSideClassification|CashflowSummaryService|RuleEngineParity`:
21/21 passed
- `bun run test rule-engine` (vitest): 13/13 passed
- `vendor/bin/pint --test`: pass; `bun run lint`: 0 errors; `bun run
format:check`: clean
- `bun run types`: 157 errors (unchanged pre-existing baseline), 0 in
touched files
Note: the 6 `Cashflow*` Browser tests fail locally only on "Vite
manifest not found" (no build present); they are environmental, not
logic, and pass in CI.
## Reviewer findings
Two read-only reviewers (architecture/quality and product/behavior)
reviewed the diff.
**Addressed**
- Both flagged that `sumTransactions` silently treated any non-Income
type as expense — added a throwing `match` guard.
- Eager-load expectation documented on the `Transaction` side methods.
**Verified identical** (behavior reviewer):
income/expense/net/savings_rate across all three endpoints; net worth
for both compared dates including no-record / all-records-after-range /
same-date edge cases; category spending (uncategorized excluded,
soft-deleted categories excluded, rollup/drill preserved); rule-engine
normalization output; multi-currency conversion.
**Deferred (documented)**
- The income/expense **summation** itself is still computed three ways
with differing uncategorized-transaction handling (dashboard
`whereExists` + sign vs analytics `join` excluding uncategorized vs
in-memory `isIncomeSide`). Unifying it would change numbers, so it is
out of scope for this behavior-preserving PR — worth a dedicated
follow-up.
- `savings_rate` keeps its `int|float` union (int `0` when income is 0).
Intentionally preserved to keep JSON output byte-identical.
- `BalanceLookup`'s `empty()` guard never short-circuits a `Collection`,
so a zero-account user runs 3 empty (harmless) queries. Left untouched —
it lives in a shared, unchanged service and only affects a no-account
edge case.
Do not merge before Wave 1 (#640) is in main.
## Summary
Wave 1 hardening: privacy/security quick wins on the Inertia boundary, a
CI safety net, and stricter test isolation. Four focused changes plus
two review fixes, each in its own commit. No dependency or
product-behavior changes.
## Changes (by commit)
1. **Hide sensitive User fields from serialization** — `User::$hidden`
only covered password / 2FA / remember_token, leaving Cashier billing
columns (`stripe_id`, `pm_type`, `pm_last_four`, `trial_ends_at`) and
the legacy `encryption_salt` exposed in every serialized User, including
the Inertia-shared `auth.user` prop. None are read by the frontend and
Cashier keeps reading them server-side, so hiding them is invisible to
the UI and billing.
2. **Advisory frontend type-check in CI + duplicate `currency_code`
fix** — the CI linter job never ran `tsc`, so type errors accumulated
unseen. Adds a `Type Check Frontend` step running `bun run types`. It is
`continue-on-error: true` on purpose: the codebase already carries ~157
pre-existing `tsc --noEmit` errors, so gating on it now would turn CI
red on unrelated code. It surfaces type output today and should be
flipped to blocking once the backlog is cleared. Also removes a
duplicate `currency_code` member on the `User` TS interface (declared
twice, `CurrencyCode` and `string | null`); the TS language server flags
it, `tsc` masks it under `skipLibCheck`.
3. **Move residual-encryption cleanup out of Inertia `share()` into a
queued job** — `share()` is a shared-data provider and must be
read-only, but it ran a `DELETE`+`UPDATE` against the user on every
non-API web GET to purge the leftover encryption salt /
`EncryptedMessage` once a user had no encrypted data left. The existing
`encryption:*` commands do not cover this case (both target users who
*still* have encrypted data; this finalizes users who *finished*
decrypting). The work now goes to a new idempotent
`PurgeResidualEncryptionArtifactsJob` dispatched from `share()`,
preserving the eventual-cleanup semantics without writing during the
render.
4. **Block stray HTTP in the Feature test suite** — adds
`Http::preventStrayRequests()` in a Feature-scoped `beforeEach` so any
unfaked outbound request fails loudly instead of hitting the network. A
representative HTTP-touching subset (open banking,
exchange-rate/currency, AI categorization + AI/stats reports, analytics,
Discord, Stripe, bank logos) was run with the guard active; no test
relied on a real request, so no fakes had to be added.
### Review fixes (after the two-reviewer pass)
5. **Purge check uses `name_iv` source of truth, not the stale
`encrypted` flag** — the destructive purge decided "no encrypted
accounts" from `accounts.encrypted`, a flag the codebase already treats
as unreliable (see the
`align_accounts_encrypted_flag_with_plaintext_names` migration and
`FindsUsersWithLegacyEncryption`, which key off `name_iv`). An account
with an encrypted name but a stale `encrypted=false` flag could have its
key material destroyed. The job's guard now matches the canonical `*_iv`
predicate exactly; the loose flag gate in `share()` is kept only as a
cheap dispatch filter, and the job re-verifies with the safe predicate
before touching anything.
6. **Deduplicate purge dispatches with `ShouldBeUnique`** — `share()`
runs on every web GET, so an affected user re-enqueued the job on each
page load until a worker cleared the salt. The job is now
`ShouldBeUnique` keyed by user id, collapsing repeat dispatches into one
pending job.
## Test plan
- New/updated tests, all green:
- `InertiaSharedDataTest`: `auth.user` omits all sensitive fields; a web
GET no longer mutates the user inline and instead queues the cleanup
(and does not queue it when there is no salt).
- `PurgeResidualEncryptionArtifactsJobTest`: clears salt + message when
no `*_iv` data remains; keeps them when an encrypted transaction, an
encrypted account name, or a stale-flag-but-encrypted-name account
exists; no-op when salt already null.
- `StrayHttpRequestGuardTest`: an unfaked request throws
`StrayRequestException`; a matched fake still resolves.
- Green locally: `vendor/bin/pint --test`, `bun run lint` (0 errors),
`bun run format:check`, `bun run test` (254 frontend tests), targeted
backend
`--filter=InertiaSharedData|PurgeResidualEncryptionArtifactsJob|StrayHttpRequestGuard|Encryption`
(24 tests). Ran ~700 HTTP-touching Feature tests with the stray-request
guard active with no guard-induced failures.
- `bun run types` still reports the ~157 pre-existing errors (unchanged
set; this PR adds none) — that is exactly why the CI step is advisory
for now.
## Reviewer findings — addressed vs deferred
**Addressed**
- 🟠 Destructive purge keyed off the stale `accounts.encrypted` flag
instead of the `name_iv` source of truth → fixed in commit 5 (job now
mirrors `FindsUsersWithLegacyEncryption`; added a regression test for
the stale-flag case).
- 🟡 Per-request dispatch amplification with no dedup → fixed in commit 6
via `ShouldBeUnique`.
**Deferred (with rationale)**
- 🟠 "Three divergent copies of the legacy-encryption query" —
substantively resolved: the job now matches
`FindsUsersWithLegacyEncryption` exactly. The only remaining
`encrypted`-flag use is the `hasEncryptedAccounts` **UI prop** in
`share()`, which is a separate, pre-existing frontend concern; changing
it would alter which accounts the UI treats as encrypted and is out of
scope here. A full extraction into one shared scope would require
restructuring the trait (it builds a `User` query, not a per-model
boolean) and is not a Wave 1 quick win.
- 🟢 Redundant `->fresh()` / null guard in the job — kept deliberately:
it is the idempotency guard that makes the re-check read committed state
on the sync path (the second reviewer credited it as what makes repeat
dispatches safe).
- 🟢 `continue-on-error` shows the type-check step green — acknowledged;
flipping to blocking (or failing on an increase over a committed
baseline) is the follow-up once the ~157-error backlog is cleared.
- 🟢 (Product review) Theoretical one-request SSR/client
`hasEncryptionSetup` diff from async salt clearing — invisible in
practice (the lock button gates on `hasEncryptedAccounts ||
hasEncryptedTransactions`, false in both SSR and client), and `ssr.tsx`
is untouched. No action.
Product-bug reviewer verdict: no user-facing regressions. Hiding the 5
fields does not affect Cashier (raw attribute access),
`EncryptionController`, notifications, or any API/JSON path; the
`currency_code` dedup is runtime-identical; the cleanup timing change is
client-absorbed.
## Why
The drip email family was near copy-paste. Each of the eight mailables
repeated the same constructor, `$tries`/`$backoff`, `drip_from`
envelope, `markdown` + `userName` content, and `RateLimited` middleware
— differing only in subject and template (plus a reply-to on one, an
extra view var on another). Each of the eight jobs repeated the
`canReceiveEmails` / `hasReceivedEmail` guards, the `Mail::send` call,
and the `UserMailLog::create` block — differing only in the email type,
the mailable, and a handful of per-email eligibility checks.
## What
Two abstract bases, so each subclass declares only what varies:
- **`DripMail`** — owns the shared mailable shape. Subclasses declare
`dripSubject()` and `template()`; optional `contentData()` (PromoCode's
`FOUNDER` var) and `repliesToSender()` (PaywallFollowUp) hooks. The
hooks are named `dripSubject()`/`template()` deliberately to avoid
colliding with `Illuminate\Mail\Mailable`'s existing public
`subject()`/`markdown()` methods.
- **`SendDripEmailJob`** — owns the shared `handle()` flow (shared
guards → send → `UserMailLog`). Subclasses declare `emailType()` and
`buildMail()`; an optional `shouldSend()` hook carries the per-email
eligibility rules (pro-plan, onboarding, transactions, bank connections,
AI consent).
Each mailable drops from ~68 to ~13 lines and each job from ~48 to ~18.
Net **−459 lines**, with the send/log logic and envelope construction
now in one place.
## Behavior
No functional change. Subjects, `from`/`reply-to`, queue name, rate
limiting, templates, view data, and every per-job eligibility guard are
preserved. Notably, passing `replyTo: []` for the seven non-reply mails
is identical to the previous omission — `Envelope`'s `replyTo` defaults
to `[]`. Verified: `pest --exclude-testsuite=Browser` **1872 passing, 0
failing**; `pint --test`, `phpstan` (level 5), `format`, `lint` clean.
## Review-driven follow-up commits
Two review agents (architecture/duplication/coverage, and
behavior/regressions) ran against the first commit. The behavior agent
verified all eight jobs' guard logic invert correctly (order +
negations) and that `replyTo: []` ≡ omitted — no regressions. The
coverage agent found the two hook-driven behaviors were untested;
applied, each as its own commit:
- Extended the drip-sender parity test to all eight mailables and
asserted `PaywallFollowUpEmail`'s reply-to (and that a default drip mail
sets none).
- Asserted `PromoCodeEmail` injects the `FOUNDER` promo code into its
view data.
## Deliberately out of scope
- Non-drip mailables (`Waitlist*`, `Banking*`, `UpdateEmail`, …) share
the same `RateLimited('emails')->releaseAfter(1)` middleware and could
later share a queued-mail base too — separate concern.
- **Pre-existing** (not introduced here, noted by review): the job sends
the mail before writing `UserMailLog`, so a failure between the two can
re-enqueue a duplicate; there is no lock/unique constraint on `(user_id,
email_type)`. Worth a separate hardening pass.
## Why
The five API-key OpenBanking "connect" controllers — Binance, Bitpanda,
Coinbase, Indexa Capital and Interactive Brokers — each carried a
~60-line `store()` that was near-identical: subscription gate →
credential validation → `Bank::firstOrCreate` → create the
`BankingConnection` (Pending) → update it to `AwaitingMapping` with
pending accounts → auto-map during onboarding or redirect to mapping.
Any change to that flow, or a bug in it, had to be made five times.
## What
Extract the shared flow into a small class hierarchy, so each controller
declares only what actually varies per provider.
- **`OpenBankingConnectController`** (abstract) owns the whole flow in a
`connect()` template method, with per-provider extension points:
`provider()`, `providerName()`, `bankLogo()`, `aspspCountry()`,
`fetchProviderData()`, `credentialErrorMessage()`,
`buildPendingAccounts()`, and an optional `emptyProviderDataMessage()`
guard.
- **`CryptoPortfolioConnectController`** (abstract, extends the above)
implements the two hooks the crypto providers share: `aspspCountry()`
(from the request) and the single "Crypto Portfolio"
`buildPendingAccounts()` (uid derived from the provider enum value, so
the generated uids are unchanged).
- The five controllers shrink to their genuine differences (client,
names, logo, error copy, and — for IB — the Flex error mapping and
empty-statement guard).
Net **−110 lines** across the five `store()` methods, and the connection
lifecycle now lives in one place.
## Behavior
No functional change. The credential-failure warning log is now a single
structured message with a `provider` key instead of five free-text
variants (the only observable difference; HTTP responses are
byte-for-byte identical). Verified by the full suite: **1869 passing, 0
failing** (`--exclude-testsuite=Browser`); `pint --test`, `format` and
`lint` clean.
## Review-driven follow-up commits
Two review agents (architecture/duplication/coverage, and
behavior/regressions) ran against the first commit. Applied, each as its
own commit:
- Finished the crypto dedup via the intermediate base (the reviewers
flagged the still-duplicated crypto hooks).
- Dropped the over-engineered per-provider
`validationFailureLogMessage()` hook in favor of a structured base log.
- Added the missing test for the Interactive Brokers empty-statement
guard (the only conditional hook, previously unexercised): asserts 422 +
NAV-section message, no connection row, and no warning logged.
## Deliberately out of scope
- **Wise** is left as-is: it builds pending accounts mid-flow (needs the
client), creates the connection in one step, and has its own
empty-portfolio guard — it does not fit this template.
- **Pre-existing** (not introduced here, noted by review):
`Bank::firstOrCreate` returns an existing bank without refreshing its
logo, so `aspsp_logo` can copy a stale/null value. Worth a separate fix.
## Problem
The Discord **Plan changed** notification only showed the *new* plan
value in the `Changed` field, so it was impossible to tell what actually
changed:
```
Changed
Plan: €3.99 / month
```
Was it a price change? An interval change? From which plan? No way to
know.
## Fix
Compute the previous plan from Stripe's `previous_attributes` and render
it as `old → new`, matching the existing status-change behaviour:
```
Changed
Plan: €9.99 / month → €3.99 / month
```
`planLabel()` was generalised to read both the modern
`items.data[].price` shape and the legacy top-level `plan` shape Stripe
still includes in the diff, so the same helper works on both the current
object and the `previous_attributes` diff. Falls back to the old `Plan:
X` output when no previous value is detectable.
## Test
Added a feature test asserting the `old → new` output for a
`customer.subscription.updated` plan change. All 11 tests in the file
pass.
## Why
Two issues surfaced from a real mis-categorization corrected via the
**Edit Transaction** modal (not the table quick-edit).
### 1. The modal never told the user the AI learned
Both the table quick-edit and the edit modal hit the same backend
(`PATCH /transactions/{id}` → `CategoryOverrideHandler` →
`AiRuleLearner`), so both learn a forward rule from a correction. But
the modal **discarded** the update response and never read
`learned_rule`:
- No *"Learned: similar transactions will be categorized automatically"*
confirmation.
- No **Undo**.
- It still showed the generic *"Automatize"* prompt — offering to create
a rule that already exists.
The modal now reads `learned_rule` and mirrors the table's toast + undo,
skipping the automatize prompt when a rule was learned. Same i18n
strings as the table (no new keys).
### 2. The learner could key a rule on a single weak token
A correction on `"Pago en MADRID SUC 04 MADRID ES"` produced a rule
keyed on `suc` alone. The tokenizer drops noise by **per-user** document
frequency, so `madrid` (20% of this user's descriptions) is correctly
dropped — but `suc` (0.14%) survives, even though it is a generic
banking abbreviation that matches broadly as a substring. The per-user
filter can't see that `suc` is generic in general.
`AiRuleLearner::descriptionClause()` now refuses a single distinctive
token shorter than 5 characters. It learns only when there are **two or
more** distinctive tokens, or a single token of **at least 5
characters**. Merchant-keyed rules are unaffected.
## Tests
- `does not learn a description rule from a single short token`
- `learns a description rule from a single sufficiently long token`
- `learns a description rule from two short tokens`
All `AiRuleLearnerTest` pass (13/13).
## Not in this PR
The one already-learned prod rule still carries the stale `suc`
clause/title (low impact, matches ~2 txns). Cleaning that data is a
separate manual step.
Víctor Falcón
Jesús Mejías Leiva