The client's in-flight flag covers the menus of one page only. A push reads YouTube, then writes
both sides for seconds and commits step by step, so a row lock ends at its first commit: two tabs,
or a push and the scheduled mirror refresh, could interleave into a doubled YouTube item or a
baseline recorded over the other's writes. `sync_claim` takes a session-level advisory lock on a
connection of its own (it survives those commits; a crashed worker's lock dies with its
connection, so no claim column is left stuck). Push and revert answer 409 `sync_busy`; the
scheduled refresh skips a list being synced and picks it up next run.
Two adds read the same max(position) and a reorder racing an add collided with
it. Every add/insert/reorder takes the parent row FOR UPDATE; the YouTube list
breaks position ties by id and a Plex reorder renumbers rows it was not sent.
channels.list simply omits a terminated channel, so nothing was stamped and the
channel page spent a quota unit on it every time. The miss is stamped like a
fetch; the published_at sentinel that kept it stale is gone.
One invalid byte sent a whole UTF-8 sidecar to a legacy code page, an untagged
file fell to Windows-1252 (ő/ű became õ/û) and Serbian was read as Cyrillic.
The cache key moves to sub3_ so a track the old rules mis-decoded is read again.
Channel metadata (banner, avatar, description, counts) was fetched only the first time a
channel was seen: the subscription import asked only for channels with no details, and the
channel page only for ones missing a join date. 394 of 397 subscribed channels had July
metadata. Both now also refresh details older than DETAILS_MAX_AGE (7 days), through one
shared staleness rule; channels.list costs 1 unit per 50 channels.
- Channels: the row's full-history, hide and unsubscribe buttons become its "…" at the row end
(useChannelActions: open, pin, YouTube, copy name / link, full history or the admin reset,
hide with an undo toast, unsubscribe behind its danger confirm); the card layout docks the "…"
beside the name (ChannelLayoutGrid moreKey). The admin reset's confirm now names the reset.
- Plex playlist page: a title, a show group and a season each open the page's one row menu (a
title's is its usePlexCardActions list plus the removal); no X at rest; removals take an undo
toast whose Undo puts every row back at its own index in one request (bulk add `positions`);
j/k walk the visible rows, Delete removes the walked one; rename and delete in the header's "…".
- notify: a toast with an action lives at least 8 s (the owner's call), whatever the toast setting.
- Playlist rows: the X column became the row's "…" (play, pin, copy title/link, move, order,
remove). Remove runs at once, with no confirm; its toast's Undo puts the row back at its index.
The Delete key removes a keyboard-walked row, and focus moves on to the next.
- POST /api/playlists/{id}/items takes an optional `position`, so an undo is one request that
renumbers the list around the row.
- A new `listDelete` key scope. Only a list with `onDelete` registers it, so the help sheet does not
offer Delete where nothing handles it.
- A playlist has one list, `usePlaylistActions.items`: rename, pin, YouTube open/push/revert,
delete. The rail row's "…" and the page header's "…" both open it. Push and revert moved into
the hook; the header keeps Play all.
- Phone-width playlist table: "#" and Channel are hidden below sm (`Column.phoneHidden`). Their
fixed widths had left the title no width.
A Hungarian .srt beside the video (Windows-1250) was decoded as UTF-8 with
errors=replace: one episode served 1391 U+FFFD and no accented letter.
decode_subtitle now lets a BOM decide, keeps valid UTF-8, and otherwise
reads the legacy code page of the stream's language (cp1250 for Central
European, cp1251 for Cyrillic, else cp1252) — on the ffmpeg path too.
The cache has no eviction, so its key is versioned (sub2_): a track cached
broken is read again once instead of being served for good.
Deep-review round 3, targeted at the surfaces round 2's fix commit changed.
Five of the eight findings are the same shape: the guards written to stop
half-done sweeps were themselves half-done.
test_upload.py's display-field guard walked only `ast.Assign`, so the one
writer with an unbounded REQUEST HEADER behind it — `spool_upload`'s
`display_name=display_text(...)` keyword — was invisible to the guard meant
to protect it. It also stopped at `app/routes/`, missing the four writers in
`downloads/` that actually construct the row, and asserted `not offenders`,
which enforces nothing once a refactor moves the writers out of its view.
It now walks assignments AND keywords across `app/`, and holds exact set
equality against a registry of all twelve write sites with each one's value.
test_body_limit.py's release guard used `any(... ast.walk(fn))`, which
accepts a `release_read_connection` that runs AFTER the await it exists to
precede — satisfying the letter of the rule while pinning a connection for
exactly the transfer it was supposed to free. Position is checked now, and a
late release is its own, louder failure. The registry keyed sites by file
basename while scanning recursively, and `admin.py`, `discover.py` and
`playlists.py` each exist both at the top level and under `plex/` — the same
entry-collapse the registry was widened to close. Keyed by relative path now.
And all three guards recognised a body read only by the `capped_` naming
convention, so a route spelling `await request.body()` inline declared no
ceiling, released no connection, and was invisible to every one of them.
`release_read_connection`'s precondition claimed the caller "must have
NOTHING PENDING, and that is checked rather than described" while checking
only UNFLUSHED ORM work. Measured: after `add`+`flush` all three collections
are empty and the rollback loses the row, and Core DML never enters them at
all — both of them this codebase's canonical write shapes. It now also asks
the transaction directly via `pg_current_xact_id_if_assigned()`.
`errors.tooLarge` was reworded to "try a smaller file", but that key answers
every route's body cap: the issue composer's over-size JSON draft got advice
about a file it does not have. Back to payload-neutral; the file-specific
sentence keeps its own key.
The upload dialog's new danger border outranked `dragOver`, so a user
replacing an over-cap file got no sign the drop zone was live — and with a
file picked the label shows its name rather than "drop it here", so the
border was the only feedback left.
Four further findings were refuted 3/3 and are not fixed: `aria-invalid` on
the drop zone (measured in Chromium — the AX node does carry it on a native
button), a bidi-override spoof through `display_text` (the served filename
already strips those categories, and the proposed filter would have broken
every ZWJ emoji), and `capped_body` not draining (the screenshot route's edge
ceiling equals its own cap, so that refusal is unreachable, and the log sink
is sent by beacon with nobody reading the answer).
Deep-review round 2, on round 1's own fix commit. It found MORE than round 1
did (28 raw, against 13), and most of it was mine -- a new piece of UI attracts
findings a backend-only diff never does. The pattern in almost all of them is
the same: a principle applied to the site that prompted it and not to its
siblings.
* `display_text` reached three of the FIVE writers of a user-supplied name.
`POST /api/downloads` and `POST /api/downloads/edit` still handed Postgres an
unbounded string, so a 300-character name pasted into the Add-download dialog
or the editor was a bare 500 (main.py re-raises every DataError but the NUL
case). Four of the seven reviewers found it independently. The bound now lives
in `models.py`, the three columns are DECLARED with it, and the guard for the
next writer is parsed rather than remembered: every `display_name` /
`display_uploader` assignment under `app/routes/` must go through the helper.
That guard immediately found two more -- the Google sign-in callback writes
the profile name into the same 255-wide column -- so it is six, not five.
* The connection release reached one of the THREE routes that read their own
body. The screenshot route (10 MiB, from a phone) and the client log sink both
still held a pooled connection idle-in-transaction for the length of the
upload, which is the invariant db.py sizes the pool on. All three now call
`db.release_read_connection`, whose precondition -- nothing pending on the
shared session -- is CHECKED rather than described, because a whole-session
rollback discarding a caller's staged write is a mistake this repo has already
made once. `test_body_limit.py` holds the rule for the next such route.
* The AST drift registry keyed its sites by (module, limit expression), so a
second route copied from the first collapsed onto the same entry and went
uncovered -- the exact hole the registry exists to close. Keyed per call site
now, and it carries the release requirement too.
* The dialog's own refusal was announced once and never again: two over-cap
picks leave an identical node with identical text, and an untouched live
region says nothing. It is keyed per pick now, the drop zone reads as invalid
and points at the sentence, and the reason is reachable from the control
rather than only from a one-shot announcement.
* Two tests rebuilt the blind spot they were written to remove: the dialog's cap
test sat nowhere near the boundary (a `>` loosened to `>=` survived it, which
is the mutation round 1 fixed on the server side), and the header-precedence
test only covered an exact-case duplicate while a differently-cased one would
have been COMBINED onto the wire rather than replaced. Both fixed, and the
merge now dedupes case-insensitively because header names are.
Also: the upload dialog's unstubbed usage query was firing a real request from
jsdom in a neighbouring suite and leaking a delayed "connection lost" toast
across test boundaries; `GET /api/downloads/usage` had no test at all though the
whole client-side check hangs off it; and the comment claiming the edge 413 is
readable now says what was measured -- readable to 200 MiB over loopback, a bare
`onerror` through a proxy -- so the pre-send check is named as the mechanism and
that branch as the backstop it is.
Every test added or changed here was mutation-checked. One mutation SURVIVED and
is recorded rather than papered over: the `tooBig` term inside `submit()` cannot
be reached, because `ui/Button` swallows the activation itself, so it is marked
as belt and braces instead of being described as the guard.
The epic-end deep review found three defects in P1 and four tests that did not
hold what they claimed. They land together because they share the upload route
and its test module; splitting by hunk would have produced commits that do not
individually run green.
* The edge ceiling and the route's own cap are the SAME number at the default
configuration, so the edge always answers first: a declared Content-Length
over the cap is refused before a byte is read, and an undeclared body trips
the middleware on the same chunk the route would have tripped on. The route's
numbered 422 -- and the drain written for it last week -- are reachable only
once an admin LOWERS the knob, which is the one shape the browser UAT tested.
Measured: headroom does not fix this, because the declared-length check fires
first for any body big enough to matter and the drain pushes the count back
over the edge for the rest. So the edge's own refusal is made readable
instead: `errors.tooLarge` localizes `too_large` the way `upload_too_large`
was already localized, and the dialog refuses an over-cap file BEFORE sending
it, off the `upload_max_bytes` that GET /api/downloads/usage now reports.
Measured in Chrome against the API directly: that 413 arrives as `onload`
with a readable body in 43 ms.
* Reading the body behind the auth dependency put the whole transfer inside the
request's database session -- the auth SELECT autobegins a transaction, so one
of the twenty pooled connections sat `idle in transaction`, with nothing to
time it out, for as long as the client cared to keep sending. That is the
invariant db.py sizes the pool on and plex/playback.py records an incident
for. The read transaction now ends before the await. Measured on the dev
server: an 1818 ms upload shows no idle-in-transaction sample while the bytes
move, only afterwards, when the handler actually writes.
* The display name moved into a request header, where nothing bounds its length,
while the column stayed varchar(255) -- so a long enough name was a DataError
500 after the whole file had already transferred. One `display_text` helper
now serves all three writers (the metadata PATCH already truncated, the upload
ingest did not), and the dialog stops at the same number.
* The gate invariant asked `route.dependant.body_params`, which holds only the
route's OWN callable's parameters: a body declared in a SUB-DEPENDENCY -- the
upload route's own idiom -- leaves it empty while FastAPI still reads the body
before auth. It asks `route.body_field` now, which is what FastAPI itself
consults. The AST registry of body-reading call sites had also never followed
`capped_stream_to`, so the one route the whole invariant exists for was the
single site the drift guard could not see.
Four tests that passed over their own mutation are fixed: the cap boundary (a
`>` loosened to `>=` survived the entire suite), the percent-decoding (every
filename the suite sends is ASCII, where quote() is the identity function), the
display-name header (no test at all), and the staging cleanup (deleting the
unlink left the suite green while every refused upload leaked its partial bytes
onto the volume). Every test added or changed here was mutation-checked.
Found in the browser UAT, not by the gate. Moving the cap onto the arriving
bytes meant it fires while the client is still sending, and answering + closing
mid-upload is what XHR reports as a network failure: the upload dialog said
"the server couldn't be reached" while the access log showed a correct 422
carrying the limit. A refusal the client cannot read is an outage as far as the
user is concerned.
`capped_stream_to` now reads the remainder to nowhere before raising, so the
client finishes sending and reads the real answer -- re-measured in the
browser, which now names the limit. Nothing past the cap is written, and the
request is still bounded by bodylimit's ceiling, so the cost is bandwidth an
AUTHENTICATED caller was sending anyway. The anonymous path is untouched:
auth still resolves before a byte is read.
The regression test sits on the helper, not the route, and the reason is worth
keeping: the route-level version was written first and PASSED with the drain
deleted. httpx's ASGI transport pumps the request body whether or not the app
reads it, so "drained" and "abandoned" are indistinguishable through
TestClient. Its own mutation check is what exposed it.
`POST /api/downloads/upload` declared `file: UploadFile = File(...)`, and
FastAPI parses and spools a declared body before the first dependency runs --
`require_human` included. An unauthenticated caller could therefore make the
process write the whole per-file allowance (500 MiB by default) to temp, once
per request. SEC-20's ceiling bounded that number but could not stop the
writing, which is why E27 parked it rather than closing it.
The route now takes no body parameter. `spool_upload` names `require_human`
first and only then streams `request.stream()` to the staging temp file, so a
stranger is refused before a byte moves -- the shape the screenshot route has
used since E16. Filename and display name travel as percent-encoded headers
(latin-1 on the wire, so a Hungarian filename needs the encoding) instead of
multipart parts.
`capped_stream_to` joins `capped_body` in routes/_common.py: the same counting
rule, written to a file descriptor rather than accumulated, because holding
500 MiB would trade a disk problem for a memory one. Its refusal is the
caller's, so the upload route keeps answering 422 `upload_too_large` with the
admin knob's own number.
With no envelope to allow for, the edge is now exactly that knob, and
MULTIPART_ENVELOPE_ALLOWANCE is gone.
The invariant stops being prose: test_body_limit.py asks FastAPI itself
(`route.dependant.body_params`) and fails the gate if any route with a raised
ceiling declares a body. A comment could not have prevented the defect it
described; that test would have.
Both new tests were seen red for the right reason -- before the fix the
anonymous probe got its 401 only after the server had read 4236 bytes of a
multipart body, and re-declaring the File parameter turns the invariant test
red naming the route and the parameter.
Two findings, both in the review's own work; nothing in the product code. That is
the shift that ends the loop: 22 → 13 → 6 → 2, and the last two are a test and a
comment rather than a defect.
The "an override that is fine is still honoured" leg of the bounds-fallback test
stored `spec.max` — which, for this key, IS the env default, because the spec's
max is derived from it. So the assertion was satisfied by the fallback it existed
to rule out: mutating `_coerce` to reject EVERY int override left all three legs
green. It stores `spec.max - 1` now and asserts the value differs from the
default, so the leg can actually fail.
And the shrink test's comment claimed `3 - 5 = -2` for an expression that computes
`chips.length - visible` = `2 - 3 = -1` — naming neither operand nor the result,
in a comment that was rewritten one round earlier precisely for miscounting.
Round 3 over round 2's fixes. The trend is 22 → 13 → 6, and the character has
shifted: these are mostly defects in the REVIEW's own work, not in the epic.
The ordering test I added to hold the middleware's position could not fail for
either arrangement it names. All four `@app.middleware("http")` layers — the
access log, the two header decorators AND the setup gate that must be inside —
register as the same `BaseHTTPMiddleware`, so asserting on the class passes for
the pre-move stack and for a cap moved back outside CORS. It names the layers by
their dispatch function now, which is the only thing that distinguishes them.
The `sysconfig` bounds check CLAMPED, and clamping is a silent rewrite — the thing
this module refuses to do everywhere else, where an unusable override falls back
to the env default and logs once. On the max side the difference is cosmetic; on
the min side it is a live bug. `download_upload_max_bytes` had `min=0` before E7,
and a stored 0 means "no per-file cap" to the route that reads it. Clamped to 1 it
becomes a ONE-BYTE cap: uploads dead, nothing in the log, the admin page showing
"1". It raises `OutOfRange` now and takes the existing fallback, with its own
event code.
The `capped_body` sweep grepped for an identifier as the limit argument, so a site
written `capped_body(request, 4 * 1024 * 1024)` — or with the parameter not named
`request` — was MISSED, and a miss reads as "no new sites". Parsed with `ast`
instead: every call is seen whatever it looks like, and an unfamiliar argument
shape fails the equality instead of vanishing.
Frontend: the shrink test drove the mock in a way the real hook cannot produce
(it moved `visible` in the same step as the shrink, and the comment miscounted the
tags), so it reproduced the right END STATE by accident rather than the sequence.
`visible` is now left alone across the shrink, which IS the precondition.
Ops: the baseline guard answered "did this run rewrite it?" and therefore went
green on the second run over an already-tightened, uncommitted baseline — the
exact state its own comment says marks the localdev stack stale ever after. It
asks both questions now. And `.dockerignore` joins BAKED_PATHS: it decides what
the COPY lines actually take, so it is an input to the build context exactly as
the copied trees are.
A second review round over the first round's fixes, which is where the remaining
real bugs usually are. All four are the same failure as the round it reviewed: a
claim stated in a comment that the code does not hold.
The middleware moved inside CORS, but the setup gate is registered LATER and is
therefore OUTSIDE it — so the comment's "the setup gate never runs for a body that
is already too large" was false the moment it was written. On an unconfigured
instance that gate opens a DB session per request, which is exactly the anonymous
pre-auth work SEC-20 exists to remove. The cap now sits between them, and the
order is asserted against `app.user_middleware` rather than described: prose about
a stack that `add_middleware` builds inside-out is how this got backwards.
The edge ceiling and the route's own cap were pinned EQUAL, but they measure
different things — the edge counts raw body bytes, the route counts the decoded
file. A file of exactly the advertised maximum was refused one layer before the
route that would have taken it. The edge now allows for the multipart envelope,
and the test asserts the relation and the gap instead of equality.
`sysconfig`'s new `max` was enforced on write only. That bound did not exist
before this epic, so a value that was legal when it was written can be sitting in
the database — served unclamped it produces exactly the drift the comment says is
impossible, with the admin page showing a number outside its own range. Int
overrides are clamped on read now, which also survives an env default being
lowered later.
And `DOWNLOAD_UPLOAD_MAX_BYTES=0` meant "no per-file cap" to the route but a hard
zero to the edge: every upload refused, including the admin page's own way back,
recoverable only by editing env and restarting. The setting takes `ge=1`.
Two test gaps from the same round: the header assertion that is the ONLY check of
the middleware's position was DB-gated and skipped on a host without Postgres, and
the headroom "ratio" the docblock calls load-bearing was held as `<=`, i.e. no
ratio at all. Both fixed, plus a sweep that every `capped_body` call site in the
routers is known and fits under its ceiling — the point of that list is the third
entry, not the two that exist today.
The module admitted its two opt-outs under a stated rule — "both enforce their own
limit on the bytes they actually read, which is what makes opting them out safe" —
and that rule is false for one of them. `POST /api/downloads/upload` declares
`file: UploadFile = File(...)`, so FastAPI parses and spools the whole multipart
part to a temp file BEFORE the first dependency runs, `require_human` included:
its in-handler cap measures bytes that have already landed. Measured on the
installed fastapi 0.141.1 / starlette 1.6.0 — starlette's max_part_size guards
non-file parts only, so an anonymous chunked upload was bounded by nothing. In the
shipped self-host compose that lands in a RAM-backed tmpfs.
The exposure predates SEC-20 (no route was capped before it); what this branch
added was the claim. So the list becomes a mapping of per-route CEILINGS instead
of a set of exemptions: every request now gets a finite number, the upload route's
being the same one the route itself enforces. `sysconfig`'s knob is bounded by it
so an admin cannot raise the handler's cap past the edge and start collecting 413s
from a layer the admin page never mentions — held by a test, not by prose.
Two more things the review caught in the same module:
The 413 was not byte-identical to `capped_body`'s after all. `json.dumps` puts
spaces where JSONResponse does not, and the test that promised to hold them
together compared PARSED JSON, so it could never have seen it. Built through the
same JSONResponse now, and compared as raw bytes.
And the middleware sat outside CORS and the security headers, so its 413 carried
neither — on the split-origin dev setup the browser drops it and the user gets a
generic network failure instead of the translated too_large, while the identical
refusal from capped_body one layer down renders fine. It moves to just inside
CORS: still ahead of the session, the setup gate and every auth dependency.
Also corrected: the docblock's "largest non-exempt body is 64 KiB, ~16x headroom"
(it is 512 KiB and 2x — the diagnostics sink, named eleven lines further down),
and CLAUDE.md's Request SIZE row, which forbade the one correct use of
capped_body that is actually in the tree.
0.93.2 shipped a week's worth of confusion about which Node this repo
runs on; the Python side had the identical shape and nobody had looked.
Prod (Dockerfile), the test image (Dockerfile.test) and CI (check.yml)
each spelled 3.12 as an independent literal, there was no
`.python-version`, no `requires-python`, and ruff had no target-version
so it judged our syntax against some other interpreter. The dev machine
had no 3.12 installed at all — only 3.13 and 3.14 — so every ad-hoc host
script ran two majors ahead of production.
`.python-version` is now the single source of truth, and the contract is
held by tests rather than by prose. It is split in two because no single
runner can see every link: the pytest suite runs inside the test image,
which mounts backend/ alone and cannot read the repo-root Dockerfile or
the workflow.
test_python_engine_contract.py : running interpreter == pyrightconfig
== ruff.toml
pythonEngine.contract.test.ts : .python-version == pyrightconfig ==
ruff.toml == Dockerfile ==
Dockerfile.test == check.yml
Together they close the loop, and the backend half carries the assertion
that pays — that the interpreter running the suite is the pinned one. A
green suite on another minor does not mean the shipped image is green;
it means they have drifted and nobody has noticed yet.
FastAPI reads a declared body IN FULL before the first dependency, so
`POST /auth/password-login` drained an arbitrary upload from an anonymous
caller and only then answered 401. 70 of the 72 body-declaring routes
behaved the same way: one unauthenticated client could spend the
process's memory and time at will.
A raw ASGI middleware, not `@app.middleware("http")` — BaseHTTPMiddleware
cannot wrap `receive`, so it could only trust `Content-Length`, which a
client may omit (chunked) or understate. This refuses an oversized
declaration outright and counts the bytes of everything else as they
arrive, the same rule `capped_body` already applies per-route. Registered
directly inside the access log: a refusal is still logged with its
request id, but the session, CORS, the setup gate and every auth
dependency never run for a body that is already too large.
Two routes opt out, under one stated rule: an exemption is ONLY for a
route whose own cap is larger than the global one. That is why
`/api/diagnostics/events` is absent — it caps itself at 512 KiB, below
the ceiling, so exempting it could only widen what it accepts. The list
names route templates and a test resolves each against the live route
table, so a rename fails the gate instead of silently dropping an
exemption in prod.
The 413 is byte-identical to `capped_body`'s and the test compares the
two against each other rather than against a literal — two shapes for one
fact is how the `cancelled`/`canceled` bug happened.
The composer's preview did not fail a call — it killed the Gitea process, and
with it the git server and the container registry. `POST /api/v1/markdown` with
`{"Mode": "gfm", "Context": "<repo>"}` makes Gitea resolve every hex-shaped word
as a commit link, and on the API's render path the commit checker it asks is nil:
panic: runtime error: invalid memory address or nil pointer dereference
renderhelper.(*commitChecker).IsCommitIDExisting(0x0)
<- markup.hashCurrentPatternProcessor (html_commit.go:217)
Every report carries hex — a tab id, a request id — so this was not an edge case
but the only case: the preview answered "the tracker isn't answering" every time,
because the tracker had just died.
Measured on throwaway instances, not argued. Same body, same call:
gfm + Context + "5645b1c0ae01" -> 1.26.4 CRASH, 1.27.1 CRASH, 1.27.3 200
gfm + Context, no hex -> 200
gfm, no Context -> 200, and the HTML keeps its tables, task
lists and chroma-highlighted code blocks
Upstream fixed it in 1.27.2 ("fix: avoid markup render panic", #38698), but a
self-hoster runs whatever version they run, so the payload is what gets fixed.
Sending was never affected: the created issue's own page renders the same body
fine (measured) — a real repo context has a real commit checker. The only loss is
that a hash or a `#123` is not yet a link in the preview; the issue page links it.
The test pins the payload dict, because the defect is in what we send.
Round 4 was targeted at the surfaces rounds 2 and 3 changed. 29 findings raised,
26 survived deduplication, 12 survived three adversarial readings each.
`set_delivery` renewed the claim but never checked it was still ours, while its
sibling `release_send` always did. `expire_on_commit` is off, so the token it
re-stamped came from the caller's own memory, not the row: a send that outlived
its 600 s lease came back, overwrote the take-over send's committed links with a
full-column write, and put its own dead token back. It now takes the token, reads
the row's claim with a targeted scalar (not a refresh, which would discard the
`issue_url` assigned one line earlier) and raises `SendClaimLost` rather than
writing. An issue created by a send that had already lost its claim is logged with
its number — that record is the only one that will exist.
The tracker's settings are read above the pre-call checkpoint instead of inside
`_client`. Reading them there was a SELECT one line after the commit that was
meant to end the transaction, and held it — idle, snapshot pinned — across the
repository check and the create. The comment and the module docstring said
otherwise; measuring it in pg_stat_activity is what settled it. Two more comments
were saying things the code had stopped doing: the `**state` spread does not carry
the claim (`set_delivery` strips and re-stamps it), and react-query does not
debounce the preview — the editor being unmounted is what paces it.
The composer stopped stranding the focus on `<body>`: a send that creates the
issue and then fails on an attachment unmounts the button that was just pressed,
and only the success path handed the focus on. Export no longer saves into a
refusal it does not need — the zip is the way out of a stuck send, and its
pre-save was the one step that could not survive one. The delivery banner
announces through a role instead of an `aria-live` created with its own message.
`historyState` returns a union, so `chipTone` matching on the same keys is checked
by tsc rather than by hope.
PlexPlayer was the only full-screen overlay in the tree with no focus containment
at all — Tab walked out of an opaque player into a page nobody can see. It now
carries the shared trap. Closing that hole exposed the one underneath it:
`useDismiss` registered as a layer that traps the Tab while installing no trap,
so the player stood its own down for its own menus. Measured in a real browser:
30 Tabs with the settings panel open, 9 escapes into the nav rail, and 0 after.
Escape ordering is unchanged — the panel first, then the player.
Three tests that did not hold their line: the advisory lock's call site (deleting
it left the suite green), the trail assertion whose `all()` was vacuous over an
empty filter, and a `layerStack` assertion the stand-in dialog already forced.
All eleven executable fixes mutation-checked red for the right reason; the two
comment corrections have nothing to pin.
Round 2 made the step prompt a blocking layer so that `j` and the `g` chords
would not reach the page behind it. The dialog it sits in gave up its Tab trap
because of that, while the prompt's own replacement trap covered only its own
two controls — so Tab from anywhere else in the dialog (the bar's Screenshot or
Stop, the dialog's own buttons) walked out behind the scrim. `layerStack` now
separates the two questions it had folded into one flag: a layer can own the
keyboard and still leave the Tab trap to the dialog below it. The mirror case,
a dialog opening over the prompt, falls out of the same split.
A send's claim is now a lease with an owner. Every progress report re-stamps
it, so a send slower than the lease cannot lose it while it is still running,
and only the send that took a claim can give it back. The issue_create audit
row is staged inside the send and rides the same checkpoint as `issue_url`, so
the trail cannot be missing for an issue that exists. The attachment-name
prefix has one definition, shared by the producer and both readers.
A running send is reported to the UI. A reload or a second tab used to render
it exactly like a send that STOPPED, and offered a retry the server was certain
to refuse; the composer now says the report is being sent, holds the draft, and
puts both the retry and Create issue out. The past-sessions list says the same
for the same reason — it is the list a user opens to check on a send, and both
"draft" and "half sent" describe one that died. `/preview` and the retry path
got a pace of their own, and the per-user byte cap took the user-scoped lock
that makes it a cap instead of a guess.
Verified in a real browser on localdev: Tab cycles the whole dialog with the
prompt open, the prompt still swallows the page's keys, and the issue link is
focusable where the composer puts it. All 24 regression tests mutation-checked.
Creating the issue rebuilt the delivery state from scratch and dropped the
claim with it, so for a new issue the claim covered only the creation — a
draft, a screenshot or a second send could change the report under the uploads
and the body edit that follow, which is exactly what the claim is for. The
state is now built on what was read, and a test walks the send's checkpoints
to see the claim still held at each one.
Found by mutation-checking the round-2 tests: removing the claim's release
left them green, because for a new issue there was nothing left to release.
The send no longer holds the session's row lock while it talks to the tracker:
it claims the session, gives the lock back, and checkpoints each step. A Stop
is never held behind a send, while a draft, a screenshot or a second send are
refused at once instead of queueing. The claim ages out, so a request that
died mid-send does not strand the session.
What the composer says is what happened: an attachment that went is no longer
named as one that did not (the missing body edit is its own line), and the
issue's screenshot count comes from the server when the list cannot be read,
so a failed read no longer blocks saving the draft. The zip is fetched as a
request, so its pace is a message and not a saved error page. Screenshots of a
finished report can be removed again; only one still being sent is held.
A recording's log elevation can no longer outlive it. The step prompt keeps
the keyboard (the page's keys stay quiet) and traps Tab itself. An image
pasted into a dialog opened over the composer stays that dialog's.
The send, the draft and the screenshots take the session's row lock, so an
autosave or a paste during a send waits for it and then sees it, and two sends
cannot open two issues. A save keeps the delivery's progress; a failed edit
that shows the screenshots is finished by the retry; an answer that is not the
tracker's API (a redirect, a proxy page) is refused, not a crash that loses the
issue's record. Only an attempt that opened an issue counts toward the pace.
Screenshots: deleting one after the send is refused, a user's storage is
capped across sessions, new sessions and zip exports are paced, images are
stored uncompressed in the zip. One liveness rule, one session view, one image
type table.
Logging: a session's elevation is tracked apart from the user's own, so the
browser's own requests (no header) no longer lower the floor a session needs;
Stop ends it at once.
Composer: an image pasted anywhere is taken; what is still unsaved is saved on
close, and "saved" is shown only when it is; the draft is held while it is
sent; a failed screenshot read says so instead of reading as none; focus moves
to what stays; the preview follows the text size. Pill: the step prompt and a
running Stop survive the pill moving between hosts, the prompt keeps the
dialog's Tab trap, the toast region lets clicks through, other tabs hear of a
Start or Stop, the last seconds are sent before the cap, a capture with no
frame is an error.
- admin config group "Bug reports": Gitea address, repository, token (secret),
and "Check the connection" — the repository must answer, take issues and
be PRIVATE (a report carries its reporter's log); checked on every send too
- POST /api/debug-sessions/{id}/issue sends the SAVED draft: create the issue,
upload each file on its own request (the measured API), then show the
screenshots inline; progress is kept in the draft, so a send that stops
half-way keeps its issue and the retry sends only what is missing; 5 new
issues an hour per user; audited as issue_create
- the composer: Create issue (only where a tracker is set up, naming the
repository), a partial-send banner with retry, and a Preview — the tracker's
own rendering in a sandboxed frame, or the zip's markdown without one
- the recorder's refusals answer in the localized family (debug_session_*,
screenshot_*, issue(s)_*), not bare codes the UI could only call generic
- a tracker refusal logs the status only: its message can quote the title back
The shared test session kept whatever a failing handler flushed and never
committed, so a later read in the test saw a write production loses when
get_db closes the session — and a handler whose failure path must commit
(E16: the tracker's issue link) looked right without the commit.
After Stop (or at the cap, or from Settings → Diagnosztika's past sessions)
one dialog drafts the issue: title, description, the steps drafted from the
session's markers and page changes (a marker without a label is named by its
time), the environment (version, browser, system, window, language). Beside
it, what goes with it: the browser and server logs — each reviewable line by
line, an unticked line is not sent — and the screenshots, removable, with
paste / drop / pick to add more.
- it saves as it goes (debug_sessions.draft, migration 0090): the rendered
markdown in the user's language, the excluded rows, and its own fields to
reopen as it was; a sent session is read-only
- the zip export (issue.md, client.log, server.log, screenshots) is built
from the saved draft by app/issues/bundle.py — the one definition the Gitea
delivery (S4) will share; .log, not .jsonl, which Gitea refuses
- the log line's level tags and clock move to components/logLineParts so the
review list draws a level the way both log viewers do
- the start dialog is the consent: level (Részletes/Teljes), what is recorded,
retention and the cap (now served as diagnostics.session_minutes)
- the pill docks under the toasts: elapsed time, Step (an optional label,
its own Escape layer), Screenshot (tab capture where the browser has it),
Stop; compact on a phone
- while recording, lib/log tags every event and every request path (req,
beacon, uploadReq via one identityHeaders) names the session; applyDiagnostics
raises the level for the session's run and never lowers a raised one, and
lapses at whichever ends first
- Stop sends what is waiting before the server closes the session; the cap
says so and opens the composer
- Settings → Diagnosztika and the command palette start/step/stop it
- the evidence leaves the log upload's own request lines out
- the step shortcut is deliberately palette-only: the keymap's D1 rule bars a
new modifier chord, and a bare letter collides with the players'
A session is one recording, capped at 60 minutes by the server. While it is
open, a request naming the caller's own session in X-Debug-Session is logged
at the session's level with every line tagged; a header naming anything else
is ignored. The browser's batches count a session as consent to its level and
keep the tag only on its own events. Screenshots are judged by their bytes,
capped in size and count, and served back to their owner only.
- debug_sessions / debug_screenshots tables, one open session per user
(partial unique index), log_events.debug_session_id indexed
- /api/debug-sessions: start (joins a running one), stop, list, get, events,
screenshots; /api/me reports the live session
- the hourly log job closes expired sessions and drops them with the logs
- logsink.serialize and routes._common.capped_body shared by their readers
Round 2 of the deep review, on round 1's own fixes:
- `ORDER BY position … FOR UPDATE` sorts before it locks, so the reorder that waited got the
fresh rows in its STALE order and dealt the pins it left out (a tab with Plex off) back to their
old slots. It locks in id order — one global order, so two reorders cannot deadlock — and sorts
the fresh rows itself. The race test now sends that partial list and fails unless tab B really
blocked.
- The request check saw only writes the handler flushed: a `db.delete` left staged, or a flush
followed by a released savepoint, passed. Staged changes count at the end of the request, and
only the ROOT transaction's end clears it (a savepoint test drives the dependency directly).
- The `ok` test exempted every call as a serializer, hiding four domain REPORTS (YouTube sync,
revert, push; Plex sync) without `ok`. They are spread into `{"ok": True, …}`; the exemption is a
named list of real serializers.
Helpers flush and handlers commit — but every test request shared the test's session, so a
follow-up GET or an assertion read the flushed write and the suite stayed green over a handler
whose commit was missing (production's get_db closes the session: the write is rolled back behind
a 200). The db fixture now hands out `db.info["request"]`: the same session, recording each request
that ends normally with a flushed, uncommitted write, and failing the test on it at teardown. All
28 get_db overrides use it (a convention test keeps it so); the full suite passes under it, and
dropping the commit from add_item turns a playlist test red.
Deep review of the debt burn:
- `refusal_reason` told the user to reconnect on ANY token-endpoint failure — a 5xx, a 429, a
misconfigured client — while the grant stayed and the account still read as connected, with no
connect button. Only `invalid_grant` (the one that drops the grant) means reconnect now.
- `pins.reorder` writes only the rows that change, so two overlapping reorders each skipped rows
that matched in their own stale snapshot and interleaved into two pins on one slot. It locks the
user's pins first; the second waits and deals from the first's result.
- Adding a whitelist entry committed the invite and the activation it implies separately, and its
audit row was written before a new invite had an id (target NULL). One transaction, flushed for
the id.
- The mutation convention test skipped async routes and dicts returned by name: request-access,
purge-discovery and the subscription sync answered without `ok`. They do now, and the test
covers both. The helper-commit check looked only at `_`-names; it reads every non-route
function, which listed two deliberate committers (upsert_pending_invite, ensure_welcome) — now
allowlisted with their reasons.
Two rows of the canonical-patterns table had drifted, found while closing the roadmap audit:
- 63 mutation responses carried no `ok` — `{"unsubscribed": true}`, `{"status": "ok"}`,
`{"added": 3, …}` and so on. Each now leads with `"ok": true` (additive: every field a client
reads is still there).
- Twelve route helpers committed on their own. Six were just the end of their handler's work and now
flush: the playlist item add/remove and Watch later get-or-create, the Plex series/season bulk
mark, and the invite decision + account activation — which an approval used to land in separate
transactions. The other six commit on purpose (a token durable before its link is mailed, a
single-use token burned as its own unit, the Google callback's tail, a background registration
with its own session, the staged search ingest around YouTube calls), each listed with its reason.
`tests/test_route_conventions.py` holds both rules in code; CLAUDE.md's two rows point at it.
- `overflow-clip` gets a safelist anchor in index.css (its only occurrence is a `.ts` constant),
and the chip-lane contract keeps it there.
- PlexPlayer's Backspace ownership rule is pinned on all three branches: nothing above, a foreign
layer above (it defers), and its own panel open under a layer (it still closes the panel).
- Removing a playlist item bumps the list's `updated_at`, like the three other item-only writes.
- Seen in a real browser on localdev: an exhausted comfort-shelf tile (an empty pinned playlist) is
focusable, aria-disabled, and its Shift+F10 menu offers Play off and Remove from shelf.
22 test files set `app.dependency_overrides[...]` by hand and tore down with a blanket `.clear()`
in a `finally`, which wipes every override rather than the test's own and stops fixtures composing.
They now take the `override_dep` fixture (helpers get it passed in), whose teardown restores each
key's prior value; no clear() carried meaning between two phases of a test. A convention test fails
any test file that writes `dependency_overrides` directly.
- A13: with Plex off, the Home shelf and the hero's comfort tier go YouTube-only like the rails
beside them (owner's call: hide the tiles). The pins are kept; GET /pins still lists them.
- Reorder deals the listed pins into the slots they already hold, so a partial list (hidden Plex
pins, a stale tab) no longer puts two pins on one position; repeats are ignored.
- A21: a comfort hero carries `pinned_from`, the shelf entry it was resolved from; its menu acts on
that pin, so a pinned show's next episode offers "Unpin" instead of creating a second pin.
- B12: the palette's "Meglepetés" asks for the surprise tier (the next press leading with it); the
rotation carries on from there.
- B13: the surprise token names the history entry it was requested for (minted by setPage's new
`then`, after the push) and is dropped on any other entry, so a navigation that never mounted
Home cannot replay it later.
~150 HTTPExceptions carried a hand-written English sentence as their detail,
shown verbatim under the Hungarian UI — in the blocking error dialog and on
every inline surface that printed HttpError.detail (sign-in, setup wizard,
change password, the passphrase gate, live search). Four also quoted an
upstream exception's text back to the user.
- app/refusal.py: `refused(status, reason, **params)` → {code: "refused",
reason, …params}; localizeDetail translates errors.refused.<reason> with the
params interpolated. "Unknown X" 404s use the existing not_found(what).
- sysconfig.set_value raises InvalidSetting (reason + bounds) instead of an
English ValueError; the three writers answer it through setting_refused.
- The Plex connection/write/image failures log what the upstream said and
answer a reason; the search scraper's failure is logged and answered as
youtube/unavailable (it crashed refusal_reason before).
- HttpError.detail now only ever holds a translated reason: a string detail
(only the framework's own can remain) is left out, so reading .detail is
safe everywhere — including destructured, the hole the raw-detail source
scan could not see. That scan and its five-file exception list are gone.
- Enforced: test_http_details.py fails on any HTTPException with a sentence
for a detail; the contract test requires every refused reason and every
not_found "what" in both languages, and no key for one that is gone.
- local/jsx-comment-text: a // inside JSX children renders as text; the rule
is ten lines in eslint.config.js rather than eslint-plugin-react's ~1800
lockfile lines.
HLS sessions were keyed by the item, so a second account opening the same
film killed the first one's session; the displaced player reclaimed at its
playhead and evicted the other in turn. The registry is now keyed by
(viewer, rating_key), segment requests resolve by both, and the session
directory carries the viewer (the orphan sweep still recognises the older
shapes).
Only transcodes counted toward a cap, so remuxes — each a whole segmented
copy of the film on the scratch volume — were bounded by the library size.
Three bounds now apply after every start, each dropping the least recently
used: two sessions per account, the transcode cap, and a new admin knob,
plex_max_sessions (default 8), over every session. A superseded session
request returns at once, since it waits on the killed ffmpeg, so an account
cannot park more request workers than it may hold sessions.
The subtitle extractor's full-pass slots were global and minutes long each;
one account opening subtitles on film after film could hold them all. An
account may now hold three of the six, and still gets windows past that.
A full pass that ran out of time kept what it reached, but its retry an hour
later started again at 0, so a file too big to read inside the extraction
timeout never completed. The pass now starts where every embedded track's
coverage from the head ends (less the window pre-roll, so a cue across the
join is read whole). The seek needs -avoid_negative_ts on this read as on a
window, measured on a real file: without it the cue running at the start
shifts the whole output.
A partial track whose full pass had failed never reached a settled state, so
the player polled it every 2 s for the rest of the film. The status now says
whether the track is still growing, and a stalled one is polled every 15 s —
still inside the 60 s lookahead that reads the next window before an edge.
The comfort shelf resolved each pinned channel with three queries (the
row, its unwatched count, its newest video) and each playlist with two.
channels_summary gets the count and the newest row per channel as window
functions over one filtered set, playlists_summary reads every pinned list
in one query, and the rows come in one IN() each — so the shelf's cost no
longer grows with its length. A test counts the statements.
RAIL_LIMIT had two definitions held in step by a comment; the Plex Discover
route now imports the domain's. And rollup's "continue the latest-started
episode" was false: it takes the last in-progress one in broadcast order,
which is also what Home's next_episode picks.
move_item checked "already in the target?" and then re-parented without a
lock, so a second tab's add or the scheduler's YouTube merge committing the
same video in between hit uq_playlist_video and answered a raw 500. The
re-parent now runs in a savepoint and a conflict falls back to the
already-present outcome. The target is touched only when the row landed in
it (an untouched list was promoted in the rail's "recently changed" sort),
and _touch stamps the database clock like the mixin's own onupdate.
The canonical-patterns table names owned_or_404 against ad-hoc ownership
helpers; six were left (_own_playlist, _own_playlist_or_404, _own_view,
_own_tag, notifications._owned, downloads._own_job), two of them
hand-rolled with a bare English 404. All call sites now use owned_or_404,
and the download/link/notification 404s are structured like the rest. The
contract test reads every "what is gone" the routes name, and the YouTube
refusal reasons, and requires both languages to name each.
Two near-end checkpoints arrive together routinely (the 10 s tick and the
debounced seeked flush); both read "not watched" before writing and both
scrobbled, bumping Plex's viewCount twice. The transition is now decided by
the write: an insert that did not conflict, or an update conditional on the
old status.
The finish also popped the push-ordering entry that _resume_push_superseded
reads, so an older resume push still in flight could land at Plex after the
scrobble and revive a resume point on a watched item. It now stamps the
entry instead; the existing eviction bounds the map.
The two manual syncs (subscriptions import, playlist pull) had no YouTubeError
handler, so a quota, token or transport failure was a raw 500 and the blocking
"Server error" dialog (E9 F1). The other YouTube routes answered either the
upstream's English or a playlist-scoped "youtube_failed".
One helper, routes/_common.youtube_refusal: 422 {code: "youtube", reason} with
the reason sorted by what the user can do — quota (wait for the reset),
reconnect (the token step refused), unavailable (retry). Used by every route
that surfaces a YouTube failure; localizeDetail translates it.
"Reconnect" had no path: a grant Google revoked still read as granted, so
Settings and the wizard offered no connect button while every call failed.
An invalid_grant from the token endpoint now clears the stored grant.
A failed connection or token step answered nothing about the channel, yet stamped it: one proxy blip
during an RSS blackout delayed every due channel by a whole window. YouTubeError now names the
endpoint that answered; a failure no endpoint gave, or a spent quota, ends the round and stamps
nothing.
A channel whose uploads playlist also failed was never stamped, so a terminated channel was charged
again on every 20-minute poll instead of once per rss_fallback_hours. An upstream quotaExceeded now
stops the round instead of charging every remaining channel the same refusal.
An INFO line outside a request never reaches log_events, so the fallback's
own summary was invisible in the Server log. Its counts ride on the
sync.rss.failed warning, the one line of a poll the console keeps.
Measured on localdev: 375 of 375 dark channels read, 403 new videos.
The server now logs polled reads at DEBUG, and the browser's copy of each
request line promises the server's level: at INFO the unread-count polls
filled My log within minutes and, at Detailed, the shipped log. The server
marks a line it logged at DEBUG with X-Log-Quiet, so the polled-route list
stays in one place.
peter