5703c4d7136751eb85a8358b7a6a680478e7654c
461
Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
 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
|
9f28f46839 |
fix(e12): unbreak the gate, and stop printing a command that can strand the restore
Round five. 47 -> 34 -> 15 -> 7 -> 6 raw findings, and this round's sharpest was the loop's own blind spot: **the branch did not pass the gate.** `prettier --check` is one of its eight lanes and I had been running the other seven, so an unformatted `onError` from round one sat on the branch for four rounds — `siftlode publish` would have refused it, with no --skip flag to get past. **The restart hint could still strand the restored data.** Round four said the scripts no longer print a command they cannot resolve; they did. The COMPOSE_FILE fallback only fires when `docker-compose.yml` is ABSENT, so in a source checkout it never fires and the hint expanded to a bare, pasteable `docker compose up -d` — which reads the other file, whose volumes are `pgdata`/`downloads` rather than `siftlode_pgdata`/`siftlode_downloads`, and recreates the database on an empty one. When the file is unresolved the scripts now print `-f <the compose file you started with>`: not runnable, which is the point. **A comment naming the wrong owner.** `formats.py` still said a zero budget is "a refusal for enqueue to make" — the premise round one disproved, three files away from the two docstrings that now say the opposite. Anyone acting on it would delete the worker's refusal and re-open the original inversion, so it now says where the refusal actually lives. Also: the reset-flag rule was rewritten in three consecutive rounds and pinned by nothing — two tests now cover a fragment losing its token, both by a pasted verify link and by navigating away entirely; `backup.ps1` deletes a partially-copied dump before throwing, the guard round four gave the docs but not the script it mirrors; and one comment that read as if the asset were untouched by a line four below it. Gate green, all eight lanes: ruff, ruff format, 3x tsc, eslint, prettier, knip, vitest 687, pytest 950. |
||
|
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. |
||
|
peter
|
876cece881 |
fix(auth): match identity on a canonical email, so one Gmail mailbox is one account (E9.5 S3)
Gmail delivers x.y@, xy@ and xy+tag@ to one inbox; the invite gate only lowercased. On 2026-09-03 that turned an approved user's Google sign-in into a silent SECOND access request -- OAuth returns the account's stored primary, which need not be the spelling they typed -- and left prod with two user rows for one mailbox (ids 13, 14). The +tag case fails every time: a Google account cannot contain +, so OAuth can only ever answer with the base address. Mail still goes to the address AS TYPED; only identity matching uses the canonical form. utils.canonical_email is the single definition, and a model validator derives users.email_canonical / invites.email_canonical on every write, so no call site can forget it. Scope is narrow on purpose: dots are significant on most providers and the Gmail rule does not hold for Workspace custom domains. Swept every sibling of the same principle, not just the reported one: is_allowed, upsert_pending_invite, the Google-callback adoption, _register_account (so no new duplicate pair can be created), _activate_user_by_email, admin _accounts_for (an invite backing a live user under another spelling used to look deletable) and add_invite, plus purge_user, which now erases every canonically-equal invite instead of leaving one behind that still grants access. Migration 0079 backfills both columns and indexes them NON-uniquely: prod already carries canonical duplicates (approved invite pairs since 2026-06-16), and merging accounts is a judgement call about people's data, not something a migration does on its own. A test pins the backfill SQL against the Python helper. |
||
|
peter
|
c134b514c1 |
fix(auth): confirm an emailed verification link wherever it is opened (E9.5 S2)
The token was only ever consumed inside Welcome.tsx, which App renders ONLY when there is no session. Opening the emailed link in a browser that already held a Siftlode session posted nothing, the fragment was then stripped, and the user got no signal at all -- the link simply did nothing. Incognito worked, which is why it survived. Confirmation now happens in App, above that branch: signed out the outcome still drives the auth card's banners, signed in it drives a top-of-app bar. The entry URL is snapshotted once at import time (lib/entryUrl) instead of per component, so no future URL writer can strip a token before whoever needs it has read it -- that ordering race is what broke this in the first place. POST /auth/verify now names the address it confirmed, which matters when the browser holds a session for a different account; the same reasoning as password_reset_required already covers the disclosure. Also here, from the 2026-09-02 review tail: the dead GET /auth/verify goes (both senders emit the fragment form, and the last query-token link expired 24h after the 0.78.7 deploy), a spent #verify=/#reset= fragment is cleared while signed in, and the auth-card banners get role=status. The verifyOkReset copy no longer blames admin approval, which E9.5 S1 made false. |
||
|
peter
|
243869401a |
fix(auth): stop discarding a legitimate password on admin approval (E9.5 S1)
SEC-10 revoked any password set while an account's email was unverified, on both the Google-adoption and the admin-approval path. On the approval path that was redundant: password_login refuses an unverified account before it ever reads is_active, so approval hands a pre-emptor's password nothing it could use. All the revoke achieved there was destroying the password a legitimate registrant had just chosen -- and since the admin normally decides before the user reads their mail, 'your password was cleared for security' was the DEFAULT experience, not an edge case. The guard stays on the Google-adoption path, where the real owner does attest ownership. password_reset_required and the ok_reset banner stay: accounts stripped on prod before this change have no other way back in. Two existing tests pinned the old rule, so they are rewritten rather than deleted, and the SEC-10 recovery pack now builds the revoked state through the surviving producer. |
||
|
peter
|
7019e3afdf |
fix(email): give every mail an HTML alternative so its link is clickable
The bodies are text/plain only, so whether the URL in a verification or password-reset mail is a link at all was the client's decision. Gmail, Outlook and Apple Mail linkify plain text; lighter web clients render the body verbatim and leave the link as dead text — in a message whose entire purpose is that one link. _send now attaches an HTML part generated from the SAME plain body, rather than per-template HTML that would have to be kept in sync with it: one converter, all eleven templates, and text/plain stays the primary part and the source of truth. Escaping runs before linkification — bodies interpolate untrusted values (a requester's address, the operator contact) — so no caller can inject markup, and a query string's `&` is already `&` when it reaches the href, which is what an HTML attribute wants. Sentence-final punctuation is trimmed out of the href while url-safe token characters (`-`, `_`) are left alone. Inline styles only, no images, no external CSS, no tracking. |
||
|
peter
|
3ac3d382e7 |
fix(auth): stop the address-bar mirror eating emailed auth tokens, and unlock revoked accounts
Two independent bugs made an email+password registration unrecoverable in prod. The verification link did nothing. urlWithParams/clearParams rebuilt the URL as pathname+query and dropped the fragment — but the one-time auth tokens ride the fragment (#verify=/#reset=, SB3), and the providers that mirror nav params (useUrlMirror in FeedFilters/Playlists/Plex/Channels) sit ABOVE the lazily-loaded <App> in main.tsx. Their mount effect replaceState'd the token away before the auth card's chunk had loaded, so the card read an empty hash: the link opened a bare sign-in page, no request, no banner. Both helpers now carry the fragment over. The account was then locked out for good. SEC-10 nulls password_hash when an unverified account is attested (here: admin approval landing before the user could verify — which the bug above made impossible). That is indistinguishable from a Google-only account, and reset eligibility keyed off password_hash alone, so no reset link was ever issued: no password to sign in with, and no way to get one. users.password_revoked_at records the revocation, reset eligibility honours it, and setting a password again (reset-confirm or set-password) clears it. An account that never had a password stays ineligible on purpose — its owner adds one from Settings. /auth/verify now also answers password_reset_required so the card can say why the registered password stopped working, instead of leaving the user at the uniform "Invalid email or password". Telling the holder of a valid verify token leaks nothing: they have already proved they own the mailbox. The 0078 migration backfills the stamp for rows carrying the exact lockout signature (local, non-demo, no password, unverified), so existing victims recover unaided. |
||
|
peter
|
8d7aaad9a8 |
fix: warn before the delete, not after — plus round-2 copy and a11y corrections
Round 2 on the feature diff (15 → 12): - still_allowed_by_env now rides the invite LIST as well as the delete response, so the confirmation warns while the admin can still change their mind. It mattered most for a DENIED row on an env-allow-listed address: deleting it silently converted a refusal into a grant, and the only warning arrived after the irreversible click. - Both inbox bodies claimed to name "the most recent" dropped message. The throttle records the FIRST of each hour and never updates it, so they now say "one of them", and the not-configured body finally names where to fix it. - settings.invites.emailOff told the admin they "get no notice when someone signs up" — rendered directly above the queue that shows exactly that. Only the email is lost, and it now says so. - The disabled delete buttons put their reason out of reach: `disabled` drops a control from the tab order and suppresses its title. Switched to aria-disabled + preventDefault, the pattern Playlists.tsx already established for the same reason. - deleteInvite is quiet, so the 409 the UI handles with a translated toast no longer also raises the server's English detail as a modal. - After a completed password reset, Back landed on the reset form again with a consumed token — the history change made that state reachable. The token is cleared on success so the reset view stops being the card's root. - The hu deleteHasAccount named a tab that renders under a different label. Tests: a dom test for the Access-requests panel covering both has_account variants and all three delete outcomes, /api/me flag cases, an ADMIN_EMAILS case, and the env-allowlist test now patches the raw setting so the parsing stays under test. |
||
|
peter
|
85043013f0 |
fix: address the feature review — accurate copy, reachable controls, real coverage
Round 1 on the feature diff found 15; the substantive ones: - The delete control VANISHED on any invite whose address had registered, which is the ordinary sign-up shape (register writes the User and the pending invite together). The guard stays broad — that row is the account's approval gate, and orphaning it strands someone who can then neither sign in nor be approved — but the control is now a disabled button carrying the reason, on both lists, so an admin never sees a row silently missing a button its neighbours have. - Deleting an entry is not always a revoke: is_allowed falls back to the env ALLOWED_EMAILS / ADMIN_EMAILS lists once no row exists, and an ADMIN_EMAILS address is force-promoted on its next Google sign-in. The route now reports still_allowed_by_env and the UI says so instead of claiming access was removed. - "Deny keeps the row so they can't immediately re-ask" was simply false — upsert_pending_invite reopens a denied row. Corrected everywhere it was stated. - The Configuration warning reads me.email_configured, which save() did not invalidate, so it kept warning about fields the admin had just filled in. - The read-only variant was an aria-hidden span explained only by a Tooltip that the hints toggle can switch off; now a disabled button with title + aria-label. - The delete error path threw away the 409's reason (the stale-list case) and left the stale row on screen. - The throttle window was spent even when no notice was written, silencing the next hour on the path whose job is to stop silent drops. - email_enabled() opened a second session and decrypted the SMTP password on every /api/me poll; it now takes the caller's session. - welcomeMode.dom.test.ts never rendered the card, so reverting the history wiring to useState left it green. A render test now drives the real links and fails on that revert (verified). Plus /api/me flag tests, the registration-shaped pending case, the env-allowlist case, and a release note that matches what the code does. |
||
|
peter
|
44f626f732 |
feat(admin): delete a whitelist entry, and stop hiding a broken mail setup
Three operator gaps, all found the same way — a person could not sign up and the
app showed nobody anything.
- DELETE /api/admin/invites/{id} with a delete control on both the pending
requests and the previously-decided list. Deny keeps the row so the person
cannot immediately re-ask; delete forgets the address. Refused with 409 while
a User row exists for it, because is_allowed reads that row and removing it
would revoke a live user's access with no other sign — the list ships
has_account so the UI withholds the button instead of offering one that fails.
- Email is fail-soft by design, which had come to mean invisible. _send() now
records an email_undelivered notification for every admin when a message is
skipped or rejected, naming the message that was lost, throttled to one an
hour. /api/me carries email_configured, driving a standing warning on
Configuration → Email / SMTP and above the access-request list.
- The sign-in card's Create account / Forgot password / Didn't get the email
views are history entries (useHistorySubview), so Back returns to the previous
view instead of only moving the address bar.
Also: the per-test rate-limiter reset sweeps every loaded app.* module rather
than a hand-kept list of three — this change added a limiter outside it.
|
||
|
peter
|
cb1f4f00b0 |
docs(auth): correct the last round of over-claiming comments and notes
Round 5's correctness/security pass came back clean; what was left was text that said more than the code does. - The register-path notify comment (and its test) claimed the admin "is still told". The call fires, but on a no-SMTP instance _send drops it — which is the open half of the problem, not something this branch fixed. Both now say so. - The release notes tell admins what to actually do: an account whose address was whitelisted in advance has no pending request to approve, so the whitelist box is what activates it. That instruction only existed in the roadmap and in chores, which the dialog does not render. - "That one line is the whole production change" now accounts for the test-only RateLimiter.reset() that also ships. - The rate-limiter test claimed to pin that blocked attempts are not counted, on an assertion that held either way. Replaced with one that drives the clock across the window boundary and fails if hammering extends it. |
||
|
peter
|
f6e1d1e69f |
fix(auth): keep the admin notice unconditional on the register path
Round 4 showed the `inv is not None` gate added in
|
||
|
peter
|
b64c3f514d |
fix(auth): drop the invite reopen — keep the fix to the lost account alone
Round 3 of the review killed the reopen from
|
||
|
peter
|
b5a1002412 |
fix(auth): reopen a pre-dated approval instead of auto-granting it
Replaces the activation added in
|
||
|
peter
|
ea931dca30 |
fix(auth): honour an approval granted before sign-up instead of parking the account
Follow-up to the commit fix, from the deep-review. Persisting the account was only half the story: when the admin had approved the address BEFORE the person registered, the account landed is_active=False against an invite that is no longer pending — absent from the Access-requests queue, and approve_invite 409s on an already-decided invite. The sign-up worked and still could not sign in. Derive activation from is_allowed() at the moment ownership is attested: - verify_email_confirm activates a pre-approved account when the emailed link is confirmed (only the mailbox owner can follow it), so SEC-10 stays intact; - _register_account activates only when there is no SMTP to verify with, where the admin whitelist is by design the sole gate — the same one add_invite already activates on. Tests: the pre-approved no-SMTP case and the verify-activates case are real regression tests (both fail without this change); guards cover the unknown-email and denied->pending branches, verification of an unapproved account, and the first end-to-end coverage of the /auth/register route and its allow_registration gate. Release notes rewritten — the previous entry miscounted its regression tests and advertised an email fix this release has no code for. |
||
|
peter
|
d954e71a77 |
fix(auth): commit the account on registration so a pre-approved sign-up isn't lost
_register_account() flushed the new User but relied on someone else to commit: upsert_pending_invite() commits only when it actually writes an invite row, and _issue_token() runs only when SMTP is enabled. For an email an admin had ALREADY approved, on an instance where email is unavailable, neither fired — the session closed and the account was rolled back. The would-be user got the uniform "ok" and no account, could not sign in, and re-registering was a silent no-op. Commit explicitly after the invite upsert (a no-op when upsert already committed the same transaction). Three DB-lane regression tests cover the pre-approved no-SMTP path (fails without the fix), the unknown-email path that always worked, and the pre-approved path with SMTP on. |
||
|
peter
|
d7e9240e54 |
merge main (0.78.2) into playback-class before release
# Conflicts: # backend/app/plex/stream.py |
||
|
peter
|
375f6e58be | merge main (0.78.1) into tonemap before release | ||
|
peter
|
1102ad6a28 |
fix(downloads): make the probe verdict authoritative over yt-dlp info by probe-ran, not per-field
Deep-review round 1 on bug/download-mediakind-probe found a real HIGH the prior review missed: - resolve_media_facts used a per-field `probe or info` fallback, so a probe that RAN and deliberately found no video track (cover-art-only audio: attached_pic skipped; a still image: format-guarded) had its None video overridden by yt-dlp's stale vcodec. An audio-mode download from a progressive source (info carries the source's video codec, embed-thumbnail on) was then classified VIDEO -> a Play button + a broken in-app <video> with no video track. Only the download path regressed (upload calls probe_media directly). Fix: gate precedence on whether ffprobe RAN, not per-field. If the probe read the file, its codec verdict wins outright (a None means 'no such track'); only a full ffprobe failure falls back to info. Dimensions still gap-fill from info. Refactored the parsing into _facts_from_probe so resolve_media_facts can see the raw probe-ran signal; probe_media keeps its 5-tuple (upload + TestProbeClassifier unchanged). - tests: the cover-art-audio and still-image resurrection cases, a differing populated-info precedence case (the prior-flagged gap), and the ffprobe-failed info fallback — now stubbing run_ffprobe_json so the real parsing + precedence run together. Backend gate green (ruff + full pytest). |
||
|
peter
|
c1a3b7b935 |
fix(plex): gate the part-key re-probe to single-version items; drop dead i18n key; cover cross-facet narrowing
Deep-review round 2 on the round-1 fixes (all low): - part_key churn (state): round 1's part-key reset re-probed EVERY sync for a multi-version item (an original + a Plex 'Optimized' copy), because _media_facts reads media[0]/part[0] whose order Plex doesn't guarantee stable, oscillating transcode_hw between the two versions. Gate the part-key signal on a single-version item (len(Media)<=1); codec-change reset still applies to all. A multi-version same-codec re-encode not self-healing is an acceptable rare edge; a per-sync re-probe loop is not. - dead i18n key: this diff's playableClass() collapses to transcode_hw/sw, so the bare plex.playable.transcode key it orphaned is now unused (knip can't see dynamic keys). Dropped from both locales, parity kept. - test: cross-facet narrowing by an active playable class (a co-facet's year range collapses to the selected subset) — the flip side of self-exclusion, previously unpinned. The aria-label+title double-announcement on the card dot is a deliberate accepted trade-off (reliable SR name + mouse tooltip). Backend + FE gates green. |
||
|
peter
|
bf98b48517 |
fix(plex): re-probe on file replace, HU parity, public class const, playback-class test gaps
Deep-review round 1 on feature/plex-playback-class-filter: - Sticky watermark (medium): _apply_item reset the bit-depth probe watermark only on a codec-string change, so a same-codec re-encode (8-bit->10-bit HEVC) kept a stale video_bit_depth/transcode_hw forever (the backfill only re-probes media_probed_at IS NULL rows). Also reset when the Plex Part key changes (the file was replaced), not just the codec. - i18n parity (medium): the HU playback-class labels were mixed-language (only 'direct' translated). Translated remux/transcode_hw/transcode_sw and the tooltips consistently to Hungarian (Kozvetlen streameles / Atkodolas). - Layering (low): _PLAYBACK_CLASSES was module-private yet imported across into routes.plex.browse; renamed to public PLAYBACK_CLASSES (it is the shared source of truth for facet order + zero-fill). - a11y (low): the browse-card playback-class dot conveyed meaning only via a mouse-hover title; added role=img + aria-label so SR/touch users get the class. - tests: the NULL/unprobed transcode_hw -> SW fold (filter + facet), facet self-exclusion under an active playable filter, the migration codec-set == HW_DECODABLE_CODECS drift guard, the probe-batch bound + oldest-first order, the transient probe-failure retry path, and the same-codec re-encode re-probe. Backend gate green (ruff + full pytest); FE gate green (tsc/lint/knip/prettier/ vitest). NOTE: the facet-vs-runtime disagreement for 10-bit HEVC (C1) is the known merge-order constraint — this branch's runtime is codec-only; the facet becomes truthful once BUG1/tonemap's bit-depth-aware routing lands with it. Left as a sequencing constraint, not code-changed here. |
||
|
peter
|
b563155a37 |
docs(plex): correct the cull-yield fallback comment to name the real route 404s
Deep-review round 3: the only surviving finding — a comment-drift nit. The fallback's cull-yield comment named the route's 'Not available yet' path, but a culled-key segment request actually returns 'No active playback session' (the substantive invariant — never a None that 404s as file-not-found — is unchanged). Reworded to match the route. Concurrency + correctness came back clean. |
||
|
peter
|
915319fa35 |
fix(plex): make the HW->SW fallback supersede-check atomic; honest cull yield
Deep-review round 2 on the round-1 fix: - Race (medium): the round-1 identity guard did the supersede check and the respawn in TWO separate _lock sections. A newer same-key session installed in the gap was still torn down by _spawn's unconditional pop+kill (the round-1 comment even had the direction backwards). Fold the check into _spawn via a new replace_only= param: the identity test and the pop+replace now share ONE lock, so the respawn is a true no-op when superseded/culled. - Honest states (low): the superseded branch returned current_session(key), which is None when the key was cap-culled during our out-of-lock wait; the route turns None into a dishonest 'Media file not found on disk' 404 for a file that exists. Yield 'current_session(key) or s' so a culled session degrades to the route's retryable 'not available yet' path instead. - Hygiene: _want_hw docstring corrected (it is a test-only shim now; production decides via _select_encode) and the two stale _want_hw references refreshed (stream.py header, config.py plex_transcode_hw). - tests: the cap-cull no-respawn/no-None sub-case; the _opencl_present except OSError branch (glob raising, not just empty); tightened a tautological -vf assertion. Backend gate green (ruff + full pytest). |
||
|
peter
|
a2a8f5681d |
fix(plex): don't tonemap 10-bit SDR; identity-guard the HW→SW fallback
Deep-review round 1 on feature/plex-hdr-hw-tonemap: - F1 (correctness): _select_encode gated tonemap on bit-depth alone, so a 10-bit *SDR* (bt709) source got the HDR hable tone curve applied, visibly altering an SDR picture where only a 10->8-bit downconvert was needed. Split the ffprobe gate into a 3-way classifier (_probe_video_kind: hwsafe / hdr / sdr10); only true-HDR (PQ/HLG) takes the OpenCL tonemap graph, 10-bit SDR drops to software libx264 (an exact downconvert). Applies to both auto and on. - F2 (race): the runtime HW->SW fallback popped+killed _sessions[key] with no identity check. Because wait_for blocks OUTSIDE _lock, a concurrent same-key _spawn (rapid seek-restart) or _enforce_cap cull could kill this session; the fallback then respawned software over the NEWER session and breached the transcode cap. Guard the respawn: only fall back if _sessions[key] is still this session, else yield the currently-registered one. - tests: 3-way classifier (incl. HLG + empty-probe), real _opencl_present glob, real wait_for proc fast-fail branch, tonemap-variant fallback, the F1 regressions (auto + on), the supersede/no-respawn race, and the on/opencl matrix cell. Backend gate green (ruff + full pytest). |
||
|
peter
|
4827aebb7f |
feat(plex): HW HDR→SDR tonemap for 10-bit/HDR transcode (OpenCL on the iGPU)
Upgrade mode-3's 10-bit/HDR branch from "route to software libx264" (BUG 1's behaviour) to a full-GPU pipeline: VAAPI decode → OpenCL tonemap (hable, HDR10/HLG → bt709 SDR nv12) → h264_vaapi. Gen 9.5 (UHD 630) can't tonemap in the VAAPI VPP (tonemap_vaapi fails), so OpenCL is the only HW route — proven on the prod iGPU at ~4.2x realtime, ~1 CPU core. - stream.py: `_select_encode` returns (hw, tonemap); a 10-bit/HDR HW-decodable source picks the tonemap graph when the OpenCL runtime is present, else drops to software. `_opencl_present` gates on a registered ICD. `_venc_opts` / `_hwaccel_in_opts` gain the tonemap variant (named vaapi+opencl devices, `-filter_hw_device ocl`, hwmap→tonemap_opencl→hwmap reverse). `_want_hw` kept as the hw-half wrapper. The runtime HW→SW fallback still backstops a session that emits no segment. - Dockerfile: add the Intel NEO OpenCL runtime (intel-opencl-icd). It was removed from Debian trixie, so it's pulled from bookworm (22.43 still supports Gen 9.5; the .deb runs on the trixie base — verified live). Harmless without /dev/dri. - route + FE: thread `tonemap` through the session response; the stats overlay mode row reads "transcode · GPU · HDR→SDR" for the tonemap path. - tests: tonemap command branch, the (hw, tonemap) selection matrix (auto/on × 8-bit/10-bit × opencl), start_session tonemap wiring, and the fmtMode tag. |
||
|
peter
|
840526b1b8 |
feat(plex): filter the feed by playback class (Direct Play / Direct Stream / Transcode HW/SW)
Adds a movie-scoped "Playback" filter to the Plex feed so a user can isolate items by how they play — the groundwork for collecting HW-transcodable (or no-transcode) versions. Four classes derived from the stored `playable` + a new `transcode_hw` facet: - Direct Play (direct), Direct Stream (remux) - Transcode · HW — playable=transcode AND the iGPU can hardware-transcode it - Transcode · SW — playable=transcode, software libx264 only The HW/SW split is precise, not codec-only: a 10-bit/HDR HEVC is HW-decodable yet breaks the VAAPI nv12 pipeline (the same case plex/stream special-cases), so it must count as SW. Backend: - New PlexItem columns (migration 0077): transcode_hw, video_bit_depth, media_probed_at. Migration backfills transcode_hw codec-only so the filter works immediately. - `plex.sync.transcode_hw_capable` is the single classifier; the HW-decodable codec set moves to sync as `HW_DECODABLE_CODECS` and plex.stream imports it, so the catalog facet and the runtime encoder choice can't disagree by codec. - Bit-depth backfill (`_probe_media_batch`, wired into sync): probes only HW-decodable transcode items' video Stream for bit-depth + HDR and refines a 10-bit/HDR HEVC to software. Bounded per run, watermarked by media_probed_at. A codec change resets the watermark (stale probe). - Filter + self-excluded facet on a shared `playback_class_expr` CASE (movie-leg only — a show is an aggregate with no single class). transcode_hw threaded onto the feed/detail/card JSON. Frontend: - Multi-select "Playback" Combobox in the sidebar (movie scope, facet-empty drop-out), active chips, 4-way corner-dot badge (green/blue/amber/red) + tooltip, and an info-page playback chip with a codec · bit-depth subtitle. New `bg-info` (blue) token, i18n HU/EN parity. - Fix a regression where the card's playback dot lost its hover tooltip: the full-card reveal overlay swallowed the cursor; the dot now sits above it (z-20). Gate: backend full lane + frontend typecheck/lint/knip/prettier/vitest all green. Browser-UAT'd on localdev (filter narrows to the SW set, facet counts, badge colour + tooltip, info chip). |
||
|
peter
|
10d69cb316 |
fix(plex): route 10-bit/HDR HEVC to software + runtime VAAPI→libx264 fallback
A 10-bit/HDR HEVC source (e.g. an HDR10 x265 file) died at scale_vaapi=format=nv12 on the UHD 630 (Gen 9.5 VPP: "VAProfile not supported"); ffmpeg produced 0 segments and the item never played, with no fallback — a retry just re-picked the failing HW path. An 8-bit HEVC and the same file on libx264 both work. Two defenses: - `_want_hw` auto mode now also gates on an 8-bit-SDR source via a new `_probe_video_hwsafe` ffprobe (bit depth from pix_fmt/bits_per_raw_sample, HDR from color_transfer) — a 10-bit/HDR source takes the software libx264 path, which downconverts to yuv420p cleanly. The Plex mirror stores no bit-depth signal, so this is probed at session start. "on" still trusts the admin and skips the gate. - `start_session` adds a runtime HW→SW fallback: a HW session that emits no first segment respawns on libx264 and reports hw=False (the route/stats overlay honestly show CPU). `wait_for` gained an optional `proc` so a dead ffmpeg fails fast instead of burning the 25s budget before the fallback fires. Tests: TestProbeVideoHwsafe (parsing + probe-failure-assumes-safe), the 10-bit auto gate + "on" skip in TestWantHw, and the no-segment HW→SW respawn in TestTranscodeStartSession. Backend lane green. |
||
|
peter
|
a86131e062 |
fix(downloads): classify media_kind from a file probe, not yt-dlp info
A downloaded video was silently demoted to a download-only "file" (no in-app play/edit) whenever yt-dlp's info dict omitted vcodec/acodec — which it does for Facebook and other sources. Finalize derived media_kind from those nulls (`derive(None, None)` -> file), so a genuine H.264/AAC MP4 became non-playable in the library. Fix: probe the finished file on disk (the ground truth) at finalize and derive from that, falling back to the info dict only for a field the probe couldn't read. This is the same ffprobe the upload ingest already used, now extracted to a shared downloads/probe.py so the download worker and the upload path share one classifier (and can't drift). Also backfills the stored vcodec/acodec/width/height from the real file. Tests: TestResolveMediaFacts pins probe-wins-over-info (incl. the Facebook no-codec case and yt-dlp's literal "none"); upload's probe tests repoint to the shared module. |
||
|
peter
|
00539a4d97 |
fix(plex): deep-review follow-ups on E6 transcode + player polish
Eight verified findings from the sprint deep-review: - Transcode cap: _enforce_cap now counts only transcode sessions, so a live remux (stream-copy) stream can no longer be LRU-evicted mid-play — matching the config comment + the HU/EN Config hint that promise remux is uncapped. - Player Back: the browser/mouse Back interceptor (BackClose) now also mounts for the stats overlay and up-next card, so Back closes them first instead of ejecting the whole player (keyboard/Esc already did). - Honest cold-start: loadSession clears sessionIsTranscode up front, so a transcode->direct/remux switch no longer flashes a stale "Preparing stream..." during the session request. - Stats overlay a11y: the close (X) buttons in the stats, help and info overlays now read as "Close"/"Bezaras" (new plex.player.close key), not "Back"; dropped aria-live from the per-second stat grid so a screen reader is not flooded with telemetry. Tests: route-level transcode routing (transcode item -> HLS re-encode session, remux stays copy, direct opens none); start_session threading the preset/crf/max_height knobs + hw/preexec into the ffmpeg argv; UpNextCard countdown auto-advance, Play-now, Cancel and the S?-E? heading fallback. Also brings UpNextCard.tsx / plexStats.ts / plexStats.test.ts to prettier. |
||
|
peter
|
ce96032edb |
feat(plex): transcode resource discipline + admin knobs (E6 S1c)
The software-fallback re-encode now runs deprioritized and admin-configurable. The SW transcode ffmpeg child is niced (preexec_fn → os.nice, applied per-child because the Popen lives in the long-lived API process, which must NOT itself be niced — unlike the download worker's whole-process _deprioritize) so a CPU-bound re-encode can't starve request handling or playback; the VAAPI path needs none of it. New sysconfig knobs (plex group, rendered on the admin Configuration page, i18n HU/EN): plex_transcode_hw (auto/on/off, layered onto _want_hw), plex_transcode_ preset, plex_transcode_crf, plex_transcode_max_height (software-only downscale-to cap; the GPU path encodes at source resolution). plex_max_transcodes stays the concurrency cap (comment corrected — the fallback is shipped, not "later"). pytest extends the command-builder + _want_hw coverage (preset/crf/max_height plumbing, the on/off/auto policy). Browser-UAT'd: max_height=240 drops the live resolution to 330x240; the SW ffmpeg child runs at nice 10 (ppid=api process). |
||
|
peter
|
0658f9550b |
feat(plex): transcode playback mode — VAAPI/libx264 re-encode (E6 S1b)
Non-h264 items (playable="transcode": HEVC/VP9/AV1/MPEG-2/XviD…) now play by re-encoding the video to H.264 inside the existing HLS session, instead of the old HTTP 501. VAAPI hardware encode (h264_vaapi, full GPU decode→scale→encode pipeline) where the render node is present and the source is HW-decodable; libx264 software fallback otherwise (yuv420p 8-bit, -threads reserve-one-core). The re-encode mints a keyframe at every segment boundary (-force_key_frames) so the HLS muxer still cuts independent segments and seeking works; everything else — -copyts, the HLS tail, multi-audio renditions, the AAC audio path — is the same machinery as remux, so hls.js/subtitles/seeking are unchanged. The session route returns transcode/hw so the player can surface the active mode. pytest pins both command branches (software + VAAPI shape) and the HW-selection gate (render node present AND source HW-decodable). Browser-UAT'd on the XviD fixture (Winning London): 501→plays, real 528×384 H.264 frames, 0 dropped, seek lands clean; stats overlay Mode row reads "transcode". |
||
|
peter
|
614c5528ad |
fix(downloads): harden upload serving + ingest against stored-XSS and leaks (E7 review)
Deep-review remediation for the E7 upload path, all findings adversarially verified: - Serve every library file with a Content-Type chosen from the ffprobe-verified media_kind + a strict container allow-list (storage.served_media_type), never guessed from the user-controlled file extension. Closes a stored-XSS vector where a media/HTML polyglot uploaded as "evil.html" was served inline as text/html on the login-free /watch surface. Applied to both the authenticated /file and the public watch_file endpoints. - Classify uploaded still images (jpg/png/gif/webp/…) as media_kind='file' so they are force-downloaded and don't get a broken in-app video player. - Roll back the on-disk side of an upload when the ingest transaction fails after place_file (unlink the placed file + its deterministic poster path), so a failed commit can't orphan bytes in the GC-skipped .uploads tree. - Floor download_upload_max_bytes at min=1 so a stored 0 can't silently disable the per-file cap. - Dedupe the ffprobe-JSON invocation into edit.run_ffprobe_json (shared by probe_codecs and upload._probe) and the upload extension into storage._clean_upload_ext; drop the now-unused parse_probe_streams. Tests: public + authenticated force-download and the polyglot-not-served-as-HTML guard, the real _probe classifier (image/cover-art/dims/duration), the ingest rollback/cleanup path, the max_jobs branch, the cap-floor invariant, and run_ffprobe_json error handling. |
||
|
peter
|
fe0b34cd00 |
feat(downloads): local file upload ingest — media library backend (E7 S5a)
Add the non-yt-dlp ingest path so a user-uploaded file joins the same download
library and inherits its function set. Any file type is accepted: media
(video/audio) gets the full set (metadata, share, play/edit), a generic file
(pdf/zip/docx/…) is download-only.
- MediaAsset.media_kind ("video"|"audio"|"file") — the stored discriminator that
gates library actions. Set at every finalize (download probe, editor carries the
source's kind, upload probe); consolidates the ad-hoc audio-detect that lived only
in links.py into one invariant (downloads.mediakind.derive). Migration 0076 adds
the column + backfills existing rows.
- downloads/upload.py: ingest_upload probes the file (ffprobe → codecs/dims/duration),
places it in a per-user .uploads tree, and creates an already-ready asset + done job
(the same cache-hit shortcut enqueue uses). Uploads never expire (irreplaceable).
- POST /api/downloads/upload: streaming multipart with the per-file cap
(download_upload_max_bytes, default 500 MiB) enforced on actual bytes read; quota
charges the known size up front (quota.check_upload).
- Security: a generic file is always served force-download (attachment +
application/octet-stream) on both the in-app and public endpoints, so uploaded
HTML/SVG can't execute inline. Edit + redownload reject non-media / upload sources.
- python-multipart added (first multipart endpoint in the app).
|