Commit Graph
4 Commits
Author SHA1 Message Date
peter 3cd0c525e2 fix(keys): epic-end review round 2 — focus comes back to the card, the right rail and the reason
Whole-epic deep-review of E11 (18 raised, 12 kept; B and G refuted, four latent test-hardening notes dropped),
plus the admin refusal copy gap round 1 surfaced.

- A context menu opened from a card the keyboard is on gives the focus back to the card. floating-ui returns
  focus through the opener's first tabbable descendant even when handed a ref, so for a non-tabbable opener the
  surface restores it itself (measured in Chrome: j, Shift+F10, Escape left it on the thumbnail link).
- Back finds a named list (a rail) by its name: a rail that appeared above it after the cache expired sent the
  focus into the wrong rail. An unnamed list is matched only when the page has the same number of lists.
- The slot kept for a removed item is forgotten once focus moves on to page content (search, a chip), so j after
  new results starts at the first visible item; a menu or dialog opened from the item keeps it.
- The list keys reach the cheat sheet only while a list has an item.
- A j-focused card root is a named group while it holds the focus.
- The channel page's Block and Hide toggles keep the focus while their request runs.
- A split export that fails part-way says how many clips were added and deselects them, so a retry adds only
  the rest.
- Config save, the one-click share link and "log out other sessions" report failures with their reason.
- VirtualGrid uses the one scroll-parent walk listNav measures against.
- Admin refusals (activate, resend, invite, set-password link) say why in HU and EN instead of "try again";
  the email-off special cases collapse into that one reason. The contract reads the codes from localizeDetail.
  HU copy points to Konfiguráció → E-mail / SMTP; HU "unreachable" matches the EN change of round 1.
2026-09-13 19:18:04 +02:00
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.
2026-09-05 04:18:33 +02:00
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.
2026-09-05 04:02:57 +02:00
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.
2026-09-05 03:41:58 +02:00