Second review pass over #1910: races, duplication, dead code

[johan-bell]

Second review pass over #1910 (duplication, comments, races, dead code, best practice). Every change here is decision-free; the findings that need a call are listed at the end and are not in this PR.

Race conditions and lifecycle

• A slow encode could land on the wrong document. EditContent is reused across post/edit/:id routes, so if the encoder's trust prompt held start() open while the editor moved from post A to post B, the late encoding event wrote A's hlsUrl/hlsKey onto B and leaked the EventSource. useMediaEncoder now carries a disposed flag checked after every await, onMediaReady names the document the result is for, EditContentMedia refuses a result for any other document, and the parent-id watcher stop()s the previous stream first. (cms/src/composables/useMediaEncoder.ts, EditContentMedia.vue)
• The Encode button went dead under the cursor. refreshAvailability flipped to "checking" on every 10 s poll tick and every window focus — i.e. exactly when the editor switched back from the encoder and clicked. Only the first check shows as "checking" now.
• The poller could re-arm after unmount if the unmount happened mid-await; guarded by the same flag.
• Encrypted video loaded twice in the app. source was truthy before keyHex arrived, so LuminaryPlayer loaded, failed on the key, and reloaded. VideoPlayer.vue now withholds the source until the key question is answered (keyResolved). Unencrypted content is unaffected — the flag is set synchronously when there is no key to fetch.
• Stale key fetch in the CMS preview. VideoPreview survives a document switch; a slow sidecar fetch for A could become B's key. Sequence-guarded.

Duplication

• unmaskKeyHex existed three times. The CMS copy is gone; the CMS imports luminary-shared's. api/src/util/maskKey.ts's comment named the CMS copy as its counterpart — it now names shared.
• isBucketRelative/toAbsoluteMediaUrl lived in both cms/src/util/mediaUrl.ts and app/src/util/videoSource.ts. Moved to shared/src/util/mediaUrl.ts; both consumers use it.
• The sidecar fetch → cast → unmask sequence was inlined in VideoPlayer.vue and VideoPreview.vue. Now fetchHlsKey(parentId, { cms }) in shared, which also gives the previously unused HlsEncryptionKeyData type its job. Specs mock fetchHlsKey instead of getRest; the mask/unmask vector moves to shared/src/util/hlsKey.spec.ts.
• The "persisted bucket, else the lone media bucket" rule was written twice (EditContentMedia, MediaBucketSelect). Now storageSelection().effectiveMediaBucketId().
• deleteMediaCollection and migrateMediaCollection each hand-rolled the "is this URL external?" test that isInOurStorage already implements, deleteMediaCollection inlined its own loadBucket, and the two files disagreed on MASTER ("/master.m3u8" vs "master.m3u8"). One loadBucket, one MASTER, one predicate.

Dead code

• app: video.js, videojs-mobile-ui, videojs-youtube, @types/videojs-mobile-ui, m3u8-parser, @types/m3u8-parser, iso-639-2 — no imports in app/src since the player moved to the encoder's player-web-legacy. Their two .d.ts shims and two stale vi.mock("video.js") calls go with them, as does app/test-coverage.md (a coverage snapshot of files that no longer exist).
• cms: LDialog.vue's wide/preventBackdropClose pass-throughs (nothing uses them; VideoPreview talks to LModal directly).

Best practice

• GET /storage/encoderconfig returns live S3 write credentials without Cache-Control: no-store; the far less sensitive sidecar endpoint had it. Added.
• MediaDto.ts imported from "src/enums" where every sibling uses ../enums.
• (b: any) in processPostTagDto → StorageDto; children as any[] in v21 → ContentDto[].

Comments

Deleted the orphaned JSDoc in useMediaEncoder.ts (a stale block stacked on top of the live one), the // Process media label, the configuration.ts header that reproduced a type's shape, and the "crypto object" comment in EditContentVideo.spec.ts (it's a sidecar). Cut the multi-paragraph // blocks in processPostTagDto, migrateMediaCollection, sidecar.controller, useEditContentSource, VideoPlayer.vue and videoSource.ts to one or two lines of why, per CLAUDE.md.

Separate commit: v21 stamps parentMedia

v21 copied child.video onto parent.media.hlsUrl but never set child.parentMedia — only a change request does that (processPostTagDto.ts). The app reads parentMedia?.hlsUrl || video, so after the upgrade every migrated video disappeared until someone re-saved the parent in the CMS, and the idempotency guards mean a re-run cannot repair it. The migration now stamps parentMedia/parentMediaBucketId on all of a migrated parent's children (including translations that never had a video of their own). It is its own commit so it can be dropped if you'd rather handle it elsewhere.

Verified

• shared: vue-tsc clean; vitest src/util — 697 pass, 1 fail in responseCache.spec.ts (pre-existing: fails identically on the untouched epic tip 990d8bc3).
• api: tsc --noEmit clean; Jest for documentProcessing/*, v21, maskKey, encoderConfig, sidecar — 210 pass.
• cms: vue-tsc --build clean; full vitest suite green.
• app: vue-tsc --build clean; full vitest suite green; package-lock.json refreshed with npm install.
• ESLint clean on every touched file.

Not in this PR — needs a decision

1. MediaDto drift (blocker). Verified empirically again by a third reviewer: a legacy audio-only post fails validation (hlsUrl must be a string), and a post with both video and fileCollections has the audio silently stripped on its next save (whitelist), then re-stamped onto every translation's parentMedia. shared says hlsUrl? + fileCollections; the API says hlsUrl required and no fileCollections. Resolve one way or the other before 1878 api cms hls media data model #1910 merges.
2. Sidecar written before the parent. processMediaDto overwrites the key sidecar during processing; the parent is upserted later in processChangeRequest. A throw in between leaves the stored document pointing at the old collection with the sidecar holding the new key — the live video is unplayable until the editor retries. Fix is to persist the sidecar after the parent write, or key sidecars per collection.
3. Migration deletes source objects before the document write (migrateMediaCollection) — same shape as Add API based on NestJS #2, the other direction. Rare; a design choice.
4. Sidecar rate limiter is check-then-act across a burst: N in-flight requests all pass before the first strike is recorded. Fine for /query; weaker than intended for the key-harvesting defence (ADR 0019).
5. isParentAvailable fetches every child Content doc (include_docs, incl. fts arrays) to test three scalars, per encrypted video start. Wants a view or fields projection.
6. Back-patch can discard an unsaved encode result when the previous save's echo arrives after the encoder event (media is a back-patch field). Documented server-wins semantics; changing it is a contract decision.
7. Dead API audio chain: MediaFileDto.ts, MediaUploadDataDto.ts, IsAudio.ts, s3-audio/audioFormatDetection.ts, music-metadata — zero non-test consumers on the epic. Same question as Add unit test workflow for app #1.
8. Raw Minio/CouchDB error.message is surfaced verbatim in CMS warnings (endpoint host, bucket name). Seven new CMS specs are flat rather than in __tests__/ per cms/CLAUDE.md. getHlsKeySidecar is test-only. All low.