133 Commits

Include renames

Author	SHA1	Message	Date
Paul LütznerandClaude Opus 5	b38aaac04e	Give the Mercure tokens an identity no installation has to configure ... v0.2.31 could not mint a token at all on a TrueNAS install, and nothing an operator set would have fixed it. That release bound `iss` and `aud` to MERCURE_PUBLIC_URL. truenas.compose.yaml leaves that BLANK ON PURPOSE — the setup screen asks for the public URL on first boot and stores it in the database, which is long after the hub container has had to be told what to trust. An empty issuer is not a weak issuer: the factory refuses it outright with `The "iss" additional claim is required by RFC 9068 access tokens`, so there was no subscriber cookie, every publish threw, and the symptom was a red dot indistinguishable from a hub that was simply down. The mistake underneath was deriving a value that must exist at CONTAINER START from one that only exists after SETUP. So it is a literal now, shared by the application and every compose file, and no installation has to know it. An identifier does not have to resolve — `.invalid` is reserved by RFC 2606 for exactly this — and all that matters is that both halves agree on the same string, which a literal guarantees and a derived value cannot. The same string everywhere costs nothing: the signing secret is generated per installation, so one install's token has never been usable against another's hub however it identifies itself. truenas.compose.yaml had been left behind twice more. It still carried the "/healthz" check that v0.2.30 replaced in the other two files, so its hub would have gone unhealthy the moment the image moved; and it had no pull_policy, so the image never moved at all. That is the combination that produced the original report: an application on the 1.0 protocol talking to a 0.x hub that nothing had ever pulled. The regression test empties MERCURE_PUBLIC_URL rather than reading the config, because what broke was the derivation — a test that read the same placeholder the code did would have passed throughout. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>	2026-09-21 13:09:36 +02:00
Paul LütznerandClaude Opus 5	d35418c40a	Speak the Mercure 1.0 protocol, because the hub does now ... Live updates were dead: every subscription came back 401 and the inbox sat on "Live updates unavailable" while the hub answered its own healthcheck happily. The hub had moved to the 1.0 protocol and now wants RFC 9068 access tokens — an `iss` it has been told to trust, an `aud` naming the hub the token is for, and `authorization_details` in place of the bare `mercure` claim. plMail was minting the older shape. NOT HAND-ROLLED CLAIMS. symfony/mercure already implements 1.0, so this is configuration: `protocol_version: '1.0'` plus the three claims the bundle requires (it refuses to build the container without `iss`, `sub` and `client_id`, which is how the requirement was found). They are merged into the token FACTORY as well as the publisher provider, so the subscriber cookie gets them too — one block covers both halves. The protocol switch also changes the subscription query parameter from `topic` to `match` and the cookie's name. Both belong to the bundle — the `mercure()` Twig helper writes the URL, Authorization writes the cookie — which is why no JavaScript and no controller is touched here. `resource_identifier` IS PINNED ON THE HUB, and that is not decoration. A 1.0 hub works its expected audience out from each request, and plMail reaches it by two roads: the browser subscribes at MERCURE_PUBLIC_URL, the application publishes to the internal MERCURE_URL. One token cannot name both, so a hub left to derive its own answer would refuse every publish. Issuer and audience are both read from MERCURE_PUBLIC_URL so the app and the hub cannot be configured into disagreeing. THE `__Secure-` COOKIE is the one thing an operator may have to know about. 1.0 names the cookie `__Secure-mercure_access_token`, and a browser drops a `__Secure-` cookie that did not arrive over TLS. Right for every real install, fatal for the test stack, which serves plain HTTP on 127.0.0.1 — so the name is a parameter defaulting to the secure one, and only the test stack overrides it, with the hub told the same name. Verified in a browser against the current hub image: the indicator reads "connected", and the subscription is GET /.well-known/mercure?match=mail%2Fuser %2F1 → 200. Full suite green. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>	2026-09-21 12:12:36 +02:00
Paul LütznerandClaude Opus 5	1ec38e2b97	Retry a Graph read that comes back 5xx ... Graph answers 502 with an empty body and "code": "UnknownError" often enough that it is weather rather than news. Nothing retried it. GraphApiClient sorted 410 into a resync and 429/503 into a throttle, and everything else became a plain GraphApiException — which GraphApiSyncer catches per folder, logs at ERROR with the whole trace, and skips. The skip is safe (the coverage guard stops anything being erased, the delta link is not advanced) but the folder still sat out a fifteen-minute cycle for a blink that one immediate retry would have cleared. GET ONLY, and that is the reason this is a strategy of its own rather than Symfony's defaults. GenericRetryStrategy retries 502 for EVERY method. A 502 means the gateway gave up on the answer, not that Exchange never acted: /me/sendMail, $batch carrying writes, createMasterCategory and /move are all POSTs that may well have landed, and repeating those is how one send becomes two. DELETE is left out too, though IDEMPOTENT_METHODS would have included it — deleting a subscription the first attempt deleted answers 404, which turns a success into an error somebody then has to teach the caller to ignore. 429 AND 503 STAY OUT. Throttling is already answered better one layer up: GraphThrottledException carries Microsoft's own Retry-After and is Recoverable, and the handlers re-slice per-sub-request throttles by hand. Mail throttling is per-mailbox at about four concurrent requests, so a retry here would spend a scarce one of them arguing with a limit that has just said how long to wait. Scoped to graph.microsoft.com, so Ollama, Paperless, Immich, Nextcloud and the CalDAV servers keep their own callers' judgement about what a 500 means. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>	2026-09-21 11:27:50 +02:00
Paul LütznerandClaude Opus 5	ab9ecbc9e0	Hand a whole-view bulk action to a worker, which it never was ... RunBulkStatusHandler exists to take a mark-read or archive over a whole view out of the request — its docblock quotes the thirty-second timeout that produced it, and BulkStatusController answers with an indicator so nobody waits. Neither was true: the message was never listed in messenger.yaml's routing map, and Messenger handles an unrouted message inline in whichever process dispatched it. Every whole-view action ran inside the web request, under the limit it was written to escape, at request concurrency — which is where the production deadlock came from. It gets a queue of its own rather than joining maintenance, because somebody is watching an indicator for it and queueing it behind an embedding backfill would be slower than the inline version it replaces. That means one new container, worker-bulk, in every compose file and in the handbook. The deadlock itself is retried now instead of failing the job, three times per chunk, and a collision that outlasts those hands the job back to the queue rather than ending it. The failure path no longer writes through an EntityManager Doctrine has closed, which is what it was doing for every failure that came out of a flush. And the list catches up: nothing ever refreshed it on the strength of the job, only on the toolbar's own "the write landed" event, which is now the instant the work was QUEUED rather than finished. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>	2026-09-14 20:18:57 +02:00
Paul LütznerandClaude Opus 5	d53e7c4963	Report browser errors to the admin panel, in a card of their own ... A JavaScript fault was visible only to whoever had the console open, which on a self-hosted mailbox means nobody. Uncaught exceptions and rejected promises from plMail's own scripts now reach Admin → Logs, together with whatever the Content-Security-Policy blocked. Its own card rather than rows in the log, because it holds a different kind of thing. A server error is one event in one request; a broken line in a controller runs on every page load for every user. So a row is a distinct fault — message, file, line — with a count and a lifetime, and the same fault reported again increments it. Merged into the log, one bug would be four hundred rows and the log would be unreadable past it. THE FILTER IS THE FEATURE. Most of what window.onerror sees is not ours: the console screenshot that prompted this was an ad-attribution metrics script failing inside requestIdleCallback, injected by an extension. A panel that accepted that fills with other people's bugs and stops being read, which is the same as not having one. A report is kept only when it can be attributed to a script this server sent — an injected script has no URL (Chrome's `VM947`), and a cross-origin one arrives as the bare string `Script error.` with no file, line or message. The rule is applied in the browser and again in the recorder, because the browser half is part of the code being reported on. CSP violations come to the same card through /csp-report. That endpoint cannot be CSRF-protected and is public in security.yaml, because the browser generates the request itself: no token, no header we chose, and on a login page no session. The rate limiter is the bound. It is worth having because the full policy is enforced in production, so what arrives is what this installation actually refused to load in a real browser. The count means page loads, not executions — the client reports each fault once per page, capped at five. Measured rather than assumed: throwing the same error twice on one page records one sighting. Documented that way on both sides after the first version claimed the server would count what the client never sent. `column` is a reserved word in Postgres, so the field is `column_number`. The quoted DDL created the table happily and every insert into it was a syntax error — same trap User::$usr already avoids. Verified on a running stack by throwing real errors in a real browser: an error from a script on this origin recorded, an anonymous injected one refused, the card rendering both kinds grouped with counts. Its first catch was a false alarm, and a useful one to record. It reported the Mercure connection as blocked by `connect-src` — which was true, and was me browsing `localhost` while MERCURE_PUBLIC_URL names `127.0.0.1`. Different origins, so the app's own policy refused its own hub. Browsing the host the config actually names produces no violation at all. The compose comment calling that URL "same-origin" is correct on its own terms; it just stops being true if you reach the stack by another name. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>	2026-09-04 16:02:42 +02:00
Paul LütznerandClaude Opus 5	39dd7e5857	Ask the model again about recent mail, and let a verdict reach the tab ... Two things, and the button is useless without the first. A VERDICT NEVER REACHED A TAB. Nothing recomputed a message's category after the classifier stored one: ingest files a message the moment it arrives, the classifier runs afterwards and asynchronously, and it wrote the answer and stopped. That was invisible while sorting was rules-only — the verdict was a tie-break nobody could see — and it stopped being invisible the day sorting became a setting. Somebody chooses "the assistant", new mail keeps arriving into whatever the rules said, and the choice appears to do nothing at all. ClassifyMailHandler now re-files what it just asked about, using each owner's own preference, and resolves only the threads it touched — recomputing per ACCOUNT would scan every message the account has, after every batch of arriving mail. recomputeCategoriesForThreads() is that statement with the threads named, pinned conversations still excluded. AND THE ANSWER GOES STALE WHILE THE MAIL DOES NOT. A message is asked about once, which is right. What changes is the QUESTION: the prompt is editable, the model is a setting, and what plMail sends alongside a message changed one commit ago — every verdict stored before the bulk-header line was reached without evidence the model now gets, and demonstrably differs because of it. So: a button, bounded to the newest 100, 200 or 500 messages, clamped server side because the field is a select and anything else arrived by hand. Nothing re-asks on its own and nothing should — a model call per message is the most expensive thing here, and spending an afternoon of somebody else's GPU because an administrator edited a prompt is not a decision to take for them. ClassifyMailMessage gains a `force` flag, defaulted and last so envelopes already on the transport still deserialise. The stamp means "already asked", which is the right reason to skip on ingest and the wrong one when the answer is what has gone stale. The re-run INVOKES the classify handler rather than dispatching to it, and the transport is why: ClassifyMailMessage routes to `ingest`, correct for a message being classified as it arrives and wrong for several hundred at once — arriving mail would queue behind a button press for the best part of an hour. Called directly, the work stays on `maintenance` and reuses the asking, the batching, the per-message permission check and the re-filing rather than growing a second copy of them. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>	2026-09-04 12:32:53 +02:00
Paul LütznerandClaude Opus 5	e3a4b78d92	Re-file the mailbox when the sorting setting changes ... The previous commit shipped the setting and told people to go and find an administrator if they wanted their existing mail re-sorted. That was wrong, and the argument against it is the one that should have been made for it. MessageCategorizer reads only PERSISTED data — stored headers, the provider's labels, the sender, the model's stored verdict — which is the entire design of the class. So a re-sort asks no other machine anything: no IMAP, no model, no network. It is a scan of rows the database already has, a PHP cascade over each, and a column write, followed by one statement per account that resolves each thread from its messages. The caution behind the old answer was real — a hundred thousand things do not belong inside an HTTP request — but the fix for work too big for a request is a worker, and this application has four. A summary job was written one commit earlier on exactly that reasoning and it was not applied here. Leaving it undone was the worse option, not the safe one. Mail arriving after the change would be sorted the new way and everything already there the old way, so one inbox would be filed two ways depending on when each message landed — an incoherent state nobody chose, produced by a setting they did, with no symptom that leads back to the radio button that caused it. Dispatched only when an answer actually CHANGED. The card submits on change and a browser will happily post the option already selected; re-filing for that is minutes of a worker writing the answers back unchanged. Threads pinned to a tab by hand are untouched — recomputeCategoriesForAccount has always skipped `category_pinned_at IS NOT NULL`, and it is worth saying out loud on a feature that re-files somebody's inbox. Safe to run twice: the categoriser is a pure function of persisted state, so a retried envelope writes the same answers over the same rows. maintenance rather than ingest, so arriving mail never queues behind somebody's settings change. The command stays and is unchanged — an operator re-sorting every mailbox after a rules change wants exactly this work, for everyone at once. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>	2026-09-04 11:29:52 +02:00
Paul LütznerandClaude Opus 5	408fcd1bed	Run full-conversation summaries as a job, with nothing held open ... A full run is silent for minutes while the model reads, and for the whole of that silence the answer depended on a browser connection staying open. That dependency is what has been failing, and no timeout could have fixed it: full: true, transcript_chars: 67720, elapsed_seconds: 40, chars_so_far: 0 Forty seconds, four heartbeats already delivered, nothing written yet, and a model host that was up for seven days either side of it. The connection was alive and being written to and then was not. Nothing in plMail closed it — a held-open connection has a proxy, a browser and a network in it, and any of them may end it without telling this application. Three rounds went into tuning timeouts before the metrics panel said `cancelled` and named the real shape. So the full run stops needing a connection. It dispatches SummariseThreadMessage and answers 202; a worker on the maintenance transport does the work, writes to the store and publishes a Mercure nudge; the card shows it as queued and fills in when it lands. The reader may close the conversation, the tab, or the laptop and still find the summary waiting — which is what somebody just told "this takes several minutes" would reasonably expect anyway. maintenance rather than ingest, deliberately: this holds a consumer for minutes, and mail arriving must not queue behind somebody's summary. The nudge carries no text. Same shape as JobNotifier and the insights strip — the store is the record and a missed message costs one stale render — with one reason of its own: what it would be carrying is a paragraph about somebody's mail, on a channel that has never held any. TWO BUGS RETIRED ON THE WAY. The streamed path is now never full, so the transcript it hashes for freshness is always the one the reader's page recomputes; a full run used to store a hash that could never be reproduced, which would have shown every full summary as permanently out of date. And the handler hashes the ORDINARY transcript for the same reason while sending the full one to the model — the hash answers "has this conversation changed", and the only answer that matters is the one the reader will ask. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>	2026-09-03 14:28:25 +02:00
Paul LütznerandClaude Opus 5	422fd1f489	fix(imap): read the flag notification instead of re-listing the folder ... An idling server announces a flag change as `* 12 FETCH (UID 5001 FLAGS (\Seen))` — which message, and its complete new flag set. The IDLE loop tested that line for the substrings "FETCH" and "FLAGS", learnt only that something had happened, and answered by dispatching a full mailbox sync: a fresh IMAP session and `UID FETCH 1:* (FLAGS)` over the whole folder, to rediscover what the line had just said. On a 1832-message inbox that is one full listing per message somebody marks read on their phone. It is also self-sustaining: the listing opens a second session, the server broadcasts flag state to the idling one, and that dispatches the next listing. Seen in the wild at roughly two per second indefinitely, nine identical jobs in the queue at a time, a sustained ten megabits a second of an installation talking to itself. Nothing was lost or corrupted, which is why it went unnoticed — the only symptom is bandwidth. readFlagChange() now parses the line into an ImapFlagNotice. When it carries a UID — optional in RFC 3501, required in RFC 9051, and sent by Dovecot and Gmail — the change is applied to that one message through ApplyRemoteFlagsMessage: a point lookup on the unique (mailbox, uid) and a write through ThreadStatusUpdater, which is where the echo guard lives and therefore the only door inbound flags may use. No connection, no listing, no round trip. Without a UID the line names a sequence number, and sequence numbers move under every expunge, so there is no honest way to resolve one here — that case still lists, and guessing would write the wrong message's flags with no error anywhere. The listing is now debounced at five seconds, leading edge plus a trailing flush from the read-timeout path so a burst that ends immediately still gets its sync. markSweepDue() moved into that flush: it forces the expensive full listing, and it was being called once per notification rather than once per burst. New mail is unaffected on the leading edge. Second line of defence in the handler, because the debounce is a property of one caller: a sync whose mailbox was synced within three seconds returns instead of reconnecting, so a duplicate from any future dispatcher costs a lookup rather than a folder listing. The parse is stricter than the substring test it replaces and refuses more: a sequence number is now required, which is what excludes the `* FLAGS (…)` and `* OK [PERMANENTFLAGS (…)]` lines every SELECT answers with. An empty `FLAGS ()` is kept — it means every flag was cleared, which is how "marked unread elsewhere" arrives. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>	2026-09-01 13:23:29 +02:00
Paul LütznerandClaude Opus 5	cd1e18230b	feat(caldav): plMail's calendars, in anything that speaks CalDAV ... One endpoint and an app password now give Thunderbird, Akonadi, DAVx5 or iOS every calendar on an account — as separate collections carrying the names and colours the plMail sidebar already shows, which is the thing an ICS feed cannot do however many events it carries. /caldav/ the root /caldav/principals/{user}/ who you are /caldav/calendars/{user}/ calendar-home-set /caldav/calendars/{user}/{calendar}/ one collection /caldav/calendars/{user}/{calendar}/{uid}.ics one event WHY THIS IS WRITTEN RATHER THAN sabre/dav ───────────────────────────────────────── sabre/dav was tried first and reverted. 4.7.1 is its current release and it still requires sabre/xml ^2.0.1 while sabre/xml is on 4.x, so installing it downgraded sabre/xml and sabre/uri underneath sabre/vobject — the library every ICS path here is built on — and closed off vobject 5.0, which needs the newer xml. The full suite passed with the downgrade; the cost was not correctness but a five-year-old dependency floor under the most load-bearing library in the calendar, permanently. Doing it by hand is smaller than it sounds because the hard parts already existed for the other direction. CalDavEventConverter maps an event to and from a VEVENT in 981 lines. MultiStatusParser reads the document MultiStatusBuilder now writes. And CalendarChangeReader already answered the only genuinely hard question — what changed since a token — because that was built first for JMAP, and CalDAV counts in the same numbers. WHAT sync-collection COSTS NOW ────────────────────────────── One request and the rows that moved, instead of a full collection listing on every poll. A deletion comes back as an href and a bare 404, which is why the change log copies the UID onto the tombstone: by the time the client is told, there is no row left to read it from. An unusable token answers 403 with valid-sync-token rather than an empty delta. "Nothing changed" to a client whose token cannot be placed would leave it stale with no way ever to notice. DECISIONS WORTH FINDING LATER ───────────────────────────── ETags taken from the change log's own sequence, so a resource's ETag and its collection's sync-token cannot disagree about whether something moved — they are the same number. Events predating the log fall back to updatedAt, which is good to the second; that set only shrinks. read-only a mirror advertises a read privilege set and refuses PUT and DELETE with 403. A write that appeared to succeed would show the user an edit that never leaves plMail. If-Match checked before anything is written, on both PUT and DELETE. Without it the second of two clients wins and the first edit is gone with nothing raised. no scheduling, no free/busy. plMail does iTIP its own way through ItipReplyBuilder and InviteResponder; a second system that also sent invitations would double every RSVP. Not advertising them is the supported way to say so. time-range matched on the series' bounds, so a recurring event is included whenever its rule could still reach the window. That over-answers for a series that skips it and never under-answers — one costs a resource the client draws nothing for, the other loses an appointment from a week. /.well-known/caldav is public where /.well-known/jmap is authenticated, and the difference is what they return: the JMAP one answers with the session object, this one with a 301 to a constant path. A client asks it before it has anything to authenticate with, which is the whole point of "just type your server address". Writes go through IcsImporter — the same door the web UI's .ics import uses, which already parses, maps and refuses a read-only mirror. Deletes go through CalendarEventWriter::markLocallyDeleted(), which decides whether the row may go now or has to survive until the remote it mirrors has been told. No deployment change. No schema change. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>	2026-08-31 22:40:29 +02:00
Paul LütznerandClaude Opus 5	755f3cf257	feat(users): an administrator can run the whole life of an account, and you can change your own password ... Managing users was reachable and worked — list, search, create, promote, demote, remove — but three things around it did not, and one of them was load-bearing. ── You could not change your own password ────────────────────────────────── Nowhere. Not in Settings, not by mail, not at all; only an administrator setting one for you. Settings → Security now leads with a Password card that asks for the current password (an unmapped UserPassword constraint, so it is checked against the signed-in token rather than against anything the form was handed) and takes the new one twice. The route carries no id and the form has no data_class, so it structurally cannot become the admin form by accident. The floor those passwords are held to was three literal 12s in three files that happened to agree. It is now User::PASSWORD_MIN_LENGTH, read by all three. ── An account could only be removed, never switched off ──────────────────── Nothing sat between "leave it alone" and "destroy it", so the answer to "this person has left, keep their mail" was to delete them. There is a deactivated_at now, and a UserChecker registered on BOTH firewalls — the JMAP one included, because an app password outlives the browser session it was minted from and a check only on `main` would shut somebody out of the web UI while every mail client they had already connected kept syncing. The refusal is in checkPostAuth rather than checkPreAuth on purpose. checkPreAuth runs before the password is verified, so refusing there would answer "is this address a suspended account here?" to anyone who types an address — the question hide_user_not_found exists to keep unanswerable. After verification, the only person told is the one who could otherwise have signed in. Symfony runs user checkers at authentication only, so a session already open would have carried on until it happened to end. UserProvider::refreshUser() refuses the same two states on every request. Suspension also travels through config backup, which would otherwise have handed a suspended account its sign-in back on restore. ── Two smaller lies ──────────────────────────────────────────────────────── A search under three characters silently returned the unfiltered list, which a QueryBuilder has no way to complain about, so the floor was dropped rather than kept as something that could only mislead. And an admin declining to demote themselves got a success toast for a change that never happened; the refusal now says so, while the rest of the form still saves. ── Restoring a removed user is deliberately absent ───────────────────────── Removal overwrites the address, the name and the hash specifically to free the unique address for reuse, so a restore would have to invent all three and the original address may by then belong to somebody else. Doing it properly means snapshot columns and a guided flow that asks for a fresh address — a different change, which alters what removal means. Documented as absent on the screen an administrator would look for the button on, because switching an account off is the answer to the question people are actually asking. Also fixed en route: the modal form theme dropped help_translation_parameters, so the first field to use one rendered "at least %count% characters" on screen. And GermanIsInformalTest only ever globbed messages.de.yaml, leaving validators and the new security catalogue — exactly where a translator reaches for "Sie" — outside the test that exists to catch it. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>	2026-08-31 16:02:12 +02:00
Paul LütznerandClaude Opus 5	92d6e8ffab	feat(ai): index new mail when the model is warm, and once a night ... New mail was embedded the moment it arrived. That is the wrong trade on a box with one GPU: it is rare to search for mail that has just landed, and paying to index every arrival competes with the two things somebody is actually waiting on. Two triggers replace it. A nightly sweep is the backstop, and a search is the invitation — a search has just loaded the embedding model, so the cheapest possible moment to index is immediately after one, when the model is warm and nobody is waiting on it any more. THE PART THAT MADE THIS MORE THAN A SCHEDULE CHANGE The backfill stood aside after ANY interactive AI use, and a search counted. That is precisely backwards for search, and it took both halves of the signal to say so: lastInteractiveCallAt() matched `feature IN ('search_query','writing_help')`, and the request subscriber stamped `app_mail_search` alongside `app_compose_assist`. The distinction the old code could not make is that there are two models with completely different costs. The composer spends the 20.3 GiB writing model, which is worth standing aside for and is the case the yielding was written for. Search spends the sub-gigabyte embedder — the same model indexing uses. So a finished search is not a reason to keep away, it is the best moment there is. Both docblocks now cross-reference each other, so restoring one half alone reads as obviously wrong. The opportunistic trigger claims its rate-limit marker BEFORE it queries, so the mailbox scan is inside the window rather than only the dispatch, and a search that produced no vector at all spends nothing — one unreachable moment must not cost the next five minutes of indexing. EmbedMailStep is deleted rather than emptied. An empty step would still be built and iterated on every ingest batch while carrying a name asserting that embedding happens on arrival — an active lie in the one directory somebody reads to find out what happens to new mail. The finder matches on model and not on width, deliberately. What consumes its ids asks about the model alone, so a width-aware finder would hand over a full budget the handler then drops as already done: a nightly sweep that reports itself busy and indexes nothing, for ever. The width mismatch already has an owner in the coverage report's "stale" count, which tells the user to re-index. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>	2026-08-27 01:50:25 +02:00
Paul LütznerandClaude Opus 5	377d6026b8	feat(ai): make the language model observable, and the composer honest ... "I click the AI button in the composer and nothing happens" turned out not to be about the model at all. #editor() queried data-compose--toolbar-target while the composer renders data-compose--compose-toolbar-target, so run() returned at its first guard: no request, no status, no error. The feature had never worked once. #subject() was wrong the same way and quieter — every prompt went out with an empty subject. And the status element lived INSIDE the dropdown that the same click closes, so even once working it would have written into a hidden subtree. The rest of this is what makes the next such thing findable. RECORDING One row per model call, tagged by the workload that caused it, and nothing about what was in it — no prompt, no completion, no message id, no subject, no address. error_kind is a category from a closed set for the same reason: an HTTP client's exception message routinely quotes the request body back, and here that is somebody's mail. OllamaClient was discarding Ollama's whole timing block; it now returns it, and AiAssistant — the only place that knows both the feature and the model — writes the row. DBAL and deliberately no entity, following EmbeddingStore: BackfillEmbeddings clears the EntityManager after every chunk, so an unflushed ORM row would be detached and silently dropped, fifty at a time, for as long as a backfill runs — which is exactly the window the panel exists to show. AiFeature could not carry this. A four-character search query somebody is waiting on and two thousand characters of mail body written unattended a hundred thousand times both went through embed() with no discriminator, and averaged together the second buries the first — the panel would report backfill throughput under the heading "search". AiCallFeature splits them. THE PANEL Live state from /api/ps, which is passive and safe to poll. Six states that look different, and the one that justifies the work is "partly on the CPU": size_vram < size means layers spilled, throughput collapses, and the model still answers correctly — invisible everywhere else, so everything is simply slow forever. THE COMPOSER Tokens stream into a provisional surface and commit in one operation, because hundreds of per-token insertions destroy the undo stack and Ctrl+Z is the first thing anyone tries. Waiting, generating, done and failed are distinct states with reasons: outside the keep-alive window the first thirteen seconds produce nothing while 20 GiB loads, and a featureless spinner for that IS the bug. Cancellation propagates to the host — an abandoned draft otherwise holds the GPU and everything else queues behind it. SEARCH Silent degradation to keyword-only is replaced by the reason it degraded, and rows found by meaning say so. The semantic arm and its join are now matched on model AND width: vectors from different models are not comparable, and after somebody changes the search model every existing vector is dead weight. On a pgvector install comparing different widths raises an error — a 500 on /mail/search caused by an admin dropdown — and on the shipped loop it silently compares the overlap, which is worse. Coverage is stated where results are shown, because semantic search over an 8%-indexed mailbox is a trap. BACKFILL Progress is counted from the database, never accumulated, so a killed worker resumes. It stands aside while anyone is using the AI and comes back on its own: both compete for one integrated GPU, and a batch in flight when somebody presses the button puts them straight back to "nothing happens". Also carries the cron line for app:jobs:reap from the commit before this one. Verified against the real host: reachable, both configured models installed, timings parsed, 1024-dimension embeddings. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>	2026-08-27 00:38:43 +02:00
Paul LütznerandClaude Opus 5	469a2e8107	feat(ai): semantic search, writing help, and the pass that embeds a mailbox ... The three remaining pieces. Written to the seams the survey found, and keeping the traps it named. SEARCH — ANOTHER ARM OF THE SAME UNION A vector hit is a candidate on exactly the same footing as a lexical one: it lands in COUNT(*) OVER (), it pages correctly, and it needs no second statement to reconcile with. Four things had to hold and each is written down where it applies: · The query is embedded ONCE, in the controller. buildSearchSql() runs up to four times per search — the cheap pass, the body rescue, and twice more when a page past the end recovers its total — and one of those is inside a statement-timeout transaction, where a slow model would be reported as a database fault. · The candidate set is BOUNDED to the most recent 2,000 messages. The distance function costs 0.11ms a row; over a whole mailbox it is eleven seconds, worse than the pathology the UNION shape exists to escape. · A THRESHOLD, not a top-k. The total rides on the same statement, and "lexical matches plus up to k" is a number that is only true on page one. · The inner LIMIT is parenthesised, because these arms are joined with a bare ' UNION ' and an unparenthesised LIMIT binds to the whole union rather than to one arm. The rank join is 1:1, which is the property the label joins lost and had to be given back in v0.1.40. Three tests: a message found by meaning that no lexical arm can reach, one rejected by the threshold, and one with no embedding still found by words — that last is what proves the LEFT JOIN does not drop rows. WRITING HELP — THE SERVER MAKES THE CALL connect-src 'self' is enforced, so a browser could not reach the model host even if it should — and it should not: that address is on the operator's private network, and putting it in a page hands it to every script the page loads. The route carries NO draft id. A composer opens before one exists, which is exactly when somebody wants a first draft; a URL built at render time with an id in it is stale in that window. Nothing is saved as a side effect either. Four tasks, a closed set. A free-form instruction parameter would be a prompt an attacker could write, with the user's own mail as context and the answer pasted into the user's own draft. The message being replied to is read from the DATABASE by id, ownership checked — never from the page. Generated text is INSERTED, never substituted. A rewrite that overwrote the original would be a model deleting somebody's words with no undo they can see. It goes in as a plain text node at the end of the body: a styled wrapper would be subtracted by the composer's typed-length calculation and could stop the draft autosaving, and "at the caret" is a position the writer did not choose, because opening the menu moved focus. The menu is behind a Twig FUNCTION, not a variable — this window is included from four places, two of them undo streams that include it `only`, and strict_variables makes a variable threaded through three of four a 500 on the undo path rather than a missing button. EMBEDDING — ARRIVALS ON INGEST, THE REST ON DEMAND A post-ingest step for new mail, and a command for everything already there. The backfill re-dispatches itself with a cursor rather than looping: a hundred thousand messages at a round trip each is hours, and a single job holding that is killed by any worker restart with nothing to show for it. Ascending id, because it is the one ordering nothing can change under a walk that long — mail arriving during it gets a higher id and is met by the pass still coming. The vectors already stored ARE the progress record, so there is no second one to keep in step, and the cursor advances past everything LOOKED AT rather than everything stored, so one message the model cannot answer for does not become a wall it restarts at forever. On the maintenance transport, not ingest: a backfill running for hours would otherwise sit in front of every arriving message, and new mail would stop appearing until an old mailbox had finished being catalogued. KNOWN FAILING, AND NOT FROM THIS compose-send-optimistic.spec.ts:124 fails in the FULL e2e run and passes in isolation and in pairs — verified with the AI menu both present and removed, 24 passed either way. It is the same spec that broke in the earlier seed race, and the row it cannot click is present but still mid-entrance. Cross-file interference, pre-existing, and left for the debugging pass. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>	2026-08-26 19:29:39 +02:00
Paul LütznerandClaude Opus 5	ac5b5360fe	feat(ai): a second opinion on mail no rule recognised ... Categorisation, built so that the model can only ever break a tie. WHERE IT IS ALLOWED TO COUNT At the very bottom of the cascade, after every rule has declined. At that point the honest answer is "we do not know, so Primary", and that is the one place another opinion helps without doing harm. Anywhere above it, the model could only overrule something that actually matched a header — which would make a tab's contents depend on which model happened to be installed the week the mail arrived, with nothing on the row to say so, decided unattended on every message that arrives. MailClassificationTest asserts it cannot: a matched header, a correspondent and a Gmail label each outrank a verdict that disagrees. STORED BESIDE THE CATEGORY, NEVER IN IT `category` is what the deterministic rules produced and what every list query filters on, and it has to stay explicable without reaching for a machine that may since have been switched off. So the verdict lives in its own column and the cascade reads it only in the branch it would otherwise have answered "default" from. Which makes switching the feature off sufficient on its own: nothing clears the column, nothing needs re-processing, and a stale verdict from a model removed months ago is inert as long as a rule recognises the mail. There is a test for exactly that. NO CALL ON ANY READ PATH MessageCategorizer never talks to anything — it reads a verdict stored asynchronously. explain() is called from the details panel, from ingest and behind list rendering; an HTTP round trip in there would put another machine on the path of drawing a page. The one request lives in a Messenger handler, on a worker, after the transaction, over a batch, for a subset. The step that dispatches it checks the feature is on before enqueueing anything, and the handler checks again — not redundantly: almost every install has this off, and without the first check every one of them would queue a job per batch for a handler whose whole body is `return`; without the second, a job already queued when somebody switches it off would still run. WHAT IT REFUSES TO ASK ABOUT Anything a rule recognised — the answer is known, explicable, and would be discarded anyway. That test needs the correspondent set, which is a query, so it happens in the handler rather than in the step that runs inside a sync holding a mailbox connection. The attempt is stamped even when the answer is unusable, so a backfill can tell "asked, got nothing" from "never asked" and does not retry the same unanswerable message forever. An ambiguous answer — two category names in one reply — is discarded rather than resolved by first match: the model has not answered the question, and leaving the mail where the rules put it is the honest result. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>	2026-08-26 18:01:42 +02:00
Paul LütznerandClaude Opus 5	e00fe25a14	feat: save a signature once instead of drawing it on every document ... Settings → Profile gains a pad beside the avatar. Draw a signature there and the PDF reader offers "Use saved signature", which places it without asking again; remove it from the same place. It goes through exactly the same placement path as a fresh scribble, because a stamp has been an IMAGE rather than a set of strokes since the day signing was written. That was the point of expressing it that way: this is six lines, not a second implementation to keep in step. The two pads share trimAlpha from assets/pdf_geometry.js for the same reason — they have to agree about what a stamp is. Deliberately NOT the "Signature" section in the sidebar. That one is the block of text appended to outgoing mail; the two are unrelated and the wording keeps them apart. SignatureStorage is AvatarStorage's shape — one file per user under var/uploads, replaced in place, random filename so a changed image is not served from cache. PNG only, and that is a requirement rather than a narrow allow-list: the image is stamped onto somebody else's document and anything without alpha lands as a white card over their text. It is not a credential. There is nothing to verify against it and it is encrypted by nothing, which is the honest treatment for a picture of a squiggle. FILES OUTLIVE ROWS Erasing a demo visitor now deletes their avatar and signature too. DemoUserEraser finds what to delete from Doctrine's metadata, and a PNG under var/uploads is not a row — so on a public demo minting a throwaway user per visitor, every signature drawn was staying on disk for good. TWO BUGS THE TESTS HID The save endpoint redirected to a route named `app_settings`, which does not exist — every POST was a 500. Three of the four tests passed anyway, satisfied by a signature an earlier failed run had left on disk. They clear any saved signature in beforeEach now, so "it saved" cannot pass without anything being saved. And the first version could not draw at all: the pad sits below the fold, page.mouse works in viewport coordinates, and boundingBox() happily reports a box off-screen. The strokes landed nowhere. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>	2026-08-25 16:38:46 +02:00
Paul LütznerandClaude Opus 5	748e1cdcdf	feat(demo): a privacy notice, and two fixes found by using the demo ... **The privacy notice gets its own page and its own link.** A section inside the Impressum is not one — it has to be reachable in its own right — and it is required even though this instance asks for nothing. A web server writes the caller's IP address into a log on every request and an IP address is personal data, so "we collect almost nothing" is a statement that has to be made rather than a reason to omit the page. Written from what the software does rather than from a template. The cookies named are the three it actually sets, observed on a real visit; the claim that nothing reaches a third party is the content security policy, which permits connections to this server only; and the retention period is read off APP_DEMO_TTL through DemoMode::ttlDescription() rather than typed into the prose, because a notice stating a period the reaper does not honour is worse than one saying nothing, and two copies drift the first time somebody tunes the setting. APP_DEMO_PRIVACY_HOST names whoever runs the machine, who is a processor. Unset, the page says so in red — the same rule the Impressum follows, for the same reason: a legal notice naming nobody looks compliant from a distance. **Pressing Receive twice looked broken.** The second press appeared to do nothing for up to thirteen seconds. _refreshList() coalesces bursts through a fifteen-second floor measured from the LAST refresh, so the press right after a delivery was the one certain to be inside it. The floor is right — a sync publishes one event per mailbox per account, and it turns eight fetches into one — but it was being applied to somebody who had just pressed a button and was watching for the result. refreshNow() skips it; onAccountSynced() still does not. The difference is whether anybody is waiting. **The user menu still wore its own tooltip.** The previous fix stopped a NEW bubble appearing over an open menu but left the one already on screen, which is why it vanished only when the pointer left. There is a document-level click listener whose whole job is to take it down, and ui--user-menu#toggle calls stopPropagation(), so the click never reached it. The listener runs in the CAPTURE phase now — at the document, before the event reaches its target, where no handler can hide a click from it. The same reason the scroll listener above it already uses capture. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>	2026-08-25 12:44:35 +02:00
Paul LütznerandClaude Opus 5	6e9a441333	fix: an install page a demo cannot close, and eleven flashes nobody translated ... Two bugs found by using the thing. **A demo visitor could lock an install out of /install.** InstallGuard closes that page as soon as a user exists, and demo mode mints one per arrival — so on a fresh demo instance the first passer-by or crawler took the install page away with them, permanently, before the operator had made themselves an administrator. Nothing announced it either: the page 404s, which is exactly what it does on a correctly installed instance, so the symptom is indistinguishable from working. Both the guard and createFirstAdmin's advisory-locked re-check now count real users only. **Flash messages reached the page as raw translation keys.** The flash bag on this install holds sentences, not keys — the toast region prints what it finds, and cannot translate, because most of these interpolate a name or a count only the controller has. Eleven call sites across TwoFactorController, SecurityStepHandler and AdminDashboardController pushed keys instead, so a rejected code produced a red toast reading "two_factor.flash.code_rejected". The strings existed and had been translated into all three locales months ago; nothing looked them up. A static test now scans src/ for flash calls passing a dotted literal, because nothing about that failure is visible in review — the call site reads exactly like a correct one. That fix broke two E2E tests, which is the interesting part: they asserted on "Two-factor authentication is on" and had been matching the panel heading ALONE because the toast beside it was showing a key. Translating it made both match and strict mode rightly refused. They were passing because of the bug. **Demo mode, from running one publicly.** The account form opened in a turbo-frame, so the guard's redirect had nowhere to land and the modal span forever — it now renders a panel saying why the form is not coming. The receive button was a full browser navigation, which reloaded the page on top of mail Mercure had already delivered AND wiped motion.js's in-memory set of seen rows, so the whole list replayed its entrance; it is a stream touching only the demo bar now, and mail simply arrives. Plus an Impressum, because a public demo in Germany needs one — blank by default and loudly saying so, since § 5 TMG wants a real operator and a notice naming nobody looks compliant without being so. **Interface, all reported from use.** The snooze menu's two distant options were taller than the near ones, because anything past two days carries the date and "Sat, Aug 29, 08:00" wrapped inside min-w-56. Clicking a dropdown trigger focused it, and focus shows a tooltip — so opening the user menu painted its own hint across the menu. The eight recovery codes ran together on one line: they were joined with real newlines and <code> collapses those without whitespace-pre-line, so nothing was rendering what was already there. Confirming a 2FA code gave no sign anything was happening, and the reply carries codes shown exactly once. Layout was the last <select> on a panel where every other short choice is a segmented group. Solar and paper put a white field on a warm off-white surface, which under the boxed layout's translucency left the search box with no visible edge. Read receipts had a tab of their own coming — the card holds nothing else, and signatures left the alias tab before it for the same reasons. And /settings opened on its fourth entry. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>	2026-08-25 11:03:08 +02:00
Paul LütznerandClaude Opus 5	231d0ab0da	feat(demo): a public demo mode, with mail that arrives on demand ... plMail is hard to show. It is a mail client, so everything interesting only happens once there is mail — and a stranger cannot reasonably be asked to hand a demo server their IMAP password to see it. APP_DEMO_MODE fills that gap: mail that is nobody's, and a button that makes more of it arrive. One switch for the whole install, not a per-account flag. The promise is that nothing reaches the network, and that is only checkable if there is one place to check; a per-account flag would move the promise into every outbound path individually, where it holds until the first one that forgets to ask. Read through `default:` so an install that never heard of the variable is a normal install — the accident worth designing against is a real deployment becoming a demo, not the reverse. **A mailbox per visitor.** /demo provisions a throwaway user, signs them in and drops them in a seeded inbox. One user each rather than one shared account: two people arriving at once would otherwise be marking each other's mail read and reading each other's drafts, and the product would look broken while working perfectly. app:demo:reap deletes them on a timer. **Receiving.** A bar at the foot of the page walks a scripted queue — a reply, an invoice with a real attachment, a delay notice, an HTML newsletter — and wraps at the end, naming what comes next. Delivery goes through the real PostIngestPipeline rather than writing rows by hand, because what is being demonstrated is that plMail *does* something when mail arrives: it threads, it categorises, it runs the filters the visitor just built, and it appears without a reload. A hand-written row would put mail on screen and none of that would be true, which is a worse demo than no button. **Sending.** DemoMailSender is registered above every real sender and claims every account while demo mode is on, so SmtpMailSender is never asked — "a demo cannot reach a relay" becomes a property of the wiring rather than a rule each sender has to remember. It answers a few seconds later, from whoever the visitor addressed, on the thread they are still looking at. **What is switched off**: adding or editing a mail account, OAuth and CalDAV connect, calendar subscribe — a stranger typing their real credentials into a box on someone else's server has handed their mail away, and there is no version of that form that is safe to show. Mail sync, push renewal and calendar sync no-op, since a demo's accounts point at documentation domains. Everything else stays clickable: locking down all the non-mail surfaces produces a tour of a product with the interesting doors locked. Two things this needed that did not exist. The demo mailbox moved out of SeedDemoMailboxCommand into DemoMailbox, shared with the provisioner, so the README screenshots and the hosted demo cannot drift into being pictures of different things. And deleting a user — this project had no such path, because a self-hosted install's users leave by having their password changed. The eraser discovers owned classes from Doctrine's metadata rather than a list: twenty entities point at User today under two different property names, and a hand-written list would be right until the next entity was added, at which point the reaper would start failing on a foreign key at three in the morning. compose.demo.yaml hardcodes APP_DEMO_MODE rather than interpolating it, and ComposeEnvironmentTest now holds it to that. Compose reads Symfony's committed .env, which ships the variable as 0, so `${APP_DEMO_MODE:-1}` would have resolved to 0 for everyone who cloned the repo — and the failure is the bad kind, where the stack comes up healthy and simply is not a demo. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>	2026-08-25 10:05:40 +02:00
Paul LütznerandClaude Opus 5	e4a642b1bf	fix(mail): the message frame is authorised by a hash, so Turbo cannot break it ... Reported after the last fix: the frame still does not measure itself on the FIRST open of a mail, only after a reload. Sharing the request's nonce was right and not enough. A page's policy is fixed when the page loads, and a conversation is normally rendered by a LATER request — fetched into a Turbo Frame — which has a nonce of its own. The srcdoc inherits the page's policy on top of its own, so the script carried a nonce that policy had never heard of. The two coincide only on a full page load, which is exactly the shape of the report. There is no request that renders both, so no nonce can work here. A hash can. It is a property of the script text: identical in every request, so the page's policy and the frame's own can both name it without either knowing when the other was rendered. It is also tighter than the nonce it replaces — a nonce authorises anything carrying it, a hash authorises one exact script and nothing else. The script has no Twig in it at all, which is what makes this possible. The script moved into templates/mail/_frame_script.js so the hash is DERIVED rather than maintained beside it. A written-down hash drifts the first time somebody edits the script, and that failure is silent, remote and production-only: under debug the policy is Report-Only, so every test would still pass while every real message sat at its 80px floor. Three tests. The one that matters asserts the PAGE's header names the hash — the half the frame cannot check for itself, and the one that fails on the original bug; verified by reverting the listener. The K-07 rendering-security spec now pins the whole directive rather than the prefix, so 'self' or 'unsafe-inline' creeping in beside the hash fails too. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>	2026-08-24 13:52:34 +02:00
Paul LütznerandClaude Opus 5	59cbbc9827	feat(mail): delete for good, and a way back out of the bin ... Reported as: deleting a mail that is already in the trash takes the row off the list and leaves the mail in the trash. And, separately: why can spam and trashed mail be snoozed and archived? Both are the same cause. Trashing is "add the Trash label", so trashing something already trashed is a no-op on the server — while the list removed the row anyway, which is why the mail looked deleted twice and was there both times. Archive and Snooze in the bin are the same question from the other side. ## The action set now depends on where the mail sits MailPlacement reads the conversation's own labels rather than the current route, because a thread reached from a search result, a live update or a URL has no list behind it and a route check would go quietly back to offering the wrong controls in exactly those cases. Archive, Delete and Snooze are hidden in trash and spam, on the row, in the thread header and in the overflow menu. It is a service the Twig function delegates to, not a Twig function alone: the purge route refuses anything MailPlacement does not call discarded, and an interface that hides a control while the route behind it stays open is a habit rather than a rule. ## Delete forever Hiding the control left the bin with no way to be emptied, which is the wrong place to stop, so this is the real thing. Most of it turned out to be written and unconnected: ApplyImapFlagsHandler has implemented `'delete' => $imapMessage->delete(expunge: true)` all along, under a comment saying the row was "on its way to having no row at all", and MailChangeRecorder::emailDestroyed() existed with no callers. What was missing was the part that removes the local copy — and one thing that could not have been reused. **LabelChangePropagator cannot carry a purge, and that is the whole design.** Every propagation message names local Message rows and lets the handler look them up. That works while the row survives the operation. A purge deletes it, so a handler would find nothing, log "no messages found" and return — leaving the mail on the server to come back at the next sync as a message the user had explicitly destroyed. Invisible against a fake provider. So PurgeRemoteMessagesMessage carries the ADDRESS instead: IMAP folder and UID, Gmail id, Graph id. Onto the export queue with every other outbound push, so a purge cannot overtake a relabel already queued for the same mail. Note propagator::delete() is NOT dead code and is untouched — it is ComposeController::discard(), where TRASH at Gmail is right rather than a compromise: a draft that was never sent belongs in the bin its own client would put it in. Its docblock now says so, because I misread it as dead on the way past. Gmail gets messages.batchDelete, needing the full mail scope. Without it the API answers insufficientPermissions, which assertSuccess() already classifies as unrecoverable — so it lands in the log once rather than retrying forever. Falling back to TRASH would be the worst available answer: the user is told it is gone, plMail can no longer see it, and Google still has it. The files go too. A deletion that leaves the bytes on disk is a lie in the way that matters most for a mail client. Parts are removed explicitly rather than by adding cascade to the mapping, which would change every other path that touches a message. ## Restore The counterpart the bin never had: mail could be thrown away and not fetched back, except by finding it and attaching Inbox by hand from the label menu. Removes both Trash and Spam, not only the one it was found under — a spam mail somebody then deleted carries both, and one returned to the inbox still marked SPAM gets filed away again by Gmail without plMail doing anything. ## Also here - Pasted markup is reduced to content (CODE-REVIEW U-02): style, class, id, bgcolor, font and every alignment attribute go; lists, links and bold stay. Whitelisted rather than flattened to plain text, which is simpler and throws away what people are usually pasting for. The `<style>` block was the worst of them — it restyled the whole composer, not just the pasted text. - Confirmations go through the app's dialog, not the browser's. Note the row's Delete forever asks in JavaScript rather than through data-turbo-confirm: Turbo only consults that attribute for links and form submissions, so on a fetch-posting button it would have looked like a confirmation and never been read. Verified: 4009 unit tests green, PHPStan clean over src/. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>	2026-08-22 13:16:18 +02:00
Paul LütznerandClaude Opus 5	a4d09b8433	feat(security): a content security policy on the app's own document ... CODE-REVIEW S-03. The reading frame carries a carefully built policy and the image proxy carries one; the document holding the session — composer, settings, admin — carried none. That is the amplifier that turned S-01 and S-02 from "script in a contenteditable" into "take the account". With a policy both would have been malfunctions. Enforced in production, reported under debug, because the profiler toolbar and VarDumper inject inline scripts nothing can nonce and they exist only when debug is on. The always-safe directives — object-src 'none', base-uri 'none', form-action 'self', frame-ancestors 'self' — are enforced everywhere. base-uri is the one worth naming: an injected <base> silently re-points every relative URL on the page, module imports included, and is invisible in the markup. It got to "enforced" by removing the violations rather than allowing them, and the last one was the application's own stylesheets. AssetMapper implements `import './app.css'` as a `data:application/javascript,` module that appends a <link> at runtime, so every stylesheet was a script as far as the browser was concerned. Permitting them would have meant `script-src … data:`, which readmits one of the oldest injection vectors there is and gives the directive away. Both stylesheets are <link>ed from the layout now — which is what the AssetMapper documentation recommends for CSS anyway, and removes a flash of unstyled content as a side effect, since the browser no longer has to run the module graph to discover them. The nonce is one per request, shared by the header and the layout through a service rather than generated at each end — two nonces is the classic way this is got wrong. It implements ResetInterface because FrankenPHP's worker mode keeps the container alive between requests, and a nonce that survives into the next visitor's page is not a nonce. The spec asserts both: same value in header and document, different value on the next request. Also, unrelated but reported while this was in flight: the entrypoint's symfony/skeleton branch is gone. It was stock scaffolding under a comment saying it could be deleted, it could only fire when /app held no application, and what it did then was fail with Invalid stability string "", expected one of stable, RC, beta, alpha or dev from six containers in a restart loop — because $SYMFONY_VERSION and $STABILITY are unset in this image. Nothing in that names the actual problem. It now exits with a message naming both ways to reach it: running the dev override from outside the checkout (it bind-mounts ./ to /app), or a stale local pl_mail-dev:latest being used without the override that builds it — compose.override.yaml.dist predicted exactly this at its `image:` line. Reproduced against an empty /app before and after. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>	2026-08-22 12:29:47 +02:00
Paul LütznerandClaude Opus 5	8c3080d00a	fix(security): an SVG attachment is not an image, and a shipped install gets headers ... CODE-REVIEW S-02 (high), S-04, S-08, S-05, S-06. S-02 — three controllers decided what may render inline with `str_starts_with($contentType, 'image/')`, and `image/svg+xml` satisfies it while executing script as a top-level document. nosniff does not help: nothing is mislabelled, the type genuinely is what it says. The type comes from the MIME headers of an incoming mail. Reachable from the mail itself. MailBodySanitizer::resolveCids() rewrites EVERY `cid:` in the document rather than only `img src`, and the sanitiser allows relative links — so `<a href="cid:logo">Rechnung ansehen</a>` has the part id filled in by the application, and the click opens the attachment route without the `?download=1` that every template adds. Now an allow-list (InlineDisposition), which is the difference between "everything not yet known to be dangerous" and "the formats actually wanted", plus `default-src 'none'; sandbox` on the response — so a later widening of the list is a bug rather than a vulnerability. Parameters are stripped before comparing, because `image/svg+xml; charset=utf-8` would otherwise walk past the one entry meant to stop it. S-08 pulled along: the JMAP download had the same prefix test. Guarded today only by the jmap firewall being stateless and wanting a header a browser navigation does not send, which is a thin thing to depend on. S-04 pulled along: the file-picker preview carried no Content-Disposition at all — implicitly inline, in our origin, with a type chosen by Nextcloud, Immich, Dropbox, OneDrive or Drive. A file an attacker drops in a shared folder is a realistic shape for the setups these integrations exist for. A non-image now falls back to the placeholder rather than to a download: a download prompt fired by a thumbnail is worse than a grey box. S-05 — the Caddyfile now sends HSTS, X-Frame-Options, nosniff, Referrer-Policy and Permissions-Policy, and drops the Server banner. None of these existed anywhere in the repository, so the ones a live instance showed were its operator's proxy — while the README's own install instructions are `docker compose up -d` and "open https://localhost". `defer`, so the per-response policies on the attachment and image-proxy routes still win. Referrer-Policy is the one that matters most for a mail client: it sends its users to strangers' links all day. S-06 — mercure subscribe defaults to [] rather than '*'. Never reached today, since both cookie-minting paths pass explicit topics, but a future createCookie($request) without a topic argument would have issued a subscription to every user's mail events and looked entirely ordinary in review. Also: mailpit removed from the dev override. MailerInterface is injected in no service in src/ — plMail's own outgoing mail leaves through the user's account via MailSenderRegistry — and the override never set MAILER_DSN either, so the container has been catching nothing. Both configuration pages said the dev stack pointed at it; they now say what is actually true. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>	2026-08-22 12:16:02 +02:00
Paul LütznerandClaude Opus 5	80f3fb5932	fix(test): the browser suite ran on a session store production does not have ... The E2E flake, and it was never in the specs. framework.yaml carried the Symfony skeleton's `when@test` line pinning session.storage.factory.mock_file. That is right for what the skeleton means by a test — PHPUnit, one CLI process, one request at a time — and this project also SERVES its test environment: compose.test.yaml runs the real FrankenPHP under APP_ENV=test precisely so CI exercises the app as shipped. That stack inherited the mock along with the environment. MockFileSessionStorage reads the session with file_get_contents and writes the whole bag back with file_put_contents, no lock. Two overlapping requests in one session therefore read the same bag and the later writer stamps the earlier one out. The native handler production runs on flocks, and they queue. So: a page render mints a stateful CSRF token into the session, the JavaScript on that page fires its own requests while the response is still being written, whichever finishes last writes its copy of the bag — and when one of those won, the token the form was holding had never existed as far as the next request could tell. "Invalid CSRF token", on a different spec every run, which is why it read as several unrelated flakes rather than one bug. twofactor.spec.ts is where it was pinned: enrolling with a code the server never got as far as checking. csrf.yaml documents the other half of this from the application's side — `compose` was moved to a stateless token id because of this exact race, and the comment says it cost a day. Several dozen stateful ids remain, many with per-entity ids no stateless list can name. They are now exercised under the concurrency semantics they actually ship with. A deletion rather than a per-process switch because PHPUnit does not need the mock: the full suite passes on native storage (3982 tests). That mattered — gating it on an environment variable would have been wrong, since CLI and server share var/cache/test and whichever compiled the container last would have decided for both. Verified: three consecutive full Playwright runs, 537 passed, no failures and no retries. Worth noting 537 rather than the 517 a passing run used to report — the exclusive project (integrations, mercure) declares a dependency on the main one, so every flake was also silently skipping those 19 specs. The flake cost more coverage than it appeared to. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>	2026-08-22 11:55:28 +02:00
Paul LütznerandClaude Opus 5	a5f53a5b47	fix(compose): a reply could be silently thrown away by a concurrent request ... The compose form was the one form in the app still protected by a SESSION-STORED CSRF token. ComposeType::configureOptions() overrides `csrf_token_id => 'compose'`, and that id was not in the stateless list — so while every other form derived its token, compose minted one into the session and needed it to survive there for the whole life of the window: minutes of autosaves, not the seconds an ordinary form is on screen. Anything writing the session in the meantime takes it away again, and the storages differ on whether that can happen. The native handler flocks the file, so concurrent requests queue. MockFileSessionStorage — what `when@test` serves the E2E stack with — and every non-locking store a scaled deployment reaches for (Redis, memcached) write the whole bag back and let the last response win. The sequence, measured: opening the inline reply box renders the window, which mints the token; the browser fires POST /mail/threads/listed at the same instant; the two responses finish about 4ms apart. When threads/listed finished last it stamped its copy of the session over the other's and the token was simply gone. The autosave two seconds later was refused with "The CSRF token is invalid", DraftPersister was never reached, and the reply was not saved. Found as a Playwright flake — "keeps the correspondent in the row after you reply", failing 5 runs in 12 — which is the cheap version of the same bug. The expensive version is a user on Redis sessions losing a reply they watched the composer say "Draft saved" about, because a refused form still re-renders with HTTP 200. That second half is a separate fault and is not fixed here. Stateless, the token is derived rather than stored, so there is nothing left for a concurrent write to lose. Verified: 5 of 12 fail without this, 12 of 12 pass with it. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>	2026-08-18 20:12:59 +02:00
Paul LütznerandClaude Fable 5	8533993c4b	feat(insights): user-facing configuration, straight off the registry ... Settings → Insights lists whatever the extractor registry holds — name, hint and an On/Off segment per extractor, so a new extractor appears here by existing. Off is stored as the disabled set, so next release's extractor starts working without anyone finding a new toggle. The endpoint validates keys against the registry and carries the same CSRF discipline as its neighbours; an extractor without catalogue lines falls back to its key rather than printing a translation path. Handbook gains "The radar" in both languages. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>	2026-08-17 19:35:38 +02:00
Paul LütznerandClaude Fable 5	c7979c7d85	feat(appearance): the pl mark drops Google's colours and becomes a choice ... The topbar monogram wore Google's literal brand hexes, which is why it read as someone else's product. The mark is redrawn — shared top and bottom lines, a square-ish bowl, a tighter tail turn, ink centred in its frame — and its geometry now lives in exactly one place, a Twig macro that the topbar, the settings tiles and the favicon all draw through. The paint is a per-user setting: thirty-two colourways in LogoStyle (single colours, duotones, ink with one coloured flick, and sweeps), picked under Settings → Appearance → Logo, applied to the topbar live and saved through the same pipeline as every other appearance knob. Berry — the product's own sweep — is the default every account starts on. The favicon follows the person now, so it is a route rather than a file: /branding/favicon.svg answers in the viewer's colourway, revalidated by an ETag that IS the style value, and anonymously it serves the product default. The static icon set (favicon.svg, the PWA PNGs, apple-touch) is repainted in Berry as the sessionless fallback, and the manifest and theme-color meta follow its tile. Two colourways from the design board are deliberately absent: the Google original, and the four-distinct-hues restatement of it. A test pins both out, along with the seven-strokes-per-style contract the macro depends on. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>	2026-08-17 17:49:54 +02:00
Paul Lützner	adc70aec0a	refactor: three classes move to where they belong ... DevSyncController was not a dev controller. It is the topbar's sync button — something every user sees on every page — and it served /dev/sync from App\Controller\Dev under a docblock opening "TEMPORARY". A route under /dev is one an operator could reasonably block at the proxy, and the sync button would go with it. It is App\Controller\Mail\SyncController at /mail/sync now. Its queue probe was inline SQL in a controller, which is the one thing §5.3 says a controller may not hold. The reasoning for why it has to be raw — the transport's table has no user column, so the count matches serialised envelope bodies — moves with it into MessengerQueueRepository, beside the other reads over that table. SidebarCounts and NewMailMarkers were in src/Twig and are neither Twig extensions nor Twig-only: they hold no Twig types, they decide things (TOTAL_ROLES, SILENT_ROLES, forRoleBadge), and MailController injects both to answer a JSON endpoint. Being a Twig global does not require living in src/Twig — FilterDescriber and AppVersion are already globals from src/Service. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> (cherry picked from commit 43e376c0e07bee30a58da46e5b89ceb0a25f04c5) (cherry picked from commit 68e2d2fdf10238389c1c2dc8b19f6371940043e6)	2026-08-17 11:25:14 +02:00
Paul Lützner	e25a3a3eed	refactor(security): one CSRF check, and a SameSite that is stated rather than assumed ... Fifteen copies of the same three lines, under two names — validateCsrf() and assertCsrf() — and they had already drifted: some read `_token` with an empty default and some without, some passed a message to the exception and some did not, and one baked its token prefix into the helper so the call sites read `validateCsrf($request, $id)` for a token actually called `admin_failed_$id`. None of that was a bug. The point of one copy is that the next divergence cannot be one either. It takes the token from `_token` or `X-CSRF-Token`, because the app genuinely uses both and the choice is the frontend's. `cookie_samesite: lax` is now written down. It was already the default and already what every current browser assumes, so nothing changes — the point is that several state-changing POSTs were protected by a token AND by the browser refusing to attach the cookie cross-site, and the second half of that was true only by accident. NOT `strict`, and that is load-bearing: the OAuth callbacks are top-level cross-site navigations and verify a state value held in the session, so under `strict` every account connection would fail its own check. Two authenticated POST controllers that checked nothing now do. Web push takes its own token from the template, and the remote-image trust pair gets the templates for the per-message tokens its controller began asking for. `/settings/push/verify` keeps a documented exemption: a service worker has no document to read a meta tag from, and it already proves itself with a per-subscription secret this server pushed to the device. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> (cherry picked from commit 0a7e92727a2d1e37107824caace9920adff0da3d) (cherry picked from commit fa48d0c06585003aea1c4cb5e9c07c83bd25a01e)	2026-08-17 11:25:14 +02:00
Paul Lützner	d2a2806764	fix(push): revoke what teardown registered, and say which way push is broken ... Three faults with one root: a push registration lives both here and at the provider, and plMail only ever managed its own half. **Teardown left the provider's half live.** ResetDataCommand had zero references to any push manager — it truncated the rows and left Google and Microsoft holding channels aimed at this install, which then delivered notifications nothing could resolve until they expired days later. That is the "notification for an unknown channel" trickle in the reported logs. Worse, no caller anywhere reached GoogleCalendarPushManager::unsubscribe() or its Graph twin on ANY calendar-removal path, so deleting a calendar, unticking one, or disconnecting a connection did the same thing. Account deletion had always done it correctly, which is what made the omission easy to miss. PushTeardown is that missing call, wired into DataResetter (so the admin panel gets it too), CalendarSubscriber::unsubscribe() and the calendar delete route. It runs BEFORE the truncate — afterwards there is no credential left to revoke with, so revoking late would pass a naive test and fix nothing — and it is best-effort: a provider that refuses, 404s an already-expired channel, or cannot be reached is logged and stepped over. A reset that will not run because Google is unreachable is a worse failure than a channel that lapses on its own within a week. **A dead mail push was invisible for a day and a half.** The old check fired after 36 hours of SILENCE, and silence cannot tell a broken push from a quiet mailbox — which is exactly why the threshold had been made so generous, and exactly why it said nothing while a user watched push deliver nothing for twelve hours. Rather than trade one arbitrary constant for another, the quantity changed: Gmail pushes on any history change, so a history that advanced more than one sweep period (15 min — the poll's own cadence, not a tolerance) after the last push is a change push demonstrably missed. A mailbox that never changes never produces that evidence and never raises anything, at any hour. Both push verdicts are now facts, so both are Warnings that light the indicator; the false-alarm case that justified Notice no longer exists. **Nothing said WHICH failure it was.** Lapsed (expiry passed — renewal did not run) and Degraded (alive, but mail arrived unannounced — the Pub/Sub leg) are separate kinds with separate copy, and each card carries the three dates it was judged from: when the registration expires, when it last delivered, when renewal last ran. PushRenewCommand now records each completed run as a ProcessHeartbeat milestone, exempt from the stale sweep because a milestone that stopped beating IS the finding — that is what makes a scheduler that simply stopped firing visible at all. Also found and fixed while verifying: the accounts pane compared health to the literal 'degraded', so a LAPSED account would have been offered the plain toggle, whose only effect there is to turn push off; the health page's repair returned a turbo-frame fragment for a frame that page does not have, so pressing it reported nothing whatsoever and could not be told from a no-op; and fa-bolt-slash is a Pro icon absent from the Free build, so the degraded card had always rendered a blank where its glyph should be. The re-arm repair already existed and is reused rather than duplicated; it is CSRF-guarded and ownership-checked, and both are now pinned. Migration is additive, nullable, no backfill.	2026-08-13 22:54:08 +02:00
Paul LütznerandClaude Opus 5	7dbd6cc890	feat(health): surface broken accounts, and repair them without re-adding ... One expired Google grant on the reporting install had stopped mail for two days and produced ~5 000 log lines, and the app knew: OAuthTokenManager wrote `invalid_grant` to Account::$oauthLastRefreshError, CalendarSyncService wrote the calendar failures, IntegrationTokenManager already composed "X needs to be reconnected". Nothing read any of it back — grep for oauthLastRefreshError over templates/ and src/Controller/ returned nothing, and no template contained the word "reconnect". The condition was detected, stored, and discarded. WHAT THIS ADDS A health model (AccountHealthInspector) that reads that stored state and answers "what is wrong, and what would fix it". It reads; it never probes. Five problem classes: dead OAuth grant, calendar sync failing, degraded push, integration needing reconnection, and abandoned queue work. Consequences are attributed to their cause. One dead grant taking three calendars and 500 jobs down with it is one thing to do, not nine, so the calendars are listed under the account that explains them and the indicator counts root causes. Severity is drawn by consequence, not by how alarming the error looks. Degraded push is a Notice and deliberately does NOT light the indicator: mail still arrives, and a page that paints "slightly delayed" the same red as "stopped" is a page people learn to close. RECONNECT IN PLACE OAuthAccountLinker::relink() writes a fresh grant onto the existing Account row. Mail, threads, labels, rules, aliases, calendars and settings all key on that id and all survive — the lighter alternative to delete-and-re-add, which is what people resort to today and what costs them their mail. Guarded on identity: the authorised mailbox must be the same address and the same provider, compared case-insensitively, and a mismatch is refused outright rather than resolved. Signing in as the wrong Google account is a mistake anyone makes, and writing those tokens here would file a stranger's mail into these threads with no undo. The repair intent travels in the session, never through the provider redirect, so no account id is user-editable. STOPPING THE LOG FLOOD A calendar that cannot sync never updates lastSyncedAt, so findDueForSync() returned it on every sweep forever. Calendar now carries syncFailureCount and syncBackoffUntil; the sweep skips a calendar inside its window, and the handler logs only when the failure is news. Not a mute button, and not a reclassification — GoogleCalendarApiClient::token() is right to refuse to call a refused refresh permanent. The first failure always reports, a CHANGED message always reports, and the delay is capped at a day so a condition that heals is retried within one. SURFACING Its own settings section rather than a block on Accounts: the problems span accounts, calendars, connections and the queue, and a variable-height alert stack would push the real account controls below the fold exactly when something is wrong. Plus a topbar badge, following LogAlertGlobal's shape including ResetInterface (worker mode) — but not dismissible, because "seen" is the right model for a log and the wrong one for a condition that is still true. Every string is translated (en/de/en_PI) and written for a person: what broke, what it means for their mail, and what the button will do before they press it. Provider error codes stay behind a disclosure. Verified: phpunit 3411 tests green, composer stan clean, 5 Playwright specs green, both themes at 1440px and 393px. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>	2026-08-13 09:21:35 +02:00
Paul LütznerandClaude Opus 5	c7ec3b36f9	Merge feat/new-mail-marker: a thread is new until its row has been shown ... Adds MessageThread::listedAt -- new iff null -- with a Gmail-style New badge on the row and a dot on the inbox category tabs and sidebar rows. Lists render first and mark afterwards, so the badge is always seen at least once, and only the rendered page is marked. The migration backfills every existing thread as already-listed. New is deliberately a separate axis from unread: a thread scrolled past but never opened is no longer new, yet is still unread. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>	2026-08-12 18:52:07 +02:00
Paul LütznerandClaude Opus 5	42246c35d9	feat(mail): a Gmail-style "New" marker for mail you have not been shown ... "New" is deliberately not "unread". Unread asks whether a conversation has been OPENED; this asks whether it has been SHOWN — whether its row has ever been put in front of you in a list. Scroll past something in the inbox and never click it and you have stopped being surprised by it while still not having read it. Both statements are true at once, so neither is derived from the other: MessageThread::$listedAt is its own nullable column, and seenAt and unreadCount are left saying what they already said. A plain column rather than a per-user table, because a thread hangs off an Account and an Account hangs off exactly one User — "was it shown" already has one answer per row. A per-user "last seen" timestamp in the settings bag was considered and rejected: it cannot say "this specific thread was displayed" when fifty threads arrive and only the first page of twenty is looked at. The migration backfills listed_at = NOW() on every existing row, and that half is the point of it. A bare ADD COLUMN leaves every row null, null means new, and the first person to open plMail after the deploy would find their whole archive badged with no way to clear it but to page through years of mail. A one-off UPDATE rather than a column default, which would keep firing and make every newly ingested thread born already-seen. The rethread backfill carries the value across a rebuild for the same reason, by COALESCE, so a maintenance command cannot relight an account either. Marking happens server-side after the list has rendered, following the logsSeenAt precedent in AdminDashboardController: the "is new" set is read before anything is written, so the badge is drawn from the state that was true when the request arrived and only then retired. Scoped to the rendered page, never the query, or page one would clear page two's badges unseen. Three ways a render could retire a badge nobody saw, and what each does: the list refresh is already deferred while document.hidden and held during a write, so a background poll cannot; a response the client discards is one the server rendered no list for, so it marked nothing; and a Turbo 8 hover prefetch — every sidebar row is prefetched, only thread rows opt out — is now detected by X-Sec-Purpose / Sec-Purpose and renders without marking. The row badge is a filled accent pill carrying a word, next to the sender at both container widths, and nothing like the bare green unread dot further down the same row. Categories, roles, labels and Starred get a quiet accent dot instead — "something arrived here", not a count — rendered at every value and hidden at zero so the sidebar controller can patch it from the counts endpoint, which now emits namespaced "new:" keys beside the unread ones. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>	2026-08-12 18:51:07 +02:00
Paul LütznerandClaude Opus 5	a1dd747d08	feat(mail): read receipts end to end, and message priority ... Wave 3/3a of the compose more-options plan: the two new Message columns, both directions of RFC 8098 read receipts, and the per-alias setting that governs them. Outgoing. MessageSendService::buildEmail() — the single MIME assembly point all three transports share — grows two header pairs. Priority writes X-Priority and Importance together, because a message setting only one is urgent in half the world and ordinary in the other half. A receipt request writes Disposition-Notification-To and Return-Receipt-To, both naming the sending identity rather than the account: mail sent from an alias must have its receipt come back to that alias, or the correlation on the way in has nothing to match — and a request naming anything else is the shape this same codebase refuses to auto-answer. Incoming. ReadReceiptStep queues a scan of each ingested batch; the handler flags the messages whose senders asked, and recognises the ones that ARE receipts. Detection at ingest rather than at read time, because the header bag is already on the row and re-deciding it on every mailbox open puts a security decision on a hot path. Sending. Hung off ThreadStatusUpdater::markRead(), which is the user-initiated path and nothing else. Its inbound twin applyRemoteFlags() deliberately does not call it, so a sync discovering a message was already \Seen elsewhere sends nothing — a receipt claims a person displayed the message, and a sync pass learning about a read that happened last week cannot make that claim. Only genuine null->set transitions fire, so the bulk toolbar re-marking a mixed selection cannot re-send. Async through the bus: opening a mailbox must never block on an outbound SMTP call. The setting is per alias in the existing jsonb bag, no migration, and defaults to Never through ReadReceiptMode::fromSetting() — every unreadable stored shape resolves to silence, so a user who never opens the panel cannot emit a confirmation that their address is live. A Disposition-Notification-To disagreeing with From and Return-Path is pulled back to Ask however the mailbox is configured; not to Never, because the legitimate version of that shape is a bulk sender collecting at its bounce address. Return leg. An inbound MDN stamps readReceiptAt on the message it answers — which is what the sent message renders as "Read at …", without which requesting a receipt tells the user nothing — and the report is marked read and taken off the Inbox rather than deleted. Third column beyond the plan's two: read_receipt_at, in the same migration. The plan asks for "Read at …" on the sent message and there was nowhere to put the timestamp. Verified in a one-off container on the test stack: phpunit 3266 tests OK (baseline 3248, +18 new), composer stan clean with no baseline growth, migration up/down/up clean against the real schema. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>	2026-08-12 17:47:16 +02:00
Paul LütznerandClaude Fable 5	363f5df6cc	fix(mail): the sandboxed frame can load a remote image again ... Remote images were broken in the client for two independent reasons, both measured on a live install. BUG 1 — the proxy was behind the session firewall. The mail body renders in a sandbox with no allow-same-origin (opaque origin, the XSS containment), so the frame sends no session cookie with its <img> loads. Behind ROLE_USER those cookieless requests were answered with the login page (200, text/html), which an <img> renders as a broken icon. The route is authorized by its HMAC signature instead — hash_hmac over the URL with %kernel.secret%, no user in the payload, so it is a global capability token only our pipeline can mint (cf. GitHub Camo). Dropped the controller's IsGranted and added a PUBLIC_ACCESS rule for ^/mail/image-proxy$ before the catch-all; every SSRF rule in ImageProxyFetcher still runs on every fetch. BUG 2 — the fetcher threw on every request. It passed extra.curl => [CURLOPT_REFERER => null], and Symfony's HttpClient reserves that option for its own headers handling and throws InvalidArgumentException on any request that sets it there. The exception was swallowed into a null return, so the proxy served its 42-byte placeholder for every image and never fetched one. Removed the option; the client sends no Referer unless one is set, and none is. Also: gate Stimulus debug logging to the runtime environment. startStimulusApp() reads a debug flag baked into the compiled asset at build time, which shipped ~1000+ console lines to prod; app.debug is now read from <html data-app-debug>, the flag of the kernel actually serving the request. Tests: ImageProxyAccessTest (anonymous access + signature matrix), ImageProxyFetcherTest guard for the forbidden curl option, and an E2E spec that asserts a proxied image decodes to real pixels inside the opaque frame — the render the old rendering-security suite never checked because it points at an unfetchable .invalid host. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>	2026-08-12 12:43:24 +02:00
Paul Lützner	ade0f9ea6d	Merge fix/sync-data-polling ... Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> # Conflicts: # src/Controller/Settings/AccountController.php # src/Controller/Settings/SettingsController.php	2026-08-11 18:39:49 +02:00
Paul Lützner	c8dffdf0a1	fix(mail): a message is read on our terms, not the sender's ... Opening a message told the sender it had been opened. Eight images loaded from images.ctfassets.net the moment a newsletter was rendered — one of them 526x5, which is what a tracking pixel wears when it is pretending to be a spacer — and each one carried the reader's IP address, their user agent, the time, and, from a per-recipient URL, which recipient it was. Read receipts, working perfectly, for anyone who cared to send one. Remote images are blocked now, and the block is not the interesting half. The interesting half is that saying yes does not undo it: an opted-in image loads through /mail/image-proxy, which fetches server-side and streams the bytes back, so the sender learns that somebody's plMail asked and nothing else. There is also "always for this sender", per user, in a table with a unique constraint — because trusting a sender is a button in a message, the same message can be open in two tabs, and an allowlist that a double click can duplicate is an allowlist that a race can lose an entry from. The proxy is a service that connects wherever a stranger's mail tells it to, from inside the container network, so its rules are stated in order and tested one by one: https only; port 443 only, which removes port-scanning rather than blacklisting the interesting ports; every address the host resolves to must be public, and the connection is then PINNED to a checked address through the client's `resolve` option, because checking and letting the client resolve again is the whole rebinding hole; redirects followed by hand, three at most, each hop re-running all of that, since max_redirects would follow a public host to 169.254.169.254 past every check; an image content type; 8 MB. Every refusal answers with the same transparent GIF, because telling the caller "refused" rather than "timed out" against an internal address hands back exactly the port-scan result the rules exist to deny. The signature on the URL is NOT the SSRF defence and says so in its own docblock — anyone with an account can mail themselves a link and be handed a valid signature for it. Deliberately not IntegrationUrlValidator, which has an allow-list and an admin-pinned base URL because a user is naming their own Nextcloud on a LAN. Importing its escape hatches here would mean an admin who allowed a LAN host for Immich had also allowed every sender's mail to reach it. The docs there claimed pinning the resolved IP "needs something Symfony's client does not expose"; that is not true, `resolve` does exactly that, and the correction is in this commit next to the code that disproves it. The body no longer renders in the app's own DOM. The sanitizer held — nothing got through it — but "no gaps, forever, against every sender" is a claim that has to keep being true, and the price of it being wrong once was a same-origin script with the session cookie. It renders in an iframe srcdoc whose value is what is absent from its sandbox: no allow-same-origin, so an opaque origin with no cookie, no storage and no reach into this document; no allow-forms, so a phishing form cannot even be submitted; no top-navigation, no modals, no downloads. allow-scripts IS present, and that is a deviation from the brief worth stating plainly: height is a fact only the framed document can measure, a frame without allow-same-origin cannot be measured from outside, and the alternatives were a fixed height or a guess. It is paid for by the frame's own CSP restricting script-src to a per-render nonce, which our thirty lines of height-and-hover reporting carry and no markup arriving in an email can. That CSP also names our origin in img-src explicitly — not 'self', which in an opaque origin matches nothing — so the blocker and the browser both have to fail before a pixel fires. Every warning is drawn OUTSIDE the frame. A banner inside the sandbox is a banner the mail can paint for itself, and a "Show images" button a message can forge is worse than no button. The hovered-link preview follows the same line: detected in the frame, where the links are, drawn by the parent, so a message cannot paint a status bar claiming a different destination. A Hetzner-branded invoice phish sat in Spam and rendered at full fidelity with nothing said about it. Messages in Spam now carry a banner, and the sender-name check is two rules that fit in a sentence each, because a warning nobody can check is a warning people learn to click through. Rule 1: the display name spells out a domain that is not the sender's. Rule 2 runs only when the display name carries a legal form as a whole word — GmbH, Inc, Ltd — which is what separates an organisation from a person; then, if none of its non-generic words of four letters or more occurs in the sender's registrable domain, the names disagree. "Hetzner Online GmbH" <support@ownkhalsick.com> fires; the same name at hetzner.com does not; "Deutsche Bahn AG" at bahn.de does not; "Jane Cooper" is never judged at all. DKIM passing for the sender's own domain suppresses both, since a domain that signed the message may write what it likes. A lookalike domain containing the brand is a known miss and has a test saying so — catching it means edit distances against a brand list, and every false positive that bought would be spent on ordinary mail. A trusted sender still gets no images inside Spam. The allowlist records a belief about a sender; a message in Spam is the provider disagreeing about whether this really is that sender, and the cautious reading of a disagreement is the right one. Blocking runs at RENDER time, not at ingest, and both reasons matter. Every message already stored was sanitized before this existed, so an ingest-time block would have protected only mail that had not arrived yet. And the answer depends on who is asking, because the allowlist is per reader — a stored form cannot carry an answer that varies by reader. Two layout fixes ride along, from the same report. Attachments rendered after the full HTML body, some 750px below the fold on a long message, and an attachment nobody scrolls to is an attachment nobody was sent; the chips move under the header, the same partial, relocated rather than duplicated. And the message header is sticky within the reading pane, so the answer to "who is this from" is not a scroll away from the sentence that raised the question. Verified in a browser, because every claim here is a claim about what the browser does. tests/e2e/rendering-security.spec.ts asserts on route interception — no connection to the sender's host, before or after opting in — and its matcher is a hostname predicate rather than the obvious glob, which would have matched the proxy's own URL because the sender's URL lives inside its query string, and reported a leak at the exact moment the feature worked. Two traps found on the way, both now in the handbook. Dom\HTMLDocument rejects LIBXML_NOWARNING with a ValueError rather than ignoring it, and the blocker fails closed, so the symptom was every message rendering as plain text: perfectly private and completely unreadable. It also needs the UTF-8 override, because MailBodySanitizer drops <head> and the body has no meta charset left.	2026-08-11 17:36:23 +02:00
Paul LütznerandClaude Fable 5	b82c9bac9a	fix(mail): stop the idle inbox polling four times a second ... Measured on an idle inbox: /mail/sidebar/counts 32x per ten seconds, /mail/inbox 80 KB of HTML 8x per ten seconds. Three separate causes. The sidebar partial is included twice — mobile drawer and desktop column — so two controller instances listened to the same sync events on document and every sync fetched the counts twice. The fetch belongs to the page rather than to the instance, so it moves to module scope: one request, both instances patching from one answer, with a floor between requests. A sync run publishes one mailbox.synced per mailbox per account, so "a sync happened" arrives as a burst. The in-flight guard only stopped two fetches overlapping, which turned a burst of eight into eight sequential requests. A burst is now one refresh plus one trailing one. And the refresh asked for the current URL and got the whole document back, then parsed it, lifted one frame out and threw the rest away. It asks for the frame now — 63.9 KB becomes 12.6 KB — via a header of our own, deliberately not Turbo's: Turbo sets that on ordinary frame navigations and reads the response title to advance history, so answering those with a fragment would have stopped the tab title following the folder. Both pollers stop while the tab is hidden and take the held refresh on return, and the trailing timer is cleared on disconnect — Turbo replaces <body> every visit, so an uncleared one is a fetch per navigation with nothing to write to. Also drops the '[mail-pane] list refreshed' console.log that was shipping. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>	2026-08-11 17:08:17 +02:00
Paul Lützner	84a5eed8b3	Merge branch 'feat/push-monitoring'	2026-08-06 18:00:46 +02:00
Paul Lützner	2bf3c38add	small fixes that claude couldnt commit because who knows	2026-08-06 16:29:22 +02:00
Paul LütznerandClaude Fable 5	e5596a9b47	Send push over FCM for clients that ask for it, configured from the admin page ... A second transport beside Web Push: PushSubscription/set create accepts a vendor fcmToken, the dispatcher fans out per transport, and delivery goes through FCM HTTP v1 with an OAuth token minted from the service account the admin pasted - no new dependency, openssl and the http-client do it. The admin page takes the service-account JSON (encrypted at rest) and the instance's google-services.json, refuses mismatched pairs, and the session advertises fcm plus the public client config a stock Play Store build needs to initialise Firebase at runtime against this very instance. Committed by the supervisor: the implementing session hit its usage limit after writing code and tests but before running them; results recorded in the merge that follows. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>	2026-08-06 15:09:43 +02:00
Paul LütznerandClaude Opus 5	b0a1d07977	Share a calendar by link, and let people book a time on it ... Two public-facing halves of one idea: showing somebody your calendar without giving them an account, and letting them take an hour of it. **What a link reveals is a per-link decision.** Busy/free is the floor; ticking boxes adds title, location, description or participants, and the ticks are editable afterwards without breaking the URL that has already been sent. The redaction is a DTO rather than a set of conditionals in a template: ShareLinkReader answers SharedOccurrence, and the public templates never see a CalendarEvent at all. A busy/free link therefore cannot leak a title through a tooltip, a data attribute, a JSON payload or an .ics, because the object it is rendering has not got one. SharedIcsBuilder is deliberately not IcsExporter for the same reason — it serialises the redaction, so there is no code path where the real event reaches a stranger. The shared .ics carries a synthetic UID derived from the link digest, so a recipient who also holds the genuine invitation cannot correlate the two. EventPrivacy finally does the job it was named for. Privacy is the ceiling and the checkboxes are the floor: Secret never appears at all, because its existence is the detail, and Private stays a busy block however many boxes are ticked. **Booking.** The owner says which weekdays and hours, how long a slot is, how much buffer sits either side, how much notice they need and how far ahead they will go, which calendar the booking lands on and which calendars count as busy. A stranger sees the slots that are actually free, picks one, and gets a confirmation carrying a real .ics. Double-booking is refused by the database, not by the code. A unique index on (page, starts_at) over a booking row that is written in the same flush as the event, so Postgres rejecting the second insert takes its event with it and the loser is told the slot has gone. Check-then- insert was rejected outright: two requests both read a free slot and both send a confirmation, and the constraint then rejects one of them after a stranger has already been told they have the time. A check is not a lock. starts_at is denormalised onto the booking on purpose — if the constraint spanned the event's own column, the owner dragging a booked meeting on the time grid would silently free an hour that somebody else is holding. A booked event is its own EventSource case: trusted, because it must reach the provider, and never rewritable by mail, because a booker has no standing to move an hour by sending a message. Not an ExtractionKind, which would have put bookings behind every "found in your email" affordance and into Happening Soon. The booker's name and address live on the booking row and deliberately not in JSCalendar participants — pushing an attendee list makes the provider mail a stranger a meeting request nobody asked it to send. Tokens are hashed at rest for both halves, following the device-pairing precedent, which costs the ability to ever show one again: regenerate is the recovery, and there can be no copy button. The public pages get a layout of their own rather than the app's. The app layout renders a CSRF token into a meta tag, which starts a session — one per visitor and one per .ics poll on a link that may be in a stranger's calendar client. The booking POST therefore carries no CSRF token, and the guard is the unguessable URL plus a rate limiter of six attempts an hour per address. The GET is deliberately unbounded: limiting it would let one stranger take a published page offline. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>	2026-08-05 19:12:03 +02:00
Paul LütznerandClaude Opus 5	ffd77d0019	Put the week on a time grid, carry calendars in and out as ICS, and ... make a reminder go off Three pieces of calendar work that were built together and are committed together, because they interleave: the reminder checkboxes live in the same editor the drag-to-move endpoint writes through, and the ICS importer and the alert reader both go through CalendarEventWriter. **A time grid for the week.** _view_days.html.twig said a time grid "belongs with drag-to-move, which is its own piece of work", and this is that work. Hour rows, blocks sized by their real times, concurrent meetings sharing the width; all-day events in a strip of their own rather than as zero-height blocks at midnight. The docked pane keeps the column list — that constraint was already reasoned out and stands, because a 380px pane has no business rendering a grid. Dragging a block, or resizing it by an edge, posts to a route of its own rather than to the save: a drag says two things and nothing else, so a route that accepts only those two cannot silently blank a description. Dragging one occurrence of a series asks the same question the editor asks, and applies the delta rather than rebasing — that bug has been fixed once already and is not being re-introduced. Read-only calendars refuse the drop and say so. **ICS in three directions.** Download one event or a whole calendar, upload a file into a calendar, or subscribe to a feed by URL. One iCalendar implementation behind all three: IcsDocumentReader splits a VCALENDAR into the per-UID resources CalDAV's shape already means by "one event", carrying every VTIMEZONE into each, and the existing CalDavEventConverter and RecurrenceRuleConverter do the mapping. There is no second parser and no second RRULE conversion. A feed is a fourth sync driver — read-only, no push, conditional GET on ETag and Last-Modified. A changed feed surrenders its token rather than reporting a listing, because a feed states what exists and says nothing about removals; that is the same trade CalDAV's ctag fallback makes, and it costs two downloads per actual change. webcal:// is rewritten to https:// in the scheme only, never anywhere else in the URL. The address goes through the SSRF guard that already exists for CalDAV and the integrations rather than a second one — which is why the successful subscribe is proven through the controller over a scripted transport and not in the browser: every address the e2e stack can reach is loopback or private, and the guard is right to refuse it. Export streams; import is bounded at 4 MiB and a feed at 8 MiB on bytes arriving rather than on a Content-Length, because sabre has no incremental component API and the parse is necessarily whole. **Alerts that fire.** RFC 8984 `alerts` on the event, in the canonical object, so they round-trip to Google, Graph and CalDAV instead of being a plMail-only column. Six one-click offsets plus an arbitrary number of minutes, and no JavaScript: a client-side repeater would have to build the same labels in a second language with a second set of plural rules. The checkboxes post KEYS, not offsets — the key an alert lives under, resolved on save against the list the template rendered. That is what lets a plain checkbox list carry an alarm the form cannot express, such as one imported from a CalDAV server at an absolute instant, rather than dropping it the first time somebody corrects a title. It also means a crafted post can untick a reminder but cannot invent one. app:calendar:alerts every minute, because a minute is the unit people set a reminder in and the interval is the bound on how late one can be — at five minutes a "five minutes before" reminder could arrive after the meeting started. Delivery goes over the Web Push stack that already exists or as mail, chosen per alert by its action. Not firing twice is structural: a delivery row with a unique constraint, not a comment. A backfill cannot fire the past, and a recurring event means one alert per occurrence, bounded by the materialiser's own horizon. calendar_event_occurrence gains a start-only index. Both existing indexes lead with a user or a calendar, and the sweep asks "what starts near now, anywhere on this install?" once a minute — without it that is a sequential scan of every occurrence ever materialised. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>	2026-08-05 17:31:52 +02:00
Paul LütznerandClaude Opus 5	455af44608	Open the push channel when the calendar is connected ... Subscribing left the calendar polling until the hourly sweep noticed it, so a calendar connected at ten past waited fifty minutes for its first push channel. Registration now happens on subscribe. Dispatched, not called. Opening a channel is an HTTP round trip to Google or Microsoft, and doing it inline would put a third-party call inside the request that ticked a checkbox — slow when it works and a 500 on the subscribe form when the provider is having a bad afternoon. Routed to maintenance rather than ingest, because on ingest it would queue behind the first full calendar read and open the channel minutes later on exactly the large calendars where push matters most. Routing it at all is load-bearing: an unrouted Messenger message is handled in the dispatching process, which is the thing being avoided. The hourly sweep stays, and the two are not redundant. Registration fails for deployment reasons — no public HTTPS address yet, a Cloud project whose domain verification is pending — that have nothing to do with the click that connected the calendar, and a subscribe that failed for one of those must not leave the calendar polling forever. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>	2026-08-05 13:46:20 +02:00
Paul LütznerandClaude Opus 5	25f1cf74c8	Say which build this is, in the admin header ... Migrations run on boot and `latest` moves, so "is this the build I think it is?" is a question with real consequences and no cheap answer. The release and the commit now sit in the admin header — in the header rather than in a panel, because it is asked while looking at something else. Stamped at image build and never discovered at runtime. A container has no .git: the source is copied in and the history stays behind, so anything that shelled out to git in production would answer from a mounted volume or from nothing. The release workflow passes both values from the metadata action it already computes; a plain `docker build` passes neither, and an image nobody stamped says "development" rather than guessing, which is also why the chip is absent entirely on a checkout — a permanent "development" badge is furniture. There is deliberately no test that a real checkout describes itself. That would assert a property of wherever the suite runs rather than of the class: the test container has the source but not the history, and where it has both, git refuses the directory for dubious ownership — so it passes on a laptop, fails in CI, and says nothing either way. This suite has been bitten by that exact shape twice already (an id sequence gap that only existed on a well-used database, a sidebar state that only existed after another spec ran). The contract is tested instead: an unreadable repository means "development". config/reference.php is gitignored and untracked. Symfony regenerates it on every cache warm-up and not deterministically — two keys inside one @psalm-type block swap places — so a tracked copy showed a phantom diff after any command that booted the kernel, all session. Its own header says it is auto-generated and for apps only. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>	2026-08-05 12:56:23 +02:00
Paul LütznerandClaude Opus 5	a674c7fc28	Answer a changed calendar instead of waiting for it, and change one day of a series ... Two halves of the same complaint: a calendar that only ever catches up on a timer, and a repeating event that could only be changed all at once. Push, for the two providers that have it. Google events.watch channels — a plain webhook, not the Pub/Sub path Gmail is on — and Graph subscriptions on a calendar's events, which is the mechanism GraphSubscriptionManager already runs for mail. Both notifications are content-free, so a webhook does exactly one thing: dispatch the sync that already worked. All the intelligence stays where it was. CalendarPushSubscriptionManagerInterface sits BESIDE the mail one rather than widening it. Every method over there takes an Account and reads columns on Account, but Graph subscribes to me/calendars/{id}/events, so one Microsoft mailbox mirroring six calendars needs six registrations, six secrets and six expiries. Widening to Account|Calendar would hand the two mail managers a type they cannot serve and turn a compile-time contract into an instanceof cascade in three classes. Four columns on calendar rather than keys in its settings bag, which is the one storage decision here worth arguing: push_channel_id is what two unauthenticated, internet-facing endpoints resolve a notification by. It needs a unique index and a constant-time comparison against a known column, and neither is something to build on a key inside a jsonb document whose readers assume their own defaults. push_resource_id is there because channels/stop takes the pair — discard it and a channel can never be closed. Push is never load-bearing. A self-hosted install may have no public HTTPS callback at all, so a refused registration means "stay on polling", and the fifteen-minute sweep is untouched. Both stored expiries are the ones the provider RETURNED, not the ones requested, which is what makes renewal work rather than nearly work. Single-occurrence editing on the other side. A chip carries the occurrence's original start into the editor, and that instant is the key its patch is filed under — so a second edit of the same instance updates its patch instead of stacking another beside it. A patch states start, duration and a title only when the instance has its own; writing the whole posted object would claim a location and an all-day flag for that one day that nobody set. The bug worth naming, found reviewing the agent's own flagged judgement call: the editor is prefilled with the INSTANCE's times, because "this event" has to move the occurrence that was clicked. That made the posted fields useless as the series' times — "all events" rebased the whole series onto the clicked day, so renaming a weekly meeting from its fifth occurrence moved it four weeks on, silently. The difference the user made is applied now, not the value they saw, so a title-only edit shifts by zero and the series keeps its day to the second. What reaches a remote is honest about itself. CalDAV round-trips a per-instance change; Google and Graph each model a moved instance as a separate resource and neither mapper writes one, so on those the change is pushed as a series update that says nothing about the instance and stays local. Documented where somebody will meet it rather than left to be discovered. The admin note about Google's domain verification is translated now rather than an English literal with an apology beside it. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>	2026-08-04 18:03:03 +02:00
Paul LütznerandClaude Opus 5	4e92d148e2	Mirror the calendars people already keep somewhere else ... Google, Microsoft and CalDAV, subscribed from settings and synced both ways. One CalendarSyncDriverInterface behind three providers; the engine has never seen HTTP, OAuth or a VEVENT and must not learn to. The decision that shapes the rest: Google and Microsoft calendars ride the MAIL grant. todo.md said to give them their own Provider cases with their own grants, and that note is overruled here on purpose — a second OAuth app is a second thing to register, consent to and keep working, and the requirement was that enabling calendars cost an administrator one more permission checkbox and nothing else. Only CalDAV is its own Provider case, because only CalDAV is its own server. Google's consent screen can still refuse the scope by itself, so that surfaces where the calendars would have been rather than being tracked at grant time. Seven conflict rules, written into CalendarSyncService's docblock rather than discovered later. Two decide whether "changes sync back" is a promise or a hope: Push before pull, always. Pull first and every local edit collides with its own echo. A row changed on both sides loses its local copy LOUDLY — the whole discarded object logged before the overwrite. An edit that vanishes without a trace is the one outcome nobody can debug afterwards. Recurrence was the weak spot and is fixed in the same pass, because shipping it broken would have been shipping a wrong calendar. RRULE to JSCalendar existed in one driver's private copy while two other paths stashed the rule unconverted, and the expander reads only the converted form — so a weekly meeting that arrived by mail, or lived on a CalDAV server, drew exactly one occurrence. One conversion now serves all three, and a rule that cannot be converted faithfully is still refused whole: an event on the wrong days is worse than one that does not repeat. An instance moved out of its series had nowhere to live, and all three drivers hit it independently — a duplicate on the new day from Google, the old time from Microsoft. recurrenceOverrides was already read by the materialiser and produced by nothing. Now produced by all three, and CalDAV writes it back, because a PUT replaces the resource and reading overrides without writing them would delete every moved instance at the server the next time somebody renamed the series. Dates in ordinary prose are proposed, never filed. A guess is an EventProposal and not a CalendarEvent, and that is the whole design: the writer materialises occurrences, every range query reads occurrences, and a guess living in calendar_event leaks into a view the first time somebody forgets to exclude it. A proposal materialises nothing and therefore cannot. Accepting converts it through the one writer; dismissing writes the suppression row that already exists. The rule nobody would have predicted came from running the parser over a real mailbox: 56 of 57 gated messages yielded a date and every one was a reply's own furniture — "On Thu, Jul 2 … wrote:", "Sent: Friday …". The horizon rule refused them all, but only because a quoted message is older than the one quoting it, which is an accident. Quoted text is skipped now. Date:/Datum: is not, because German business mail states appointments that way. Two bugs fell out of asking whether a claim actually held. EventSource::isTrusted() was `return self::Llm !== $this`, so every case written after it silently inherited *trusted* — the exact permission the method exists to withhold, and invisible in a way a default arm at least is not. And an event accepted from a proposal could be rewritten by a later message: the guard covered extracted events only, and what really kept mail off these was that plMail mints UIDs no sender collides with. Hand the reconciler the event's own UID and it overwrote it. Both closed on the enum, exhaustively. Groundwork that made the above possible is in the previous commit. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>	2026-08-04 16:50:51 +02:00
Paul LütznerandClaude Opus 5	24c7df83cd	Make room for a calendar connection before adding one ... Groundwork only: nothing new is offered to a user yet, and the mail grant now asks for one more thing. Integration was written as "a file or photo service", and every list of providers walked Provider::cases() on that assumption. A calendar connection is the same shape — a base URL, a credential, a health check, a settings bag — and belongs nowhere near "Save to…", so services now declare a ServiceKind and both lists ask for the file ones. Declared rather than inferred from an empty capability list, which would be true today and quietly wrong the first time a calendar service also served files. The two matches todo.md asked to close are closed. capabilities() fell through to the full file set, so a case nobody had thought about claimed Browse, Upload and ShareLink and only admitted otherwise after a user had picked it; authKind() fell through to OAuth2 and would have sent a password-only service through a flow it has no endpoints for. Both are exhaustive now, which is what makes adding a case a compile-time conversation instead of a runtime one. verify() moves to VerifiableDriverInterface, which IntegrationDriverInterface extends. It is the one thing every connection owes whatever it connects to, and the connect, test and onboarding paths call it for all of them — the alternative was five throwing file stubs on a driver that holds no files. Same argument SearchableDriverInterface already makes, and the tag moves to the broader interface so the file drivers are unaffected. The mail scopes gain calendar access, on purpose and against the note in todo.md that said to add separate provider cases with their own grants. A second OAuth app is a second thing for an administrator to register, consent to and keep working; the requirement here is that enabling calendar sync costs them exactly one more permission checkbox in Google Cloud or Azure. The admin help text now says so, per provider, next to the credentials it applies to. Google's consent screen lets a user untick a single scope, so a token can still come back without calendar access. That is left to be discovered when a calendar call is refused and reported there, rather than tracked at grant time — there is nothing useful to do with the knowledge until something asks for a calendar. Two browser specs fell out, both newly exposed rather than newly broken. Which account is expanded in the sidebar is a stored preference, and it only started surviving reliably when the chevron learned aria-expanded a commit ago. An expanded account lists its own Inbox and its own copy of each label, so a locator reading "any text saying Inbox in the sidebar" and another reading "any link named E2E Label" each matched two elements. Both now name the row they meant. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>	2026-08-04 12:36:59 +02:00
Paul LütznerandClaude Opus 5	05f8f8feea	Ask Graph for the invite flag through the type that declares it ... meetingMessageType lives on microsoft.graph.eventMessage, not on the base message, and naming it unqualified is not ignored — the whole $select is rejected, every sub-request in the batch answers 400, and the account stops syncing. That is what account 10 had been doing: RequestBroker--ParseUri: Could not find a property named 'meetingMessageType' on type 'Microsoft.OutlookServices.Message' Found the same day the failure started carrying Graph's own reason, which is the whole argument for logging it. Selected with the cast now. Mailboxes served by the older Outlook broker refuse even that, so a batch that fails on this property is retried once without it and the mailbox is remembered for the rest of the process — one wasted batch per worker instead of an account that never syncs. The unknown-subscription warning is throttled to one an hour per subscription. Microsoft holds a registration this install no longer has, re-delivers several times a second for up to three days, and nothing can cancel it from here without the account it belonged to — so it filled the log browser and lit the unread-log ring over something no admin can act on. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>	2026-08-03 22:34:36 +02:00
Paul LütznerandClaude Opus 5	3c176d7e79	Outline the user menu when the log has something nobody has read ... An error only exists if somebody opens the admin panel to find it, and nothing on any other screen suggested there was a reason to. The avatar now carries a ring — amber for warnings, red once anything reached error — with the count in its tooltip and next to the Admin entry, which then goes straight to the log browser rather than the dashboard. Seen is "the browser was opened", marked from that moment rather than from the newest entry on screen, so anything logged while it is open is still unread afterwards. The mark goes in the user settings bag: it is a read marker for one screen, of no interest to anything that queries users, and that keeps it out of the schema. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>	2026-08-03 17:37:22 +02:00