2089 Commits

This Branch

This Branch

All Branches

Author	SHA1	Message	Date
Paul Nothaft	fb7af502ec	chore(main): release 3.116.0-beta.0 (#1203) v3.116.0-beta.0	2026-08-26 19:20:50 +00:00
Paul Nothaft	da802169a8	fix(gallery): show colour labels in the Carousel layout (#1189) (#1196) ... Carousel paints its own markup instead of going through PhotoCard, so it inherited none of the colour-label treatment — not other viewers' marks from #1178 and not the viewer's own from #1044. A photo the client flagged green looked identical to one nobody had touched, in a layout a photographer can select like any other. It has been missing since the feature landed. Rendered in both places the carousel paints a photo. The thumbnail strip is the one that matters: it is the only place the layout shows more than one photo at a time, so it is the only place a label can actually be scanned. Two small additions to ColorLabelBadge, both defaulting to today's behaviour so every existing layout renders byte-identically: - `size="sm"` shrinks the dots for the strip's 80px tiles, where the grid-sized 20px dot plus three 10px ones covers most of the image. - `position` is overridable because this layout has different corners free. Its top-left carries the counter and category chips and its top-right the play/fullscreen buttons, so the badge goes bottom-left on the main frame — the only corner left — and top-left in the strip, where nothing competes. This is the per-layout position override #1178's review said would start to pay for itself the first time a layout genuinely needed a different corner. Co-authored-by: Paul Nothaft <paul@MacStudio-von-Paul.local>	2026-08-26 21:13:52 +02:00
Paul Nothaft	d3e9a7cf0d	feat(auth): make the admin "Remember me" checkbox actually do something (#1186) (#1195) ... The checkbox had no `checked`, no `onChange`, and no place in the login request; `rememberMe` existed only as an i18n label. On the backend establishAdminSession hardcoded `expiresIn: '24h'` and the cookie always got DEFAULT_MAX_AGE_MS, so there was nothing to receive it anyway. Wired end to end: state on the page, `remember_me` in the login body, and a 30-day JWT plus a matching 30-day cookie when it is set. Opt-in on purpose. An absent or malformed value means "no", so a client that never sends it keeps exactly the 24h session it always had, and a stolen cookie is still worth a day by default. The JWT and the cookie take their lifetime from the same flag. If they can disagree the session either dies early (long cookie, short token) or outlives what the user consented to, so the tests assert them against each other. Review found the feature was non-functional as written, which is the important part: sessionTimeoutMiddleware and isSessionExpired enforce security_session_timeout_minutes — 60 minutes by default — against a session's idle time regardless of how long its token lives, so a remembered admin was logged out within the hour with a 30-day token sitting unused. rememberMe now travels in the JWT payload and both checks exempt a remembered session from the IDLE timeout. Not from expiry: the token still dies on its own 30-day exp, and revocation, deactivation and password-change invalidation are untouched. Also: /api/admin/auth/change-password reissued a hardcoded 24h token without the flag, so a remembered admin dropped back to 24h the moment they changed their password — which is mandatory for new and reset accounts. It now inherits the choice from the session it replaces, carried on req.admin.rememberMe. Through MFA the choice rides inside the signed mfa_pending token rather than being resent, so the second leg cannot ask for longer than the first agreed to. The tests drive POST /api/auth/admin/login and read the real Set-Cookie and token rather than minting a local clone of the ternary they are meant to be checking, boot one database per file before anything reads it, and generate their credential per run so no literal that looks like a password lands in the repository. No visual change — the checkbox was uncontrolled, so it already toggled on click; it just did nothing. Co-authored-by: Paul Nothaft <paul@MacStudio-von-Paul.local>	2026-08-26 21:13:23 +02:00
Paul Nothaft	95e7301909	chore(main): release 3.115.4-beta.0 (#1200) v3.115.4-beta.0	2026-08-26 19:08:03 +00:00
Paul Nothaft	edef4d7365	fix(images): backfill orientation for libraries that predate the fix (#1199) ... * fix(images): backfill orientation for libraries that predate the fix (#1198) #1194 corrected the generators and every ingest path, but did nothing for photos already in the database. Those rows end up worse than untouched ones: before the fix a rotated photo was CONSISTENTLY wrong — a sideways image in a tile shaped to match — and afterwards the regenerated thumbnail is correct while photos.width/height still describe the raw sensor order, so masonry and justified size a portrait photo with a landscape ratio. The dimension repair cannot reach them: it only selects rows with a NULL dimension, and an affected row has both, just transposed. Its own job rather than a mode of that one. They look alike but are not the same operation: the repair FILLS missing values and touches nothing else, while this RECOMPUTES and invalidates the derived data generated against the old orientation. Sharing a lease would also mean one blocks the other. A first attempt at this was reverted from #1194 after review found five problems. All five are addressed here: - Originals are read through resolvePhotoStorageKey + withLocalCopy + withProcessableImage, so the job works on S3 installs and on RAW/DNG. The dimension repair's direct fs read does neither, which stops being an edge case in a job that walks the whole library. - The canonical preview is cleared BEFORE faces are requeued. ensurePreviewImage returns a cached preview whenever it is still a valid image, and a pre-fix unrotated one is perfectly valid — so requeueing alone made the rescan read unrotated pixels and scale those boxes by the corrected dimensions, which is worse than leaving the data alone. - Invalidation keys off the EXIF transform, not a dimension delta. Orientations 2, 3 and 4 move every pixel while leaving width and height unchanged, as does 5-8 on a square image; a delta check skips exactly those rows. - Archived events are excluded — archiving deletes the originals and keeps the rows, so every one of them would fail its read. - The dimension write and the invalidation share a transaction. Split, a failure between them leaves stale face data that no retry can fix, because the retry computes "already correct". Tier deletion stays outside the transaction on purpose: it touches storage, and a failed object delete must not roll back a correct database write. A leftover tier regenerates on next read; a rolled-back write is silent corruption. * fix(images): invalidate every stale rendition, fence the writes, and give the job a button (#1198) Three things from review, one of which mattered a lot. The invalidation was too narrow. Clearing only preview_path fixed the face data and left the gallery worse off: ensureThumbnail and ensureHeroImage return their cached file whenever it is merely VALID, and a pre-fix sideways thumbnail is perfectly valid — so a corrected row rendered the old sideways image inside a newly-corrected portrait tile. All three canonical renditions are cleared now, their stored objects deleted, and both responsive tier sets with them. The responsive tiers also needed handling rather than a hopeful catch. Their helpers swallow delete errors, and ensurePreviewImageAtWidth treats storage.stat(key) as a cache hit — so a tier that survived deletion keeps serving unrotated forever and never regenerates. The keys are re-checked after deletion and survivors are counted into the result, so a run that could not clear them does not report itself as clean. Writes are fenced on the identity that was measured, not just the id. replacePhoto swaps a new file under an existing row and rewrites path/filename, and it IS reachable — from the replace_by_name upload path in adminPhotos.js. A replacement landing while this job read the old original would otherwise have had the previous file's dimensions written over it and its fresh renditions cleared. And the job had no way to start it: the endpoint existed with no caller, so an upgrade would have left every affected library untouched unless an operator found the API themselves. It gets a Status card like its two neighbours, with strings in en/de/fr/sl. No backlog counter, because unlike the other two it cannot know how many rows need it without doing the work. * fix(images): make the backfill idempotent, and stop it lying about what it did (#1198) Six things from review round 2. The job was not idempotent, and the way it failed was expensive. Its trigger is the EXIF tag on the ORIGINAL, which correcting a photo never changes — so every re-run threw away the renditions it had just regenerated and requeued every completed face scan. On a face-enabled install, running it twice meant re-detecting the whole library for nothing. Migration 191 adds photos.orientation_checked_at, written in the same transaction as the work it records, with `force` as the escape hatch for an interrupted run. The candidate query selected preview_path but not thumbnail_path or hero_path, which the deletion loop reads — so those two pointers were cleared in the database while the objects stayed in storage, still reachable through previously issued URLs. watermark_path was missed entirely. gallery.js serves it ahead of the original when branding watermarking is on, which makes it the most visible rendition of the lot. (Its generator needed rotating too — that went into #1185, where the other three live.) storage.stat() RESOLVES with null for a missing key rather than rejecting, so counting "the promise settled" marked every deleted — and every never-created — tier as a survivor. A perfectly clean run told the operator to re-run. Now a null means gone, and a rejection counts as stuck, since a storage error is not proof the object went away. Face data is invalidated whenever the stored dimensions change, not only when the change came from rotation: boxes are scaled by photo.width at read time, so any dimension change strands them. And `corrected` now comes from the affected-row count. If the fence rejected the write because the file was replaced mid-run, the photo was not corrected and the run must not claim it was. * fix(images): stop the backfill doing unnecessary work, and make its retry advice true (#1198) Round 3, four points, all narrower than the last two rounds. It re-processed photos that were already correct. A 5-8 rotation changes the dimensions, so a tagged photo whose stored dimensions are ALREADY oriented must have been ingested after #1185 — its renditions are fine and clearing them deletes valid files and rescans a completed face detection for nothing. Those are now skipped and simply marked. Orientations 2, 3 and 4 (and 5-8 on a square image) leave the dimensions identical either way, so they carry no such evidence and are still done once. The retry advice was impossible to follow. When a responsive tier could not be deleted the row was still marked, so the ordinary re-run the UI recommends found nothing and the stale tier kept serving unrotated forever. The marker is withheld when a tier survives, which is what makes that message honest. Storage cleanup now only runs when a fenced write actually landed. If the file was replaced mid-run every update matched zero rows, but the deletion went ahead anyway and could destroy renditions belonging to the REPLACEMENT — watermarks especially, which are keyed by photo id and alias straight onto the new file. And the full-photo ETag includes the backfill's timestamp. It was built from the ORIGINAL's mtime plus the watermark settings hash, neither of which this job touches — so a guest holding a pre-fix ETag would go on getting 304 and their cached sideways image no matter how many times the backfill succeeded. --------- Co-authored-by: Paul Nothaft <paul@MacStudio-von-Paul.local>	2026-08-26 21:01:51 +02:00
Paul Nothaft	c18f54ede0	fix(images): respect EXIF orientation in thumbnails, heroes and previews (#1194) ... * fix(images): respect EXIF orientation in thumbnails, heroes and previews (#1185) generateThumbnail, generateHeroImage and generatePreviewImage went straight from sharp(imagePath) to .resize(), so a photo whose Orientation tag is not 1 — routine for portrait shots on bodies that tag rather than rotate the sensor data — was resized from the raw frame and came out sideways. The same pipelines then call .withMetadata(false), stripping the tag from the output, so nothing downstream could correct it either. The download path already had this right: resizeToBox calls probe.rotate() for stills, which is why the same photo looked correct on download and rotated in the gallery. All three generators now do the same, guarded to stills for the reason resizeToBox already documents — .rotate() flattens a multi-frame source. The reporter also spotted the half that compounds it: photos.width/height were stored from sharp's metadata, which reports pixels as STORED, not as displayed. For orientation 5-8 those are swapped, so a portrait photo landed in the database as landscape and masonry/justified sized its tile with the wrong aspect ratio on top of the image being unrotated. A shared orientedDimensions() helper now does that conversion at all four capture sites — managed upload, background processing, external import and the dimension repair — so the stored numbers describe the rotated result the generators now produce. Existing rows keep their pre-rotation dimensions until the photo is reprocessed; the images themselves correct on the next thumbnail/preview regeneration. Tests fail on the unfixed generators — verified by reverting the rotate calls and the swap, which fails 4 of the 7. * fix(images): orient dimensions on every ingest path, and stop guarding rotate where it protects nothing (#1185) Review found the first cut covered four of eight dimension-capture sites. The filesystem watcher, the S3 auto-importer, the v1 upload API and replace-by-name all still persisted raw metadata.width/height, so an orientation 5-8 photo arriving that way got a correctly rotated thumbnail and a database row describing it as landscape — the same aspect-ratio mismatch this PR set out to remove, just on the paths I had not grepped. (I searched for `metadata.width` and the v1 route aliases it to `meta`.) The animated guard was also wrong in two of the three generators. generateThumbnail and generateHeroImage never pass `animated: true`, so they already flatten a multi-frame source to its first frame — skipping .rotate() there protected an animation that was being discarded anyway, while leaving the output in raw orientation against swapped stored dimensions. Both now rotate unconditionally. generatePreviewImage keeps the guard, because it genuinely does open animated sources as animated and .rotate() would flatten them. That leaves one corner unsolved rather than papered over: a multi-frame source that also carries an orientation tag keeps its raw orientation in the preview while the thumbnail and stored dimensions describe the rotated one. GIF has no EXIF and animated WebP effectively never sets it, so it is a real gap but not a common one, and closing it means rotating frame by frame rather than quietly dropping the animation. Documented at the guard. * fix(images): add a recompute mode so existing libraries get corrected too (#1185) The orientation fix only helped new photos. A row affected by the bug has BOTH dimensions stored — just in the raw order — so the repair job's NULL filter could never reach exactly the rows that needed it. Worse, once their thumbnails regenerated rotated, those rows went from consistently-wrong (sideways image in a matching tile) to inconsistent: correct image, wrong-shaped tile. `recompute` widens the candidate set to every image row. Opt-in, because it re-reads every original. It also has to deal with the consequence for faces. Detection runs against the preview and stores boxes in ORIGINAL pixel space, scaled by `photo.width / previewMeta.width` (faceProcessor.js:220-224) — so a photo whose stored dimensions change has face data recorded against a coordinate system that no longer exists, and the overlays crop the wrong region. Photos whose dimensions actually change are requeued for scanning; ones that were already correct are not, or a routine repair would rescan the whole library. Rows with face_status NULL are left alone so installs that never enabled the feature don't start scanning because of a dimension repair. Writing the test for that last rule caught a real bug in it: the candidate query never selected photos.width/height, so `photo.width` was undefined and every row compared as changed. Both columns are selected now. * Revert "fix(images): add a recompute mode so existing libraries get corrected too (#1185)" This reverts commit cb771d08. Review round 3 found five problems, all of them in this addition rather than in the orientation fix itself, and one of them an own-goal: requeueing face scanning makes processPhotoFaces call ensurePreviewImage, which returns the CACHED pre-fix preview when it is still a valid image — so the rescan reads unrotated pixels and scales those boxes by the newly corrected dimensions. That is worse than leaving the data alone. The rest need work this PR should not be carrying: the dimension repair reads originals through resolvePhotoFilePath and plain sharp, so it does nothing on an S3 install and rejects RAW/DNG; recompute pulls archived rows whose originals were deleted on archive; orientation 2, 3 and 4 change the pixels without changing width or height, so a dimension-delta test never notices them; and the dimension write and the face invalidation are not atomic, so a failure between them leaves a row that no retry will ever requeue. Split out so it can be designed and reviewed on its own. The orientation fix — .rotate() in the three generators and orientedDimensions() at all eight ingest sites — is unaffected and stays. * fix(images): the watermarked rendition needs orienting too (#1185) A fourth generator with the same bug, found while reviewing the backfill that builds on this. watermarkService composites and re-encodes through its own sharp pipeline with no .rotate(), and gallery.js serves photos.watermark_path ahead of the original when branding watermarking is on — so on a watermarked gallery the sideways image is precisely what a guest sees. Two details this needed beyond the .rotate() itself: metadata() is read from a separate, unrotated handle. .rotate() does not change what metadata() reports — a 400x200 source tagged orientation 6 still reads 400x200 — and every use of those numbers here is positioning: watermark scale, font size, composite extent. They have to be the DISPLAYED dimensions or the mark is placed against the wrong axis, so they go through orientedDimensions. The composite offsets are floored. getPositionCoordinates derives from the SVG's estimated text extent and returns fractional pixels; sharp rejects a non-integer offset and applyWatermark catches its own error and returns the image unwatermarked. Landing on a whole pixel was luck, and changing the dimensions it is computed from ran out of it — the test surfaced a real "Expected integer for left but received 92.8". --------- Co-authored-by: Paul Nothaft <paul@MacStudio-von-Paul.local>	2026-08-26 20:55:54 +02:00
Paul Nothaft	2d98f5fa9f	chore(main): release 3.115.3-beta.0 (#1192) v3.115.3-beta.0	2026-08-26 07:02:21 +00:00
Paul Nothaft	3991dc3ccb	fix(admin): gate the dimension repair as system maintenance (#1182) ... * fix(admin): gate the dimension repair as system maintenance (#1181) The endpoint's candidate query is unscoped, so it walks every event in the install, reads every original off S3 or the NAS mount, and rewrites their metadata. It required only photos.edit, which the built-in team_photographer preset holds (175_granular_permissions_and_presets.js:106) — a role that exists for a contributing second shooter, not for someone who should be able to start a whole-library scan or touch another owner's events. Now system.manage, whose own description is "run system maintenance actions", with the status endpoint on system.view to match. Nobody who should have it loses it: super_admin is granted every permission, solo_photographer is 'ALL', and migration 175 already projects every settings.edit holder forward onto system.manage on upgrade. The capture-date sweep next to it was gated this way in #1179; this brings its older twin in line. * fix(admin): gate the dimension status card on the permission the button needs (#1181) Same mismatch as the capture-date card: system.view and system.manage are independent grants and StatusTab renders its card and enabled button purely on a successful status payload (StatusTab.tsx:558), so a system.view-only role got a live Repair button whose every click 403s. * fix(admin): stop the dimension status card polling a 403 (#1181) With the endpoint correctly requiring system.manage, anyone who can open the Status tab but lacks it would have had a 403 and a logged denial every ten seconds for a panel they were never shown. The query is now gated on the same permission the endpoint requires, so it never starts. * fix(admin): gate the dimension card's render on the permission too (#1181) TanStack keeps the cached status after `enabled` flips false, so checking only the payload would still show the card — and an enabled Repair button whose POST 403s — to a lower-privileged admin logging in behind a system.manage user inside the cache lifetime. * fix(admin): name the dimension-card permission flag for the card it gates (#1181) #1179 adds a second system.manage-gated card to this same component with the same flag name. Two identical declarations merge WITHOUT a conflict and then fail to compile — TS2451, cannot redeclare block-scoped variable — and since each PR is green on its own, nothing catches it until main's build breaks. Verified by trial-merging both into main: no conflict, two declarations, tsc fails on both lines. Naming this one for the card it gates removes the trap; once both have landed the two flags can collapse into one. --------- Co-authored-by: Paul Nothaft <paul@MacStudio-von-Paul.local>	2026-08-26 08:54:04 +02:00
Paul Nothaft	51d20c5920	fix(gallery): show other guests' colour labels in the grid (#1178) (#1180) ... * fix(gallery): show other guests' colour labels in the grid (#1178) A colour set by one guest was visible to others in the lightbox and invisible on the tile. The lightbox reads /photos/:id/feedback, which returns per-colour tallies across everyone; the grid reads /photos, whose payload carried only `my_color_label` — so PhotoCard could render nothing else. The feature simply was not extended to the grid. /photos now also returns `other_color_labels`: the DISTINCT colours other viewers put on each photo, gated on show_feedback_to_guests like every other aggregate. `my_color_label` stays ungated, because a viewer's own selection is not shared data — that distinction is unchanged. Distinct colours rather than counts, and capped at three dots: a tile has room for a couple of marks, and "who marked this, and how many" is a question the lightbox already answers properly. The viewer's own colour is excluded from the dots so the badge and the dots never say the same thing twice, and they sit in opposite corners so they do not read as one group. The inset ring stays the viewer's own signal, which is what the badge was built for. Not addressed: the same issue asks for an identity-less shared colour tag — one tag per photo that any guest can overwrite. Neither existing identity mode does that (`simple` scopes by device fingerprint, `guest` by guest_id), so it is a third model touching the feedback schema, the per-guest caps, moderation and the admin aggregates. That is a feature with its own design, not part of this fix. * fix(gallery): carry other guests' labels into the premium and story grids too (#1178) PhotoCard was not the only place the badge renders. GalleryPremiumLayout and StoryPhotoCard have their own copies, and both still passed only my_color_label — so the fix would have covered the default grid and left the two full-bleed layouts showing nothing, which is the same shape of gap the original bug had. Found by driving a real gallery rather than reading the diff: the masonry grid rendered the dots correctly, and a grep for the remaining call sites turned up these two. * fix(gallery): keep the other-viewers colour dots out of the contested corner (#1178) The dots were placed bottom-left, which is the busiest corner in every layout: Timeline paints a timestamp chip there on every tile, and Grid, Mosaic and Masonry a media-type badge. All of them render after the badge, so the dots sat underneath them. Moved into a single row in the corner the colour-label dot already owns, next to the viewer's own mark. Nothing new is contested, and the grouping reads better anyway — your mark and everyone else's are the same kind of information. --------- Co-authored-by: Paul Nothaft <paul@MacStudio-von-Paul.local>	2026-08-26 08:53:52 +02:00
Paul Nothaft	849a5807b7	fix(admin): make "Storage used" report storage used (#1164) (#1170) ... * fix(admin): make "Storage used" report storage used (#1164) The tile summed photos.size_bytes — the catalogued size of the ORIGINALS, which has no relationship to the disk PicPeak runs on. In reference mode those files are never copied and sit on the NAS; duplicate rows counted the same file twice (#1162); and it ignored everything PicPeak genuinely does write locally: thumbnails, previews, hero renditions, watermarks and the per-event download cache. The reporter's tile read ~80 GB against 21 GB of real usage. Worse than the label: the same number drove the storage soft-limit warning bar and, via /storage/info, the recommended soft limit — so a reference-mode install got a disk-capacity recommendation computed from bytes that are not on the disk. - new localStorageUsage service walks the storage root and reports the total plus a breakdown. Walking rather than summing DB columns is the point: thumbnail/preview/hero rows record a key and never a byte count, and orphans from a deleted event or an interrupted import are real bytes. Symlinks are not followed, so a link into the media mount cannot put the NAS back in the total. Cached for 5 minutes, since the dashboard polls. - the dashboard tile and /storage/info now report that, with the catalogued figure kept and labelled as such next to it. A failed measurement reads as "unavailable" rather than substituting a number that means something else. On the local rig: 64.37 MB used against 15.75 MB catalogued, of which 27.9 MB is watermarks and 6.8 MB is download cache — none of which the old figure could see. Not addressed here: `.download-cache/all.zip` still has no TTL or size cap. It is now at least visible in the breakdown, which is what makes the case for capping it. * fix(admin): exclude the media share from local storage usage (#1164) External review found the walk could reintroduce the exact over-count it replaces. EXTERNAL_MEDIA_ROOT's compose default is `<storage>/external-media`, where the NAS is bind-mounted. That is a plain directory, not a symlink, so the symlink guard did not cover it and the walk descended into the share — putting every referenced original back into a figure whose whole purpose is to leave them out, and comparing NAS bytes against statfs() of the local disk. On the reference-mode installs this issue is about, that is the failure mode reappearing inside its own fix. The configured root is now skipped when it lies inside the storage root, and the result reports which path was excluded. A directory that merely shares the name is still counted, because those really are local bytes. Also from the review: - concurrent cold-cache callers now share one walk. /dashboard/stats, /storage/info and the sidebar are routinely requested together, and each was starting its own stat-per-file traversal of the whole library. - storage_partial is surfaced in the StorageInfo type and the sidebar tile, not just the dashboard and analytics cards. An unreadable subtree makes the total a floor, and a floor silently compared against a soft limit reads as "safely under". * fix(admin): do not report a disk walk on an S3 backend (#1164) Second review round. S3 installs were regressed. With STORAGE_BACKEND=s3 the originals, renditions, archives and download caches are objects in the bucket and STORAGE_PATH holds only incidental local files — so the walk reported near-zero and the soft-limit recommendation was derived from it. Those installs now keep the catalogued figure, which is the approximation they had before this PR, and the response says which measurement it is (`storage_measurement: 'disk' | 'catalog'`) so the UI labels it instead of implying a disk measurement that never happened. The Settings → Status storage card ignored storage_partial, formatting a lower bound as exact and deriving the limit percentage from it — so an unreadable subtree could read as safely under the limit. It now carries the same `+` marker as the sidebar and dashboard. * fix(admin): stop rendering an absent measurement as zero usage (#1164) Third review round, two findings. The analytics storage bar coerced a null measurement to 0, drawing an empty bar labelled "0% of limit" and suppressing the over-limit state — reading as plenty of room at exactly the moment nothing is known. It now shows the catalogued figure on S3, where that IS the available answer, and says "no measurement available" rather than inventing a percentage when there is none. /storage/info walked the filesystem before checking the backend and then threw the result away on S3. The sidebar polls that endpoint, so a migrated install still holding a large local tree paid a full stat-per-file traversal on every cold cache for nothing. Gated before the walk, as the dashboard route already was. * fix(admin): tell "no disk to measure" apart from "the measurement failed" (#1164) External review of the stable twin. Both were reported as `storage_measurement: 'catalog'`, so a failed local walk made the dashboard claim the objects live in S3. They are different things — one is a fact about the install, the other is a fault — and there is now an `unavailable` state for the second. The analytics percentage could reach the billions. `safeSoftLimit` fell back to `storageUsed || 1`, and on S3 that is null → 1, while the figure beside it came from `catalogedBytes`. An editor or viewer holds `analytics.view` but not `settings.view`, so `/storage/info` 403s for them and `storageInfo` is undefined — which is exactly when that fallback fires. It now falls back to the measured figure, and suppresses the percentage entirely when there is no real limit rather than dividing usage by itself and always reading 100%. Also lands the AnalyticsPage half of the previous round, which the commit message claimed but the commit did not contain — only its backend counterpart was staged. The stable twin has carried it since it was written, so this is the parity gap in the unusual direction. --------- Co-authored-by: Paul Nothaft <paul@MacStudio-von-Paul.local>	2026-08-26 08:53:39 +02:00
Paul Nothaft	1366d6d14c	fix(previews): preserve alpha and animation in the preview tier (#1171) ... * fix(gallery): stop the lightbox loading originals to display a photo (#1166) The lightbox read `preview_url`, which the server only emits once an admin has flipped lightbox_preview_enabled — off by default. So a stock install fell straight through to `url`, the untouched original: a reporter measured 16.5 MB for a photo whose preview is 345 KB. The lightbox renders its neighbours too, so opening one photo pulled three originals. `slideshow_url` is the same /preview/:id URL, watermark query included, and has been emitted unconditionally for images since #1015 — the slideshow never had a fallback worth taking. Preferring it fixes every existing install with no migration and no admin action, and `url` still backstops videos, where both derivative URLs are null. Verified on the local rig with the toggle off, so the photos API returns preview_url: null exactly as filed. Opening one photo: before GET /photo/82, /photo/81, /photo/21 (3 originals) after GET /preview/82?w=1280, /preview/81, /preview/21 397 KB -> 23 KB per image on that gallery's test photos. The toggle no longer decides whether the lightbox uses previews, so its copy said something untrue; it now describes what it still does, which is pre-generate rather than wait for the first guest to open a photo. Updated in en/de/fr/sl, the locales that carry those keys. * fix(gallery): cover the layouts the lightbox fix missed (#1166) External review found the fix was incomplete, and the review of it found one more. Premium galleries were untouched. PhotoGridWithLayouts returns early for gallery-premium, which builds its own yet-another-react-lightbox slides with `src: photo.url` — so those galleries kept pulling full originals and the reported bandwidth problem remained. They now use lightboxImageUrl for the display source; `download` deliberately stays on photo.url, because what a guest saves must be the original. The Story layout was worse, and neither the issue nor the review caught it: StoryPhotoCard rendered the full original as its GRID TILE, at object-cover in a small card. That is the one place where "hundreds of megabytes for a gallery" was literally true. It now uses the per-device thumbnail tier like PhotoCard, and its PhotoSwipe source uses the preview tier. Animated GIFs keep the original. generatePreviewImage always encodes JPEG, so routing an animated source through the preview tier would have replaced the animation with its first frame — a regression the toggle-off default never had. Animated WebP has the same problem and cannot be distinguished by MIME alone; that needs the backend to report it (Sharp's `metadata.pages > 1`) and is left rather than costing every static-WebP gallery the bandwidth fix. The settings copy claimed too much. "Pre-generate lightbox previews" does not generate anything on save — it unlocks the regenerate button and keeps preview_url emitted. Reworded to say that, in en/de/fr/sl. Not changed: the review's P1 said this bypassed the secure-image route on enhanced/maximum galleries. It does not. AuthenticatedImage collects requiresToken and secureUrlTemplate into an explicitly-voided unusedProps and never substitutes {{token}}, so on those protection levels photo.url was a literal `.../secure/82/{{token}}` that returns 400 — the lightbox was falling back to the 300px thumbnail, not to a protected image. Verified against a live maximum-protection gallery. Codex withdrew the finding on that evidence. * fix(gallery): keep premium downloads working and story framing intact (#1166) Second review round, three findings — two of them regressions this PR introduced. Premium Download became a no-op. handleDownloadFromLightbox recovered the photo with `filteredPhotos.find(p => p.url === slide.src)`, and slide.src is a derivative now, so the lookup found nothing and the button silently did nothing. The slide carries the photo id and the handler resolves by that; what Download hands over is still the original. Story cards were reframed. thumbnail_fit is seeded to 'cover' on every install, so thumbnails are square centre-crops — and story cards are not square (400x500 in the carousel, fixed-height in the desktop grid), so the card's own object-cover cropped them a second time and every photo shifted. They now use the preview tier, which is fit:'inside' and therefore the whole frame: the card looks exactly as it did before, without pulling an original. APNG joins the animated-format guard. It declares image/apng and the preview route would serve a static frame. Animated WebP still cannot be detected from MIME and remains the documented gap. * fix(gallery): keep PNG on the original, alpha and all (#1166) Third review round. generatePreviewImage encodes JPEG, which drops ALPHA as well as animation — a transparent PNG came back flattened against a solid background. And an APNG is normally reported as image/png, so the image/apng check alone missed the common upload path. PNG now stays on the original: it is where transparency is the norm, and rare enough in an event gallery that the bandwidth given up is small. Animated or alpha WebP still cannot be detected from MIME and remains the documented gap; it needs the backend to report Sharp's `pages`/`hasAlpha`. Two further findings are acknowledged and deferred rather than fixed here: - Story cards now request /preview on mount, so a cold gallery generates its previews in one burst. That is a new CPU cost, not a regression — those cards previously fetched full ORIGINALS on mount, which is strictly worse. Doing it properly means viewport-gating AuthenticatedImage, which is a change to a component every gallery surface uses and belongs in its own PR. - The premium layout memoizes slide URLs, so rotating the device before opening the lightbox can leave a photo on the tier chosen for the old geometry. The result is a slightly undersized image, and the fix is a resize subscription this PR does not otherwise need. * fix(gallery): load Story images on approach, and give the hero its own tier (#1166) Every card in a Story gallery mounts at page load — `whileInView` gates the animation, not the render — and AuthenticatedImage fetches from an effect on mount, so all of them requested at once. That was tolerable while they pointed at photo.url, because nothing was generated; pointing them at the preview tier meant a gallery with cold previews would Sharp-decode every original in one burst. The image now waits until the card is within 200px of the viewport, using framer-motion's useInView — the same observer the entrance animation already relies on — with `once` so a card never unloads on scroll-away. Verified on a 62-photo Story gallery: 3 images fetched at load, growing to 15 as you scroll, where all 62 would have fired before. While confirming that, the hero turned out to be doing the same thing the cards were. StoryHero rendered photo.url as a full-bleed object-cover background — a full original on the critical path for first paint of every Story gallery — when hero_url exists for exactly this and is a 1920x1080 cover crop emitted unconditionally for every photo (gallery.js:1139). That gallery now issues no /photo/ request at all: hero_url for the hero, the preview tier for the cards, and only as they come into range. * fix(previews): preserve alpha and animation in the preview tier Follow-up to #1166, which had to bypass the preview tier for GIF, APNG and PNG to avoid a visible regression. This removes the cause. generatePreviewImage encoded JPEG unconditionally. JPEG has no alpha channel and no second frame, so a transparent PNG came back flattened onto a solid background and an animated GIF came back as its first frame — for every consumer of this tier, not just the lightbox: the slideshow (#1015), admin previews, and the face avatars that read it as a whole-frame rendition. It was only invisible by default because the lightbox served originals. Sources with alpha, or more than one page, are now encoded as WebP, which carries both and is still far smaller than the original. Ordinary photos stay JPEG — the common path pays nothing. Two things had to move with it: - The output extension now matches what was written. A PNG source previously produced `preview_foo.png` holding JPEG bytes; harmless while the route hard-coded image/jpeg, wrong once the encoding varies. Existing keys keep working — they are still JPEG and still served as such. - The preview route derives Content-Type from the key. With `nosniff` set, mislabelling would show a broken image rather than being silently corrected. The watermark branch re-encodes to JPEG, so it labels itself explicitly; preserving animation through the watermark compositor is a separate problem. The frontend guess-by-MIME goes away entirely — including the case it could never get right, since a still and an animated WebP declare the same type. Verified on the local rig: a transparent PNG round-trips as `Content-Type: image/webp`, `hasAlpha: true`, 8.3 KB; an ordinary photo still serves `image/jpeg` from a `.jpg` key. * fix(previews): retire the legacy preview keys, and stop mislabelling watermarked ones External review of the stable twin found two defects, both on this branch too. Legacy keys collide with the new naming. The old generator kept the SOURCE basename verbatim while always writing JPEG, so a `.webp` upload produced `previews/preview_shot.webp` holding a JPEG. My PR body claimed "pre-existing keys have no .webp suffix and are JPEG" — that was simply wrong. The route now derives Content-Type from the key and the response carries nosniff, so every photo uploaded as WebP would have rendered as a broken image in the lightbox. Legacy `.png` keys are wrong the other way: flattened JPEGs of what may have been transparent sources, which isPreviewValid would have let stand forever. Migration 188 clears photos.preview_path outright — all of it, not just the suspicious extensions, because a `.jpg` key can equally be a flattened rendition and nothing in the key says so. Previews regenerate lazily on next view under the new encoder, so the cost is one regeneration per photo actually viewed. Storage is untouched, as elsewhere. The watermark branch mislabelled its output. applyWatermark PRESERVES the source format (watermarkService.js:200-211: png stays png, webp stays webp), and its input is the preview — so the output already matches the key the header was derived from. Forcing image/jpeg mislabelled every watermarked WebP preview, and nosniff means the browser would not correct it. The override is gone; the animation loss through the compositor is documented where it happens. * fix(gallery): make the Story hero fix actually work on external galleries (#1166) External review of the stable twin, both applying here too. hero_url was inert for external media. ensureHeroImage only ever called resolvePhotoStorageKey, which returns null for external/reference photos by design — and that null was handed straight to withLocalCopy, which throws, so the hero route caught it and redirected to the full ORIGINAL. #1078 fixed exactly this shape for ensurePreviewImage and nobody carried it across. It stayed invisible until this PR pointed the Story hero at hero_url: on a managed gallery that is a real saving, on a reference-mode gallery it quietly changed nothing. ensureHeroImage now has the same external branch ensurePreviewImage does — direct fs read, per-photo output basename — and returns null instead of throwing for a reference-mode row with no source_origin. The format bypass trusted mime_type, which is not trustworthy here. Migration 039 backfilled every pre-existing photo to image/jpeg regardless of what it was, and adminExternalMedia inserts rows with no mime_type at all — so a mislabelled PNG sailed past the guard and came back flattened. It now checks the filename extension as well. --------- Co-authored-by: Paul Nothaft <paul@MacStudio-von-Paul.local>	2026-08-26 08:53:05 +02:00
Paul Nothaft	77953c15c1	fix(gallery): stop the lightbox loading originals to display a photo (#1166) (#1169) ... * fix(gallery): stop the lightbox loading originals to display a photo (#1166) The lightbox read `preview_url`, which the server only emits once an admin has flipped lightbox_preview_enabled — off by default. So a stock install fell straight through to `url`, the untouched original: a reporter measured 16.5 MB for a photo whose preview is 345 KB. The lightbox renders its neighbours too, so opening one photo pulled three originals. `slideshow_url` is the same /preview/:id URL, watermark query included, and has been emitted unconditionally for images since #1015 — the slideshow never had a fallback worth taking. Preferring it fixes every existing install with no migration and no admin action, and `url` still backstops videos, where both derivative URLs are null. Verified on the local rig with the toggle off, so the photos API returns preview_url: null exactly as filed. Opening one photo: before GET /photo/82, /photo/81, /photo/21 (3 originals) after GET /preview/82?w=1280, /preview/81, /preview/21 397 KB -> 23 KB per image on that gallery's test photos. The toggle no longer decides whether the lightbox uses previews, so its copy said something untrue; it now describes what it still does, which is pre-generate rather than wait for the first guest to open a photo. Updated in en/de/fr/sl, the locales that carry those keys. * fix(gallery): cover the layouts the lightbox fix missed (#1166) External review found the fix was incomplete, and the review of it found one more. Premium galleries were untouched. PhotoGridWithLayouts returns early for gallery-premium, which builds its own yet-another-react-lightbox slides with `src: photo.url` — so those galleries kept pulling full originals and the reported bandwidth problem remained. They now use lightboxImageUrl for the display source; `download` deliberately stays on photo.url, because what a guest saves must be the original. The Story layout was worse, and neither the issue nor the review caught it: StoryPhotoCard rendered the full original as its GRID TILE, at object-cover in a small card. That is the one place where "hundreds of megabytes for a gallery" was literally true. It now uses the per-device thumbnail tier like PhotoCard, and its PhotoSwipe source uses the preview tier. Animated GIFs keep the original. generatePreviewImage always encodes JPEG, so routing an animated source through the preview tier would have replaced the animation with its first frame — a regression the toggle-off default never had. Animated WebP has the same problem and cannot be distinguished by MIME alone; that needs the backend to report it (Sharp's `metadata.pages > 1`) and is left rather than costing every static-WebP gallery the bandwidth fix. The settings copy claimed too much. "Pre-generate lightbox previews" does not generate anything on save — it unlocks the regenerate button and keeps preview_url emitted. Reworded to say that, in en/de/fr/sl. Not changed: the review's P1 said this bypassed the secure-image route on enhanced/maximum galleries. It does not. AuthenticatedImage collects requiresToken and secureUrlTemplate into an explicitly-voided unusedProps and never substitutes {{token}}, so on those protection levels photo.url was a literal `.../secure/82/{{token}}` that returns 400 — the lightbox was falling back to the 300px thumbnail, not to a protected image. Verified against a live maximum-protection gallery. Codex withdrew the finding on that evidence. * fix(gallery): keep premium downloads working and story framing intact (#1166) Second review round, three findings — two of them regressions this PR introduced. Premium Download became a no-op. handleDownloadFromLightbox recovered the photo with `filteredPhotos.find(p => p.url === slide.src)`, and slide.src is a derivative now, so the lookup found nothing and the button silently did nothing. The slide carries the photo id and the handler resolves by that; what Download hands over is still the original. Story cards were reframed. thumbnail_fit is seeded to 'cover' on every install, so thumbnails are square centre-crops — and story cards are not square (400x500 in the carousel, fixed-height in the desktop grid), so the card's own object-cover cropped them a second time and every photo shifted. They now use the preview tier, which is fit:'inside' and therefore the whole frame: the card looks exactly as it did before, without pulling an original. APNG joins the animated-format guard. It declares image/apng and the preview route would serve a static frame. Animated WebP still cannot be detected from MIME and remains the documented gap. * fix(gallery): keep PNG on the original, alpha and all (#1166) Third review round. generatePreviewImage encodes JPEG, which drops ALPHA as well as animation — a transparent PNG came back flattened against a solid background. And an APNG is normally reported as image/png, so the image/apng check alone missed the common upload path. PNG now stays on the original: it is where transparency is the norm, and rare enough in an event gallery that the bandwidth given up is small. Animated or alpha WebP still cannot be detected from MIME and remains the documented gap; it needs the backend to report Sharp's `pages`/`hasAlpha`. Two further findings are acknowledged and deferred rather than fixed here: - Story cards now request /preview on mount, so a cold gallery generates its previews in one burst. That is a new CPU cost, not a regression — those cards previously fetched full ORIGINALS on mount, which is strictly worse. Doing it properly means viewport-gating AuthenticatedImage, which is a change to a component every gallery surface uses and belongs in its own PR. - The premium layout memoizes slide URLs, so rotating the device before opening the lightbox can leave a photo on the tier chosen for the old geometry. The result is a slightly undersized image, and the fix is a resize subscription this PR does not otherwise need. * fix(gallery): load Story images on approach, and give the hero its own tier (#1166) Every card in a Story gallery mounts at page load — `whileInView` gates the animation, not the render — and AuthenticatedImage fetches from an effect on mount, so all of them requested at once. That was tolerable while they pointed at photo.url, because nothing was generated; pointing them at the preview tier meant a gallery with cold previews would Sharp-decode every original in one burst. The image now waits until the card is within 200px of the viewport, using framer-motion's useInView — the same observer the entrance animation already relies on — with `once` so a card never unloads on scroll-away. Verified on a 62-photo Story gallery: 3 images fetched at load, growing to 15 as you scroll, where all 62 would have fired before. While confirming that, the hero turned out to be doing the same thing the cards were. StoryHero rendered photo.url as a full-bleed object-cover background — a full original on the critical path for first paint of every Story gallery — when hero_url exists for exactly this and is a 1920x1080 cover crop emitted unconditionally for every photo (gallery.js:1139). That gallery now issues no /photo/ request at all: hero_url for the hero, the preview tier for the cards, and only as they come into range. * fix(gallery): make the Story hero fix actually work on external galleries (#1166) External review of the stable twin, both applying here too. hero_url was inert for external media. ensureHeroImage only ever called resolvePhotoStorageKey, which returns null for external/reference photos by design — and that null was handed straight to withLocalCopy, which throws, so the hero route caught it and redirected to the full ORIGINAL. #1078 fixed exactly this shape for ensurePreviewImage and nobody carried it across. It stayed invisible until this PR pointed the Story hero at hero_url: on a managed gallery that is a real saving, on a reference-mode gallery it quietly changed nothing. ensureHeroImage now has the same external branch ensurePreviewImage does — direct fs read, per-photo output basename — and returns null instead of throwing for a reference-mode row with no source_origin. The format bypass trusted mime_type, which is not trustworthy here. Migration 039 backfilled every pre-existing photo to image/jpeg regardless of what it was, and adminExternalMedia inserts rows with no mime_type at all — so a mislabelled PNG sailed past the guard and came back flattened. It now checks the filename extension as well. * test(gallery): the hero fixture follows the root-relative relpath contract (#1166) external_relpath has been resolved from EXTERNAL_MEDIA_ROOT rather than from event.external_path since #1163 landed. This fixture still carried the base-relative form — its own comment noted the change was 'a separate stack' — so the two tests stopped resolving and ensureHeroImage returned null the moment that stack merged. The production path was never affected. --------- Co-authored-by: Paul Nothaft <paul@MacStudio-von-Paul.local>	2026-08-26 08:46:29 +02:00
Paul Nothaft	05e23ef1a1	fix(admin): move the maintenance sweeps' run state into the database (#1181) (#1184) ... Both photo sweeps tracked whether they were running in a module-level variable. Correct on one replica, wrong behind a load balancer: the status poll answers from whichever process it reaches, so an idle replica reports isRunning false while another is mid-run, the UI re-enables the button, and the next POST lands elsewhere and starts a second pass over the whole library. The .whereNull() guards mean nothing is corrupted; the cost is duplicated S3/NAS I/O and an operator who cannot tell whether a job is running. Migration 189 adds one row per job. The claim is a conditional UPDATE whose affected-row count is the answer — the shape backgroundProcessor already uses to hand a photo to exactly one worker — so two replicas cannot both match. The lease is fenced on a per-claim token: taking over a stale claim does not stop the old runner, so without fencing a superseded runner finishing late cleared the new owner's flag and overwrote its result. heartbeat() reports renewal failure and the loops stop on it. Renewal runs on a timer spanning the claim through release, including the candidate query, because one hung NAS read can outlast the stale window inside a single iteration. maintenance_jobs is excluded from .picpeak archives — an archive taken mid-sweep would otherwise restore a live lease with no runner to release it. The importer filters the same set, so older archives are skipped too. Response shape is unchanged, so the frontend needs no change. Co-authored-by: Paul Nothaft <paul@MacStudio-von-Paul.local>	2026-08-26 08:45:41 +02:00
Paul Nothaft	410b8f8f6f	fix(external-media): record capture dates on import, and backfill existing libraries (#1172) (#1179) ... * fix(external-media): record capture dates on import, and backfill existing libraries (#1172) External imports never read EXIF, so photos.captured_at stayed NULL for every row they created. The gallery sorts "Date Taken" with COALESCE(captured_at, uploaded_at), which on a bulk import is the import timestamp — so the sort silently degraded into "order by import batch" with no error and nothing in the UI to say the sort key was missing. The reporter's 12-day trip came back with its first two days at positions 4204-5296 of 5555, because those folders happened to be imported second. - the import reads the capture date next to the sharp().metadata() call that already opens the file, so this costs one more read of the same source rather than a second pass over the mount. Best-effort like the dimensions: a source without EXIF imports with captured_at NULL, as before. - POST /api/admin/photos/repair-capture-dates backfills existing libraries, modelled on the dimension repair beside it — background pass, in-flight guard, status endpoint, and resolvePhotoFilePath, which is what reaches an external row at all. Not a migration: the originals sit on a mount that may be down at upgrade time, reading 8000+ of them would block the boot, and a run that found nothing has to be repeatable. - "no EXIF date" is counted separately from "could not read the file". An operator needs to tell "these files carry no date" from "the mount is broken" before deciding to re-run. - the update is guarded whereNull, so an import finishing mid-run is not overwritten by a slower pass. - every sort branch now carries photos.id as a tiebreaker, not just capture_date. A bulk import writes hundreds of rows inside one second, so uploaded_at and the COALESCE fallback both collapse and the grid reshuffles between loads. id is insertion order, which makes the fallback meaningful. Not addressed: extractCaptureDate reads no OffsetTimeOriginal, and exifr resolves a naive EXIF timestamp against the HOST timezone — so captured_at is not a true instant, and the same file imported on two machines yields two values. That predates this and applies to managed uploads equally; the tests here deliberately assert ordering rather than an absolute instant so they do not encode the bug. Worth its own issue. * fix(capture-dates): read managed originals through storage, skip archived, claim the run flag (#1172) Four holes in the backfill endpoint, all found in review: - Managed photos were resolved with resolvePhotoFilePath, which builds a STORAGE_PATH filesystem path. On an S3 install nothing is there, so every managed row failed. Now split the way the thumbnail regenerator does: external rows read from the mount directly, managed rows go through resolvePhotoStorageKey + withLocalCopy. - Archived events keep their photos rows but their originals are deleted on archive, so those rows failed every run and kept the button lit forever. Excluded from both the job and the status counts. - isRunning was claimed after the candidate query, so two concurrent POSTs could both pass the guard and start a pass. Claimed before the await, with every early exit releasing it. - The noExif comment promised a distinction extractCaptureDate does not make (it returns null for unreadable files too). Reworded to what it is. * chore: drop a stray node_modules symlink committed by mistake The .gitignore pattern is `node_modules/`, which matches a directory and not a symlink of the same name, so a local convenience link slipped past it. It pointed at an absolute path on one machine and would dangle everywhere else, breaking `cd backend && npm install`. * fix(capture-dates): gate the backfill as system maintenance, stop overstating the counters (#1172) The endpoint walks every event in the install and rewrites their metadata, but required only photos.edit — which the built-in team_photographer preset holds (175_granular_permissions_and_presets.js:106). That role exists for a contributing shooter, who should not be able to start a whole-library S3/NAS scan or touch another owner's photos. Now system.manage, with the status endpoint on system.view so the panel simply stays hidden for everyone else. The "without EXIF date" wording also promised a distinction the code does not draw: extractCaptureDate returns null for an unparseable file as well as for one that genuinely carries no date, so both land in that bucket. Reworded to "no date found" / "unreachable" in en, de and fr, which is what the two numbers actually separate. * docs: point the permission note at the follow-up PR (#1172) The dimension repair's matching gate landed in #1182, so the comment no longer needs to describe it as unaddressed. * fix(i18n): align the Slovenian capture-date wording with the other locales (#1172) sl was missed when the counters were reworded from 'without EXIF date' / 'unreadable' to what they actually measure. * fix(capture-dates): gate the status card on the permission the button needs (#1172) system.view and system.manage are independent grants, and StatusTab has no permission gate of its own — a successful status payload is what renders the card and its enabled button (StatusTab.tsx:637). Gating the status endpoint on system.view therefore handed a system.view-only role a live Backfill button whose every click 403s, with no error surfaced by the mutation. The comment above it already claimed this endpoint matched the POST. Now it does. * fix(gallery): make the Date Taken sort correct on SQLite (#1172) photos.captured_at does not hold one type on SQLite. Three writers put three different things in it: integer managed uploads — photoProcessor.js:488 hands knex a Date, which the sqlite3 binding stores as epoch milliseconds text external imports and the backfill, which write ISO-8601 null no capture date, so the sort falls through to uploaded_at, itself text in knex's 'YYYY-MM-DD HH:MM:SS' default shape A plain COALESCE over that is not an ordering. SQLite sorts INTEGER before TEXT unconditionally, so every managed photo carrying EXIF came back ahead of every photo that did not, whatever the dates said — a 2027 capture landing before a 2020 one. Among the text values 'T' (0x54) also outranks the space (0x20), so a same-day ISO 01:15 sorted behind a fallback 23:00. Both failures predate this branch — the first needs only two managed photos — but making that sort correct is what #1172 is about, so it is fixed here rather than left for the issue it belongs to. Normalised in the ORDER BY rather than by rewriting the column: the data fix would have to touch every existing row and every writer, which is a far heavier change than the sort it corrects. The cost is that this sort no longer uses idx_photos_captured_at on SQLite — an acceptable trade on the fallback engine, where the alternative is an index-assisted wrong answer. Postgres is untouched: captured_at is a real timestamp there and COALESCE already compares correctly. The regression tests drive the real gallery route on real SQLite. They write the epoch-millisecond integer directly, because the Date that produces it in production cannot be reproduced inside jest — there the binding's type dispatch misses sandbox Dates and stores "[object Object]" (CLAUDE.md). All four behavioural tests fail on the unfixed ORDER BY; verified by reverting it. * fix(gallery): normalise epoch-integer uploaded_at too, and stop polling a 403 (#1172) Two follow-ups from review. uploaded_at is not always text on SQLite either. A legacy archive restore leaves epoch milliseconds in it — there is a test pinning exactly that (__tests__/integration/sqliteEpochTimestamps.test.js) — and the fallback branch read it with substr(), so '1830297600000' was compared against '2020-01-01 00:00:00' as text and a 2028 upload sorted first. Both columns now get the integer/real branch. The status card also polled every ten seconds regardless of permission. With the endpoint correctly requiring system.manage, anyone who can open the Status tab but cannot run the job would have had a 403 and a logged denial every ten seconds for a panel they were never shown. The query is now gated on the same permission the endpoint requires, so it never starts. * style: quote convention in the capture-sort test (#1172) * fix(capture-dates): skip watcher-imported videos, and make the status counts consistent (#1172) Three follow-ups from review. fileWatcher.processNewPhoto sets type='video' and a video/* mime but never media_type (fileWatcher.js:128-130), so those rows keep the 'image' default from migration 048. Filtering on media_type alone queued every such video on every run — extractCaptureDate returns null for a video, captured_at stays null, and the backlog never cleared. Candidate query and status scope now check all three markers. The status counts were two separate queries, so an import committing a dated photo between them could be counted by the second and not the first: the card then showed withCaptureDate > total and a negative backlog, with the button enabled to "fix" it. One aggregate now. And the card's render checked only the cached payload. TanStack keeps that after `enabled` flips false, so a lower-privileged admin logging in behind a system.manage user inside the cache lifetime would still have seen the card and a button whose POST 403s. The permission is part of the render condition now. --------- Co-authored-by: Paul Nothaft <paul@MacStudio-von-Paul.local>	2026-08-26 08:43:13 +02:00
Paul Nothaft	9426ade2d3	chore(main): release 3.115.2-beta.0 (#1191) v3.115.2-beta.0	2026-08-26 06:42:21 +00:00
Paul Nothaft	a7b74bcd87	fix(external-media): store external paths from the media root (#1163) (#1168) ... * fix(external-media): store external paths from the media root (#1163) Importing a second folder into an event silently invalidated every photo already in it. photos.external_relpath was stored relative to events.external_path, and every import overwrites that column — so the older rows were rebased onto the new folder and their originals resolved to paths that do not exist. Nothing errored, and the grid still looked intact: thumbnails are written to local storage during the import while the base path is still correct. Only what needs the original broke — preview generation, the lightbox, downloads — which presents as a gallery that looks slow rather than one that is broken. The reporter had 7547 of 8004 rows pointing into the void and spent a while chasing it as a CPU problem. - external_relpath is now relative to EXTERNAL_MEDIA_ROOT, so a row is self-describing and nothing an admin does to the event afterwards can move an already-imported photo. - migration 187 folds each event's base path into its rows. Where the current resolution is missing on disk it walks up the base path for an ancestor under which the file IS there — the already-rebased case — and where it finds nothing it leaves the row resolving exactly where it resolves today. Skipped entirely when the media root is unmounted, since every file looks missing then. - the fold also runs after a .picpeak restore: knex_migrations is excluded from the archive, so a pre-#1163 backup would otherwise land base-relative rows on a migrated instance. - drops the duplicate-leaf-segment guess in photoResolver. It papered over this same double-prefixing and actively corrupts a root-relative path whose first segment legitimately repeats (base 'Trip', row 'Trip/x.jpg'). * fix(external-media): verify provenance and fold atomically (#1163) External review found four real defects in the fold. Repair could adopt the wrong file. Existence alone was accepted as proof that an ancestor candidate was the row's original — so a row whose file an admin simply deleted would adopt any same-named file one directory up (base `Trip/Sub`, relpath `photo.jpg`, an unrelated `Trip/photo.jpg`), and downloads would then serve a different photo. Worse than a dead link. An ancestor must now also match photos.size_bytes, which the import recorded from the very file the row describes; rows carrying no size are never repaired from an ancestor. The CURRENT base is still accepted on existence alone, because nothing is being inferred there — that is where the row already resolves. The fold was not atomic. Every UPDATE committed independently and the marker came last, so a process killed mid-fold left converted and unconverted rows with no marker — and the next run folded the converted ones a second time, putting every original one directory deeper with no undo. Probing is now a read-only first phase (so a slow cold NAS does not hold a write transaction open), and every rewrite plus the marker commit together. Failed rewrites certified a partial conversion. The per-row catch counted any error as a collision, carried on, and wrote the marker anyway — leaving that row in the old format for a resolver that now reads it differently. It also could not tell a genuine duplicate from a SQLite lock or I/O fault. Target collisions are now resolved in the planning phase, where they can be identified honestly, and a write that fails rolls the whole fold back. Restore ordering. The fold ran after the face requeue, with the worker live — so a worker could claim an external row while it was still base-relative, resolve it against the wrong path, and burn it to 'failed', a state only an explicit Re-scan clears. The fold now runs first, for the same reason the requeue already sat after restoreFiles. * fix(external-media): close the fold's remaining stranding paths (#1163) Second review round, three findings. A collision loser was left stranded. When an event imported one file through both `Trip` and `Trip/Sub`, two rows folded to the same path and the loser was skipped — keeping a base-relative value that the root-only resolver then reads as `<root>/<relpath>`, permanently wrong, with the marker claiming conversion was complete. It is a duplicate by construction, so it now goes through migration 186's deleteDuplicatePhotos, which reparents its feedback and marks and reconciles the face clusters instead of orphaning them. This branch is rebased onto #1162 for that helper. The other restore path had the same face-ordering bug. restoreService queued face scans in step 6, before step 7c runs pending migrations — so a pre-187 full or database restore handed the live worker rows whose paths were still event-relative, and it burned them to 'failed', a state the later fold does not clear. The requeue now happens after the migrations, where the files already are. A failed conversion was reported as a clean restore. The fold is transactional, so a failure leaves every external path in the old format under a resolver that reads from the media root — every original unreachable. It was logged as a warning and the restore returned success. It now returns externalPathsConverted/externalPathError, and suppresses the face requeue, which would otherwise mark those photos failed on top. * fix(external-media): make the fold safe against its own intermediate states (#1163) Third review round, four findings. A one-pass rewrite could collide with itself. Every FINAL path is distinct, but a final value can equal another row's CURRENT one — `photo.jpg` repairing to `Trip/photo.jpg` while the row already holding `Trip/photo.jpg` folds deeper — so the update violated migration 186's unique index halfway through. On Postgres that surfaces as 23505, which run-migrations-safe.js mistakes for "schema already exists" and records 187 as applied after the rollback, leaving every path unconverted with nothing to retry. Rows now park on a per-row staging value first, and migration 187 re-throws without the driver's code so the runner cannot misread it. The bulk update targeted rows the plan never saw. Phase 1 probes outside the transaction and can run for minutes; an import finishing in that window inserts an already root-relative row, and `where event_id` prefixed it again with the stale base. It now updates by the ids phase 1 captured. The restore UI never showed a conversion failure. The API carried externalPathsConverted, but PicpeakBackupCard neither declared nor read it and showed a green success either way — so an admin whose external originals were all unreachable was told the restore worked. restoreService requeued faces even when the migrations failed. The step 7c catch is deliberately non-fatal, so a pre-187 backup whose fold never ran still handed the live worker event-relative paths to burn to 'failed'. * fix(external-media): the fold's staging value must be storable on Postgres (#1163) External review of the stable twin caught this, and it was on both branches. The two-pass rewrite parks each row on a temporary value, and that value was written with a leading NUL. SQLite stores NUL in TEXT without complaint; Postgres rejects it outright — "invalid byte sequence for encoding UTF8: 0x00" — so migration 187 rolled back on exactly the installs that need the two-pass repair, and only on the engine most of them run. Restores hit the same wall and reported the conversion as failed. The prefix is ordinary text now. It still cannot collide with a real relative path and is still obviously wrong if a crash leaves one behind. Adds a gated Postgres test alongside the existing picpeakRestorePg one, because a SQLite-only suite structurally cannot catch this class: restoring the NUL makes exactly the two-pass repair case fail with that error, and nothing else. --------- Co-authored-by: Paul Nothaft <paul@MacStudio-von-Paul.local>	2026-08-26 08:36:45 +02:00
Paul Nothaft	36192708ab	chore(main): release 3.115.1-beta.0 (#1190) v3.115.1-beta.0	2026-08-26 06:34:49 +00:00
Paul Nothaft	06da1b9f7e	fix(external-media): one row per external file per event (#1162) (#1167) ... * fix(external-media): one row per external file per event (#1162) Two overlapping import-external runs against the same event inserted every file twice. The route checked for an existing external_relpath and then inserted, with an fs.stat and a sharp().metadata() read sitting in between — a window wide enough for both runs to see "not there". A reporter's event held 8004 rows for 6012 distinct paths. Nothing at the storage layer stopped it: migration 041 created only a NON-unique (event_id, source_origin) index. - migration 186 removes the duplicates that already exist and adds a partial unique index on (event_id, external_relpath). The survivor is the lowest id that has a thumbnail, so a half-finished import does not cost a grid tile, and hero references are repointed first because the FK is SET NULL. - the route treats a unique violation as a skip and carries on, so a second writer this process cannot see (another replica) converges instead of duplicating or 500ing. - a second import while one is already running now gets a 409 rather than walking the whole tree to have every insert bounce. The duplicates' thumbnail files are left behind as unreferenced bytes — a migration is the wrong place to reach into storage, which may be S3. * fix(external-media): keep dependent rows and legacy restores intact (#1162) External review found two real defects in the dedupe half of this fix. Dangling rows on SQLite. Every FK into photos declares ON DELETE CASCADE, but PicPeak never sets `PRAGMA foreign_keys = ON` — the codebase says so where it deletes an event (adminEvents/helpers.js:245) — so on every SQLite install the cascade is inert and deleting a duplicate photo left its face embeddings, guest feedback and admin marks behind, pointing at an id that no longer exists. Biometric data outliving its photo is exactly the invariant the event delete goes out of its way to hold. Dependents are now handled explicitly, and moved rather than discarded where they can be: the duplicates were separate tiles in the grid, so a guest's comment or an admin's rating could legitimately be on either, and dropping it inside a fix for silent data loss would be its own bug. Where the target already holds an equivalent row — the same guest's like, the same admin's mark, the same transfer's entry — the loser is dropped, because those tables mean one row per (photo, actor). photo_faces is the deliberate exception: both rows were scanned, so moving would duplicate every embedding and split the person clusters built from them. Legacy restores. Suspending FK enforcement does not suspend a UNIQUE index on either engine, so a .picpeak backup taken before migration 186 — carrying exactly the duplicates it removes — would hit the new index mid-batchInsert and roll the whole restore back, after every table had already been emptied. The restore now drops the index for the load and rebuilds it after running the same dedupe. Also: a failed CREATE INDEX is no longer swallowed. Recording the migration as applied without it leaves the install permanently racy, with nothing to trigger a retry. The shared work moves to services/externalPhotoDedupe.js, which the migration and the restore both call. * fix(external-media): reconcile derived state around the dedupe (#1162) Second review round, four more real findings. The index throw did not actually stop anything. run-migrations-safe.js treats 23505 as "schema already exists" and marks the migration applied (run-migrations-safe.js:138) — and a CREATE UNIQUE INDEX that finds duplicate rows raises exactly 23505 on Postgres. A replica inserting one between the dedupe and the index lock is a real rolling-deploy shape, and the outcome was the thing the throw was added to prevent. The index is now verified against the catalog afterwards, and failure raises a code-less error the runner cannot mistake for idempotence. Two people sharing a device were treated as one. photo_feedback carries both guest_identifier (per device) and guest_id (per person, migration 078), and feedbackService scopes by guest_id when present. Keying equivalence on the identifier alone deleted one of two different people's ratings. It now uses the same COALESCE rule the service does. Deleting faces raw left ghost people. event_people counts and centroids are derived from the photo_faces rows being removed, and #1132's separation snapshots hold a copy of each side's centroid — which is why faceProcessor exposes purgePhotoFaces and says it is "called from every photo-deletion path". The dedupe now goes through it. Reparenting feedback left the survivor's totals stale. photos carries denormalized feedback_count / like_count / average_rating / favorite_count and the later reaction and colour counts, so a survivor that now owns feedback kept rendering zero. updatePhotoFeedbackStats takes a trx so the dedupe can recompute on its own connection. Also: the equivalence-key delimiter was a literal NUL byte, which made git classify the whole file as binary and hide its diff. Escaped. * fix(external-media): stop the dedupe discarding half-states (#1162) Third review round. Five findings, four applied. - is_hidden joins the feedback equivalence key. feedbackService lets a moderator-hidden row coexist with the guest's visible replacement and counts only the visible one, so ignoring it deleted the visible row as redundant. - admin marks merge instead of dropping. rating and color_label are written independently, so the same admin can have rated one tile and coloured the other; the loser now hands over any field the winner has no value for. - a survivor that loses the only completed scan is requeued. Otherwise the purge takes the sole embeddings and nothing re-queues it — the photo just silently stops having a face. - view_count and download_count are carried over. Those are real interactions recorded per row, and dropping them quietly lowered the engagement the admin grid shows. Not applied: repointing a category hero can in principle land on a survivor in another category. It needs the two duplicate rows to have been re-categorised apart after the racing import, and the result is a cosmetic hero mismatch that the admin category routes already guard on write. Not worth the extra branch in a data migration. * fix(external-media): invalidate the download zip when duplicates are removed (#1162) External review of the stable twin. Applies to both branches. The pre-built "download everything" archive still contained the duplicate rows the dedupe had just deleted, so guests kept receiving them until something else happened to invalidate it. Every ordinary photo-deletion path calls downloadZipService.invalidate for exactly this reason. The columns are cleared rather than the service being called: that service carries debounce timers and a regeneration queue, which is not something a migration should start. getZipInfo already treats a cleared record as a cache miss and rebuilds on the next request, so this is the durable half of what invalidate does. The stale object is left in storage for the same reason the duplicates' thumbnails are — a migration is the wrong place to reach into a backend that may be S3. --------- Co-authored-by: Paul Nothaft <paul@MacStudio-von-Paul.local>	2026-08-26 08:28:48 +02:00
Paul Nothaft	bb5d496495	chore(main): release 3.115.0-beta.0 (#1158) v3.115.0-beta.0	2026-08-23 20:17:29 +00:00
Paul Nothaft	c305ad4146	feat(faces): make "not the same person" survive a re-scan (#1132) (#1145) ... A separation — an explicit dismissal, or the implicit one a Split records — was stored as a pair of event_people.id. Those ids do not survive re-derivation: recluster() deletes every person, and a full re-scan replaces a photo's faces outright, so face ids die too. The only thing that survives both is the embedding, so the decision is keyed on the two centroids the pair had when the photographer separated them. It binds while both sides still look like the clusters that were separated, and lapses once they have drifted past recognition. The constraint is now honoured at assignment time as well as in consolidate(), which is what makes it hold across a re-scan rather than being reformed before any later pass could object. Six review rounds shaped the matching itself: each candidate must resolve to the OPPOSITE side rather than merely matching something (a split leaves two similar halves, and the loose test fragmented the person the split was not even about); assignment judges both sides at the ordinary match threshold, since a single face — or a cluster of one part-way through a recluster — cannot resemble a settled centroid; separations carry their own model_version; and the projections are hoisted out of the innermost loop, which took a 2000-photo scan from ~15s of dot products to 0.23s. Lifecycle closed three ways: purgePhotoFaces re-anchors each side onto the live cluster it still describes and drops rows that describe nothing left, deleteEventCascade and the permanent archive delete clear the table (which deliberately has no event FK), and a later manual merge drops the separations it reverses. All of it matched on vectors rather than ids, since a row that has outlived a recluster names people who no longer exist. Merged with admin privileges: the author cannot self-approve.	2026-08-23 22:10:28 +02:00
Paul Nothaft	2c81888eaf	fix(gallery): a guest's own hidden feedback is hidden from them too (#1150) (#1153) ... Everything in the system treats a hidden row as absent — getPhotoFeedback drops it even for the guest's own feedback, and updatePhotoFeedbackStats does not count it. The per-viewer is_liked heart and my_color_label badge read the row without looking at is_hidden, so a like the photographer had hidden still showed as liked on a photo whose like_count was zero. Making those agree exposes why it had not been fixed: the duplicate check behind like/favorite toggling did not skip hidden rows either, so the now-empty heart, when clicked, found the hidden row and toggled it OFF — the click did nothing visible and the moderation was silently undone. Skipping hidden rows there makes the click create a fresh, visible row. Review found four more surfaces still treating a hidden row as present: the per-guest caps (an at-cap guest with one hidden met their own click with limit_reached), /my-feedback (which drives the Liked/Favorited/Rated chips in guest identity mode), getEventFeedbackSummary (disagreeing with the photo counters in the same response), and unhide (leaving two visible rows for one guest). The rating-clear and single-value delete scopes are visible-only now, so a follow-up mutation no longer destroys the admin's hidden record, and the unhide collapse is skipped when there is no stable identity to scope by — that fallback was 'guest_identifier IS NULL', i.e. other visitors' rows. Not taken: refusing to hide non-comment feedback, which the issue recommended. #839 and #1044 both ship hiding for reactions and colour labels with tests asserting a hidden one stops counting; only the admin UI's Hide button is comment-only. Merged with admin privileges: the author cannot self-approve.	2026-08-23 22:09:07 +02:00
Paul Nothaft	00b20b2d72	fix(gallery): guest filters respect show_feedback_to_guests, and marks survive a mid-write clear (#1147) ... Two follow-ups from the review of #1137. Filters were a second way to read hidden feedback. Every token on /photos is an OR of two halves: what THIS viewer marked, and what ANYONE marked. The response fields built from the second half — like_count, comment_count, color_label_count — are all gated on show_feedback_to_guests. The filter was not, so with the setting off a guest could still send ?filter=liked and get back exactly the photos other people liked, across all five tokens. Reachable by a direct API caller holding a gallery token; the frontend never sends filter to this endpoint. The half it left standing was also the wrong half. It read guest_identifier from the guest_id QUERY PARAMETER, which never matched anything — the frontend invents that string in localStorage and never sends it when submitting feedback, while submissions store generateGuestIdentifier(req). So gating the aggregate would have emptied these filters rather than narrowing them to 'mine', and accepting a caller-supplied identifier was a way back through the gate. Resolved from the request now, hidden rows excluded to match what the viewer can see. A mark whose row is cleared mid-write lost its value. #1137 fixed two calls both writing; this is one clearing while another sets. The clear empties the row, the row is deleted for being empty, and the setter's update matches nothing — the caller told 'no mark'. A zero-row update now reports itself and the caller re-reads, bounded at three passes, throwing rather than reporting a success that did not happen. Merged with admin privileges: the author cannot self-approve.	2026-08-23 22:07:50 +02:00
Paul Nothaft	e4a8be8e7e	fix(gallery): no Logout button on galleries that don't require a password (#1149) (#1152) ... showLogout was hard-coded true, so a gallery with no password showed a Logout button. Logging out of it is meaningless — no credential to drop, nothing to return to — and it stranded the visitor: GalleryPage's auto-login is a one-shot latch, so clearing the session left the page on its skeleton until a manual reload. That is the 'turns blank' in the report. The button is gated on requiresPassword || isClient || viaCustomer at both call sites. The full-page layouts render it on the callback being present rather than on a flag, so withholding the callback is how the gate reaches them. Session kind now comes from /auth/session rather than sessionStorage, which is per-tab while the cookie is per-browser: a gallery reopened in a second tab lost 'client' while the backend kept serving it as one. viaCustomer marks a portal token, which bypasses reveal mode and so is a credential that does not look like one. The public-gallery branch no longer returns the skeleton unconditionally — once auto-login has run and left us unauthenticated it shows the reason and a Retry. That state was otherwise unrecoverable, and it also swallowed loginError entirely. Merged with admin privileges: the author cannot self-approve.	2026-08-23 22:06:47 +02:00
Paul Nothaft	b581267031	fix(scripts): regenerate-thumbnails resolves external sources through ensureThumbnail (#1148) (#1151) ... The CLI fallback carried the defect #1129 fixed in the admin route: it computed `storage/events/active/<photo.path>` and fs.access'd it, a location that does not exist for external or reference rows. Every one failed the check and was counted as an error, so on an external-media install the script was inert while reporting one error per photo. Resolution now goes through ensureThumbnail, which already branches on source_origin and owns the per-photo ext<id>_ output name — sharing it is what stops the script and the route drifting apart again. Also: videos skipped on every marker they can carry (fileWatcher writes type and mime_type but never media_type), responsive tiers backfilled alongside the canonical rendition, skip-vs-generate asked from isThumbnailValid rather than inferred from an unchanged path, tier failures counted rather than swallowed, and a nonzero exit when the backfill was incomplete. The script is now importable with the CLI behind a require.main guard; it previously ran on require and called process.exit, so it could not be tested at all — which is why this survived #1129. Merged with admin privileges: the author cannot self-approve.	2026-08-23 22:06:11 +02:00
Paul Nothaft	66a9b5eba1	chore(main): release 3.114.0-beta.0 (#1146) v3.114.0-beta.0	2026-08-23 09:20:30 +00:00
Luca	e2844d1909	feat(gallery): colour labels for client proofing, and one global default per feedback type (#1044) (#1137) ... Colour labels for client proofing, plus the photographer's own stars and colours in the admin grid. - Guest colour labels alongside likes/reactions, opt-in per event (defaults off so live galleries do not change mid-proofing), with 'colors' and 'lightroom' keybind schemes. - One global default per feedback type, replacing the per-type scatter. - Admin marks live in their own table (photo_admin_marks) so they can never reach a guest-facing surface. - XMP export prefers a real label, keeping the rating-derived mapping as a fallback. Review: concurrent-write loss on the mark update path, migration index idempotency and error classification all fixed in 7139bcae; migrations renumbered to 182/183 in 8fecdfae after 180/181 were taken on main. Merged with admin privileges: bypass-size-gate is a required check that fails on size alone for review-bypass authors and never re-evaluates on review, which is its designed behaviour once a maintainer has approved.	2026-08-23 11:15:01 +02:00
Paul Nothaft	7b77bbf243	chore(main): release 3.113.0-beta.0 (#1144) v3.113.0-beta.0	2026-08-22 19:46:22 +00:00
Paul Nothaft	3583c924da	feat(faces): consolidate look-alike clusters after a scan, and suggest the rest (#1107) ... consolidate() has existed since #1074 and described this exact symptom in its own comment, but its only caller was recluster() — i.e. when an admin pressed Re-group people. After a normal background scan the centroids converged and nobody looked, so a gallery settled with 14 people that should have been 8. It now runs when a scan drains. There is no scan-finished event to hook, so an idle worker asks whether the events it touched have actually drained — 'a worker went idle' is deliberately not treated as sufficient, because with concurrency above one the others may still be working. The uncertain band asks instead of acting: pairs between the assignment threshold and the stricter auto-merge one surface as accept/dismiss suggestions, with sticky dismissals. Nothing merges silently — a pass that merged anything reports it and points at Split. Review rounds hardened it against overruling explicit decisions: it no longer absorbs ignored clusters (mergePeople ORs is_ignored onto the survivor, which would have hidden a real person), no longer merges dismissed pairs, no longer undoes a manual Split (which now records a separation), and no longer runs after detection is switched off. The dismissal read fails closed, a failed pass is retried with backoff rather than lost or hot-looped, and the new table follows event_people out of exports and backups. Name autocomplete needs no endpoint — the people list already open is the source, and it is event-scoped on purpose. Known limitation, tracked in #1132: separations are keyed on person ids, so a full re-scan loses them. Reported by @BraynArts.	2026-08-22 21:39:15 +02:00
Paul Nothaft	25fbefc703	docs(faces): link the face-recognition guidance from where people look (#1125) ... Every other feature routes readers to docs.picpeak.app. Face recognition was the one that either pointed somewhere else or pointed at nothing — poor placement for the feature with the highest read-before-you-enable burden anything here ships. .env.example referenced docs/feature-face-recognition.md, which does not exist — and creating it is not the fix, because .gitignore:89 ignores docs/feature-*.md outright, so the file would be invisible to anyone who cloned. That was the only pointer to legal guidance an operator got while editing the variables that turn Art. 9 processing on. Also: the README linked the sidecar's developer README for the feature name and had no row in the documentation table, docs/single-container.md left readers who wanted the feature nowhere to go, ml/README.md had no backlink, and the admin consent callout had no link at all. It does now, inline at the end of the obligation. Reported by @Luca-Timo.	2026-08-22 21:38:25 +02:00
Paul Nothaft	87115b28e8	fix(gallery): give masonry tiles their real shape back (#1130, #1131) ... Two independent causes of the same symptom — an aspect-ratio layout that does not lay anything out. gallery-premium discarded the tile height MasonryPhotoAlbum computed from photos.width/height and set height:auto on both card and image, so the rendered shape came from the intrinsic ratio of whatever rendition was served. With thumbnail_fit seeded 'cover' by migration 040 every rendition is square, so the layout drew identical squares and was indistinguishable from grid. The card now uses the height it is given and the stylesheet's existing height:100% applies. The bundled CSS templates pinned images to a fixed pixel height, which has specificity (0,1,1) and beats the .h-full utility (0,1,0) six of the seven layouts use. Elegant Dark is seeded is_default, so that was the out-of-the-box result for any layout other than grid/timeline. Migrations 052/053 corrected for fresh installs; 181 repairs the rows already seeded. Whitespace-tolerant because sanitizeCSS strips newlines from any template ever saved through the editor — an exact-text migration would have silently no-opped on most real installs. The height property is matched with a lookbehind so line-height/max-height/min-height are untouched, grouped selectors are handled, and nested rules are skipped rather than mis-rewritten. Both reported, measured in the live DOM, by @BraynArts.	2026-08-22 21:38:17 +02:00
Paul Nothaft	97d92f8428	fix(thumbnails): regenerate external photos instead of dropping their tiers (#1129) ... POST /admin/thumbnails/regenerate resolved every source as storage/events/active/<photo.path> and fs.access'd it. External and reference rows are not there, so every one failed and was counted as an error — and because the tier deletion runs first, the endpoint dropped every ?w= tier and rebuilt nothing, leaving the library worse than before it ran. The UI reported success either way. Now routed through ensureThumbnail, which resolves both source kinds, uses the per-photo ext<id>_ output name, and writes thumbnail_path back itself. Review rounds also removed both destructive deletes in generateThumbnail: the pre-delete ran before sharp opened the source, so an unreadable source left the previous rendition gone and the database pointing at it — across a bulk run, the whole gallery. Neither delete was needed, since put stages to a temp file and renames atomically and is the last statement in the try. Videos are filtered out, and the superseded rendition is removed only when the storage key actually moved, compared through the same canonicalisation the backends apply so a legacy backslash path is not mistaken for a different object. Reported by @BraynArts, who also identified the fix.	2026-08-22 21:37:49 +02:00
Paul Nothaft	f735d26422	fix(gallery): a missing thumbnail tier must not take the backend down (#1128) ... The first load of a gallery whose ?w= tiers do not exist yet could exit the Node process — not 500 one tile, kill the backend. Two defects stacked. The reader: LocalFsStorage.get() returns a lazy fs.createReadStream, so an ENOENT arrives after the await returned and outside the route's try/catch. An unhandled 'error' event is a process-level throw. pipeStreamToResponse attaches the handler the routes were missing — 404 for a vanished source, connection destroyed if bytes are already on the wire, file headers cleared so the JSON error is not served as image/jpeg or cached as a broken tile for an hour. Applied to all nine streaming responses in gallery.js. The writer: ensureThumbnailAtWidth passed regenerate:true, whose first act is to DELETE the target — on a path only reached when the tier is absent. A grid fires one request per tile, so one request unlinked the file another had just published and handed to a reader. Without the flag the write is an atomic rename. Generation is now also deduped per tier key: 8 concurrent requests ran 5 Sharp passes before, 1 after. Reported with a full diagnosis by @BraynArts.	2026-08-22 21:37:15 +02:00
Paul Nothaft	e243a88410	chore(main): release 3.112.0-beta.0 (#1139) v3.112.0-beta.0	2026-08-22 16:41:36 +00:00
Luca	7223118b89	feat(deploy): make the all-in-one image installable without a shell (#1124) ... The all-in-one image could not be installed from a GUI at all — the deployment it exists for. validateEnv treats a missing JWT_SECRET as critical and exits, and the documented run command supplies it with `openssl rand`, a shell command a Synology Container Manager or QNAP Container Station form cannot run. wait-for-db.sh now generates one on first start and persists it next to the database, extending the existing /run/secrets hydration rather than adding a second mechanism. Explicit env still wins, then /run/secrets, then the generated file. The write is load-bearing: JWT_SECRET is exported only when the file actually persisted, because an unpersisted secret would mint a new one every restart and sign every session out. Creation writes to a private temp file and hard-links it into place — atomic, fails with EEXIST when another container won, and the loser adopts the winner's value. Non-regular paths are rejected before the link, since POSIX ln links INTO a directory rather than failing, which would make a mistyped -v target unrecoverable. Also repairs the onboarding paths a new install actually walks: the installer no longer rotates the secrets of a running install on re-run, deprecates the dead scripts/install.sh in place, corrects the CONTRIBUTING dev loop, and fixes the vite proxy target that had been pointing at a stray local port since 0da45e69. Reviewed over three rounds. Co-authored by @Luca-Timo.	2026-08-22 18:37:12 +02:00
Paul Nothaft	8f23118782	chore(main): release 3.111.1-beta.0 (#1138) v3.111.1-beta.0	2026-08-22 16:29:22 +00:00
Paul Nothaft	24e11df299	fix(faces): dark-mode styling for the People surfaces (#1106) (#1126) ... Every component added by #1074 was styled for light mode only. In the admin dark theme the three toggle labels rendered invisible — including "Detect people in this gallery", the switch that starts GDPR Art. 9 processing — and the Manage-people modal rendered as a light panel over a dark page because its shell was a hardcoded bg-white. Admin surfaces pair each neutral with a dark: variant; guest surfaces read the gallery theme tokens, because galleries carry their own dark themes that the admin dark class knows nothing about. Beyond the issue's inventory: the cover picker and face-in-context viewer that landed after it was filed, the magnifier chip whose bg-white/90 would have carried light glyphs, PeopleSheet's own hardcoded bg-white shell, the selected avatar's white ring-offset halo, and both dismiss buttons whose hover darkened into the background. External review found one defect, fixed: the sheet's avatars ring against --color-surface, not the page background.	2026-08-22 18:24:13 +02:00
Paul Nothaft	2a43d95ff8	chore(main): release 3.111.0-beta.0 (#1123) v3.111.0-beta.0	2026-08-21 17:41:19 +00:00
Paul Nothaft	38c27d097c	feat(faces): show a detected face in its source photo, outlined (#1120) ... Phase 2 of #1096. Stacked on the phase-1 branch — it needs the Postgres fix there, or the face list this reads comes back empty. A 64px avatar answers "is this a person", not "is this the same person as that other cluster". The reporter's revised use for this is the pre-merge decision: who they were standing next to, what the occasion was. So the face opens in its own photo with the detected box drawn, and prev/next walks that person's other appearances without leaving the modal. The box is positioned in PERCENTAGES of the original frame, not measured pixels: the container carries the photo's aspect ratio, so the same four numbers land correctly at any rendered size, with no resize listener. Verified against real data before writing the component — bbox [221.9, 174.9, 294.5, 405.8] on a 750x750 frame resolves to left 29.6% / top 23.3% / width 39.3% / height 54.1% and lands squarely on the face. Preview rendition, never thumbnail, and that is load-bearing rather than a quality preference: thumbnail_fit is seeded to 'cover' on every install, so a thumbnail has had its edges cut off and ratios taken against the ORIGINAL land nowhere on it. That was #1100, and it presented as a broken detector. Not built on AdminPhotoViewer, deliberately. It wants full AdminPhoto objects (this endpoint returns photo_id + bbox + dimensions), it carries delete and category actions that are wrong for "who is this?", and there is no seam to draw the box. Three things review caught, all real: - The container had a height cap but no width cap, so a panorama derived its width from the aspect ratio and overflowed the modal sideways, taking part of the outlined face off-screen. - The per-tile affordance was hover-only, so on a tablet it was permanently invisible and there was no way to inspect a specific tile. - The row action opened index 0, which is the TOP-SCORING face — the same thing as the cover only until someone uses phase 1 to pick a different one, at which point the row showed one face and opened another. It now resolves to the cover's own index. Round 2 found three more, all real: - The counter called a list truncated whenever it hit 500, so a person with exactly 500 faces was told their complete list was capped. It now compares against total_face_count. - facesLoading goes false with an empty array on a zero-face person or a failed request, so the panel sat on a spinner that would never resolve. - Five 32px actions plus a 64px avatar exceed a 320px row, and the name is what got pushed out. flex-wrap alone did not fix it — the toolbar still claimed its max-content width first — so its basis is capped at small sizes and the buttons wrap to a second line instead. A cover that falls outside the capped list opens the first face instead. That case implies the list IS capped, so the truncation note already explains it — real pagination is a bigger change and is not in this. Verified end to end: picked the 5th of 13 faces as cover, and the row action opened at 5 / 13 rather than 1 / 13. Frontend suite 178 passing, build clean, no new type errors.	2026-08-21 19:36:59 +02:00
Paul Nothaft	ffc6bc1530	chore(main): release 3.110.0-beta.0 (#1122) v3.110.0-beta.0	2026-08-21 17:34:11 +00:00
Paul Nothaft	bbce3cd2a2	feat(faces): let the photographer choose which photo represents a person (#1119) ... Phase 1 of #1096. Clustering picks the cover, and its idea of a good one and a human's do not always agree. A cluster whose avatar is turned away or softer than the rest stays that way in the guest-facing people strip too, and nothing in the UI could change it. A picker reachable from each person row, reusing the face list the split dialog already loads — same query, same grid, different action on a click. Making the choice actually stick took four changes --------------------------------------------------------------------------- event_people.cover_face_id has existed since migration 177 and the PATCH already accepted it, so the first version of this was frontend-only. It was also a no-op: - facePeopleService.listPeople SELECTED cover_face_id and then discarded it, recomputing the cover as the best-scoring VISIBLE face on every read. The picker saved, said so, and the avatar reverted immediately. It now prefers the stored pick whenever this audience can see it, and falls back to the score-ordered choice otherwise — so visibility scoping still wins, and a guest is never handed a crop of a photo they cannot open. - recomputeCentroid overwrote cover_face_id unconditionally. It runs on rescan and on photo replacement, so any reprocessing silently undid a deliberate choice. It now keeps the chosen face while it is still a member of the cluster. - The face list is cached per person, and split/merge move faces between people. Until now the only reader closed itself after acting, so nobody saw the stale copy; the picker is a second reader of the same key. - cover_face_id meant two things. assignFaces seeded it with whichever face opened the cluster and recomputeCentroid overwrote it with the highest scoring one, so an automatic guess was indistinguishable from a deliberate choice — and honouring it would have pinned every UNCURATED person to that guess, which is worse than the fallback it replaced (the fallback is computed per audience and skips photos a guest cannot open). Both writers are gone, migration 179 clears the stored guesses, and the column now means one thing. That also removes the need to defend the choice against rescans: nothing overwrites it, and a dangling id self-heals to the derived cover. Clearing existing values is safe rather than destructive: no install has ever been able to SET a cover, so every stored value is an automatic guess by construction. Also fixes a PostgreSQL-only 500 --------------------------------------------------------------------------- GET /admin/events/:id/people/:personId/faces joined `photos` but did not table-qualify its WHERE, and photo_faces and photos BOTH have an event_id: column reference "event_id" is ambiguous Postgres refuses it, so the endpoint 500s and the Split dialog — its only consumer until now — has been broken on every PostgreSQL install since the join was added. SQLite resolves the ambiguity silently, which is why the suite stayed green. Reproduced against a real Postgres before and after. The query is now a named builder the route calls and the test imports, rather than a copy: an earlier version of that test re-declared the query, so the route could regress to the bare form while the assertions kept passing. Merge and recluster preserve the choice as well. Both already carried labels and privacy flags across; the chosen cover is human state of the same kind, so it now rides along — through a merge when the target has none, and through a recluster by following its FACE into whichever cluster ends up holding it, rather than the majority-descendant rule the label uses. The picker and the endpoint disagree past 500 faces, so the picker now says when it is showing a capped list rather than presenting it as exhaustive. Frontend suite 178 passing, backend 23 across the touched suites, build clean, no new type errors. Mutation-checked twice: dropping the cover preference fails the new listPeople test while the visibility-scoping test still passes, and restoring the auto-seed in assignFaces fails it too.	2026-08-21 19:26:52 +02:00
Paul Nothaft	887bdbe6e5	feat(gallery): responsive grid thumbnails (#1095) (#1109) ... * feat(gallery): responsive grid thumbnails (#1095) The half of #1095 that #1099 deliberately left out. Grid tiles are ~175 CSS px at the mobile 2-column default — about 530 device px on a DPR-3 phone — so the 300px thumbnail is upscaled ~1.8x and faces visibly mush. Backend mirrors the preview tiers exactly: ?w= on the gallery thumbnail route, whitelisted to 300/600/900, cached by width in storage, never written to photos.thumbnail_path, and keyed by photo id for every source type — basenames are not unique across events and a tier is served from a cache hit without re-reading the source, which is how the preview tiers nearly leaked one gallery's photo into another. The tier is in the ETag, or a client holding the 300px file gets a 304 for its 600px request. Cleanup and regenerate invalidation are wired the same way. generateThumbnail now takes width/height overrides; it keeps the configured `fit`, because the grid renders with object-cover and tiers that were framed differently would visibly jump as the viewport changes. The srcset only advertises tiers the SOURCE can fill. Thumbnails are generated withoutEnlargement, so a 400px original asked for 900 comes back at 400 — advertising "900w" would have the browser pick that candidate and upscale it, which is the reported softness made worse. That exact trap is why this was held back from #1099; the photo's own dimensions are now the guard, measured on the SHORT edge because thumbnails are square and a 4000x600 panorama can still only fill a 600 tile. A source that clears only one tier gets no srcset at all rather than a single pointless candidate. Two things this surfaced, both worth knowing separately: `npx tsc --noEmit` type-checks NOTHING in this project — the root tsconfig is `files: []` with project references, so the real command is `tsc -b`, which is what build:check runs. Under tsc -b the repo has 43 files with pre-existing type errors; this branch adds none, and the one error in a file I touched (PeopleManagerModal:91) is on main already and unrelated to the line I changed. * fix(gallery): wire grid tiers into the component that actually renders The srcSet landed in PhotoGrid.tsx, which nothing imports — GalleryView renders PhotoGridWithLayouts, and every grid layout funnels its tile through the shared PhotoCard. The frontend half of #1095 shipped nothing. Moved to PhotoCard, and switched from srcSet to a single sized URL, the same shape PhotoLightbox already uses for preview tiers. AuthenticatedImage fetches its src with the gallery bearer token and renders the blob; an <img> carrying a w-descriptor srcSet ignores src entirely, so that fetch would have been discarded and the browser would have issued its own — unauthenticated, and resolved against the page origin rather than the configured API host. One URL keeps the auth path and halves the requests. The tier comes from the tile's measured width via the IntersectionObserver entry, read on the same render that reveals the image so nothing is fetched twice. Column counts differ per layout and shift again with thumbnailScale, so the breakpoint table is only a fallback. Also closes what the tier cache leaked or served stale: - ensureThumbnailAtWidth short-circuits videos. Their thumbnail is a poster frame, so the tier path handed the video file to Sharp — after downloading it in full on S3, uncached, once per request. - The ETag names the tier actually served, not the one requested. A fallback to the canonical thumbnail was caching a 300px image under a 900px key. - Tier height scales from the configured aspect ratio instead of forcing a square; with fit:'cover' a 300x200 canonical and a 600x600 tier are two different crops and the photo reframed between tiers. - The canonical short-circuit compares against the configured thumbnail_width, not the 300 default, so a 600px install stops generating duplicate tiers. - Tier invalidation on /admin/thumbnails/regenerate, above the local-file check that skips S3 and external rows. - Tier cleanup in replacePhoto and deleteEventCascade. Both derive keys from the photo row, so the rows have to be read before they change or vanish. Preview tiers had the same two holes and are swept alongside. The clamp no longer drops a tier when the source falls between them: a 400px short edge asked for 600 returns all 400 pixels, where clamping to 300 threw 100 of them away. Backend 18 tier tests, frontend 22. Full suites green: 293 backend across the touched areas, 185 frontend, build clean, no new type errors. * fix(gallery): measure the tile, and stop regenerating the w300 tier Follow-up to the review of #1095. Closes the three items left open there, plus a defect the previous commit introduced. **The w300 tier regenerated on every request.** Decoupling the canonical short-circuit from the hardcoded 300 left generateThumbnail still tagging against DEFAULT_THUMBNAIL_WIDTH. On an install with thumbnail_width=600 a w=300 request wrote `thumb_<name>` while the caller probed for `thumb_w300_<name>`: the cache never hit, so every request re-downloaded the original and ran Sharp, and the file it left behind was in no cleanup list. The tag now follows the configured width, and thumbnailTierKeys lists all three widths — which one is canonical is a setting, so excluding 300 stranded exactly the file a 600-configured install generates. **The tier is chosen from the tile's measured width.** The observer entry only exists for `lazy` cards, and Mosaic, Masonry and Timeline don't pass it — Mosaic is 1-up on mobile where Grid is 2-up, so they are the layouts a breakpoint guess gets most wrong. Measured in a layout effect and gated: the image is not rendered until the width is known, so AuthenticatedImage never mounts with a src it has to replace. Attaching the observer ref unconditionally instead refetches every tile, since React flushes passive effects before the sync re-render a layout effect triggers — removing the gate makes the new single-request test fail, which is how that was confirmed rather than assumed. **Gallery Premium has its own card** and never reached the shared one, so its tiles kept pulling the canonical thumbnail. MasonryPhotoAlbum already hands the laid-out width to the render prop, so it needed no measurement. **Event rename orphaned tiers.** The key embeds the basename, so the DB update is the point past which the old keys cannot be derived. Dropped inside the filename-changed branch, not the loop body: unconditional would fire four storage deletes per photo on every rename, 20k calls against S3 for a 5,000-photo event that merely had its slug adjusted. Preview tiers had the same hole and are swept alongside. Carousel is the seventh layout and deliberately gets no tiering: its filmstrip thumbs are 80 CSS px, under the canonical 300 even at DPR 3. Tests: first PhotoCard suite (6), backend tier suite 21. Both new behaviours mutation-checked — reverting the width tag, the render gate, the measurement, or the rename sweep each fails a test. Full suites green: 298 backend across the touched areas, 191 frontend, build clean, no new type or lint findings. * fix(gallery): mount masonry cards once, into a measured layout Found while capturing screenshots for this PR, by attributing every thumbnail request to a photo id rather than eyeballing the grid. Masonry columns mode starts at 3 columns and runs its greedy distribution off a hardcoded 300px estimate until the container has been measured. Cards mounted into that guess are torn down when it settles — photos move to a different parent column, so React unmounts them — and since #1095 each mount picks its tier from its own width, the two mounts request two DIFFERENT urls. Measured on a 1440px desktop, production build, 62 photos: before 45 photos fetched at canonical AND w600, 17 stuck on w600 107 requests after 62 photos, canonical only, 62 requests Mobile was already landing on one tier either way, so both mounts produced the same url and the second was a cache hit — which is why it looked clean and the desktop case did not. The fix is the gate the rows/justified mode in this same file already applies for the same reason (line 346): hold the cards back until containerWidth is known. Only columns mode was missing it. Grid and Justified take their column counts from CSS breakpoints, so they have no transient measured value to discard and are unaffected. Worth noting this was NOT visible on main: without tiering both mounts request the same url, so the browser cache absorbs the duplicate. Tiering is what turns a harmless remount into a second download — the regression is this PR's, which is why it is fixed here rather than deferred. Frontend suite 194 passed (3 new). Mutation-checked: removing the gate fails the mount-once and placeholder tests. --------- Co-authored-by: Paul Nothaft <paul@MacStudio-von-Paul.local>	2026-08-21 19:26:21 +02:00
Paul Nothaft	00776234fd	fix(security): let cors() own Access-Control-Allow-Origin on protected images (#1118) ... Closes #1116. secureImageMiddleware set its own Access-Control-Allow-Origin, overwriting the one cors(corsOptions) had already computed. server.js:247 mounts cors() on all of /api with credentials:true, so by the time the route handler ran the correct header was already there — and the local assignment replaced it with a worse answer in BOTH directions: unresolved -> '*'. Combined with the credentials:true that cors() sets, that is an invalid pair browsers reject outright. Unreachable on Docker until #1104 stopped compose injecting FRONTEND_URL; reachable on a fresh install from then until the wizard stores general_site_url. resolved -> the frontend origin, even when the request legitimately came from the allowlisted ADMIN_URL. A split admin host got a header naming the wrong origin and the browser rejected a request cors() had allowed. Deleting the line fixes both. cors() already validates the request Origin against the allowlist, sets Vary: Origin, omits the header entirely for a disallowed or absent Origin, and pairs correctly with credentials. Methods, Headers and Max-Age stay here: they are route-specific and cors() does not contradict them. Observed against a running instance before and after: allowlisted Origin ACAO: <that origin> + Vary: Origin + credentials:true disallowed Origin no ACAO no Origin header no ACAO Six tests, mounted on a real Express app with server.js's middleware order. Deliberately NOT a unit test against a response double: the first version of this fix was a guarded assignment that looked correct in isolation and still overwrote cors() whenever an origin resolved. A double cannot see middleware composition, which is exactly how that slipped through. Mutation-checked both ways — restoring the original `|| '*'` fails 5 of 6, and restoring the guarded assignment fails 3 of 6 including the admin-origin case.	2026-08-21 19:25:56 +02:00
Paul Nothaft	d241919604	fix(ui): stop iOS Safari zooming in on 14px form fields (#1113) ... Closes #1105. iOS Safari zooms the whole page in when a focused form control computes to under 16px, and it does not zoom back out. Unlocking a gallery is a client-side transition rather than a document navigation, so the zoom the password field triggers carries straight into the gallery: the layout pans horizontally and the header actions sit off-screen until the visitor pinch-zooms out by hand. A real page load would have reset it. `.input` and `.input-themed` are `text-sm`, so the field is 14px on every phone, and GalleryPage inverts the breakpoint on top of that (`text-sm sm:text-base` — 14px below 640px, where iOS zooms, and 16px above it, where it never does). Keyed to the POINTER, not a width. The zoom depends on the computed font size and a touch device, never on how wide the viewport is — and a phone in landscape is 667-956 CSS px, above any width you could call "phone". Measured on the admin login page, which has no `sm:` override: main portrait 390x844 14px zooms main landscape 844x390 14px zooms main iPad 820x1180 14px zooms fixed all three 16px fixed desktop (mouse) 14px unchanged, no zoom off touch One media query rather than flipping each call site. `.input` alone backs 334 `<Input>` usages, but there are also ~440 raw inputs, selects and textareas carrying their own `text-sm`, and Tailwind utilities sit in a later layer than @layer components — so a fix at the component definition misses most controls and any new `text-sm` silently reintroduces the bug. The `:not()` on each selector is load-bearing, not decoration: it buys the specificity to beat a utility class. Measured in a browser — input.text-sm 16px (0,2,1 beats .text-sm) select.text-sm 14px (0,0,1 loses) textarea.text-sm 14px (0,0,1 loses) 24 selects and textareas in the tree carry `text-sm`, so the bare form would have left them zooming. Checkbox and radio stay excluded so font-size never sizes their box. max(16px, 1em, 1rem) is a FLOOR, not a size. A flat 16px would make controls that are already bigger smaller: Typography -> Large sets --font-size-base to 18px on body, so anything inheriting it would be clamped down and the setting quietly ignored. Each term covers a case the others miss: normal (body 16) 16px Large theme (body 18) 18px Small theme (body 14) 16px browser default 20px 20px The viewport meta is deliberately left alone: `maximum-scale=1` would suppress the zoom by disabling pinch-to-zoom for everyone.	2026-08-21 19:25:47 +02:00
Paul Nothaft	69edee36fc	ci: give the backend job headroom over its observed tail (#1108) ... The backend job normally finishes in about 3 minutes — the last eight runs on main were 2.6 to 3.4 — but it is the only one that boots Postgres and runs the full integration suite, so it is the only one exposed to runner contention. The observed spread has reached 9.2 minutes against a 10-minute cap, and release PR #1088 was cancelled at 10.3 with every test in the log passing and jest still running. That failure mode is expensive out of proportion to how often it happens: a cancelled job is a red X on a branch that is actually green, so it costs a diagnosis and a re-run each time, and it lands on release PRs because those are the ones that run when everything else does. The cap is a runaway guard rather than a performance budget, so 20 buys real headroom over the worst run seen while still killing a genuinely hung suite well inside the hour GitHub would otherwise allow. frontend and ml keep 10: they finish in seconds and have never been close. Co-authored-by: Paul Nothaft <paul@MacStudio-von-Paul.local>	2026-08-21 19:25:23 +02:00
Paul Nothaft	ba221ba2a0	chore(main): release 3.109.0-beta.0 (#1115) v3.109.0-beta.0	2026-08-21 10:58:27 +00:00
Luca	9431b9f094	feat(setup): configure the public address and SMTP in the wizard, not .env (#1104) ... * feat(setup): configure the public address and SMTP in the wizard, not .env A fresh install could not configure its own public address. `general_site_url` and the `email_configs` row already existed as admin settings, but nothing could reach them: - docker-compose.yml injected FRONTEND_URL=${FRONTEND_URL:-http://localhost:3000} and Dockerfile.aio baked in ENV FRONTEND_URL=http://localhost:3000, so getFrontendBaseUrl() returned on its first branch every time and the setting was never read. .env.example shipped the same value as an uncommented placeholder for FRONTEND_URL / ADMIN_URL / API_URL. - the wizard never asked for the address at all, and skipped its whole config step unless a CRM-ish feature was selected — so a gallery-only install was also never offered SMTP, despite gallery links, guest invites and expiry warnings all going out through email_configs. - eleven call sites read process.env.FRONTEND_URL directly rather than the resolver, three of them defaulting to placeholder hosts that reached real recipients: https://app.example.com in payment-reminder emails, localhost:3005 in admin invitation emails, https://app.example.com in dev template previews. Stop injecting a default anywhere, and resolve the origin instead: FRONTEND_URL -> general_site_url -> the origin the request arrived on -> whichever exists -> ''. A loopback candidate is treated as unconfigured so the installs that already have http://localhost:3000 baked into their environment self-heal; the same guard previously lived inline in routes/gallery.js for the slideshow QR (#848) and is now shared. The empty return is preserved because shareLinkService and the SSO redirects in routes/auth rely on it to emit relative urls — callers needing an absolute url use getAbsoluteFrontendUrl(), which still ends at http://localhost:3000. The wizard now persists window.location.origin right after the admin account is created, so an install that skips the rest still has a usable origin for background jobs that have no request to derive one from, and offers it as an editable "Public address" field. Settings -> General shows the field read-only when FRONTEND_URL pins it, instead of silently ignoring edits. Also drop the `|| 'mailhog'` fallback when seeding email_configs: that host only exists in the dev compose profile (which does not even start by default), so a fresh install came up with a live config pointing nowhere while the wizard showed empty SMTP fields. With no row, blank fields are the truth and emailProcessor logs "No email configuration found". Developers set SMTP_HOST=mailhog explicitly. backend/src/services/emailService.js is deleted: nothing in backend/ references it, and it was the only consumer of the SMTP_* variables, which misrepresented how mail is configured. Refs #705 * fix(setup): keep FRONTEND_URL ahead of ADMIN_URL/APP_URL when resolving links The previous commit routed two call sites through the resolver but put the site-specific variable FIRST, silently reversing precedence: userManagementService was: FRONTEND_URL || ADMIN_URL || localhost:3005 became: ADMIN_URL || resolver adminEvents/crud was: FRONTEND_URL || APP_URL || '' became: APP_URL || resolver An install with both variables set would have flipped which one won. Call the resolver first instead — it starts with FRONTEND_URL, so the original relative order is preserved and only the final fallback changes: localhost:3005 (not even the frontend's port) and '' (a relative link inside an email) both become the resolved origin. Refs #705 * fix(setup): unpin loopback FRONTEND_URL, keep ADMIN_URL/APP_URL reachable Review feedback on #1104. isEnvPinned() reported ANY FRONTEND_URL as authoritative, including the loopback values getFrontendBaseUrl() deliberately demotes. An install upgrading with the old compose default FRONTEND_URL=http://localhost:3000 therefore resolved its origin from general_site_url correctly, but got the Site URL field rendered read-only in Settings and skipped by the wizard's seeding - locking the exact operators this change exists to unblock out of configuring a public address anywhere. The predicate now mirrors the resolver, and the derived general_site_url_effective the General tab reads comes from the same helper instead of re-normalising process.env inline. APP_URL and ADMIN_URL had become dead code: getFrontendBaseUrl() only returns falsy when NOTHING is configured, so `|| process.env.ADMIN_URL` after it never ran once a site URL existed - which after this PR is the normal case. A split-origin install pointing ADMIN_URL at a separate admin host got invite links on the public gallery origin instead. They are now passed as an explicit `override` that resolves directly below FRONTEND_URL, preserving the historic FRONTEND_URL-before-ADMIN_URL order while beating the database- and request-derived fallbacks. general_site_url now feeds the CORS allowlist and the Access-Control-Allow-Origin header, not just email links, so a schemeless value is an allowlist entry no browser origin can match. Validate it server-side in PUT /general (isURL with require_protocol, require_tld off so LAN/NAS installs on http://nas:3000 still work) and client-side in both surfaces that write it - type="url" never fires in either, since neither input sits inside a form. Two more wizard fixes: the General tab no longer reposts general_site_url while it is env-pinned, because the field then holds the effective env value rather than the stored one and the round-trip read as a change to a protected key, 403ing a settings.edit-without-settings.domains admin on an unrelated save. And SetupConfigStep validates the From address before posting - /admin/email/config rejects a blank one, which used to surface as a generic warning while the wizard advanced from its finally block anyway, discarding every SMTP value the user had typed, password included. A failed save now keeps them on the step. * fix(setup): surface a rejected public address instead of swallowing it Review round 2 follow-up on #1104, pushed onto the branch. saveSiteUrl() caught and discarded every error. That was defensible before round 2 added a server-side URL check, but PUT /general can now answer 400 — and the two validators disagreed: http://my_nas.local client: accepted server: rejected http://foo_bar:3000 client: accepted server: rejected validate() let those through, the 400 was swallowed, `failed` stayed false and onDone() ran. The operator finished the wizard believing the public address was stored when nothing had been. That is the silent misconfiguration this whole change exists to remove, landing on the LAN and NAS installs it targets. Three parts: - saveSiteUrl() throws. finish() resolves it before anything else is posted and puts the message on the address field rather than the generic "some settings could not be saved" warning. Skip for now still always leaves, by contract, but warns instead of dropping the value in silence. - allow_underscores on the server check, for the same reason require_tld is off: browsers resolve http://my_nas.local and the client accepts it, so rejecting it server-side only produced the mismatch above. Both validators now agree across the LAN/NAS, IDN, bare-IP and scheme-less cases. - LOOPBACK_BASE_RE anchors its host token. Bare prefix matching also demoted https://localhost-nas.example.com, and now that this predicate gates the whole resolver rather than just the slideshow QR, being demoted means a configured address is silently ignored. 127. stays a bare prefix on purpose: all of 127.0.0.0/8 is loopback. Resolver suite 31 passing, up from 26. Mutation-checked: restoring the unanchored regex fails the three new host-boundary cases. * fix(settings): don't lock the General tab on a site URL nobody typed Review follow-up on #1104, pushed onto the branch. general_site_url was free-text until this PR added a server-side check, so an upgraded install can hold something schemeless that predates it. The tab flagged that on load, and `disabled={!!siteUrlError}` then killed Save for EVERY General setting. An admin holding settings.edit but not settings.domains could not clear it either: correcting the address is a change to a protected key and 403s. The tab has no permission gating, so that role was simply locked out of the tab with no self-service way back. That is the same role adminSettings.js:85-95 documents the no-op round-trip allowance for. The allowance only helps if the request is made, and this blocked it in the browser first. Validation now waits until the field is actually edited, and an unchanged value is dropped from the payload rather than reposted — matching what the env-pinned case already does one line above, and for the same reason. stored value invalid, untouched Save works, key not sent edited to something unusable Save blocked edited to a usable absolute url saved Four tests, first coverage for this feature. Mutation-checked: removing the dirty gate fails the untouched-value case. --------- Co-authored-by: Paul Nothaft <53005142+the-luap@users.noreply.github.com>	2026-08-21 12:54:14 +02:00
Paul Nothaft	7583b4f6b0	chore(main): release 3.108.1-beta.0 (#1103) v3.108.1-beta.0	2026-08-20 12:22:34 +00:00
Paul Nothaft	b3a7ab27ea	fix(faces): face avatars were cropped against a cropped rendition (#1100) ... * fix(faces): face avatars were cropped against a cropped rendition Found while triaging #1096, which reported the People manager showing unusable cluster covers — a bare shoulder, the back of a head, a patch of background — and asked for more sample faces to compensate. Most of that is not a detector problem and not a UI limitation. It is a bug. faceCropStyle positions an avatar by scaling the WHOLE frame and offsetting so the face lands centre. That holds only while the rendition shown is the entire image at a uniform scale. Thumbnails are not: imageProcessor.js:93 DEFAULT_THUMBNAIL_FIT = 'inside' migration 040:6 thumbnail_fit seeded to 'cover' imageProcessor.js:229 fit: settings.fit The 'inside' constant is only a fallback for a missing settings row, and the row is seeded on every install — so thumbnails are centre-cropped essentially everywhere, and every face avatar rendered against one is silently offset on any non-square photo. The reporter read the setting as safe because of that constant, and the code comment at :87-92 says the same thing; all three places disagree with what is actually stored. It presents as a bad detector, which is why it survived: the boxes are right, the frame they are drawn against is not. All three surfaces — the admin manager and the guest-facing strip and sheet — now read a preview, which uses fit: 'inside' and is therefore the whole frame. At w=640: plenty for a 64px avatar at DPR 3, and small enough that a strip of a dozen people does not pull a dozen 1920px renditions. Face scanning already calls ensurePreviewImage for anything it scans, so a preview exists for every photo that has a face. Adds the admin preview route the manager needed; the gallery already had one. Both whitelist ?w= the same way. The first version of the call-site test passed with every surface still reading thumbnail_url, because an import alone satisfied it. It now matches inside the src={...} expression, and each of the three surfaces was individually reverted to confirm the test fails. * fix(faces): size the face tier by bbox, and keep admin_preview auth The face half of the external review; the tier-key and long-edge fixes live on the #1099 branch this is stacked on. Face avatars used one fixed 640 tier. In a 6000px group shot a 200px face is ~21px there, and faceCropStyle then blows that up ~9x to fill a 64px avatar at DPR 3 — mush, and indistinguishable from the mis-positioning bug this PR exists to fix. The tier is now derived from the bbox's share of the frame, so a face across a hall gets 1920 and a close-up still gets 640. The synthesized face URL also dropped admin_preview. verifyGalleryAccess only accepts the admin cookie when admin_preview=1 is on the request (middleware/gallery.js:28), and the preview flow deliberately mints no gallery JWT — so every avatar 401'd in exactly the mode an admin uses to check a gallery before sending it to a client. --------- Co-authored-by: Paul Nothaft <paul@MacStudio-von-Paul.local>	2026-08-20 14:17:25 +02:00
Paul Nothaft	56034af0f0	chore(main): release 3.108.0-beta.0 (#1102) v3.108.0-beta.0	2026-08-20 12:14:09 +00:00
Paul Nothaft	011f6ae7ec	feat(gallery): sized preview tiers so phones stop pulling 1920px (#1095) (#1099) ... * feat(gallery): sized preview tiers so phones stop pulling 1920px (#1095) A phone can display ~1170px at most, but the preview tier is a single 1920px JPEG with no size parameter — so every lightbox swipe ships roughly twice the bytes it can use, and the slide track preloads neighbours, which multiplies it. On the reporter's all-external install a null preview_url falls back to the untouched NAS original, which makes it worse again. Backend: ?w= on the gallery preview route, whitelisted to 640/1280/1920. A whitelist rather than a free-form width because every distinct value is a permanent rendition on disk — an open parameter is an invitation to fill the volume. Unrecognised or absent values fall through to the canonical 1920 preview, so old clients and hand-typed URLs behave exactly as today. Extra tiers are cache, not state: ensurePreviewImageAtWidth keys them by width, looks them up in storage and generates on miss, and never writes photos.preview_path. That column owns the canonical rendition, and threading a width through it would mean the last size anyone requested silently becomes "the" preview. Requesting 1920 resolves to the existing preview rather than a w1920 duplicate, so no install grows a second copy of every preview it already has. The tier is part of the ETag. Without it a client holding the 1920 rendition gets a 304 for its 640 request and renders the wrong size, which is this feature inverted. Frontend: the lightbox picks a tier from innerWidth x devicePixelRatio, capped at DPR 3 — uncapped, a DPR-10 device asks for 3900px and lands straight back on the desktop rendition. At the top tier the URL is left byte-identical so existing caches and ETags stay valid and desktop sees no change at all. saveData and a 2g/3g effectiveType drop one tier; both are Chromium-only, so they are a bonus rather than the mechanism. Grid thumbnails are NOT tiered here, deliberately. generateThumbnail resolves its width from admin settings rather than an argument, so tiering it is a separate change — and shipping a srcset whose candidates the server ignores would be worse than shipping none: the browser would take the "600w" candidate, receive the 300px image and upscale it, which is the reported softness made slightly worse. That half of #1095 lands separately. * fix(gallery): scope tier keys per photo, size by long edge, clean up tiers External review. Three findings against the tier work, one a cross-gallery leak. The tier cache key was the photo's BASENAME. Managed uploads keep camera basenames, so two events can each hold an IMG_0001.jpg — and a tier is served straight from a cache hit without re-reading the source, so the second gallery gets the first gallery's photo. Keys are now scoped by photo id for every source type. The RAW branch passed proc.outputBasename, which would have dropped that scoping again; it now passes the scoped name. Tier selection used viewport WIDTH, but ?w= bounds the LONG edge (fit:'inside'). On a 390x844 phone at DPR 3 a 2:3 portrait is bound by height and renders ~1755 device px, so width-only picked 1280 and made portraits softer than today; landscape on the same phone needs ~1170. It now computes the rendered long edge from the photo's own dimensions and falls back to the top tier — today's behaviour — when they are unknown. Tiers live outside photos.preview_path, so nothing else knew they existed: delete, bulk-delete and archive left them orphaned in previews/ forever, and regenerate-previews refreshed only the canonical rendition while phones kept the stale copy. previewTierKeys derives them from the same deterministic scheme and all four paths clean up. Deliberately outside the preview_path guard — a tier can exist when the canonical rendition never did, so keying cleanup off preview_path would strand precisely the photos only ever viewed on a phone. The existing tier tests encoded the old width-only semantics and were updated rather than kept; that is a behaviour change, not a test fix. --------- Co-authored-by: Paul Nothaft <paul@MacStudio-von-Paul.local>	2026-08-20 14:08:07 +02:00