Skip to content

Follow-ups from the 2026-07-02 rolling-rebase audit remediation (library-sync favorite leak, re-trim race, Dart nullable-items patch, branding/i18n, migration-collision debt) #743

Description

@Deeds67

Follow-ups surfaced while remediating the 2026-07-02 rolling-rebase audit findings (docs/plans/2026-07-02-rolling-rebase-audit-findings.md). None were in the original 34 findings; each was verified during a fix slice but left out of scope. The deferred trim-aware realtime-HLS half of M1 is tracked separately in #741.


1. LibraryAssetSync leaks the owner's isFavorite to all library members

Same bug shape as audit finding LOW#1 (fixed for shared-space sync in slice S4), but on the connected-libraries sync path. LibraryAssetSync (server/src/repositories/sync.repository.ts ~:1194) selects the raw columns.syncAsset (including isFavorite) scoped only by accessibleLibraries(userId), with no per-row ownership mask. Upstream now masks the favorite flag for non-owned synced assets. Fix: apply the same CASE WHEN asset.ownerId = <userId> THEN asset.isFavorite ELSE false END mask S4 added to SharedSpaceAssetSync.

2. handleVideoTrim race on rapid re-trim

A quick second trim of the same asset throws Input file is missing: *_edited.mp4.frame.jpg (async, post-response — does not fail a request today, but logs an error and can leave a stale thumbnail). Surfaced by the S11 e2e (video-trim.e2e-spec.ts'should re-trim (widen)'). Likely a thumbnail-regeneration step racing the replacement of the _edited.mp4 between two overlapping trims. Fix: serialize per-asset trims, or make the frame-extraction tolerant of the input being swapped mid-flight.

3. Re-author the Dart nullable_items_in_arrays patch for v3 templates (audit LOW#18)

open-api/templates/mobile/serialization/native/native_class_nullable_items_in_arrays.patch (which makes generated Dart models type nullable-item arrays as List<T?>) is orphaned — open-api/bin/generate-dart-sdk.sh no longer applies it. It can't just be re-inserted: upstream's later native_class.mustache.patch (openapi-generator three-state-DTO rework, immich-app#27231) rewrote the same region, so 2 of 3 hunks reject, and generate-dart-sdk.sh runs set -euo pipefail, so a rejecting patch would abort every SDK build. The hunks need re-authoring against the current template, covering both the {{#vendorExtensions.x-is-optional}} and {{^…}} branches. Live symptom: TimeBucketAssetResponseDto declares 8 nullable-item array fields (city, country, duration, latitude, livePhotoVideoId, longitude, projectionType, thumbhash) as non-nullable despite the spec/doc-comments; only stack is hand-patched via open-api/patch/time_bucket_asset_response_dto.dart.patch.

4. Four more un-branded i18n keys containing "Immich"

Beyond the 9 covered in slice S12 (audit M7), these en.json keys still leak "Immich" and are not in branding/i18n/overrides-en.json: admin.asset_offline_description, import_option_skip_duplicates, my_immich_title, my_immich_description. (S12 also hardened verify-branding.sh, which had been silently passing on a leaking tree — it now catches this class, so these should surface.)

5. Two other pre-existing benign migration-timestamp collisions

migrations-gallery/ has two more duplicate-timestamp pairs beyond the LOW#2/#15 one: 1775100000000 (AddAssetDuplicateChecksum + DropSpacePersonThumbnailPath) and 1777000000000 (AddSpacePersonCounts + AdminScopedClassification). All three are benign (Kysely keys by full filename; both apply in deterministic order) and are grandfathered by the S17 guard (tools/upstream-preflight/src/migration-timestamps.spec.ts). Renaming any of them is unsafe on a continuously-deployed fork (Kysely missing-migration hard-fail on already-deployed DBs). Tracked only so the debt is visible; no action needed unless a safe rename path (build-time alias) is wanted.

6. micromatch parens quirk in the ownership-manifest matcher

SvelteKit route-group segments like (user) in docs/fork/ownership.yml globs are parsed by micromatch as empty regex capture groups, so a glob such as web/src/routes/(user)/spaces/** silently matches nothing even though the files exist (currently masked in production by a broader web/src/routes/** optional_path). Handled in the S20 guard's test helper via parens-escaping, but tools/upstream-preflight/src/coverage.ts's matcher likely needs the same escaping to avoid silently under-counting owned files.


Context: docs/plans/2026-07-02-rolling-rebase-audit-findings.md ("Follow-ups discovered during remediation") and the per-slice plans under docs/superpowers/plans/2026-07-02-rolling-rebase-audit-slice-*.md.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions