4 Commits

Include renames

Author	SHA1	Message	Date
Paul LütznerandClaude Opus 5	0aa628ff32	fix: a shell that fits the phone, a menu that fits the pane, and tests that survive the fixture ... **The mobile double scrollbar.** `app.css` sets `html, body { height: 100dvh }` because mobile browsers measure `100vh` against the viewport with the URL bar HIDDEN, and its comment says the shells ask for `h-full`. Three did not: the mailbox, settings and admin shells were `h-screen`, so the shell was taller than the body containing it, the body scrolled to make room, and the page grew a second scrollbar beside the message list's own. The contract was already written down; these were the three places not honouring it. **The demo bar covered the last row.** It is `fixed` on purpose — a banner in the flow costs every page a strip of reading height — but the cost is that the last row sits behind it, which on a phone is the row you were reaching for. Scrollers get its height back as clearance. **The Label-as menu was sliced off inside a thread.** The reading pane is `overflow-hidden` and the panel was `absolute`, so it was cut at the pane's edge with no way to scroll to the rest — the thing clipping it is not the thing that scrolls. `position: fixed` does NOT fix this, which is the part worth recording: a fixed element is positioned against the nearest ancestor carrying a transform, filter or backdrop-filter, and the boxed layout gives the panes a backdrop blur. Measured rather than assumed — the panel reported `top: 125px` inline and rendered at 210, ending eighty pixels below a 420px viewport. So it stays where it is and is sized to the box that clips it, flipping above the button when there is more room there. **More demo data, from using the demo.** There was HTML mail, but List-Unsubscribe correctly files it under Promotions, so the only formatted mail sat behind a tab nobody clicks and the reading pane looked as though it could render nothing but plain text. There is now one from a person, in Primary. And there was no conversation at all — every thread a singleton, which leaves the reading pane's whole argument with nothing to demonstrate on. Threading that conversation had to be done one turn at a time. MessageThreader resolves a reply's parent with a SQL query that joins message.thread, so a parent created earlier in the same UNFLUSHED batch is invisible to it and all four turns looked like conversation openers. The code already says as much where Gmail meets the same wall from the other side. **Tests that survive the fixture growing** — CODESTYLE 9.5, and three specs brought in line. An assertion that must be edited every time the seeded mailbox gains a thread is asserting that number rather than the behaviour, and it fails in a spec whose subject has nothing to do with the change. Baselines where there is a "before", a relation where there is not: every message is its own thread EXCEPT the conversation, whose turns collapse. Proved by mutation rather than by reading: seeding a fifth thread and re-running found a third brittle spec I had not spotted — unread-view's hard-coded "3" — which now takes the badge's own starting value. Absence checks and own-fixture counts are left alone, and the rule says why. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>	2026-08-25 13:51:45 +02:00
Paul Lützner	fd47a79aed	refactor(security): one voter answers "is this yours", instead of nineteen helpers ... Ownership was a private method on the controller — nineteen of them, across eighteen files, under six names (assertOwned, assertOwnership, assertOwnsEvent, assertOwnsCalendar, assertOwnedUserLabel, denyUnlessOwner). All nineteen were correct. The problem was that each was a fresh derivation of the same sentence, so the twentieth was a fresh chance to derive it wrong, and a controller that simply never grew one looked no different from the outside. One voter rather than one per entity, which is the idiomatic shape and was rejected on purpose: owning is `$owner === $user` for all nine types, so nine classes would have been nine copies of that comparison with nine different ways to reach the owner — the same duplication wearing a different hat. Only the route to the owner varies, so only that is a match arm. It fails closed. Seven of the nine links to a user are nullable, so "no owner" is representable and must never read as "everyone's": a null anywhere on the path is denied rather than compared. Message is reached through its own non-nullable `$account`, NOT through ThreadStatusUpdater::accountOf(). That walk exists for Gmail-API messages with no mailbox and ends at the nullable `$message->thread`, so as an ownership check it could fatal on a message carrying neither. Rules that are not ownership stay with their controller, because they are a different question: a system label is still refused, an already-sent draft is still refused. CODESTYLE §5.2 documented the old pattern, using one of these very controllers as its worked example, and is updated to match. Two tests, both confirmed to fail when the voter is stubbed to grant everything: the rule in isolation, and the wiring over real HTTP with a second user's rows. One baseline entry retires with a dead null-check the voter replaced. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> (cherry picked from commit dd425f1fe29283c7b4dc5b79e2c117419d3c0179) (cherry picked from commit de4e8f9d4dd8a1a05a987d9f5777f3f1ef82d6f2)	2026-08-17 11:25:14 +02:00
Paul LütznerandClaude Opus 5	4d103cfbdf	Write the handbook, and make it fail the build when it goes stale ... README says what plMail is and gets you running; CONTRIBUTING says how to change it. Nothing covered the part in between — using it, operating it, and registering the Google and Microsoft applications it talks to. That is docs/: 34 pages across features, installation, providers and internals, mirrored to the GitHub wiki. The wiki is a mirror and never a source. bin/mirror-wiki.php flattens the tree into the wiki's flat namespace, rewrites every relative link, generates the sidebar from the index so the two cannot disagree, and stamps each page with a footer saying where it came from. A browser edit survives until the next push to main. That cost is deliberate: documentation not versioned with the code drifts from it, and the drift is invisible until somebody follows an instruction that stopped being true. Which is the whole problem, so the checkable half is checked. DocumentationCoverageTest fails the build when a command is missing from the reference or documented and gone, when a variable in .env is absent from the configuration page, when a relative link dangles, when a page is unreachable from the index or promised and missing, or when a page has no "Things that bite" section. It cannot check whether a paragraph is TRUE, and says so — this is the floor, so review attention goes to the part a machine cannot have an opinion about. CONTRIBUTING has claimed since it was written that the command table "is part of the definition of done"; nothing enforced it until now, and it was wrong in three places. Writing it found things. They are fixed here rather than documented around, because a handbook describing a bug accurately is still a handbook nobody can follow. compose.yaml mounted no volume for var/attachments, var/raw or var/uploads. Workers write those during sync and the web container serves them back, so each kept its own copy in its own layer — downloads 404, and everything died on a recreate. The override and the TrueNAS file both mount them; the file the README tells an operator to run did not. The same omission cost real data once before. See the CHANGELOG for moving what is currently stranded. app:calendar:materialise new, nightly. Occurrences are drawn when an event is saved and nothing moved that window again, so a weekly standup ran out of dates in two years and its reminders stopped with them — silently. The query it runs had existed since the materialiser did, documented as exactly this sweep, and had never been called. Microsoft wizard asked for three of the nine permissions plMail requests, and a token already issued is not upgraded when the rest are added later. GmailWatchService told you to grant Pub/Sub Publisher to gmail-api@system.gserviceaccount.com. The account is gmail-api-push@; a topic granted exactly what the docblock asked for never delivered a notification. IcsExporter said it wrote no VALARM. It has written them since alerts landed. CLIENT_DEVELOPMENT.md said there was no JMAP calendar API and that SearchSnippet/get was absent. Both shipped. CONTRIBUTING said only app:calendar:push registers a push channel; subscribing has done it since the subscribe flow learned to. Three docblocks named getters removed long ago, or described a method two below the one they sat on. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>	2026-08-05 21:13:25 +02:00
Paul LütznerandClaude Opus 5	36bc791f93	Measure the JavaScript, and cover the three controllers that had none ... Playwright collects V8 coverage when E2E_JS_COVERAGE is set, one raw file per test, aggregated by bin/js-coverage.mjs. Only this application's own scripts are counted — Stimulus and Turbo are in the same importmap and would otherwise dominate a number nobody here can act on. The aggregation is the part worth reading. V8 nests its ranges: a function that ran reports one span with a count, and the branches inside it that did not as zero-count spans within it. So the covered set has to be resolved per test and only then unioned. Pooling the dead ranges across tests instead reads "this block did not run in test A" as "this block never runs" — and since the importmap preloads every controller on every page, every spec that merely loaded a module cancelled the specs that exercised it. It reported 28.2% and did not move when three specs were added, which is what gave it away. Corrected: 63.1%. Three new spec files, for three controllers with no browser test at all: search the operator dropdown and what the parser does with an operator it cannot honour — the difference between "no results" and "your whole mailbox". 22% to 90%. admin-queue what a worker is holding, and that the filter asks the server rather than sifting the page already fetched. 0% to 79%, with app:test:seed-queue to give the panel something to be about. filters-scope removing the last condition, and the sentence naming the account once one is chosen. 18% to 77%. One bug, found the way browser tests are supposed to find them: the search box is an <input type="search">, which Chromium clears natively on Escape — so "first Escape closes the list, second clears the box" had never worked. The close branch preventDefaults now. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>	2026-08-03 23:50:37 +02:00