feat(spaces): in-space album management + shared-space-album scope helper#752
Open
Deeds67 wants to merge 280 commits into
Open
feat(spaces): in-space album management + shared-space-album scope helper#752Deeds67 wants to merge 280 commits into
Deeds67 wants to merge 280 commits into
Conversation
github-actions
Bot
added
the
documentation
Adds /spaces/:spaceId/albums: SpaceAlbumCard grid with link/unlink/toggle-timeline handlers relocated from space-linked-albums.svelte. Editors/Owners get the Link-album inline picker; Viewers see the grid read-only. Mirrors People sub-page shell (UserPageLayout, load pattern, isEditor derived).
Route /spaces/:spaceId/albums/:albumId=id opens a linked album inside the space chrome. The +page.ts verifies the album appears in getSharedSpaceAlbums before calling getAlbumInfo — an unlinked album owned by the user redirects to /spaces/:id/albums instead. The +page.svelte renders the album Timeline (enableRouting=false, options via buildAlbumTimelineOptions + grouping:month) with a canManage-gated "Add photos" button (data-testid=add-photos-button) for Task 7 to wire.
…T19) - Add 7 e2e cases to shared-space-album.e2e-spec.ts proving the checkSpaceLinkedAlbumReadAccess grant: · space viewer GET /albums/:id linked album → 200 · non-member GET /albums/:id → 400 · space member GET /albums/:id for non-linked album → 400 (boundary) · space viewer GET /timeline/buckets?albumId → 200 · space viewer GET /timeline/bucket?albumId → 200 (content path) · non-member GET /timeline/buckets?albumId → 400 · re-assert: editor PUT /albums/:id/assets still → 200 - Regenerate server/src/queries/access.repository.sql against the running immich_postgres (all fork tables present). Diff is scoped to 12 additions: new checkSpaceLinkedAlbumReadAccess block only. No lines dropped from any existing query. e2e deferred to CI (e2e compose stack not running locally).
…ry grouping in timeline options) The grouping control updated the timelineGrouping state + the Timeline grouping prop, but the TimelineManager builds buckets from options.grouping — which was missing from the browse options. Carry grouping: timelineGrouping in the options (matching the global album page). TDD: a clickable grouping-control mock + tests asserting options.grouping (default day, and month after a control click).
… control-bar overlap) Render the asset-picker (add mode) as a fixed full-screen overlay outside UserPageLayout, with ControlAppBar at the top and the picker Timeline below it in a pt-(--navbar-height) main — matching the layout of the global album page. The browse Timeline is now gated to mode==='browse' so it is never double-mounted under the overlay.
…fter rebase) The rolling/main fix that dropped the bordered grey/navy grouping-control bar in favour of a transparent container was applied to the space timeline page but not to the in-space album detail page (separate file). Port the same styling here so the Years/Months/All control sits on a transparent bar. Adds a regression test asserting the bar uses bg-transparent (not bg-gray-50/ dark:bg-gray-900/border-b).
…order) The add-photos overlay rendered the ControlAppBar BEFORE the full-height <main>. ControlAppBar is position:absolute with auto z-index, so <main> painted over it and swallowed clicks on the trailing Upload/Add-assets buttons (only the close X, outside main's content, still worked). Render <main> first and the ControlAppBar after it — matching the global album page — so the absolute bar paints on top while staying pinned to the top. Adds a DOM-order regression test (bar must follow the timeline-main).
…ge, drop dead album-reset)
…e 15) THE ONE INTENTIONAL BEHAVIOR CHANGE in this branch — reviewable/revertable on its own. Four sites resolved a linked-album access path WITHOUT the album.deletedAt IS NULL guard (A1), so an asset reachable only via a SOFT-DELETED album was still treated as in-space — leaking/retaining its faces and mis-attributing its adder: - getAlbumAssetIdsWithoutOtherSpacePath (branch 2) [review finding #5] - getAssetIdsWithoutOtherSpacePath (branch 2) [review finding #5] - isFaceInSpace (album leg) [newly found by this work] - getSpaceAssetAdder (album leg) [newly found by this work] Each album leg now joins album with deletedAt IS NULL, matching isAssetInSpace and the shared album-scope helper's default. RED-first: the 4 characterization assertions were flipped to the correct post-fix expectation, failed on the prior commit, and pass now (211 shared-space tests green). Spec: data/sa-abstraction-spec-t8/report.md
…ame) Prettier-format 4 space-album-scope files and switch the guard's server-root resolution from __dirname (banned by unicorn/prefer-module) / import.meta.dirname (invalid under CJS) to process.cwd() (matches face-identity-query-shape.spec.ts). No behavior change. Local: format --check clean, eslint --max-warnings 0 clean, tsc 0 errors, 4752 unit tests pass.
…s any surface (#754) * docs(spaces): RBAC visibility hardening spec + progress ledger * feat(spaces): canonical space visibility gate + raw-SQL showInTimeline option - Add spaceVisibleAssetVisibilities (single source of truth for [Archive, Timeline]) and spaceVisibilityGate() helper to shared-space-album-scope.ts - Add requireShowInTimeline option to SpaceAlbumAssetSqlOptions / spaceAlbumAssetExistsSql, matching the existing Kysely helper behavior - Consolidate the three local constant declarations in shared-space, face-identity, and person repositories to alias the canonical export * fix(spaces): space + linked-album downloads exclude hidden/locked assets Add spaceVisibilityGate(eb) to all three arms of downloadSpaceId (direct, library, album) and to downloadAlbumId, so bulk-download streams honour the canonical space-visible rule (Timeline + Archive only; Hidden + Locked never). Medium tests cover all four visibility values per arm, the added-then-flipped replay edge case, isOffline library exclusion, and the Locked-in-album invariant (auto-stripped by asset.service.ts:313, documented in test comments). * fix(spaces): space asset read/view/download gate excludes hidden/locked Apply spaceVisibilityGate (asset.visibility IN (Archive, Timeline)) to all three arms of checkSpaceAccess (direct / library / album). Hidden and Locked assets were previously readable by any space member by ID — full detail and original bytes. The caller's own Locked assets remain accessible via checkOwnerAccess (hasElevatedPermission) which runs before checkSpaceAccess in access.ts and is unaffected by this change. Covers livePhotoVideoId: a Locked parent's video part is no longer reachable via the livePhotoVideoId OR-match since the parent row is filtered out before the ID expansion step. Added-then-locked replay (space row survives a visibility flip) is also blocked at read time. Tests: 9 new medium specs (direct / library / album paths, livePhotoVideoId, own-asset non-over-block, added-then-locked replay, isOffline, soft-deleted album); 6 new e2e tests (Locked/Hidden/Timeline/Archive × GET /assets/:id + /original + /thumbnail); updated Test 6 in shared-space-album.service.spec.ts to assert the fix. tsc + eslint 0 + prettier unchanged. Note: checkSpaceAccess is @GenerateSql-decorated; its generated SQL changed. SQL-doc regen must be run centrally with the correct DB (not make sql locally). * fix(spaces): shared-space sync streams exclude hidden/locked assets and exif Gate all four shared-space asset sync streams (SharedSpaceAssetSync, SharedSpaceAssetExifSync, SharedSpaceAlbumAssetSync, SharedSpaceAlbumAssetExifSync) behind spaceVisibilityGate so that Hidden and Locked assets — and their EXIF/GPS — are never emitted to space members. The two EXIF classes lacked an `asset` join required to filter on `asset.visibility`; add `innerJoin('asset', ...)` to each of their backfill/creates/updates builders before applying the gate. The album-asset stream (SharedSpaceAlbumAssetSync) intentionally does NOT apply requireShowInTimeline — it is a browse/GRANT surface, not a personal-timeline projection. A showInTimeline=false album's Timeline assets must still sync; only visibility-gated. Nine new medium-spec assertions confirm the red→green path across all four streams (backfill, creates, updates), the showInTimeline invariant, isFavorite masking unchanged, and update-path suppression for a flipped asset. * fix(spaces): purge direct space assets from member devices when hidden/locked * fix(spaces): filter suggestions/facets/tags to shareable space assets Apply the same M3 visibility gate to applySuggestionScope (spaceId and timelineSpaceIds branches), getAccessibleTags, and tag.repository.getAll that searchAssetBuilder already uses. Other members' Hidden/Locked assets no longer leak their city/country/make/tag metadata through the suggestions and filter-facets endpoints. The album-path arm also gains requireShowInTimeline=true so showInTimeline=false linked-album assets are excluded. Own-M3 is preserved: the caller's own assets bypass the space-visibility gate. @GenerateSql-decorated methods whose SQL changed: getStates, getCities, getCameraMakes, getCameraModels, getCameraLensModels, getAccessibleTags, getFilterSuggestions (search.repository.ts); getAll (tag.repository.ts). * fix(spaces): representative-face thumbnail only from shareable space assets Guard all three arms of isFaceInSpace against non-shareable assets: - Direct arm: add asset join + deletedAt/isOffline/visibility guards - Library arm: add spaceVisibilityGate (deletedAt/isOffline were present) - Album arm: add spaceVisibilityGate + showInTimeline=true gate Before this fix a space person's auto rep-face thumbnail could be served from another member's Hidden, Locked, soft-deleted, or showInTimeline=false album asset. isFaceInSpace is the gate in getSpacePersonThumbnail's auto path; all three arms now mirror the shape of isAssetInSpace. 17 new medium tests (direct/library/album arms × visibility scenarios + added-then-flipped replay + Archive-still-serves + manual-path regression). * fix(spaces): linked-albums endpoint no longer exposes album-user emails * fix(spaces): scope person-asset ids and space-asset attach to shareable assets getPersonAssetIds now joins shared_space_person to resolve the space and filters via OR-of-three-paths (direct/library/album) with asset.deletedAt, isOffline, visibleSpaceAssetVisibilities, and requireShowInTimeline — mirroring the getSpaceRepresentativeFaces scoping pattern. checkSpaceAccessForSpace gains spaceVisibilityGate(eb) on every union arm, matching the Slice-3 checkSpaceAccess fix. Hidden and Locked assets are now excluded even when a space-specific asset-attach call is made by a member. * fix(spaces): de-timelined albums stop feeding faces/memory/view and stay consistent on toggle * refactor(spaces): search shows shared archive (match timeline); edit gate matches read gate Slice 10 of shared-space RBAC hardening: 1. searchAssetBuilder (database.ts): change BOTH other-member visibility terms from `= Timeline` to `spaceVisibilityGate` (IN Archive,Timeline), covering both the spaceId path (~:673) and the timelineSpaceIds path (~:685). Caller-owned branch (`asset.ownerId IN userIds`) is untouched. 2. checkSpaceEditAccess (access.repository.ts): add spaceVisibilityGate to both union arms (shared_space_asset + shared_space_library) so an editor cannot AssetUpdate another member's Hidden/Locked asset via the space path. Mirrors the Slice-3 checkSpaceAccess fix. map/memory/view repositories are intentionally unchanged (Timeline-only by design). @GenerateSql methods changed: checkSpaceEditAccess * test(spaces): exhaustive RBAC visibility matrix + structural regression guard Slice 11 regression-lock for the shared-space RBAC hardening (Slices 1-10): SCAN 1 (existing, extended): - Added getMapMarkers (shared-space.repository.ts variant) back to album-leg allowlist with a pre-existing gap note; fixed earlier premature removal. - Restored SQL keywords to NON_DECL to prevent misidentification of raw sql\`...\` template literal bodies as TypeScript function declarations. SCAN 2 (new — visibility gate): - Independent scan in shared-space-album-scope.guard.spec.ts verifying that every shared_space_asset / shared_space_library / shared_space_album read references the visibility gate (spaceVisibilityGate, visibleSpaceAssetVisibilities, AssetVisibility.Timeline/Archive). Calibrated allowlist for infrastructure functions that legitimately read space tables without asset-data exposure. MATRIX SPEC (new): - server/test/medium/specs/repositories/shared-space-visibility-matrix.medium.spec.ts - 26 tests across 17 RBAC surfaces: download, access, sync, search/tags, timeline, map markers, memory, view/folders, people-facets. - Uses real repositories via newMediumService; seeds {Timeline, Archive, Hidden, Locked} × {direct, library, album(shown), album(noTimeline), deleted-album}. RED CELLS (real gaps, NOT weakened): - getPersonsBySpaceId does NOT apply asset visibility gate to base query: persons backed only by Hidden/Locked asset faces are still surfaced. → 3 tests fail: Hidden-only, Locked-only, and added-then-flipped scenarios. This gap is in shared-space.repository.ts:getPersonsBySpaceId (no spaceVisibilityGate on the base selectFrom). Fix needed in a follow-up slice. E2E NEGATIVES (new): - e2e/src/specs/server/api/shared-space-visibility-negatives.e2e-spec.ts - Non-owner member asserts Hidden/Locked space assets return 400 on GET /assets/:id and GET /assets/:id/original, and are absent from POST /download/info manifests (spaceId and albumId-via-space-grant). Positive controls for Timeline/Archive confirm gating is selective. * fix(spaces): space people-list excludes persons with only hidden/locked faces getPersonsBySpaceId: make the visible-face EXISTS gate unconditional (was only applied when takenAfter/takenBefore was set, so the default People tab listed persons whose faces are all on Hidden/Locked/out-of-scope assets). Date bounds stay conditional inside the always-applied EXISTS. countPersonsBySpaceId: rename datePersonFilter → visibleFaceFilter and always apply it; the asset_scope CTE already enforces visibility so the date bounds are handled implicitly. getPeopleFaceStatisticsBySpaceId and getSpacePersonStatistics: already unconditional via asset_scope CTE — no change needed. Update existing tests that were inadvertently relying on the missing gate (no newSharedSpaceAsset calls, no addPersonFaces — persons with no in-scope visible faces were wrongly returned). Add faces/space membership to each fixture. For the soft-deleted-face test, add a second visible face so the person correctly remains listed. Add new matrix tests: count-consistency assertions (countPersonsBySpaceId excludes Hidden/Locked-only persons), mix regression (person with ≥1 visible face IS listed), and Hidden/Locked count exclusion. @GenerateSql methods changed: getPersonsBySpaceId, countPersonsBySpaceId. * chore(spaces): regenerate SQL query docs for RBAC visibility gates * chore(spaces): regenerate OpenAPI spec + SDK/dart for trimmed SharedSpaceLinkedAlbumDto * docs(spaces): prettier-format RBAC hardening spec * style(e2e): prettier-format shared-space visibility e2e specs * fix(spaces): timeline buckets exclude other members' hidden/locked even with explicit visibility * fix(spaces): library sync excludes other members' hidden/locked assets and exif LibraryAssetSync and LibraryAssetExifSync had no visibility gate, so a space member syncing LibraryAssetsV1 / LibraryAssetExifsV1 received the full asset row (checksum, thumbhash, dims, visibility) and EXIF (GPS, make/model, rating) for Hidden and Locked assets owned by other members of the same space, because accessibleLibraries() includes the space-link branch. Apply the M3 gate on all four builders (getBackfill + getUpserts for each class): asset.ownerId = userId OR asset.visibility IN ('archive', 'timeline') This preserves the owner's full access to their own library assets (including Hidden/Locked) while blocking other members from receiving private assets. For LibraryAssetExifSync.getUpserts, replaced the asset-id subquery with a direct innerJoin('asset') so asset.ownerId and asset.visibility are available for the gate — matching the style used by SharedSpaceAssetExifSync (Slice 4.A). Both getBackfill methods receive userId as an explicit third parameter, matching the AlbumAssetSync.getBackfill pattern. Fixes: LibraryAssetSync.getBackfill, LibraryAssetSync.getUpserts, LibraryAssetExifSync.getBackfill, LibraryAssetExifSync.getUpserts @GenerateSql annotations updated: LibraryAssetSync.getBackfill (added DummyValue.UUID for userId), LibraryAssetExifSync.getBackfill (same). * fix(web): group space albums by ownerId after linked-album DTO trim albumUsers was removed from SharedSpaceLinkedAlbumDto in Slice 7 to prevent PII leakage. The Owner grouping in space-album-grouping.ts read album.albumUsers[0]?.user.id/name for both the group key and display name, causing four TypeScript errors. Reroute the Owner case to read album.ownerId (forward-compat cast for when the server adds the field) and resolve the display name from ctx.members (the space member list already threaded into the grouping context). Albums without an ownerId fall to Unassigned — safe and crash-free. Remove the now-invalid albumUsers field from the four spec fixtures. Update the Owner grouping test to key by ownerId via Object.assign and assert that member-name resolution and the Unassigned bucket work correctly. * fix(spaces): redact album-user emails when album reached only via space grant A space Viewer can reach a linked album via checkSpaceLinkedAlbumReadAccess (no role filter), causing album.service.get() to return full albumUsers[].user.email for owner and all collaborators. Fix: after mapAlbum(), check if the caller is an album participant (present in albumUsers); if not, blank every albumUsers[].user.email. The albumUsers array shape is preserved so web album.albumUsers.some() still works. * fix(spaces): album-scoped search/facets exclude hidden assets The album-scoped (albumId) branches of applySuggestionScope and albumSharedSpaceScope applied only the caller-level not-locked filter, passing Hidden assets from other participants through to filter facets and album-scoped search results. - applySuggestionScope album branch: wrap the album_user participant EXISTS arm and the timelineSpaceIds space-path branches in eb.and([spaceVisibilityGate(eb), ...]) so Hidden/Locked assets owned by other participants are excluded, matching the sibling spaceId and timelineSpaceIds branch semantics. - albumSharedSpaceScope: wrap the shared_space_asset and shared_space_library EXISTS arms in eb.and([spaceVisibilityGate(eb), ...]) to apply the same Archive+Timeline gate used by searchAssetBuilder's space branches. Caller-owned assets are unaffected; existing album and space search tests remain green. Three new SQL-shape tests verify the gate is present. * fix(spaces): activity excludes hidden/locked album assets Gate search() and getStatistics() in ActivityRepository to only return activity rows for assets with default visibility (Archive + Timeline), mirroring the withDefaultVisibility filter applied in album.repository withAssets. Previously, search() only filtered deleted assets (leaking Hidden), and getStatistics() only filtered Locked (leaking Hidden). * feat(spaces): expose non-PII ownerId on linked-album DTO for group-by-owner Add ownerId (album owner UUID) to SharedSpaceLinkedAlbumDto so the web group-by-owner feature has a stable, PII-free key to pivot on after albumUsers was removed in Slice 7. The repo getLinkedAlbums query now selects the owner's userId via a correlated album_user subquery instead of relying on an ownerId column (the album table has none). Service maps the field directly from the row; albumUsers remains stripped from the response. * test(spaces): extend RBAC matrix to timeline/library-sync/activity/album-info/facet surfaces Regression-locks the five surfaces the re-audit found after the initial matrix was written (Fixes A/B/C/D/I): Surface 18 — timeline explicit-visibility (Fix A): asserts that passing visibility=HIDDEN or visibility=LOCKED to getTimeBuckets / getTimeBucketCovers via spaceId or timelineSpaceIds does NOT surface another member's Hidden/Locked in-space assets; owner's own Hidden via timelineSpaceIds+userIds IS present. Surface 19 — LibraryAssetSync / LibraryAssetExifSync (Fix B): asserts that a viewer's backfill of a space-linked library excludes Hidden+Locked assets (spaceVisibilityGate), while the owner's own backfill surfaces all visibilities. Surface 20 — activity.search / getStatistics (Fix C): asserts that activity rows on Hidden/Locked assets in a space-linked album are excluded from both search results and statistics counts. Surface 21 — AlbumService.get email redaction (Fix D): medium integration test that a space Viewer (not an album_user) gets all albumUser.email fields redacted to ''; album participants and the album owner see real emails. Surface 22 — album-scoped facets (Fix I): asserts that getFilterSuggestions with albumId excludes a participant's Hidden/Locked asset country from the suggestion result while Timeline and Archive values remain present. * test(spaces): strengthen visibility-gate guard to detect helper-based space reads * chore(spaces): regenerate SQL docs + SDK for re-audit fixes and ownerId field * fix(web,e2e): add ownerId to linked-album fixtures; drop unused afterAll import * fix(e2e): import asBearerAuth from src/utils not src/fixtures * fix(e2e): add space assets while Timeline then lock/hide (add-endpoint rejects locked) * fix(e2e): add-then-lock the download-info multi-asset space test
…#753 #1+#2) (#757) * docs(spaces): design spec for album/library asset visibility purge (#753 #1+#2) * docs(spaces): review fixes — impl-loop slices, union-checkpoint rationale, Locked/showInTimeline edges * feat(spaces): add shared_space_album_asset_audit table + migration * feat(spaces): purge album-linked space assets on Hidden (A1) * feat(spaces): prune shared_space_album_asset_audit on schedule * feat(spaces): wire album visibility purge/restore into asset updateAll (A3, A8, unit) * chore(spaces): revert-to-immich + regen SQL for album visibility purge * docs(spaces): impl-loop slice 1+2 plans for album/library visibility purge * feat(spaces): add shared_space_library_asset_audit table + migration * feat(spaces): purge library-linked space assets on Hidden, owner-excluded (L1/L2) * feat(spaces): wire library visibility purge into asset updateAll * chore(spaces): revert-to-immich for library visibility purge * test(spaces): cross-path visibility purge convergence (X1) * test(spaces): empty-list no-op + idempotent double-purge (X2, X3) * test(spaces): union checkpoint correctness across audit sources (X4) * test(spaces): audit retention prune for new space audit tables (R1) * docs(spaces): impl-loop slice 3 plan (cross-path invariants + retention) * chore(spaces): regenerate query SQL for album/library visibility purge * style(spaces): prettier-format impl-loop plan docs * fix(spaces): add new audit-table migrations to revert-to-immich cleanup list * style(spaces): prettier-format visibility-purge server files * chore(spaces): use spread-apart random migration timestamps for new audit tables
…ile collapse (#751) (#755) * docs(spaces): design spec for stacked photos in Spaces (#751) * docs(spaces): add TDD, edge-case matrix, vertical slices to #751 spec * docs(spaces): refine #751 spec — space-eligible visibility set, test file naming * docs(spaces): S1 implementation plan for #751 * feat(spaces): add getOwnedStackSiblingIds repo query for stack expansion (#751) * feat(spaces): expand add-to-space to the whole stack (#751) * test(spaces): composition E2E for stack-in-space add + collapse (#751) * chore(spaces): regenerate SQL docs for getOwnedStackSiblingIds (#751) * fix(spaces): exclude offline stack siblings + tidy test helper (#751 review) * docs(spaces): S2 plan + E16 spec refinement for #751 * feat(spaces): add getStackSiblingIdsInSpace repo query for remove expansion (#751) * feat(spaces): expand remove-from-space to the whole stack (#751) * test(spaces): composition E2E for stack-in-space removal + album survival (#751) * fix(spaces): satisfy eslint zero-warnings on S2 test additions (#751) * style(spaces): prettier formatting for S2 medium spec (#751) * chore(spaces): regenerate SQL docs for getStackSiblingIdsInSpace (#751) * docs(spaces): S3 plan (mobile stack-collapse) for #751 * feat(spaces): collapse stacks in the mobile aggregated-space timeline (#751) * test(spaces): exercise none-count builder + album-count guard (#751 review) * docs(spaces): S4 plan (web guard + user docs) for #751 * test(spaces): guard space timeline collapses stacks, album detail does not (#751) * docs(spaces): document whole-stack add/remove in shared spaces (#751) * fix(spaces): mobile collapse must not hide non-owned stacks (#751 final review) The collapse LEFT JOINs stack_entity, which only syncs for own/partner stacks; a viewer's non-owned space stack has stack_id set but no local stack_entity row, so the previous predicate filtered every frame out and the stack vanished. Keep assets whose stack row is absent locally (show flat) — owned stacks still collapse. Adds a regression test + documents the mobile limitation. * docs(spaces): note merged_asset.drift main-timeline follow-up (#751) * docs(spaces): prettier-format #751 spec + slice plans (Docs Build) * style(spaces): apply web-prettier + dart format to #751 test/impl files
…al follow-up (TDD) (#759) * docs(spaces): TDD remediation spec for space-albums review findings Deduped 44 review findings -> ~37 distinct fixes across 11 impl-loop slices (ordered high->low), each with TDD red/green flow and full edge-case coverage. Base: origin/space-albums-onto-main (e52d882). Source: SPACE-ALBUMS-REVIEW-2026-07-08.md. * docs(spaces): correct remediation spec fix-approaches after code-grounded review Verified spec claims against e52d882 with 5 agents. Corrections: - album-read surfaces use a FLAT spaceVisibilityGate (not applySuggestionScope's own-OR-gate) to stay consistent with the album grid - C1 (activity leak) fixed at the access layer, not the SQL visibility gate - correctness-6 single-transaction is infeasible (#595); use idempotency+reconciliation - rbac-3 must patch single-asset update() too + split owned subset BEFORE cascades - metadata.service motion-photo hide is an additional purge-bypass writer (slice 3) - correctness-4 advisory lock needs create+delete+restore triggers w/ deterministic order - rbac-7 reframed as deny-only widening; rbac-8 resolved as documented no-change - mobile-6 is 6 predicate sites (not 5); mobile-1 version gate must be the fork version - slice 8 scoped to the user-deletion window; correctness-3 needs trigger + LinkSync filter * fix(spaces): reject Hidden/Locked album-scoped timeline buckets (security-3) * fix(spaces): gate plain-album search branch on shareable visibility (security-1) * fix(spaces): gate asset-repo albumId timeline arm on shareable visibility (security-3) * test(spaces): e2e negatives for album-scoped search + timeline visibility leaks * test(spaces): hoist album-visibility medium test helpers to module scope Satisfies eslint-plugin-unicorn's consistent-function-scoping rule (--max-warnings 0): seedAlbum, seedAlbumWithVisibilities, and itemIds don't reference anything local to their nested describe block, so they must live alongside this file's other module-level seed/assert helpers. * fix(spaces): gate album map-markers on shareable visibility (security-2) * fix(spaces): deny album-level activity to space-only album readers (C1) * fix(spaces): strip album participants for space-only readers (security-8) * fix(spaces): grant PersonRead on archived-only space assets (rbac-7) * docs(spaces): pin album-download flat visibility gate (rbac-8) * refactor(spaces): extract applyVisibilityTransitionSideEffects from updateAll (security-4) * fix(spaces): route single-asset visibility PUT through the space purge helper (security-4) * fix(spaces): route motion-photo AssetHide/AssetShow through the space purge (motion bypass) * fix(spaces): emit visibility purge/restore only for boundary-crossers (correctness-8) * fix(spaces): owner-gate album + direct visibility-purge delete streams (gaps-5) The album (SharedSpaceAlbumToAssetSync.getDeletes) and direct (SharedSpaceToAssetSync.getDeletes) purge arms are now owner-gated like the library arm: the album arm INNER JOINs asset (purge-only table, asset row always exists); the direct arm LEFT JOINs asset with a null-owner disjunct, since shared_space_asset_audit is dual-purpose (also receives physical/unlink-delete tombstones whose asset row may be gone via FK cascade). Also fixes several pre-existing baseline medium tests (album A1/A5/A9/X3/X4, direct Hidden/Locked) that asserted the SAME user (space owner = asset owner = sole synced device) received their own purge tombstone — now correctly excluded per gaps-5, so those tests were updated to sync as a separate non-owner member. Regen of server/src/queries/sync.repository.sql is DB-deferred (no running DB in this worktree). * fix(spaces): visibility-gate link-row sync streams (correctness-1/security-6) Add an asset INNER JOIN + flat spaceVisibilityGate to SharedSpaceToAssetSync and SharedSpaceAlbumToAssetSync getBackfill/getUpserts, matching the content sibling streams. Blocks the restore-then-hide resurrection (a stale updateId re-adding a now-Hidden asset after the delete tombstone) and the Hidden-asset membership metadata leak. Owner-visibility is flat (no own-OR), consistent with the flat content streams and slice 3's owner-gated tombstones. * fix(spaces): exclude the owner's partner from the library purge arm (security-7) The library visibility-purge tombstone arm gated only on asset.ownerId != userId, so a member who is also the owner's partner dropped an asset their partner entitlement still covers. Add a NOT EXISTS on the partner table (sharedById = asset.ownerId, sharedWithId = userId) so a partner-and-member is never purged; a non-partner member is still purged. * fix(mobile): version-gate the SharedSpaceAlbum sync request types (mobile-1) The 5 SharedSpaceAlbum* request types were added to the /sync/stream body unconditionally. An older fork server's SyncRequestTypeSchema (z.enum) rejects the unknown values with a 400 for the whole request, taking down the entire sync stream (total outage) whenever the app is ahead of the server. Gate them behind serverVersion > 5.0.0 — a FORK version, since deployed fork servers report FORK_VERSION (stamped by apply-branding), not the upstream Immich version. v5.0.0 is the last release without space-albums; the feature ships in the next release, and > 5.0.0 also admits its release-candidates for validation. * fix(sync): drop unknown /sync/stream request types instead of 400 (mobile-1 defense-in-depth) SyncStreamDto.types rejected the whole request with a 400 on any unrecognised SyncRequestType enum value, so a newer client sending fork-only types (e.g. the SharedSpaceAlbum* types) to an older server took down the entire sync stream. Filter unknown values before the enum-array validation via z.preprocess so known types keep streaming; SyncService.stream already ignores types it does not handle. Structural errors (non-array types) still 400. No OpenAPI/SDK change (same array-of-enum shape as time-bucket.dto's preprocess arrays). * fix(mobile): keep album-reachable assets during the library sweep (mobile-2) deleteLibrariesV1's orphan sweep preserved owner/partner/direct-add (shared_space_asset) paths but not shared_space_album_asset (the album path this feature encourages) nor remote_album_asset (a pre-existing classic-album gap). Unlinking a library while an asset was also in a linked album deleted the shared remote_asset row, so the asset vanished from album detail and the space timeline. Add both tables to the sweep keep-set. No new bound parameters. * fix(spaces): require AssetShare (not AssetRead) to add assets to a space * fix(spaces): restrict visibility and livePhotoVideoId writes to owned assets * feat(spaces): album owner can view + revoke space-album links (rbac-6) * feat(web): surface album space-links to owner; pin space-album edit affordances (rbac-6, rbac-5/albums-8) * test(e2e): space Viewer cannot see space-album link/unlink/edit affordances (C4) * feat(spaces): album soft-delete/restore drives shared-space album grant + link lifecycle * fix(spaces): exclude soft-deleted albums from shared-space album link sync sync.repository.sql not regenerated (no local DB) — CI's SQL-generation check will flag the drift; regenerate on a scratch migrated DB or in CI. * fix(spaces): refresh shared-space album grant createId on re-link (albums-9) * test(spaces): end-to-end shared-space album trash lifecycle sync * fix(spaces): forbid removing or demoting the space creator * fix(spaces): unlink a departing member's own albums on removal removeOwnedAlbumLinksAddedBy carries @GenerateSql; server/src/queries/shared-space.repository.sql regen is CI-deferred (no local DB to regenerate against — never run `make sql` without one). * fix(spaces): reconcile stranded album grants after concurrent revocations getLinkedAlbumIds + reconcileAlbumGrants carry @GenerateSql; server/src/queries/shared-space.repository.sql regen is CI-deferred (no local DB to regenerate against — never run `make sql` without one). * fix(spaces): clear fork space tables on mobile SyncResetV1 * fix(spaces): show archived assets in mobile space and space-album timelines * fix(spaces): count shelf albums via visible remote_asset join * fix(spaces): space-aware mobile pruneAssets on syncComplete * fix(spaces): reconcile space face-people projection after a failed OnEvent handler * fix(spaces): match space-card metrics to the timeline (showInTimeline gate + album/library recency) * fix(spaces): repair revert-to-immich guard list and album-slice migration DELETE list * perf(spaces): add composite audit indexes for the member-join sync delete-scan * fix(spaces): redact cross-space identifiers from the space activity PersonMerge payload * test(spaces): pin album-arm trash + stack parity on space read surfaces (C5 resolved safe) * test(spaces): pin partner × space-linked visibility (Locked blocked, Hidden via partner) (C6 resolved safe) * fix(spaces): validate shared-space route params so non-UUID input returns 400 not 500 * style(spaces): satisfy eslint on slice-11 specs (set-has, no-__dirname, fn-scoping) * docs(spaces): per-slice implementation plans for the remediation run * style(spaces): prettier-format remediation files (server, e2e, docs markdown) * chore(spaces): regenerate openapi clients for param DTOs + reconcile job name * style(spaces): dart format mobile hygiene files (slice 10) * style(docs): apply docs-config prettier fixed-point to remediation slice-3 plan * fix(spaces): resolve medium-test failures surfaced by CI (Docker up) - revert the DIRECT-arm gaps-5 owner-gate on SharedSpaceToAssetSync.getDeletes: that audit table is dual-purpose (removals + purges), and gating suppressed the owner's legitimate removal-convergence deletes. album+library arms (purge-only) stay gated. - wire SharedSpaceRepository into the workflow-core-plugin medium harness (slice-3's update() visibility path uses it via the plugin-host BaseService.create); prod DI is fine. - wire FaceIdentityRepository into the search medium harness for the albumIds+personIds case. - stub JobRepository.queue in the album-permissions harness (slice-9 reconcile enqueue). - trash-lifecycle restore test: assets re-deliver via BACKFILL (fresh grant), not upsert. - update the direct-arm gaps-5 tests to the reverted (owner-included) behavior. * chore(spaces): regenerate SQL query docs for changed decorated queries Regenerated via sync-sql against a fresh migrated scratch DB (immich vchord image) — access (rbac-7), map (security-2), shared.space (slices 9/11), sync (slices 3/4/8). * test(e2e): albumId+visibility=locked returns 401 (elevated required) not 400 * fix(spaces): resolve e2e failures from DTO/behavior changes - album.service: omit sharedSpaceLinks from the album response when empty, so a plain album's shape is unchanged (fixes pre-existing album.e2e deep-equal assertions). - sync.dto: revert the drop-unknown request-type filter (slice 5 defense-in-depth) — it changed the established reject-unknown API contract and broke pre-existing sync validation tests. The mandatory mobile-1 CLIENT version gate remains the fix. Removed the filter's unit/e2e tests and restored the controller spec's 400 assertions. * test(e2e): drain metadataExtraction before querying album map-markers (GPS timing) * test(spaces): fix 3 e2e test-setup bugs in space-albums visibility specs - map-markers (security-2): uploaded the same thompson-springs.jpg twice, so Immich deduped gps + hidden into one asset — flipping hidden to Hidden also hid gps, emptying the marker list. Use two distinct GPS files. - PUT-locked purge (security-4): the raw GET /albums/:id omits the assets array once A is Locked (elevated-only), so .map() threw. Assert on assetCount === 0, which reflects the album_asset row deletion directly. - rbac-3 bulk-lock guard: freshly-uploaded victimAsset isn't populated into the assets array until async processing finishes; assert assetCount === 1 to prove the blocked lock left it an album member. * ci(e2e): free runner disk before the e2e docker build Both End-to-End jobs (Server & CLI, Web) build the full server + ML images and load the geodata (geonames) dataset into Postgres on the same runner. On a runner image with little free disk (~81 MB observed), Postgres fails the geodata_places primary-key build with 'No space left on device', the MetadataService init throws, the microservices worker exits 1, and the server never boots — every e2e test then fails on the server-ping timeout. Reclaim the unused preinstalled toolchains (dotnet, android, ghc, CodeQL) and prune cached docker images before the build, mirroring the existing step in storage-migration-e2e.yml. Absolute paths + ignored-if-missing, so it is safe on both the x86 and arm runners. * test(spaces): align gaps-5 e2e with the dual-purpose direct-arm revert The direct visibility-purge arm (SharedSpaceToAssetSync.getDeletes) was reverted to NOT owner-gate: shared_space_asset_audit is dual-purpose (visibility over-purge AND physical delete), so gating it to spare the owner the over-purge would also suppress physical-delete tombstones for the owner's own assets. The owner therefore receives the direct purge tombstone too (benign — a restore round-trips it), matching the medium visibility-purge spec. This e2e test still asserted the pre-revert behavior (owner gets 0 tombstones). Update its name, comment, and assertion: the owner now tombstones on their own /sync too (>= 1), same as the member. Verified against the local e2e stack. * test(sync): de-flake album-to-asset backfill order assertion 'should not resend an already-acked item when backfill resumes' inserted two album_asset rows back-to-back, then asserted the backfill returns them in insertion order. The backfill sorts by updateId ASC (deterministic), but the two UUIDv7 updateIds can land in the same millisecond and invert relative to insertion order — so the ordered toEqual([asset1, asset2]) flaked (passed twice, failed once in CI with identical server code). Separate the two inserts with wait(2) so asset1's updateId is strictly less than asset2's, matching the file's existing idiom (it already does this six times for the same reason). Pre-existing upstream flake, downstream of a deterministic ORDER BY; unrelated to the space-albums changes. Verified 5/5 green locally. * docs(spaces): follow-up remediation spec + source review (Phase 1 critical / Phase 2 deferred, TDD) * fix(spaces): exclude trashed album assets from album-granted search (H1) * fix(spaces): reject trashed-asset browse on space-linked album timeline (H1 secondary) * docs(spaces): slice-1 implementation plan (H1) * fix(spaces): scope person representative-faces to space-reachable assets for non-owners (M1) * fix(spaces): gate person-faces picker owner-vs-space in getFacesForPicker (M1) * docs(spaces): slice-2 implementation plan (M1) * feat(spaces): role-filtered member scope + PersonAccess.checkSharedSpaceEditAccess (M2) * fix(spaces): require owner or space-editor to set person representative face (M2) * docs(spaces): slice-3 implementation plan (M2) * fix(spaces): guard unlinkAlbum owner path against activity injection + FK 500 (M11) * docs(spaces): slice-4 implementation plan (M11) * fix(spaces): redact commenter emails from asset-level activity for space-only readers (M5) * docs(spaces): slice-5 implementation plan (M5) * fix(spaces): gate album contributorCounts to direct readers + visibility (L1) * fix(spaces): scope legacy person statistics for space readers (L3) * fix(spaces): exclude album-level activity counts for space-only readers (I2) * docs(spaces): slice-6 implementation plan (A residuals) * fix(spaces): gate suggestion facets + flat-gate album search to match grid (I1, L2) * fix(spaces): retry-convergent visibility purge + library-restore EXIF re-delivery (M3, L4) * fix(spaces): mask owner isFavorite on the library sync arm (L5) * docs(spaces): slice-7 implementation plan (purge convergence) * fix(spaces): durable async lifecycle — reconcile self-heal, atomic member removal, stale-face sweep, nightly backstop (M6, M7, L6, L7, L8) * docs(spaces): slice-8 implementation plan (async lifecycle durability) * test(spaces): pin motion-photo visibility purge through the real sync seam (M13) * test(spaces): de-vacuum visibility/creator-guard assertions + fail-closed creator guard (L19) * refactor(spaces): remove dead Locked/Hidden branches in duplicate merge (R1 cleanup) * docs(spaces): slice-12 implementation plan (test hardening) * feat(mobile): space-album arm on personal timeline/map/video/place + archived space-person assets (M4, L12) * feat(mobile): linked-spaces view + owner revoke on the album options sheet (M8) * docs(mobile): correct sync version-gate comment re reverted server filter (L13) * docs(spaces): slice-9 implementation plan (mobile parity) * fix(spaces): revert L2 flat-gate — restore album-scope timeline RBAC gate (fixes people-identity-rbac regression) * style(spaces): prettier-format slice 7-8 server + e2e files (CI format gate) * chore(openapi): regenerate clients for the SharedSpaceAlbumGrantReconcileSweep job (M6/L8) * test(spaces): allowlist library-restore emit + grant-reconcile in the scope guard (Slice 7/8) * test(spaces): drop infeasible un-lock proof in security-4 e2e (Locked needs elevated auth; row-delete proven via member sync + unit) * style(docs): prettier-format the follow-up slice plan markdown (CI Docs Build gate) * fix(spaces): align 1782000000000 trigger DDL + override with functions.ts + parity guard (M12) * test(spaces): derive revert-guard fork tables from schema, scope migration-name check (L11) * test(mobile): pin fork-only sync request types to the version gate (M14) * docs(spaces): slice-10 implementation plan (migration/self-decl hygiene) * fix(test): move migration-override-parity spec out of the migrations-gallery scan path (unblocks medium suite) * fix(spaces): unlink departing member's trashed own albums + per-viewer space-visible cover (M9, L17) * feat(spaces): log AlbumUnlink for auto-removed departing-member albums (L16) * docs(spaces): slice-11 implementation plan (product/UX) * i18n(spaces): add album-in-space + leave-warning keys (en) for L14/L16 * feat(web): album link-list $derived fix, editor-gated cover, link-to-space entry point + i18n (L9, L10, L15, L14) * feat(mobile): link-to-space picker on album options + space-album i18n (L15, L14) * style(spaces): sort en.json + prettier web spec + dart-format mobile space pages (CI format gates)
The space-album Drift migration was authored as v35 but upstream v3.0.2's uploaded_at index took v35 during the rebase, so it was renumbered to v36 (schemaVersion 36, from35To36 step). Regenerated drift_schema_v36.json, schema_v36.dart, GeneratedHelper versions, steps + .drift.dart via drift_dev make-migrations + build_runner. Replaces the pre-rebase schema_v35.dart artifact commit. Migration test (vN->v36) green.
Merging main's live-photos test group (uses flutter_test isNull matcher) with the branch's drift import (Value()/.equals() for space-album link writes) made isNull ambiguous. Hide it from the drift import so the matcher wins. dart analyze --fatal-infos lib test now clean.
Deeds67
force-pushed
the
space-albums-onto-main
branch
from
a117997 to
230b1db
Compare
…e 1782000000000 collision AddAssetExifDescriptionTrigramIndex landed on main at 1782000000000, colliding with the fork's AddAlbumSoftDeleteSharedSpaceAlbumTrigger and failing the upstream-preflight migration-timestamps collision guard. Move the fork migration to 1782050000000 (still ordered before the 1782100000000 grant-relink fix). Update the override-parity spec import and the revert-to-immich.sql migration-name list to match.
# Conflicts: # server/src/repositories/asset.repository.ts
# Conflicts: # web/src/routes/(user)/spaces/[spaceId]/[[photos=photos]]/[[assetId=id]]/+page.svelte
The add-all-to-collection button #762 wired across the timeline surfaces (spaces, albums, map, favorites, archive, people) never reached the space-album detail page, which only exists on this branch. Mirror the global album page's Pattern A: a handleAddAllToCollection that replays the active browse filters (scoped to this album via albumIds) into the SearchAddAllToCollectionModal, passed to the browse-mode ActiveFiltersBar. The bar self-gates the pill on there being active filters + results, which matches the existing {#if browseActive > 0} render guard. Extend the local mock-active-filters-bar wrapper with data-has-add-all and add per-surface coverage matching the sibling space-timeline test.
apply-branding already accepts a `version` input (→ FORK_VERSION); the RC workflow never passed one, so it always fell back to git-describe (the latest v<N>.<N>.<N> tag). That makes it impossible to test a version-gated feature on a pre-release branch whose nearest tag is the last release — e.g. mobile gates the shared-space-album sync request types on `serverVersion > 5.0.0`, but a space-albums RC built off the v5.0.0 tag stamps 5.0.0 and the gate never opens. Wire an optional `fork_version` dispatch input through to both apply-branding steps. Empty keeps the git-describe behaviour; set it (e.g. 5.1.0) to stamp a version that clears the gate.
…Albums Adds search-by-name and a reversible sort menu to the mobile SpacesPage and a space's SpaceAlbumsPage, mirroring the existing Albums page pattern. Client-side only (both lists are already loaded); the album date sorts reuse Drift columns that already exist, so no server/sync/migration changes.
… mock Adds an explicit TDD (test-first) development approach, a comprehensive edge-case test matrix (search casing/diacritics/regex-meta, sort ties & null-safety, persistence out-of-range defense, no-match vs empty, reactivity, regression of existing affordances), and commits the approved HTML mock as the visual acceptance reference the spec builds to.
Six independently-testable slices for /impl-loop: (1) expose SpaceAlbum linkedAt/updatedAt, (2) sort-mode enums + pure filter/sort helpers, (3) persist sort via SettingsKey/AppConfig, (4) reusable CollectionSortButton, (5) wire the space albums page, (6) wire the spaces page. Each is test-first with real code, exact files, commands, and per-slice acceptance.
…/members/photos Strengthens the Task 2 sort-helper matrix: default-direction coverage existed for every mode, but reversal was only proven generically via SortOrder.reverse() for 4 of 9 modes at the filterAndSort*/ function-call level. Add explicit reversed assertions for the remaining SpaceAlbumSortMode.recentlyLinked/recentlyUpdated and SpaceSortMode.members/photos so "reverse flips every mode" is verified end-to-end, not just at the enum layer.
Converts SpaceAlbumsPage to a HookConsumerWidget: a SearchField + a result-count caption + CollectionSortButton (options from SpaceAlbumSortMode) sit above the existing 2-column grid, filtering and sorting client-side via filterAndSortSpaceAlbums. The sort choice reads reactively from AppConfig.spaceAlbums and persists through the same settingsProvider.write path album_selector.widget.dart uses (SettingsKey.spaceAlbumsSortMode / .spaceAlbumsIsReverse). A non-empty source list whose query matches nothing now renders a new _NoMatch state (key space-albums-no-match), distinct from the genuinely-empty _EmptyState (key space-albums-empty, unchanged). The editor-gated Link app-bar action, card ⋮ menu, and Hidden badge are preserved unchanged and still gated by canEdit. Also bootstraps a real Drift-backed SettingsRepository in the two other test files that pump SpaceAlbumsPage directly (space_albums_link_wiring and space_b6_mutations), since the page now reads/writes through it, and widens the b6 mutation tests' logical viewport so the ⋮ menu isn't clipped by the new search+sort header at the default test surface size.
…, richer search count, dead-key cleanup
Addresses the whole-branch review of the mobile Spaces/Space-Albums
search+sort feature: relabel the "name"/"photoCount" sort options to match
the mock ("Name"/"Photo count"), localize the hardcoded "Sort: " prefix in
CollectionSortButton, add a defensive orElse to its firstWhere, drop the
dead space_albums_no_matching key, and render a richer "N of M · matches
"query"" result caption while searching on both the spaces and space-albums
pages. Adds remount-read persistence, present-but-null-Optional, and
updated result-count regression coverage.
…pace-albums list
The prior final-review fix pass deleted this key as "dead" after grepping only
mobile/. But web + mobile share the same i18n/ directory, and the web
space-albums-list.svelte renders $t('space_albums_no_matching') for its
empty-search state. Deleting it made English web users see the raw key string
(no web test asserts the text, so CI would not catch it). Restore it; mobile's
own space_albums_no_match key is unaffected.
Adds the 16 new keys the mobile search+sort feature introduced (sort labels,
search hints, no-match, result counts, "Sort: {label}") to the six core
locales, matching each app's existing terminology (de/nl keep "Spaces",
fr "Espaces", it "Spazi", es "Espacios", pl "przestrzenie") and correct ICU
plural forms (pl uses one/few/other per the repo convention). Other locales
continue to fall back to English.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
In-space album management (search / create / sort / group / list-covers)
Brings the full main albums-page toolbar into the shared-space Albums tab, and consolidates shared-space-album scoping behind a single shared helper.
Web — space Albums tab
AlbumCover,sortAlbums, and the album mapper; the toolbar/list/table live in fork-only components with a space-scoped view-settings store (isolated from the global/albumssettings). The/albumsCreate button moves to the right so both pages match.Server
SharedSpaceLinkedAlbumDtoto theAlbumResponseDtoshape (+showInTimeline/addedById/linkedAt) and removes the per-album N+1 (bulkgetMetadataForIds+ reusedmapAlbum).shared-space-album-scopehelper routed through ~9 repositories (access / asset / search / timeline / map / memory / view / person-face / face-identity), de-duplicating inline space-scoping SQL — proven equivalent by medium characterization tests.Testing
TDD throughout: server unit + real-DB medium (incl. the scope-equivalence characterization spec), full web component suite, and Playwright e2e journeys (search/sort/group/view/create/link/viewer gating). Regenerated SQL / OpenAPI / Dart clients.