33dd9ff054a6edbddf6ea6782d2f5d50dbede86c
487
Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
 peter
|
47d968eeea |
fix: review round 4 — an account switch unloads as the old account, a share link's lockout costs five networks
Deep-review round 4, targeted at round 3's changes (f569b19c..e7f38396). Accounts - Switching, adding or signing out an account moved the tab's pin before the page unloaded, so the players' resume beacons fired on the way out under the NEW account: one account's playback position saved into the other. The pin is now handed to the next page (pinOnNextPage) and taken over at import; clearActiveAccount is gone. - Switching to a suspended account was refused but left it in the switcher, refused on every click; a deactivated one was not refused at all, and made the default it signed every account in the browser out on the next request. Every account _session_user would refuse — gone, deactivated, suspended, revoked — is refused and dropped from the list. Share page (/watch) - An IPv6 client is limited by its /64 (client_ip becomes client_key, for every limiter): keyed on the full address, one subscriber could rotate through endless identities. The comments now state what the per-link ceiling costs: guessing from five networks can keep a link shut for its window, and guesses already running when it fills are still checked. - The link is looked up before the limiter records the attempt, so made-up tokens no longer grow it by an entry each. Forms - Failing the same way twice is announced again: the sign-in, passphrase and setup errors are keyed on the attempt (a clear and a set in one handler left the alert node untouched). - The install wizard's own validation showed the generic error — it threw plain objects its catch did not read. It sets the step's message. Tests: a re-pin leaving the previous account's notices behind; the pin hand-over; switching to an ended account; the /64 viewer; made-up tokens; the repeated alerts. Each mutation-checked RED for its own reason. |
||
|
peter
|
e7f38396a2 |
fix: review round 3 — a new tab keeps its account's notices, a revoked tab account lands on the default
Deep-review round 3 over main..improvement/watch-background-audio. Account boot - The notification history and settings, and the hints toggle, are read when their modules load, which in a new tab (bookmark, typed URL) is before BootGate pins the account: they read as nobody's, and the tab's first notice replaced the account's stored history (up to 100 entries) with a one-item list. They now read again when the tab is pinned (onAccountPinned, run by setActiveAccount); a notice raised before the pin stays, after the history. - A tab pinned to an account still in this browser's wallet but whose session ended (suspended, deleted, or revoked by "log out everywhere") got "signed out" from /api/me while the default account was still signed in, and signing in again as anyone else left it there. A refused per-tab account now leaves the wallet, and the probe answers as the default, so the tab reloads onto it. Share page (/watch) - The password unlock is limited per viewer (link + client IP), under a per-link ceiling that counts only wrong passwords. Keyed on the link alone, ten wrong guesses every five minutes locked out every viewer, the ones with the password included. client_ip moves to app.ratelimit so the public routes can key on it. - Its failure messages and the load-failure screen are announced (role=alert, the password input described by its error). The sign-in, passphrase, password-change and setup form errors get role=alert too. Comments that still described removed machinery are corrected: App's collapse flags, SettingsPanel's Save bar, ConfigPanel, authed_format_sig. Tests: the account's history and hints across the pin; a revoked and a suspended pinned account; the per-viewer limit and the ceiling; the limiter's exhausted(); the share page's announced failures; a share link's feed filters persisted. Each mutation-checked RED for its own reason. |
||
|
peter
|
f569b19c49 |
fix: review round 2 — a signed-out tab acts as nobody, honest share-page failures
Deep-review round 2 over main..improvement/watch-background-audio. Account resolution (server) - A request naming an account that is no longer signed in to this browser (X-Siftlode-Account or ?account=) now acts as nobody instead of the session's default account. A tab still pinned to an account signed out in another tab went on acting as the default: its player checkpoints and preference saves landed in that account, the messages socket and Plex playback ran as it, and its Sign out signed THAT account out. A hidden tab never polls /api/me, so a tab playing in the background kept doing it. The /api/me bootstrap probe alone keeps the fallback, which is how such a tab learns who it now is: the app's 401 handler reloads, and BootGate re-pins the tab. BootGate - On an identity change the tab reloads without moving its pin first. The tree being torn down still runs its unmount saves before the page goes, and with the pin already moved they wrote into the new account's local keys (a player volume still pending). The reloaded page corrects the pin. Share page (/watch) - Only a 404 says the link is gone. A 5xx, a dropped connection, and the unlock's own rate limit all rendered "no longer available ... expired or revoked" to a viewer with a valid link, with no way to try again. A load failure now offers Retry; too many password tries says so and keeps the form; an unlock that gets no answer says so instead of doing nothing. Strings in en/hu/de. Downloads - A cookie-authed asset is recognised under the current signature version and the one before it, so a job queued before a deploy that bumps the version still downloads with its cookie instead of failing on the login wall with an error that says no cookie was used. The app chunk is preloaded through lazyWithPreload, so a failed fetch no longer leaves an unhandled rejection beside the ErrorBoundary. The panel layout seed is one helper (seedLayout). Tests: the stale-tab refusal, its harmless Sign out and the probe fallback; the pre-bump authed asset; BootGate keeping the pin on reload and staying mounted through a same-account change; the reveal clock's later-commit hand-off; the prefs, panel layout and demo-scope seeds from the me cache; the share page's failure states. Each mutation-checked RED for its own reason. |
||
|
peter
|
2a9ddb78ac |
fix(downloads): E11.5 epic-end review round 2
Pause now re-reads the job under the same asset-then-job row lock as resume, cancel, delete and redownload. Its unlocked write waited for a concurrent cancel's lock and then set the released job back to paused, a holding status, so a later delete released the same hold again and could free a file another user's job still held (measured: the overlapping-request test ends 'paused' instead of 'canceled' with the old read). The lock test now calls the route handlers, so it also pins which helper each route uses. Comments follow the code: resolve_url's contract names both verdicts and that a failed page read is raised as itself; the inherited-failure diagnosis asks the cookie gate as it stands when the job is claimed, so its advice fits the cookie that job's Retry would use; the undecryptable-cookie warning no longer claims a download goes ahead, since the worker parent reaches it only to diagnose. |
||
|
peter
|
e887233210 |
fix(downloads): E11.5 epic-end review round 1
A refused Facebook cookie on a group post is no longer "no video in this post":
measured on dev, Facebook answers with its login page (no post id at all), so the
adapter now raises its own POST_NOT_SHOWN verdict and the catalog explains it as
the refused cookie. The adapter owns both post verdicts and the "cannot parse
data" wall wording, so the Chrome retry and the catalog read one constant. The
group-post "no formats" entry is cookie-scoped (with a cookie yt-dlp only gets the
post's video link) and uses the adapter's own group-post test instead of a
"/groups/" substring.
Job transitions that move an asset's ref count (resume, cancel, delete,
redownload) re-read the job under a row lock, asset first, so two overlapping
requests can no longer release the same hold. get_or_create_asset loses an insert
race inside a SAVEPOINT instead of rolling back the caller's pending writes (Retry
lost its cleared error). Every worker-side job failure goes through
finalize_if_running and writes error_detail, so a cancel that landed first is kept
and a failure never shows an earlier one's copy. The worker asks the download
child's own cookie gate (credentials.usable_cookie) when a job inherits a failed
asset.
UI: the Facebook copy no longer points at a Retry button the failure window does
not have, says "paste" like Settings, and the HU wording is fixed
("Újraletöltés" on the re-download confirm). A category this client has no copy
for falls back to the generic sentence instead of a raw key.
Tests: route serialization of error_detail, the probed duration through a real
_download, the page-read cap and a failed read, a Retry off a pending asset, the
confirm-button labels, the lock and savepoint races.
|
||
|
peter
|
98ec1ffcab |
fix(downloads): Retry and Re-download take a cookie added after the failure
The login-wall copy tells the user to add their cookies and press Retry, but Retry kept the job on the shared asset the worker never attaches a cookie to, so it failed on the same wall. Resume now moves a pending/errored job onto the asset enqueue would pick with the cookie (never off a file or a download in flight). Re-download salted its fresh asset with the job's previous asset id, which the cookie gate cannot recompute, so even a cookie download rebuilt without the cookie. The rebuild salt is now the new asset's own id (plus the user's cookie salt when one applies), and is_authed_asset recognises it. |
||
|
peter
|
18c11ca437 |
feat(downloads): a per-site failure catalog — Facebook's walls say why and what to do
A failed Facebook download read "Download failed": yt-dlp words the login wall (18+ reel, closed group) as "Cannot parse data", which no category matched. Measured on dev with the owner's cookie before writing a rule — the same text is the login wall without a cookie and a refused cookie with one. - errors.diagnose(msg, url, cookie_attached) tries a host-scoped catalog first (all-of markers, cookie state, link fragment) and falls back to the category. First entries: facebook.login_wall, facebook.cookie_rejected, facebook.group_post_video. The module doc is the recipe for the next entry. - download_jobs.error_detail (0084) + backfill of the stored Facebook parse failures that were unknown. - The pipeline's failure branch passes the link and whether the cookie went with it; resume and redownload clear code and detail (redownload never cleared error_code). - A job meeting an already-failed asset got the text but no code, so it read "Download failed" where the first job said why. It is diagnosed again, with the cookie state from credentials.is_authed_asset (now shared with the cookie gate); the redacted network text classifies back to network. - FE: failCopyKeys gives the row and the failure window one lookup — catalog copy, else the category. Copy in EN and HU; a contract test pins every backend entry to both locales. |
||
|
peter
|
f130a1c05a |
fix(keys): epic-end review round 1 — Back, menus and dialogs keep the focus where it belongs
Deep-review of E11 (19 raised, 14 confirmed; 2 refuted, 3 latent test-hardening notes dropped). - Back from a sub-view (a Plex show page, a Messages thread) restores the item it was opened from even when focus was still inside the view: the first attempt now waits one task, past the sub-view's own popstate listener (measured in Chrome: it left focus on <body>). - The restore listener is installed at load, so the first Back after a reload finds it; a push cancels a restore still waiting for the last landing's lists. - Navigating over an overlay asks it again, a render apart, while its entry stays: the Plex player peels one panel per Back step, so Shift+W over info + stats closed one panel and dropped the navigation. - j/k: a native <button> item keeps its own Tab stop; landing twice on the same item no longer leaks its tabindex; after the focused item is removed (a hidden card) the next j goes on from its slot when that slot is on screen. - An abandoned `g` no longer swallows a modifier chord (g, then Mod+K opened the browser search). - A hover card above a dialog no longer switches its Tab trap off (player, Modal, wizard, Lightbox): new layerStack `anyBlockingLayerAbove`. - "Manage views…" closes its menu before opening the dialog, through a shared `closeThenAct` that ActionMenu now uses too. - Copy: the EN welcome says "Feed"; the EN unreachable reason no longer claims a change; stale comments about PlayerModal's scroll lock and dialog contract. |
||
|
peter
|
8c51062e5b |
fix(api,ui): the demo refusal says why at every site, and the contract guard cannot fail open
Round 6 converted three of four `is_demo` 403s to a structured detail and wrote a docstring saying it
had converted all the sites there are. The account-delete route in `routes/me.py` still answered a
hand-written English sentence, which `notifyActionError` discards in favour of the caller's
translated fallback — so the demo user gets no hint that the demo account is the reason. It is
UI-gated today, so the string is unreachable; what was reachable was the false completeness claim,
which is precisely what stops the next reader from looking. The site is converted, and the list is
now DERIVED from the source: a new test walks `app/` for every `is_demo`-guarded 403, asserts each
raises a structured detail, and fails when the count moves — so this file cannot fall behind the code
the way it just did.
The same round's frontend contract guard was failing open. Its pattern required a bare identifier
before `.detail` and excluded `.detail` used as a last call argument, so `setErr(e.detail)`,
`(e as HttpError).detail`, `String(e.detail)` and four more idiomatic forms were invisible — and
because the file set is compared with `toEqual`, a file the pattern cannot see is simply never listed
and the gate stays green over a fresh copy of the contract. Widened to cover cast receivers, optional
chains and call-argument reads; the pinned set is unchanged, so the narrowing had removed no false
positive. A destructured `detail` stays out of reach (a pattern loose enough for it fires on
react-query's own `const { data: detail }`), and that hole is written into the test rather than
papered over with two innocent files.
Its anti-vacuity guard also did not guard anything: the comment says the glob must have matched
something, but the assertion under it read a sibling file by its own URL, which resolves whether or
not the glob enumerated anything. That only mattered while the pinned list was non-empty — the moment
the deferral sweep lands and it becomes [], the whole file would pass over a glob matching zero
files. The scan size is asserted now.
|
||
|
peter
|
71d7646bf1 |
fix(plex): a name this session cannot write fails at once, and the layout has one spelling
Round 6 turned `artifact_may_arrive` into a positive rule, but keyed it to the HLS vocabulary in
general rather than to what THIS session produces: the playlist regex accepted master/index/stream_*
for every session and the `seq == 0` branch accepted any variant token unconditionally. The two argv
branches are disjoint — a single-audio session is given `seg_%d.ts` + `index.m3u8` and no `%v` at
all, so `master.m3u8`, `stream_*.m3u8` and every `seg_<variant>_<n>.ts` are names it can never write,
and vice versa — so each of those was read as "an HLS name, so wait and see" and held a threadpool
worker, plus its pooled DB connection, for the full timeout. That is verbatim the round-5 outage the
route's own comment claims it prevents.
Measured through the real route against a live single-audio session, before: master.m3u8 404 after
30.0s, stream_q7z.m3u8 404 after 30.0s, seg_a0_0.ts / seg_zzzz_0.ts / seg_0_0.ts 404 after 20.1s
each. After, on the running localdev API: 404 in 44-316ms for all of them, while index.m3u8,
seg_0.ts and seg_1.ts still answer 200 — nothing is over-refused.
The session now carries the rendition names its ffmpeg was told to write, and `entry` is derived from
them instead of passed. And "one place" now means the argv BUILDS its patterns from the same
functions the parser reads back (`segment_name("%d", "%v")`, `variant_playlist_name("%v")`): the
layout was still spelled out independently in six places, with `_VIDEO_VARIANT` read by no consumer,
which is the drift that caused round 6's HIGH. A test asserts the argv's own patterns are what those
functions produce, and that every name they expand to is one the guard accepts.
The load-bearing MEASURED comment cited ffmpeg 8.1.1 — the Windows host's. The binary that writes
these files is the API container's 7.1.5; re-measured there, the conclusion holds.
|
||
|
peter
|
a0114deff1 |
fix(api,ui): a refused write says why, wherever the refusal happens
Round 5 taught `notifyActionError` to discard a plain-string `detail` in favour
of the caller's translated fallback, because `req` leaves 403/404/429 to the
caller and those routes hand-write English. Three places did not get the
message.
**The demo account lost its explanation.** `require_human` refused with a plain
string, so from that commit on the shared demo account got its caller's generic
"couldn't do that" and no hint that the demo account was the reason — and the
gate is not backed by a UI gate everywhere: the comfort shelf has no `is_demo`
check, so the pin button is live and simply fails. A structured `{code: "demo"}`
and an `errors.demo` string in both locales. The two auth routes that hand-roll
the same check were swept with it.
**`ChatThread` carried a second copy of the contract.** It filtered 404/429
itself and then did `e.detail || t(…)`, the pre-fix order, so the central fix
could not reach it and "That member hasn't set up messaging yet." stayed English
under a Hungarian UI. It goes through the shared helper now, and a contract test
pins the remaining raw-`detail` consumers as an explicit SET — never a count —
so a new copy fails the gate instead of shipping. The five on that list are
inline surfaces (a form field's error line, a setup banner) and are deferred
with the ~60 backend routes whose details they print.
**The Plex "Add to playlist" dialog was still silent.** Same shape as the
collection editor round 5 fixed and the YouTube twin that already had the fix:
no catch, so a playlist deleted on another device answered a structured 404 that
nothing reported — the tick never flipped, the gone row stayed, and the floating
promise became an unhandled rejection.
Also: the `ABORTED` sentinel's VALUE was asserted nowhere. The consumer test
hand-built the error it expected, so replacing `xhr.onabort`'s rejection with a
friendlier message left the lane green — and every upload the user cancels would
then raise a red "the upload failed" toast, which is what the sentinel prevents.
Both producer tests assert through `isAborted` now.
And two backend tests were pinning fewer sites than their own docstrings claimed:
the channel-404 loop said "all five" and issued three requests (explore and
subscribe unexercised, the excuse for subscribe naming a different call site
with a different reason slug), and the recipient/job test named three refusals
and requested two — `share_download`'s `not_found("recipient")` had no test in
the suite at all, nor did the locale keys added beside it. Every site named now
has a request behind it.
6 tests, mutation-checked against 7 mutations: the demo detail at both the gate
and an auth route, a re-hand-rolled ChatThread, the silent Plex dialog, the
abort sentinel, and each unpinned 404 — all red on the test that names them.
|
||
|
peter
|
7f17b17c13 |
fix(plex): an HLS session's file names are one vocabulary, and a name it cannot produce fails at once
Round 5 added a guard so a request for a segment beyond the produced region answers instead of polling out the full timeout — the route is a SYNC one, so every such wait parks a threadpool worker and, through `current_user`, a pooled DB connection. Measured against real ffmpeg, the guard could not fire where it mattered. `-hls_segment_filename seg_%v_%d.ts` substitutes the VARIANT first: a dual-audio session writes seg_0_0.ts (video) and seg_a0_0.ts / seg_a1_0.ts (the `name:aN` renditions). The regex read those as sequence-first, so every video segment parsed as segment 0 and every audio segment did not parse at all — the guard was inert on every multi-audio item, which is most of this library. `_seg()` carried the same inversion: the warm-up waited 8 s for `seg_1_0.ts`, a name ffmpeg never writes, on every multi-audio session start AND every seek-restart. The layout is now one constant plus one parser (`segment_name` / `_SEG_NAME_RE`), with a test pinning them to each other, instead of three comments that could drift — and they had drifted, all three ways. The guard is also a positive rule now. It was a blocklist, so anything that was not a single-audio segment name was waved through: `GET …/anything.m3u8` still parked a worker for 30 s, while the route's comment claimed a flood of hopeless names was what it prevented. A session produces its playlists and its segments and nothing else. `.vtt` leaves `_HLS_CT` with it — ffmpeg runs with `-sn` and subtitles are a separate route, so that extension could only ever name a file that will not arrive. Also here: - A 5000-digit segment index reached the client as a 500: `int()` on an unbounded digit run raises past 4300 digits and only OSError was caught. The name is bounded on the route and the index in the regex — the same bug class the subtitle routes were bounded for. - A session torn down between the wait and the read is a 404, not a 500. `_kill` rmtrees the directory and no lock spans those two steps; the player's recovery is keyed on the STATUS, so a 500 skipped the reclaim and ended playback with "couldn't play" instead of restarting at the playhead. The trigger is ordinary — `loadSession` awaits the new session before destroying the old hls instance. - Two comments claimed an open player checkpoints every 10 s "whether or not it is playing", which is the stated reason the 30 s finished-idle reap is safe. `flush` sends nothing once the finish latch is up, or before a position is known. The states it does cover — playing or paused mid-item — are the ones the rule needs; the comments now say that instead of more. - The route's tests leave the DB lane: the route reads nothing from the database since the auto-start was removed, and there those four regressions SKIP on a machine with no Postgres while pytest prints green. 14 tests, mutation-checked: the old parse, the old warm-up name, the blocklist, both bounds off the absurd name, the unguarded read, and the route dropping `proc` — each red on the test that names it. |
||
|
peter
|
1f81684b6e |
fix(ui,plex): a dialog keeps the focus its child asked for, and a hopeless segment fails at once
Both found by UAT in a real browser, after the tests for them were already green — which is the whole reason this repo settles focus and timing claims by running them. - **The autoFocus fix worked, and then the dialog took the focus back.** React 18's StrictMode runs every effect mount→cleanup→mount in development: the cleanup handed focus back to the trigger, and the second mount, finding the focus outside the dialog, moved it to the first focusable — off the `autoFocus` field a beat after the browser had put it there. Chrome measured it: INPUT (commitMount) → BODY (cleanup) → ✕ (re-mount). The effect is idempotent now — it remembers what the dialog was focused on and restores THAT on a re-mount — and the close-time restore is conditional on the dialog still holding the focus, which it tracks as it changes rather than computing at cleanup, where the panel is already out of the document. A dialog no longer yanks focus back from wherever the user has since moved it. - **Passing ffmpeg's handle to the segment wait only shortened the FINISHED case.** Measured against the running localdev API: a request for `seg_99999.ts` on a live session still took 20 195 ms, because that ffmpeg has not exited and the wait has no other reason to stop. ffmpeg writes segments in order, so a segment whose predecessor does not exist is not the next one and cannot arrive — and that is knowable at once. Same request now: 45 ms. Verified on localdev against the real library (Westworld S01E06, remux, multi-audio): Mod+K opens the command palette with the focus in its search field and typing reaches it; playback runs on the new session-scoped HLS URL; a request naming a stale session id is a 404; and a viewer paused for longer than the finished-session rule keeps their 2.4 GB session across two reaper passes — the progress checkpoint touches it — and resumes cleanly. vitest 1009, pytest 1127. |
||
|
peter
|
4a7b65d187 |
fix(api): a refused write is named, in the reader's language
Deep-review round 5, over round 4's own fixes.
- `notifyActionError` preferred the server's `detail` over the caller's translated fallback, and
`reportFailure` deliberately leaves 403/404/429 to the caller — so for exactly those statuses the
dozen call sites round 4 converted started printing whatever English a route hand-wrote into an
otherwise Hungarian UI ("No user with that email.", "Not subscribed to this channel", "Unknown
job"). A STRUCTURED detail is still preferred: `req` has already run it through `localizeDetail`,
so it is both localized and more specific. A plain string is not.
- The other half of the same defect, on the backend: round 4's `get_or_404(what="channel")` sweep
reached exactly ONE of the five identical sites in channels.py and shipped the HU/EN string with
it, so the Block button said it in Hungarian and the four controls beside it in English. All five
are converted, and the subscription / share-recipient / scheduler-job 404s with them (two new
reasons in both locales).
- `PlexCollectionEditor`'s add/remove, take-over and delete were try/finally with no catch — in the
very file round 4 edited to give `createNew` one. 403 and 404 are the caller's to report, so the
tick simply never appeared and a CONFIRMED "delete collection" did nothing, silently.
- `UploadDialog` is the only `uploadReq` caller and kept an empty catch whose comment still claimed
the global error modal had it. It had not since `reportFailure` became shared: a proxy 502 raised
only the self-clearing connection notice and a 413/403/404 nothing at all, so the dialog snapped
back to its pre-upload state with no error named. The user's own Cancel stays silent — and since
a cancel and a network drop both carry status 0, that difference is now a named `isAborted`
rather than a string literal at each end.
Regression tests for each (5 vitest, 2 pytest, plus two existing assertions corrected), all
mutation-checked: 6 mutations, each red on the assertion it is named after. vitest 1007,
pytest 1127.
|
||
|
peter
|
47af4d5c05 |
fix(plex): an HLS artifact names the session it belongs to, and a paused viewer says so
Deep-review round 5, over round 4's own fixes. - Sessions are keyed by rating_key alone while a session's DIRECTORY is per (key, offset, tag), and the artifact URL carried no session identity — so once a seek-restart or a second viewer of the same item took the registry slot, the old viewer's request for `seg_12.ts` was answered 200 with the new session's twelfth segment: bytes from a different `-ss` offset under `-copyts`, on a timeline the client is not on. Wrong content with a success status is the one failure a player cannot notice, and round 4's reclaim-on-404 could never fire on it. The id is in the path now (so the playlist's relative names resolve back into the same session), a mismatch is a 404, and the player reclaims at the playhead. The entry playlist's auto-start is gone with it: it could only ever begin at 0, silently discarding the viewer's position. - Round 4 exempted paused viewers from the reaper by keeping EVERY finished session for the full ten-minute idle window — one whole segmented copy of a film per item, on the volume the download center shares. The signal it needed already existed: an open player checkpoints its position every 10 s whether or not it is playing, so that route now touches the session and the periodic pass is back on the short rule. - POST /session's opportunistic reap was never scoped to the key being started: it walked the whole registry with the 30 s finished rule, so pressing play on ANY item — even a direct-play one, the call ran before that early return — killed another viewer's paused session. It is gone; the periodic pass owns reaping. - A segment that can never arrive no longer polls out the full 20 s: the producer's handle is passed to `wait_for`, so a finished ffmpeg answers at once. This is a sync route, so each such wait parked a threadpool worker AND a pooled DB connection, and an ordinary seek past the produced region was enough to do it. - The DataError handler's re-raise branch — the half that keeps a genuine DataError a 500 — is pinned, so simplifying it to an unconditional 422 cannot pass in silence. Six regression tests (two route-level, in the DB lane), each mutation-checked red on the assertion it is named after. pytest 1124, vitest 999. |
||
|
peter
|
6d49da17fe |
fix(api): a write that hit a connection failure is still reported, and every failure is reported once
Deep-review round 4, the "one report per failure" contract round 3 introduced. - `req` marked gateway (502/503/504) and network failures "already reported" because they raise the connection notice. But that notice is about connectivity and removes itself on the very next answer — which every caller's own resync refetch fetches at once — so a delete that hit a keepalive reset flashed the notice and was then reported nowhere, the list still there. Only the durable reports (the error dialog, the 401 sign-out) count now; the caller says the action failed. - A connection notice the user closed swallowed every later one for the rest of the visit; it is raised again. - `req` and `uploadReq` share one failure policy (`reportFailure`). The upload copy had drifted: a 502 opened the blocking "Server error (502)" dialog and cleared a notice that was still true. - A taken Plex collection name is a structured 409, localized, and the create/rename calls are quiet: it was an English dialog plus a Hungarian toast. `notifyCollectionEditError` is gone. - The mutation onErrors that toasted unconditionally after a non-quiet request (invite approve / deny / add, demo whitelist, audit clear, scheduler, share, full history, edit, config tests, tag attach) go through `notifyActionError`; the playlist delete's hand-rolled copy of it too (it takes a level now). The user role / suspend / delete actions had no onError at all: a user deleted in another tab failed silently and stayed listed — now said, and the list refetched. |
||
|
peter
|
31dc4184f5 |
fix(api): a paused viewer keeps its HLS session, a refused subtitle publish backs off, and a NUL is a 422 everywhere
Deep-review round 4, backend half. - The periodic HLS reaper (round 3) applied the "finished and unfetched for 30 s" rule that only a NEW session request may apply. A remux finishes long before the film, and a paused player with a full buffer fetches nothing, so a one-minute pause killed the session and playback died ~30 s after resuming. The periodic pass is `reap_abandoned` now (the long idle limit, finished or not). - A subtitle `publish` that could not keep its output (full / read-only cache volume) returned quietly, so every poll queued the window and the whole-container pass again. It reports failure and the pass is backed off like a failed read; a cached VTT that cannot be READ is answered `unavailable` (`cache_unreadable`) instead of scheduling reads that can never land. - The subtitle routes bound `rating_key` / `sub_ord` (they become a cache file name: a 250-digit one was ENAMETOOLONG, which pathlib's exists() re-raises -> a 500), and the cache helpers no longer throw on such a name. - One app-wide handler turns psycopg's "text fields cannot contain NUL" DataError into a 422: round 3 guarded only the by-rating_key lookup, and `contains=`, `contains_group=`, reorder/bulk key lists and the collection search still 500'd. - The hand-written `db.get(...) is None -> 404 "Unknown ..."` siblings use `get_or_404` with a `what` (channel / video / user), so the Hungarian UI no longer shows English for a gone row. |
||
|
peter
|
86971c574d |
fix(api): progress checkpoints past the int4 column are a 422, and idle HLS sessions are reaped on a timer
Deep-review round 3 of bug hunt 2026-09. - The YouTube and Plex progress bodies are bounded by POSITION_MAX_S (the int4 position columns): a finite position of 2^31 or more was a DataError, i.e. a 500 — the sibling of round 2's non-finite fix. - plex/stream.py promised "a periodic reaper", but reap_idle ran only inside the NEXT session request: after the last playback an abandoned session (its ffmpeg still working, its directory a whole segmented copy of the film) lived until someone pressed play again. A lifespan task reaps every 60 s. - Tests for what round 2 left untested: the 422's body keeps the list shape the UI localizes, and stream_session refuses non-finite start/aoff. |
||
|
peter
|
1cfe6a7f59 |
fix(api): a gone row and a refused playlist action are said in the reader's language, and a failure is reported once
Deep-review round 3 of bug hunt 2026-09.
- get_or_404 / owned_or_404 answer a structured {code: "not_found", reason: <what>}
(errors.notFound.* in HU+EN, generic fallback). The structured-404 fix of round 2 had gone into the
ad-hoc `_own_playlist` (CLAUDE.md's "Not" pattern) while the shared helpers stayed string-only, so
views, tags, pins, Plex playlists, invites and users still 404'd in English. `_own_playlist` now
goes through owned_or_404; admin's hand-rolled user lookups use get_or_404.
- A move into a list that is gone names the TARGET (reason "move_target") — "this playlist doesn't
exist any more" was read as the list on screen, which still exists.
- Every refusal in the playlists router is a {code: "playlist", reason} (playlists.errors.* HU+EN)
instead of a bare English string shown verbatim in the Hungarian UI.
- `req` / `uploadReq` mark the errors they already put on screen (the error dialog, the connection
notice, the 401 sign-out), and notifyActionError skips those. The watched toggles, the saved views
and a playlist delete toasted a fault the dialog was already showing; Playlists' two hand-written
status filters now go through the same check.
|
||
|
peter
|
dcd8a9566a |
fix(playlists): the player's Remove follows the list it was launched from, the player survives a change of selection, and a gone list is said in the reader's language
- D3 snapshotted the removal's TARGET, but whether the player offered Remove still followed the
selected list: launched from Watch later (no Remove, by design), selecting another list under it
made Remove appear — and delete from Watch later. The snapshot now carries the launch list's
editability too.
- Selecting another list under an open player (the command palette works over it) put the page into
"Loading…", and the player was rendered only in the loaded branch — it vanished mid-playback and
came back at its launch video. It now sits at one place in the tree whatever the page is doing.
- A removal takes the video out of the CACHED list as well as the rows: the player's progress
checkpoint rewrote that entry before the refetch landed, handing the membership guard a list that
still held the video — the row came back and the undo history went.
- The watched-state callback refreshed the selected list, not the launch list (D3's sibling).
- `_own_playlist`'s 404 is structured (`{"code": "playlist", "reason": "not_found"}`) and localised:
the bare "Unknown playlist" reached the Hungarian UI in English, which also made round 1's new
`removeFailed` string unreachable. Both Remove and Move report through `notifyActionError` now.
- The "viewer moved on under the confirm" test stepped back only once, which lands on the same video
whether or not the removed one left the queue; it now takes the second step that tells them apart.
|
||
|
peter
|
7360339039 |
fix(api): a non-finite number in a JSON body is refused with a 422, not a 500
The JSON parser under FastAPI accepts `Infinity`/`NaN`, and both progress checkpoints truncate the position with `int()` — a hand-written checkpoint came back as a 500 with a traceback. Their bodies now refuse non-finite floats, and the app's validation handler — FastAPI's own shape — echoes a non-finite input by name: the default one failed to serialise its own error (JSON has no NaN), which was a 500 by another route. Sibling of the subtitle routes' non-finite query parameters. |
||
|
peter
|
473d9f02f2 |
fix(plex): subtitle windows land where the full pass does, and the extractor stops re-reading, respawning and outliving the process
Deep-review round 2 of the 2026-09 bug hunt, over the round-1 fix diff. - A window was LATE by the distance to the keyframe its input seek landed on, and then doubled for good once the full pass merged in: a cue already running at the window's start came out at a negative time, and the webvtt muxer's default shifted the whole output so it sat at 0. Measured on the real Westworld S01E06 at 25:00: every window cue +5.005 s, 682 cues merged instead of 608. `-avoid_negative_ts disabled` keeps the times exact (the parser reads ffmpeg's signed-field spelling of a negative time); now 608/608 on that file, and a real-ffmpeg test pins it. - A full pass that failed was re-armed every minute while the viewer watched what it did publish — a whole-container read per minute. It now backs off for an hour; windows keep running meanwhile. - A failed WINDOW set the track's failure memo, so any uncovered position read "failed" — a final answer that ends the player's poll, even with the full pass about to deliver, and one any account could plant with an absurd `at`. It now marks only the span it failed to read; "failed" is given once the full pass AND a window there have both failed. - Windows join by what they COVER, not a bucketed identity: a backward seek no longer waits behind a window that cannot cover it, and a playhead crossing a bucket edge no longer queues a second read. The queue is bounded per track and overall. - shutdown() sets a flag a worker checks before it spawns ffmpeg (one still in its Plex lookup used to start a read after the kill sweep); queued-job cancellation and the lifespan call are now tested. - A window starts where ffmpeg is told to seek (rounded to the ms it gets), a rounded-up millisecond carries into the minute (no more `00:00:60.000`), a key's first publish claims nothing before its VTT lands, and `at`/`offset`/`start`/`aoff` refuse non-finite values (`at` also an absurd one). |
||
|
peter
|
cf7ec5e9ec |
fix(plex): the subtitle extractor's seams — doubled cues, a truncated track cached as whole, a poll that respawned ffmpeg
Deep-review of the bug-hunt round (R1). The background extractor from
|
||
|
peter
|
889d2acf52 |
feat(plex): the collection page sorts, and a linked collection has a name
Two halves of the same report ("Collectionben nem működik a rendezés?"). The pasted browse URL
sorts correctly — that part was working. What did not:
The Collection INFO page had no sort control at all and a hardcoded release-year-ascending order.
It now shows the browse grid's Sort control, with the browse grid's vocabulary, because a second
sort vocabulary on a neighbouring surface is exactly the kind of drift this repo keeps paying for:
`PLEX_SORT_KEYS` is now one tuple in `plex/query.py`, and a test pins BOTH implementations to it —
browse's key→column map (extracted from the handler to be reachable) and the collection page's
key→member map. A key added to the vocabulary without teaching one of them fails the gate instead
of putting a dead option in the menu.
Ordering a collection cannot be a single ORDER BY: its two legs are different tables, so the merged
member list is sorted in Python. Members with no value for the key sink to the END in both
directions (a movie with no rating is not "the worst rated" — the browse grid's `nullslast()`), and
ties always resolve title-ascending, so flipping the direction reverses the ranking rather than the
alphabet inside it. `lastwatched` needed the show leg's `resumed_at`, so the page now takes ONE
rollup pass that yields both the badge status and the recency instead of the status alone — without
it that menu entry would have ordered every show as never-watched. The default stays release-year
ascending: a franchise reads in release order, and that is what the page did before.
The other half: a shared Plex link carries the collection filter as bare rating_keys, so a link
opened cold had no name for them — the sidebar token rendered `17045` and the chip could only say
"Collection". `GET /api/plex/collections` takes a `keys=` CSV now, and a display-only hook resolves
just the missing ids. It deliberately does NOT write them into `collectionTitles`: that map is
hashed into the Plex query keys, so storing a resolved label there would split the cache from the
identical state and refetch the grid for a piece of text.
Verified in the browser on localdev against the real Plex library: the movie collection's menu
offers Length and the show collection's does not (the same scope rule as browse); rating-ascending
reorders 8.1/7.4/7.2/6.6/6.5 correctly; `lastwatched` on a show collection brings the in-progress
shows up; the choice survives a reload (it is a server pref, on its own key so opening a collection
never re-sorts the library behind it); and the reported URL now reads "Pirates of the Caribbean
Collection" in both the sidebar and the chip.
12 pytest + 8 vitest, mutation-checked against 14 mutations (direction ignored, nulls first, no
stable tiebreak, unknown key untouched, wrong default, show recency dropped, empty `keys=` falling
through to everything, a key missing from either sort map, every id refetched, ids unsorted,
fetched titles unmerged, the chip ignoring them, the hook's result never reaching the builder) —
each red on exactly the test that names it.
|
||
|
peter
|
ed4ab4e13c |
fix(plex): subtitles stop blocking the request, and stop re-reading the file
Bug hunt 2026-09, items 8 and 12 — the two things the Plex player was measurably
lying about.
Item 8: picking a subtitle track showed nothing for a long time, and the reported
workaround was to stop/start until it suddenly worked. Measured on prod against a
17.06 GB MKV with two embedded SubRip tracks:
full pass, one track 154 s
full pass, both tracks in ONE run 111 s
-c:s copy (no WebVTT conversion) 143 s
from 1:00:00 to the end of file 66 s
a 630 s window at 1:00:00, both 8 s
So the conversion was never the cost: ffmpeg streams every byte of the container
between its seek point and its stop point, and the array delivers ~140 MB/s. The
only lever is reading less. Three changes follow from that:
* a window around the playhead is extracted and published FIRST (seconds), and
the full pass completes behind it;
* every embedded text track rides that same single read — picking Hungarian used
to cost a full pass and then switching to English cost another one;
* none of it happens inside a request any more. The endpoint answers 202 while a
background worker extracts, the player polls /status (carrying its position, so
the window follows the viewer), and one ffmpeg run finally has a timeout.
-copyts is deliberately absent from a windowed pass: with it ffmpeg ignores -t and
reads the whole container anyway (measured — a [0, 600 s] window still took the
full 145 s), so a window comes back on a zero-based clock and is shifted to
absolute time here. Cached tracks written by older builds have no coverage sidecar
and count as complete, so nothing is re-extracted after the upgrade.
The <track> is no longer mounted before the server has something to put in it; a
"Preparing subtitles: …" notice stands in, and a track that can never be shown says
why instead of leaving the viewer to conclude the picker does nothing.
Item 12: "Bitrate" read a constant 211 kbps on every remux, because it was the
master playlist's BANDWIDTH — which ffmpeg derives from the one stream whose rate
it knows, the -b:a 192k AAC track, plus 10%. It is a measurement now: bytes per
second of MEDIA from hls.js FRAG_LOADED, accumulated per fragment type and summed
(a multi-audio session's video and audio renditions cover the same media seconds,
so pooling them would report half the truth), and an honest dash until the first
fragment lands. The panel also finally names the audio and subtitle track in play.
Verified in the browser against the real Plex library, and every new test
mutation-checked.
|
||
|
peter
|
b8bff504d8 |
fix(plex): "Clear resume point" now tells Plex about an in-progress item
The button deletes the local row and mirrors the un-mark to Plex, but it decided whether to push from the previous STATUS alone (prev_status != "new") — and an in-progress item's row is status="new" carrying a position, which is exactly how the importer maps a bare viewOffset. So for the single case the button exists for, nothing was ever sent to Plex: the offset stayed, and the next pull imported it straight back. The user cleared Protector twice and it returned to Continue watching at 92s both times. Push now when Plex holds anything to clear — a view count, or a resume point. "hidden" stays Siftlode-only and pushes nothing, which is what this endpoint's docstring always claimed and the status-only guard contradicted. One unscrobble clears BOTH viewCount and viewOffset — measured against the live server from a watched+offset state and an offset-only one, so the push itself needed no change. The client docstring that claimed "clears viewCount" is what sent the first diagnosis down a wrong path (a timeline write with time=0, which does not clear an offset at all); it now records what the server actually does. Verified end to end on the real Plex server: the button leaves viewCount=None viewOffset=None, and a full 44,885-item re-import brings back no row for it. |
||
|
peter
|
e4ffd59ca7 |
fix(e24): review round 2 — a buried history entry, a stale hero, and a spinner that was not ours
Nine of the round's ten surviving findings. The tenth (a failed shelf reorder leaving an order the server rejected on screen) is on ROADMAP.md: its obvious remedy reintroduces the overlapping-move defect the code already documents, so it wants a design, not a review edit. - `navigateOverOverlays` buried the live overlay entries before knowing whether the navigation pushed one. `setPage` no-ops when the page is already current and Home is the default landing page, so Shift+W over a player ON Home marked its entry buried under a push that never happened: the unmount consumed the mark instead of reclaiming the entry, and the next browser Back was a press that did nothing. `setPage` now reports whether it navigated, and one `buryUnder` rule serves both entry points — `closeThenNavigate` takes the same no-op navigation from a palette command for the page you are already on. - The hero could answer a new time budget with the previous one's pick. The chip row resets seed/seen/press but cannot reset an answer that already resolved, and `keepPreviousData` chained through the disabled seed-0 query — so a reroll under "10 perc" showed the film picked for "Bármennyi" while it loaded, and kept it if the request failed. `lastPick` now carries the server's own budget echo, and the placeholder is scoped to the budget. - The reroll button spun and swallowed clicks after mark-watched, bookmark, pin and shelf moves: the pick key sits under the invalidated `discover` prefix on purpose, so raw `isFetching` was true for background refetches. It now means "a press of mine is outstanding" — which still covers the first press, where there is no placeholder. - Failed writes on Home toasted the page's LOAD-failure string over a fully loaded page; they get their own key (HU+EN). The empty-state reroll had no `loading`, so the press was unacknowledged and a second click skipped a tier. `danger` moved into the shared unpin row so the same command is not red on the shelf and neutral in the rail below it. - Two comments claimed things the code does not do: the `exclude` cap guards no "unbounded scan" (it never reaches SQL), and the route test that named the cap sends three keys. Tests: the burial rule gets its own file (it is the one assertion that lets a reconcile issue a real programmatic back()); App's keymap wiring gets a source contract, because dropping it left all 818 tests green; `add_pin`'s IntegrityError arm was reached by nothing. Every new test mutation-checked red, both edges where the predicate has two. Gate: ruff, ruff format, 3x tsc, eslint (0 errors), prettier, knip, vitest 822, pytest 1005. |
||
|
peter
|
3709b71a94 |
fix(e24): the review's low findings — dead constants, honest labels, a 500 that should be a 404
`add_pin`'s IntegrityError handler assumed the unique constraint was the only way in and re-raised otherwise, so a target row pruned between `resolve_ref` confirming it and the INSERT (a Plex sync dropping a removed item) tripped the FK and handed the user a 500. Semantically that is the same "no such thing" the pre-check already returns. Untested: the race needs two concurrent transactions. The five per-kind unique constraints had no test at all — the route's own dedupe returns before reaching the DB, and the migration drift check filters constraint diffs out as noise, so they could go missing with every lane green. Frontend, all from the same review: the shelf's unpin row is now `danger` like every other destructive row in the app; the hero's artwork button says what it will actually do and names the item instead of always announcing "Play"; the empty-budget message offers a reroll rather than `StateMessage`'s "Retry", which promised a re-run of something that had not failed; `setState` and the watch-later toggle catch their rejections instead of leaving an unhandled promise and a click that silently does nothing. `StateMessage` gained a `compact` rung, which retires the frontend's only two `!important` overrides — they existed to beat the primitive's own padding from the call site, and were the copyable idiom for the next surface that wanted a compact empty state. Dead vocabulary removed or made load-bearing: `DISCOVER_BUDGETS` was exported, re-exported and consumed by nothing while `Home.tsx` re-typed the same four bands (a `readonly DiscoverBudget[]` accepts any subset, so a fifth band would have typechecked green and rendered no chip); `PIN_KINDS`' value export had no consumer at all; `useComfortPin` no longer publishes `pinIdOf`/`toggle`, which nothing outside the module calls. Three comments corrected against the code they describe: a pointer to a `resolve.py` that was never written, the file header's claim that Home defines no card component of its own, and the `seen` window still quoting the 8 that was measured and abandoned for 32. The rest of the review's survivors are logged on ROADMAP.md under "Deferred — E24 epic-end deep-review (2026-09-08)" — the ones whose dissent was substantive, and the ones whose fix is wider than the finding. |
||
|
peter
|
cedebe0ffb |
test(e24): close the review's coverage gaps, and correct a comment that was untrue
`test_only_a_show_row_pays_for_a_resolution` seeded no watch state, so every rail query inner-joined its way to zero rows and the loop body — with it the assertion — never ran. It now seeds a video, a movie and an episode, and asserts all three kinds are present before the loop, so it cannot go vacuous again. The Plex half of the surprise rail had no test for either of its promises: every test that reached `plex_surprise` passed budget="any" against a library with no state rows, so the duration bounds and the already-started exclusion could both be deleted with the suite green. The YouTube half has had tests for the same two rules since S1 — the asymmetric half of one principle. `plex_item` and `plex_show` — two of the five pinnable kinds — were pinned by no test at all, leaving the rating_key → internal-id translation and both shelf arms uncovered, including the self-reference cycle that once 500'd the shelf. And `GET /api/discover/pick` was exercised only through its domain function, so the route's own query-string translation could regress silently; the new test excludes all three candidates, which is what proves the whole list parsed rather than one unsplit string. `PIN_COLUMNS`/`PIN_KINDS` claimed in a comment to be the one place the pin vocabulary is spoken and had no readers anywhere: `resolve_ref` re-spelled the column names and the route re-typed the kind list. Both now drive off the constants. The `_NO_TS` sentinel was introduced as a crash fix and is not one. The key it replaced, `(ts is not None, ts)`, cannot raise — tuple comparison finds the first UNEQUAL element via `==`, so two null keys compare equal and `None < None` is never reached; three independent runs confirmed it over every permutation. The rationale now says what the sentinel actually buys, lives once in `plex.rollup`, and the third rail that was still on the older spelling has been swept to match. Its test is renamed and re-scoped to the mutation it does catch. Frontend: the reroll's second press asserted `expect.any(Array)`, which cannot tell an accumulating exclusion window from a replacing one — both that and SEEN_WINDOW = 1 (each reinstating the measured nine-card cycle) left the suite green. And `onInfo`'s jump type was untested while `onPlay`'s was pinned twice, so pointing the ⓘ affordance at the player typechecked and shipped green. All eight fixes mutation-checked; every mutant died on its intended test. |
||
|
peter
|
82914b56ca |
fix(e24): the Home surface's three UAT findings — Back, the reroll, and a show card
All three were measured before they were touched, and the diagnosis moved twice.
**Back from a Plex title played off Home landed on the Plex module's root.** One
click pushed three history entries: the page navigation, the sub-view opened by
the `jumpTo` hand-off, and — under StrictMode, whose effect body runs twice
before `setJumpTo(null)` re-renders — that same sub-view a second time. So the
first Back visibly did nothing and the second stranded the user on a Discover
page they had never visited.
A cross-module hand-off is the CONTINUATION of the navigation that brought you
there, not a second navigation, so it now REPLACES the page entry instead of
stacking on it (`useHistorySubview.open(view, {replace})`, asked for by the jump
target). Replacing is also idempotent, which is what disarms the StrictMode
double-run. The intent rides on `PlexJumpTarget` rather than being inferred,
because the header typeahead jumps from INSIDE the module, where the entry being
replaced would be a sub-view the user really was looking at.
**"Mást kérek" cycled nine cards and never left the resume tier.** Measured:
twelve presses, nine distinct titles, period nine, every one `tier: "resume"` —
the shelf and the surprise tier were unreachable by pressing the button. Two
causes multiplied: the tiers were tried in a fixed priority order over a 20-row
resume rail, so the button just walked down it; and the client's rolling
exclusion window of 8 guaranteed the wrap.
The press count now rotates which tier answers first (`start`), so each press
offers a different KIND of answer — the user's call, given the choice between
that, a wider window, and a surprise-only reroll. The fallthrough is kept, so a
press does not come back empty because the tier it led with had nothing. The
window is matched to the server's own cap rather than a smaller number of ours.
**A half-watched series on Home opened the show page.** A movie already went
straight to the player; only shows did not — and the comfort shelf and the hero
both resolved a show to its next episode while this rail alone did not, handing
back six seasons to choose between. The rail row now carries `next`, and the
client already played `next ?? card`, so this is a backend fix.
Affording it needed `queries.next_episode` to stop loading every episode of the
show into Python to pick one of them (236 rows for Friends, 227 for JAG) — three
ordered LIMIT 1 probes instead. The show→episode rule now lives in ONE function
(`rails.playable`), which is what the shelf, the hero and the rail all call; two
implementations are how they came to disagree.
Ten mutations run, all ten red for the right test — including a first pass where
the press count reached `api.discoverPick` and stopped there, invisible because
every Home test mocks that module. The slice now has its own URL test.
Verified in a clean browser tab against the live account: one Back returns to
Home, the rotation gives resume/comfort/surprise across nine presses with no
repeat, and Westworld opens at S1E1 rather than at its show page.
|
||
|
peter
|
f3c0b7be23 |
feat(e24): the Home surface — one question, one answer, both sources
E24 S2. A new `home` module in layout C+ (the mockup the owner picked): the time-budget chips on top, one large hero that answers them, then the comfort shelf and a cross-source resume rail. Home is the landing — deliberately OVER the remembered last module, since landing on whatever you happened to close the app on is the habit this epic exists to replace; `LS.startOnHome = "0"` opts an account back out (Settings toggle lands in S4). No third card component: each unified card carries the native feed / Plex payload, so the rails render through the existing VideoCard (`cardsSmall`) and PlexPosterCard, and `aspect` decides the tile shape without branching on source. Plex playback hands off to the Plex module via a new `jumpTo` type — the player lives there with its lazy hls.js and its provider tree, and mounting a second one on the landing page to save one hand-off would be the expensive way round. Four defects found by driving it in a real browser against the live account, each now pinned by a mutation-checked test: - **The reroll gave the same card back.** A new seed only re-draws the SURPRISE tier; the resume tier deterministically returns the most recently progressed thing that fits, so anyone with something half-finished pressed a dead button. `/pick` now takes an `exclude` list of `<source>:<id>` keys and the client keeps a rolling window of 8. Excluding a SHOW also excludes the episode it resolves to — otherwise the same suggestion comes back wearing a hat. - **"Nothing to suggest yet" flashed over a full library** whenever a reroll was in flight: the new query key has no data for a beat. An empty state standing in for a loading one is the dishonesty the house rules name outright. The hero now falls back to the loaded payload while a pick is pending, and only trusts a null once the pick has RESOLVED. - **A budget change replaced the page — chips included — with a spinner**, so the control you just clicked vanished under your cursor. `keepPreviousData`, the same reason the feed uses it. - **The hero printed twice**, once as itself and once as the first resume tile. Two smaller UI calls made from the same session: the budget caption sits NEXT to its chips rather than in SegmentedToggle's justify-between mode, which on an 80rem page throws the answers to the far edge; and the feed takes the YouTube mark its own toolbar already uses, since Home now owns the house icon. Gate green: ruff, ruff format, 3x tsc, eslint, prettier, knip, vitest 780, pytest 989. Verified end to end on localdev against the real account: budget chips re-answer (matched on REMAINING time — a 51-minute video with 2:46 left is a ten-minute answer), the reroll walks three distinct heroes, and a Plex show tile opens its drill-down in the Plex module. |
||
|
peter
|
53b50231ff |
feat(e24): the Home surface read-model — hero, resume rail, comfort shelf
E24 S1. The anti-paralysis answer layer, served from our own Postgres with no upstream call: one hero suggestion, one CROSS-SOURCE resume rail, and the user's own comfort shelf. Survey first, because the roadmap line was wrong about the starting point: Plex already ships the whole trio (E5's Discover rails, including a reshuffling Surprise me), the YouTube side ships none of it, and nothing anywhere answers the question across both. The data already existed on both sides — VideoState and PlexState are structurally identical, the feed has an `in_progress` filter and a seeded shuffle, and the YT player already resumes position. - app/discover/budget.py — the time bands. "How much time do you have" (10 perc / 30 perc / egy este / mindegy) replaces 15/D's mood questionnaire: deterministic, answerable from a column we have, and it picks the source for free. Matched against REMAINING time, so a film with 8 minutes left is a ten-minute answer. An unknown runtime fits only the unbounded band. - app/discover/cards.py — a thin unified card that carries the NATIVE feed / Plex payload, so the page renders with the existing card components instead of a third one. `aspect` is the one new field: a rail needs to know poster from 16:9, and that is a property of the artwork, not of the module. - app/discover/queries.py — the SQL, reusing feedquery and plex.rollup rather than the Plex Discover ROUTE, whose LibraryFilters narrowing belongs to a surface that has a sidebar. One seed drives both halves so a reroll moves them together. - app/discover/hero.py — resume -> comfort -> surprise, with the chosen tier in the payload so the client labels it honestly instead of guessing. - comfort_pins (0083): five nullable CASCADE-ing FKs under num_nonnulls(...) = 1 rather than a flag or a polymorphic string ref. A flag cannot order a shelf or hold a channel; a string ref would let a deleted library item leave a dead tile on the one surface whose whole job is to be reliable. Two honest-state rules the tests pin: the surprise draw excludes anything already started (the resume tier may have skipped it for not fitting, and offering it back as "Meglepetés" would contradict the question), and which source leads the draw follows the seed, so the Plex library can actually answer "egy este". Gate green: ruff, ruff format, 3x tsc, eslint, prettier, knip, vitest 770, pytest 987. |
||
|
peter
|
5703c4d713 |
fix(plex): a null progress timestamp must sort a rail row last, not raise
The Continue-watching and Surprise rails sorted on `(ts is not None, ts)`. That key LOOKS null-safe and is not: two rows that both lack a timestamp compare equal on the flag and then compare `None < None`, which raises — so a show whose rollup has no `resumed_at`, or a movie state written before `progress_updated_at` existed, could 500 the rail instead of sorting last. Replaced with an explicit epoch sentinel, and named it beside the other rail constants so the sibling rail E24 adds cannot re-derive the same broken key. |
||
|
peter
|
013f19faca |
refactor(plex): move content_libs down into the query layer
The browse scope is a pure DB query with no request state, but it lived in routes/plex/_shared.py. E24's Home surface needs the same scope from a domain package, and a domain package importing from routes/ inverts the one-way dependency CLAUDE.md draws. Moved to app/plex/query.py and renamed to content_libs (public — it is a domain helper now). Swept all five call sites and both tests in one go rather than leaving a re-export shim: the canonical-patterns table already names the routes.admin re-export as the anti-pattern. |
||
|
peter
|
b28be682fe |
feat(e10): make "move a video to another list" one atomic request
The one-step move S2 needs has no honest client-side shape: "add to the target, then
remove from the source" leaves the video in BOTH lists whenever the second call fails,
and the reverse order loses it entirely. So it is one endpoint in one transaction —
POST /api/playlists/{id}/items/{video_id}/move — which re-parents the existing item row
(so `added_at` survives), recomputes `dirty` on both sides, and is idempotent when the
target already holds the video.
Also publishes `updated_at` on the playlist summary, and bumps it explicitly for the
edits that only write `playlist_items` (add / remove / reorder / move). UpdatedAtMixin's
`onupdate` fires on an UPDATE of the playlist row, so without `_touch` the rail's coming
"recently changed" sort would have seen renames and nothing else.
|
||
|
peter
|
fc56f514b3 |
fix(e12): the restart hint pointed at the one command that undoes a rollback
Round three, on round two's diff. The trend across the loop is 47 -> 34 -> 15 raw findings; the ops half kept producing the sharpest ones because the gate cannot run it. **`docker compose start` ignores a newly pinned IMAGE_TAG.** Round two removed the automatic restart because it re-ran `alembic upgrade head` — and then told the operator to run that exact command by hand, in both scripts and the docs. `start` relaunches the container that already exists, which still carries the tag being rolled back FROM, so pinning 0.79.0 and following the printed instruction brings 0.80.0 back up and re-applies its migrations. Verified against a real daemon: after changing the pinned tag, `start` kept the old image and only `up -d` picked up the new one. Both scripts and the doc now say `up -d`, with the pin-first caveat where the operator reads it. **The reset flag outlived its token.** Reverting round one's "each link clears the other" went one step too far: a `#verify=` link pasted over a standing reset notice replaces the whole fragment, so the reset token is gone — but `resetPending` stayed true, and being checked first it hid the confirmation while promising a recovery that signing out could no longer complete. The flag is now cleared when the fragment it describes is gone, which is narrower than what was reverted and does not blank the signed-out card. **A test that could not fail.** The shared-asset regression test seeded the asset as `pending`, and `_reset_asset_pending` only acts on `downloading` — so the release half was a no-op and deleting it kept the suite green. Seeded as the worker actually leaves it now. Also: the malformed-URL guard answers with urlguard's structured detail rather than a bare English string, so both halves of one typo class get the same localized copy, and the third site in that module handing user input to `urlparse` was swept with it; the asset is released after the job is terminal, not before; the Stats 403 toast gets the wizard handle its own comment claimed it had; the pasted Windows recipe sets `$ErrorActionPreference` inside the block (a scriptblock inherits the caller's, so a `SilentlyContinue` profile swallowed every throw) and cleans up the container-side dump on the failure path too; the restore warning moved to the warning stream so a `> log` redirect cannot file it away; and two comments narrowed to what the code does. Gate green: vitest 685, pytest 950. |
||
|
peter
|
358d722ce9 |
fix(e12): close what round one broke, and revert two fixes that overreached
Round two of the review loop, on round one's own diff. Two of its findings were regressions the fixes themselves introduced, which is exactly what the loop is for: **A per-user refusal was writing onto a shared row.** MediaAssets are deduped across users, and the generic failure handler puts the message on the asset — right for an upstream failure, wrong for "your storage is full". Two users queuing the same video share one asset, so the first user hitting their cap failed the second user's job with the first one's private reason, telling someone with an empty library to free up space. The refusal now raises `_OwnerRefused`, caught separately: the job fails, the asset goes back to `pending` for whoever else holds it. Pinned by a test that drives `_download` directly — the refusal happens before extraction, so nothing needs stubbing. **The restore scripts restarted the app, undoing the restore.** `docker compose start` re-runs the container command, and that runs `alembic upgrade head` — so on the rollback path this documents as recommended (restore the dump, THEN pin the older tag) the newer image immediately re-applied the migrations the restore had just undone, and on a FAILED restore it put the app back on a half-restored schema. Both are the opposite of why the stop was added. Nothing is restarted automatically now; the scripts say what to run and when, and the docs' snippets say the same. **Reverted rather than patched:** the OnboardingWizard toast (the wizard already renders that failure inline, next to a Retry button — three recoveries for one failure), and `useVerifyEntry`'s "each new link clears the other". The latter blanked the signed-out card's only banner, where nothing renders the reset flag to replace it. The two states are independent again; what changes is that `dismiss` peels ONE layer, which is what the original finding was actually about. Also: `Stats` uses the YouTube helper rather than the bare one, so the route's English 403 detail no longer beats the translated fallback under a Hungarian UI; `urlparse` guarded alongside `.port` (an unclosed bracket raised just as late), the reason renamed `malformed` since "bad port" misdescribes it; three docstrings narrowed to what the code actually does (the worker's byte check is per-job, not an aggregate ledger, and not on the edit path; the redirect residual leaks a reachability oracle, which the note had denied; the invalidation helper does not cover suspend); the pasted Windows recipe wrapped so `throw` can stop it; the bash one-liner's failure branch no longer exits 0; and the tests that were missing for the round-one fixes — the account panel's error paths, the invalidation set, and the shared-asset case above. Gate green: vitest 685, pytest 950. |
||
|
peter
|
fe38dfbcca |
fix(e12): close the eighteen findings the E12 review round left standing
The deep review of the branch raised 47 findings, 18 of which survived adversarial verification. Grouped by what they actually were: **The size ceiling inverted at its boundary.** `remaining_bytes` clamped to 0 for a user already at their storage cap, and `build_ydl_opts` reads 0 as "no limit" — so the one user over quota was the only one downloading with no ceiling at all. `check_enqueue` could not catch it: it ran while the job was queued and the user was still under the cap. The arithmetic moves into `quota.remaining_bytes` (where every other max_bytes decision lives, and where it can be tested), and the worker now fails the job outright rather than proceeding unbounded — as a non-retryable `quota_exceeded`, since retrying cannot help until space is freed. **Silent failures in the new account panel.** All three actions went through `quiet: true` endpoints, which suppress the global error dialog on the promise that the caller reports inline — with no `onError`, that promise was broken and a refusal rendered as nothing. On the broken-SMTP instance this panel exists for, an admin would read silence as "the mail went out". The co-invalidation set and the email-off predicate move to `lib/adminUsers`, shared with the pending-access queue, so the two copies of these mutations cannot drift apart again — which is how half of this was lost in the first place. **A rollback recipe that destroyed data without working.** `alembic downgrade -1` steps back one migration, but a release routinely ships several (0.80.0 shipped four): it drops the newest one's columns and leaves the schema at a revision the older image also cannot resolve, i.e. the crash loop it was meant to escape, minus the data. The dump restore is now the recommended path, and the alembic route asks the target version for its own head revision instead of counting backwards. Also: redirects named in the urlguard docstring beside the DNS-rebinding residual they share a fix with (SEC-17); a malformed port refused as `bad_port` instead of escaping as a 500; the pull cooldown extended to the sibling route spending the same QuotaAction; restore stops the app before dropping its tables; the PowerShell cleanup can no longer decide the script's verdict on 5.1, nor report the wrong exit code; the documented one-liners hardened to match the scripts; the registry's retention window stated where pinning is advertised; the two banners made mutually exclusive so neither can shadow the other; `hover:bg-hover` (a utility this project never generated) replaced by the `selectable` recipe; the clipboard helper reused; and the sync cooldown's copy swept to the two callers that discarded it. Verified: gate green (vitest 678, pytest 949), and the panel, the row hover and the reset banner UAT'd on localdev. |
||
|
peter
|
f98b060e2b |
feat(admin): an account panel, and a set-password link that needs no email
Design option C, chosen by the user 2026-09-05. Nothing in the admin UI ever showed an account's GATE STATE: an admin could see a "Pending approval" chip but not whether the address was confirmed, whether SEC-10 had cleared the password, or which Google account was attached — so every support conversation started by guessing. Clicking a row now opens a drawer that says which gate the person is actually stuck behind, with the recovery actions under that readout rather than in the row. The drawer is `Modal placement="drawer"`, not a second dialog: the layer stack, ESC, the focus trap, Back-to-close and the scroll lock are the hard parts, and duplicating them is how two shells drift apart. It slides in, and honours prefers-reduced-motion like the rest. The link itself (SEC-15, SEC-16, and the everyday "no SMTP and someone forgot theirs") reuses the existing token machinery and the SAME `#reset=` entry the mailed link uses, so the signed-out card consumes it with no new flow. It is a distinct token KIND for one reason: a mailed reset arrives in the mailbox, so completing it proves ownership and confirms the address; an admin-issued link proves nothing about the mailbox — the admin hands it over by whatever means they choose — so it must not satisfy the email gate. Claiming otherwise would be the same false attestation SEC-15 is about. Issuing does not revoke the current password: a misclick must not lock anyone out. USING it rewrites the hash and bumps the session epoch, which is what evicts a squatter — SEC-16's actual remedy. Spending either kind burns the siblings of both, so a completed reset cannot leave a live admin link behind. The non-consuming probe learned the new kind too, or every admin link would have opened onto "this link is spent". The audit log records that a link was issued, by whom, for whom — never the link, which is a live credential-setting secret and the last thing that belongs in a long-lived log a human reads. Not gated on account state on purpose: it is useful for a pre-empted address, a revoked password, a Google-only account that wants one, and a plain forgotten password. Narrowing it would only make it refuse exactly when the admin needs it. The shared demo account is the one exception. Verified on localdev, both languages, end to end: issue from the panel -> the signed-in tab shows the honest "reset links open in a signed-out window" banner with the token still in the address bar -> signing out draws the "Set a new password" form, which is only possible because the probe recognises the new kind. Gate: pytest 925 -> 942, vitest 676. |
||
|
peter
|
bc9e34d0ae |
fix(auth): close the three leftovers E9.5 logged but kept out of its diff
DX-17, three one-liners the epic's canonical sweep did not reach:
`routes/setup.py::setup_admin` still resolved the wizard's admin by
literal `User.email ==`. The wizard can be re-run, and E9.5's premise is
that two spellings of one Gmail mailbox are one person — so a second run
under the other spelling would have created exactly the duplicate admin
row the epic exists to prevent.
`routes/admin.py::_decide` hand-rolled `db.get` + 404 where the file's
newer endpoints use `get_or_404`. There were TWO such sites, not the one
the review named; both are converted, per the house rule that a
principle gets fixed everywhere in one sweep.
`useVerifyEntry`'s `hashchange` listener recognised `#verify=` but not
`#reset=`, so pasting a reset link into a tab already running the app
was silently ignored — the same silence that made the verify half a bug.
It now notices, and says out loud that reset links need a signed-out
window; the fragment is still deliberately left in the address bar
(review round 4), so signing out from that tab recovers it.
DX-16: `.catch(() => setOutcome("invalid"))` could not tell a spent
token from a 502 mid-deploy or a dropped connection, and by then the
fragment is cleared — so a network blip was reported as "that link is no
longer valid", pushing the user to request a fresh one on the strength
of a false statement. Only a 400 carrying {code:"verify"} is the
server's verdict now; everything else gets its own outcome, with no
resend CTA, because the token is untouched and retrying it works. The
rule was already written one file away for the reset-link probe: "a
network blip must never be cached as 'this link is dead'".
Two existing tests asserted the old conflation by rejecting with a bare
Error("400"); they now reject with the real HttpError. The errors are
built inside the rejection, not hoisted: mountAt calls vi.resetModules(),
so an HttpError imported earlier is a different class identity and
`instanceof` would quietly miss.
Gate: vitest 671 -> 676, pytest 924 -> 925.
|
||
|
peter
|
0cc450d1c9 |
fix(downloads,sync): bound what one user can make the server fetch, download and spend
SEC-12: the download box accepts an arbitrary http(s) URL and yt-dlp fetches it from the WORKER, so any approved user could aim the server at the compose network, the LAN or a cloud metadata endpoint — and read the result off job.error, where "connection refused" vs "HTTP Error 401" vs "timed out" is a port scanner. downloads/urlguard.py refuses a URL whose host resolves to a loopback/private/link-local/reserved address, at enqueue AND again in the worker (DNS can move in between, and the worker is where the socket opens). A network-class failure on a `url` job now reports one fixed sentence instead of the raw transport text; the error CODE is still classified from the original, so the localized explanation is unchanged. DNS rebinding is explicitly NOT closed — that needs the connection gated inside yt-dlp — and the module says so rather than implying more than it does. SEC-13: `noplaylist` only demotes a video-inside-a-playlist URL to that video, so a URL that is purely a playlist or channel still expanded into every entry — one enqueue, unbounded downloads. `playlist_items` pins it to one. Size was unbounded too: `check_enqueue` compares the CURRENT footprint against the cap, and a download's size is unknown until it lands, so one job could overshoot by any amount; the owner's remaining budget is now handed to yt-dlp as `max_filesize`, applied AFTER the adapter merge so no source adapter can raise a limit the quota layer set. A join's cut-list is capped at 100: an accurate join emits two filters per segment into one ffmpeg filter_complex. SEC-14: the YouTube budget is instance-wide, so one user holding down a manual sync spent the day for everyone. Per-user (not per-IP — the cost belongs to the account) 3-per-10-minutes limiters on the two bulk pulls, subscriptions and playlists, with separate budgets so one does not close the other. The review also listed /rss, /backfill and /enrich; E3 deleted them, and search stays unlimited on purpose — it is interactive, not a bulk pull. Gate: pytest 896 -> 924 (none skipped), vitest 671. |
||
|
peter
|
540ca244c8 |
revert(auth): the newest registration does not own an unverified account (E9.5 review round 2)
Round 1 fixed the address pre-emption chain by letting a later registration replace the password on an unverified account and delete its outstanding verify tokens. Round 2 read that code and showed it was worse than what it fixed, so it goes back out. It closes nothing. The rule is symmetric and an attacker can simply register last — on a timer, inside the five-per-five-minutes budget — while the owner registers once. What it does add is two primitives an unauthenticated caller did not have: overwrite a real registrant's chosen password, and delete the confirmation link already sitting in their inbox (the approval mail's 7-day token is an ordinary verify token, so the blanket delete took that too). Worst of all it fired on the rows SEC-10 left stripped — password_hash NULL, password_revoked_at set, active — installing an attacker's password where the entire state means "there is none", and clearing the stamp that is the only reason password-reset still accepts them. That is a straight takeover which did not exist before, in the code written to prevent one. The owner's recourse needed no new primitive and already worked: password-reset accepts a pre-empted account, proves the mailbox, rewrites the hash and bumps the session epoch. What remains is the bounded window between a confirmation click and that reset, which is SEC-16 — logged with this attempt written down, so the next person does not reach for it again. Two smaller round-2 findings go with it. `upsert_pending_invite`'s new exact-spelling tie-break ran before any status preference, so it could deterministically pick a DENIED sibling over a pending one and reopen it — the very outcome its comment claimed to prevent; the status preference now comes first. And `send_invitation` activated the account before the send it might have to report as failed, so a request the UI called a failure had quietly let someone in; activation now follows a mail that actually left. The verify token minted to render that mail still commits either way — it is single-use, expires on its own and was mailed nowhere, and the note says so rather than leaving the next reader to wonder whether it was missed. |
||
|
peter
|
563fa0513d |
fix(auth): close what the epic's own review found (E9.5 review round 1)
Eleven findings survived adversarial verification; these are the fixes.
**Address pre-emption.** S1 dropped the approval-time password revoke, arguing `password_login`
refuses an unverified account before it reads `is_active`. Two verifiers showed that holds only
while the address stays unverified — and S6's approval mail exists to get the real owner to end that
state, at which point the squatter's password works. While an address is unverified the NEWEST
registration now owns it: the password is replaced and every outstanding verify token deleted, so
the owner takes the row back simply by registering, and a link already in flight cannot confirm it
for anyone else. The residual (an owner who confirms and neither registers nor resets) is logged as
SEC-16 beside SEC-15, where the re-attestation path it needs already lives. The comments that
asserted the old, false invariant now say what actually guards this.
**The entry URL.** S2 moved `clearEntryUrl()` out of the signed-out auth card into a hook App mounts
unconditionally, and it blanket-stripped the whole query string, the fragment and `history.state` on
the FIRST commit. That broke first-run setup outright — the wizard mounts only after its query
resolves and then finds no `?token=` — and silently ate `?link=`/`?oauth=` (no toast, no refetch
after connecting Google), `?v=`/`?chan=` deep links, and scroll restoration. It now clears only the
fragment and the two one-time flags it owns, and preserves `history.state`.
**The env allow-list.** S3 canonicalised the Invite branch of `is_allowed` and left the
ALLOWED_EMAILS/ADMIN_EMAILS fallback matching literally — the 2026-09-03 lockout with a different
source of truth, plus a shadowing trick where an unauthenticated request under a sibling spelling
permanently masked an env grant. Canonical on both sides now, and the same for the delete dialog's
warning and the ADMIN_EMAILS role bootstrap.
**Honest states.** `send_invitation` stamped "I already wrote to them" before a fail-soft background
send; it sends inline and records only what left. The admin-side resend answered `{ok: true}` for a
dropped mail; it now says so. Both also activate/serialize consistently with the other decide paths.
**Correctness sweep.** The 7-day approval link said "expires in 168 hours" — durations render in
days once there is more than one. `_register_account`, `upsert_pending_invite` and `add_invite`
picked among canonical siblings with no ORDER BY and no exact-spelling tie-break, so which account
got the re-send, and whether a denied row was reopened over a pending one, was up to Postgres.
`purge_user` deleted every canonically-equal invite, revoking a surviving sibling account's access.
`ConfirmProvider` wrapped its ReactNode message in a `<p>`. Approving no longer leaves the
stuck-sign-ups list stale. The refusal panels announce and take focus.
**Tests.** Three mailers rewritten this epic never rendered in any test; every `send_*` now renders
in both languages. The orphan-key guard could not fail (it scanned the catalog that defines the
keys) and had no reverse direction. Newly pinned: canonical activation, the surviving sibling's
grant, a failed invitation and resend, an expired reset link, a throttled login's silence, the
per-language admin fan-out, and that the entry URL keeps what it does not own.
|
||
|
peter
|
10c0ca4cf7 |
feat(email): write to people in their own language (E9.5 S7)
Twelve mails carried hardcoded English while the app itself has enforced HU/EN parity since E5.
A mail is the one user-facing surface nobody can re-render by switching the UI language: it arrives
once, in whatever language we picked, and stays that way. That made the epic's own headline mail —
"one mail, one click" — English for a Hungarian reader.
The text moved to a catalog (`mailtext.py`) with a `text(key, lang, **vars)` renderer, and the parity
rule is now a test rather than a habit: every key must carry both languages AND the same
`{placeholders}`. A translation that drops `{link}` sends a mail with no link in it; one that invents
a placeholder raises inside a background task nobody is watching. Both are silent in production.
Every `send_*` takes the language as its second argument, resolved by the REQUEST handler and never
inside the mail — these all run as background tasks, after the request's session is gone, which is
the same reason the approval mail computes its link in the handler.
Where the language comes from, in order: the account's own preference; failing that, the language the
person asked us in; failing that, English. The middle one is new — `invites.lang`, migration `0082` —
because the mails an access request produces go to an address that usually has no account yet, and
the signal does exist: the page their form was submitted from, or the locale Google reported for a
sign-in we refused. Captured rather than derived, for the third time in this epic (`google_email` in
S3, `preferences.language` in S6 taught the same lesson): a signal that exists only in one moment
cannot be backfilled later. So the admin is asked in exactly one place — the invitation to an address
they typed in themselves, where nobody has ever told us anything.
That send is now a dialog rather than a bare click, which is also where the "you already wrote to
them" warning belongs — it used to live only in a tooltip the admin may never hover, guarding the one
action whose failure mode is mailing a real person twice.
S6's registration `lang` field turned out to be dead on arrival: the backend accepted it, the
frontend never sent it, so every account has been storing nothing. The request builders now stamp the
UI language themselves — a signal that has to be passed in by every caller is one every caller can
forget, which is exactly how it shipped unset.
Admins are notified one mail per language group instead of one blast to everyone. They are people
with their own UI language; a single message would have to pick one of them and be wrong for the rest.
Also removes a duplicated `invited_at` column declaration on `Invite` (two identical definitions
landed in S6).
|
||
|
peter
|
a76bab9b34 |
feat(auth): one mail, one click — the approval mail carries the confirmation link (E9.5 S6)
The funnel's common ordering made a person open TWO messages: one saying their access was approved, another asking them to confirm their address. The approval mail now branches on what they actually still have to do — carry the confirmation link, point at sign-in, or say a confirmation mail follows once they register — so the usual case is one mail and one click. A mail that guesses wrong is worse than a plain one: telling someone to sign in while their address is unconfirmed contradicts the screen that will refuse them. That embedded link lives 7 days rather than the usual 24 hours. Nobody sits waiting for an approval — it can easily be read on Monday after a weekend, and an expired link spends the one-mail promise and puts them back on two. The risk is small: the token only ever confirms an address; it signs nobody in and sets no password. The most likely root cause of the 2026-09-03 report that a confirmation mail "never arrived" is here too, and it was never a delivery failure: re-registering an address that already had an unconfirmed account returned in SILENCE — same cheerful "check your inbox", no mail sent at all. It now re-sends the link. Nothing new leaks: the response and its timing are unchanged, and the public verify/resend already does exactly this for the same input. Whitelisting stays silent, and that is a decision, not an omission. The operator opens a door as a possibility — usually already agreed with the person — and a row can sit approved for weeks with nobody registering; a mail fired automatically there would be nudging someone who never asked. Telling them is its own act on its own button, `invited_at` records it so a second click is a decision rather than an accident, and the button only appears where nobody is behind the address yet: "the door is open, come in" is meaningless to someone already inside. Two things UAT caught that no test would have. Every fresh registration appeared in BOTH admin lists at once, because the stuck-sign-ups filter keyed on "unconfirmed" — the rule is now "the address may sign in, and this account still can't", which leaves an undecided request with the queue that owns it. And the invitation button was offered on every approved row, half of which are existing users. Also: the confirmation mail finally says what its link's rules are (once, 24 hours) — the reset mail always has, while this one left the reader to discover both by meeting a dead link. The card's "didn't get the email?" moves up under the form, and the three waiting messages name the spam folder, which is where a first message to a fresh recipient sometimes lands. Registration now records the language the sign-up page was in. Nothing reads it yet — the bilingual templates are S7 — but it is the only moment that choice exists anywhere the server can see, and a preference nobody recorded cannot be backfilled later. UAT on localdev: re-registering re-sent the link (one token became two, one account); approval issued a 7-day token and moved the account into the stuck list with only a re-send; whitelisting mailed nobody; the invitation recorded itself and the row then warned that sending again would write twice. Test data removed. |
||
|
peter
|
7acdf5d577 |
feat(admin): surface the sign-ups that are stuck outside, and let an admin finish letting them in (E9.5 S5)
Approve an address BEFORE the person registers and their sign-up produces no queue row at all — `_decide` only takes a still-pending invite — so the Access-requests screen sat empty, the tab badge read zero, and the only working move was re-typing the address into the whitelist box. Nobody would guess that, and on 2026-09-03 nobody did. This is roadmap UX-25, where two earlier attempts were reverted after review. Both moved AUTHORITY: auto-activating on registration turns a standing whitelist entry into a bearer grant for whoever registers the address first, and reopening the approved invite to `pending` lets an anonymous sign-up REVOKE a standing approval. What was actually missing was visibility, so this changes nothing about what an unauthenticated caller can cause — the admin could already do exactly this by re-entering the address; they just had no way to know they had to. The section lists every account that cannot sign in yet and says WHICH gate is shut, because the two dead ends look the same from the admin's chair: waiting for activation, or waiting on a confirmation mail. Naming the unconfirmed ones is not decoration — on a no-SMTP instance that is the difference between the owner and whoever registered the address first (SEC-15), and an admin about to let someone in should see it. A person the queue above already covers is deliberately NOT listed here; two buttons for one person is worse than one. Activate is gated on `is_allowed` — the same function the sign-in paths consult, in core, so the button that appears and the badge that counts cannot disagree. Without that gate it would manufacture an inconsistent account: password sign-in only checks `is_active` and would let them in, while their Google sign-in would be turned away and silently reopened as a fresh access request. Letting in someone with no approved invite stays the whitelist box's job, which says so out loud. The badge counts activations SEPARATELY from pending invites rather than widening an existing field's meaning, and only counts what genuinely waits on the admin: an unconfirmed account waits on its owner, so it is listed with a re-send but does not nag. The admin-side resend says plainly what happened, including refusing when the instance has no working SMTP — mail is fail-soft everywhere else, and that silence is exactly what left the 0.78.5 sign-ups with nothing to fall back on. `send_verification_link` is now one function with three callers (registration, the public resend, this one), so the link's shape and TTL cannot drift by who triggered it. UAT on localdev, both languages: the UX-25 shape reproduced end to end (approve the address, then register), the badge counting it, Activate letting them in and dropping the badge, the row losing its Activate but keeping the re-send, a fresh token issued by the re-send, and both actions in the audit log. Test data removed. |
||
|
peter
|
b1086e05dc |
fix(auth): stop drawing a reset form the link can no longer fill (E9.5 S4)
Reported from UAT: delete an account, then open its password-reset link. The card drew the full "set
a new password" form — because a `#reset=` fragment was present, and nothing more — and only after
the user had typed a password did the server answer 400. Measured first: no security hole. A user's
tokens cascade away with the row, so nothing can be set on a deleted or a re-created account. What
was left is a dead end of exactly the kind this epic exists to remove, and it is not rare: a reset
link lives one hour, mail is slow or lands in spam, and whoever got two mails clicks the older one.
password-reset/check answers whether a link is still usable WITHOUT spending it, so asking can never
cost the user their one real attempt — the property the tests pin hardest. It gets its own limiter,
because opening a link must not eat the budget for requesting one, and a throttled caller is told
"ok", never "dead": this check may only ever save someone a wasted form, so when it cannot answer it
degrades to the old behaviour instead of declaring a live link finished. The confirm endpoint stays
the authority, and its failure raises the same named screen, since the token can die between the two
calls or the probe itself may have failed.
A too-short password is deliberately NOT that screen. The length check runs before the token is
spent, so a typo costs the link nothing, and the card tells the two 400s apart by `reason` so only
one of them takes the form down.
Both email-link confirmations (verify, password-reset/confirm) now answer in the {code, reason}
shape S4 introduced. They were the last refusals a user meets by simply clicking an old link while
still being answered in hardcoded English under a bilingual UI. The seven remaining bare strings in
this module (register, request-access, the demo guards, set-password) sit behind client-side
validation the UI does first; they are logged as a follow-up rather than swept in silently here.
One bug found by the UAT itself, in the new code: the check went out FOUR times per opened link —
StrictMode double-invokes effects and the shell remounts the card as the session probe settles.
Harmless per request, but against a per-IP limit it would exhaust the budget in two reloads and then
silently fall back to drawing the form unchecked: nothing errors, the feature just stops working.
Deduped in a module-level map holding the in-flight promise (a ref dies with the remount that causes
it); a failed probe drops itself so a blip is never cached as "dead". The test renders the card
twice, because with one render the guard would rot undetected.
UAT on localdev: the reported scenario end to end (delete the account, open the link) shows the named
screen and no form; a live link still opens the form and completes; the CTA lands on the request-a-
link form; one probe per opened link. Throwaway accounts removed.
|
||
|
peter
|
ed91c83dd7 |
fix(auth): name the state and the address behind a refused sign-in (E9.5 S4)
"Your account is awaiting admin approval." was true and useless. It never said WHICH mailbox the account was on, and on 2026-09-03 that silence hid a Gmail alias mismatch for a day: a registration as np.temu@gmail.com met a Google claim of nptemu@gmail.com, and every screen in the funnel agreed to say nothing about it. Every refusal password-login can answer with now uses the codebase's structured {code, reason} detail, built in one helper. The three that arrive AFTER the password is proven carry the address on the account plus both gate flags; the two that answer before it (invalid, rate_limited) carry nothing, so a wrong password and an unknown account stay byte-identical. The frontend renders the first three as a named screen in place of the form — there is nothing to retype once the password is correct — with an icon, the two-gate checklist, the address in monospace, and a resend CTA where one helps. Reporting each gate from its own flag is the payoff of S1: approved-but-unverified is a real state now, and a screen that assumed a sequence would send that user off to wait for an admin who has already decided. The Google access-request notice names the address too, and says so when the request was already on file under ANOTHER spelling of the same mailbox — the honest version of "we didn't open a second one", stated only when it applies. It moved from the query string to the fragment: the notice now carries an address, and a query string lands in every reverse-proxy access log. Same reasoning that put the verify/reset tokens there, and the same owner already clears it. urlencode, not an f-string, because URLSearchParams reads a bare + in a fragment as a space and would mangle exactly the +tag addresses this epic exists to protect. upsert_pending_invite returns the row it settled on beside the is-new flag, since the row's stored address is what the notice has to name and what mail will go to. Deviation from the PendingScreen mockup, deliberately: no "Sign out" button and no separate "Not your address?" link. The mockup drew the panel in the app shell; the accepted decision (a) keeps the pending user session-less, so there is no session to end, and the address is named directly above a "Back to sign in" that answers the same question. UAT on localdev, both languages: all three refusals with the typed spelling deliberately differing from the stored one (the panel named the stored one), the resend CTA carrying the typed address into the resend form, and both access-request banners with a +tag address surviving the fragment round trip. Throwaway accounts and the language switch reverted. |
||
|
peter
|
a614ee2435 |
feat(auth): show the Google address beside the one the account registered with (E9.5 S3)
An account can now hold two true addresses: the one its owner typed, which a Google sign-in no longer overwrites, and the one Google answers with. Only the first was stored, so Settings could say 'Connected' without saying connected AS WHOM -- exactly what someone who registered with a +tag needs to know. users.google_email (migration 0080) records Google's answer at sign-in and at an in-app link. /api/me exposes it ONLY when it differs from the account address, so the comparison lives in one place and the majority, whose two addresses are the same string, see no change at all. Rendered in the account menu as a muted third line under the identity, and in the Settings sign-in row under the existing hint. A short 'Google' label rather than a brand glyph: the app draws one icon set (lucide), and a generic mail icon next to an email address says nothing. It also reads to a screen reader, which an unlabelled logo would not. No backfill is possible -- Google's answer is only known at sign-in -- so accounts linked earlier stay NULL until their owner next signs in with Google. |
||
|
peter
|
c74487fbb3 |
fix(auth): keep the address the user registered with, and let it sign them in (E9.5 S3)
Two halves of one rule: the address someone typed is theirs to keep, and it must be the one that gets them back in. A plus-tagged or dotted Gmail is often a deliberate choice -- registering as me+siftlode@gmail.com is how you find out later which site leaked your address. The Google callback overwrote it with the account's primary, which can never contain a '+', so the tag vanished on the first Google sign-in and, because sign-in matched the stored address, took their chosen address with it. The rewrite line is older than this epic, but S3 made it reachable here by matching adoption across spellings. It now runs only when the mailbox GENUINELY changed, and lives in a named helper rather than inline, so the rule has one home. The three lookups that resolve a typed address -- password login, password reset, verify resend -- go through find_user_by_email: exact spelling first, then the unique canonical candidate, and None when several accounts share a mailbox rather than guessing which one a password should open. Reset and resend previously answered a different spelling with silence (they reply uniformly by design), which was a dead end with no way out. Their mail now goes to the address on the account, never the one typed into the form. |