Skip to content

feat(media-server): managed TV library with author shows and collection-as-show (#411) - #412

Open
franklioxygen wants to merge 40 commits into
masterfrom
feat/issue411-playlist-tv-export
Open

feat(media-server): managed TV library with author shows and collection-as-show (#411)#412
franklioxygen wants to merge 40 commits into
masterfrom
feat/issue411-playlist-tv-export

Conversation

@franklioxygen

@franklioxygen franklioxygen commented Aug 16, 2026

Copy link
Copy Markdown
Owner

Closes #411not yet: see Outstanding before marking ready.

What this adds

An opt-in Author → playlist seasons media-server export layout that builds a separate, MyTube-managed TV library under uploads/media-library/:

media-library/
└── Kurzgesagt/
    ├── tvshow.nfo
    ├── poster.jpg
    ├── Season 00/          ← Specials / Unassigned
    ├── Season 01/          ← one source playlist
    │   ├── season.nfo
    │   ├── S01E001 - Human Origins.mp4
    │   ├── S01E001 - Human Origins.nfo
    │   └── S01E001 - Human Origins-thumb.jpg
    └── Season 02/          ← another playlist

The existing adjacent layout stays the default — nothing changes for current installations until they opt in.

Add uploads/media-library/ (not uploads/videos/) to Jellyfin/Plex/Emby/Kodi as a Shows library.

Collection-as-show

author → show does not suit every library. A TV drama uploaded by a third-party channel gets named after the uploader, and Plex shows the uploader's avatar as the poster — which is exactly what happened on the live NAS deployment while testing the layout above.

A collection can now opt out of being a season and become its own top-level show with a single Season 01, independent of the author:

media-library/
├── Kurzgesagt/                  ← the author show, unchanged
│   └── Season 01/…
└── In the Name of the People/   ← the collection, promoted to its own show
    ├── tvshow.nfo
    ├── poster.jpg
    └── Season 01/
        └── S01E001 - EP01.mp4

The title comes from one of three explicit choices — search TMDB, type it manually, or keep the collection's own name. MyTube never picks for you: TMDB matches that fail a strict token-subset gate render as suggestions and still require confirmation, and a TMDB key is needed only for the search path. TMDB supplies show identity, artwork and premiere date; episode numbers always come from MyTube's own allocator, never from TMDB, which cannot see local files.

The toggle is per-collection and appears only in the playlist_tv layout.

Design decisions worth reviewing

Decision Why
Originals are never moved or renamed Episodes are hard links back into uploads/videos/, so the mirror normally costs no extra disk. Copy fallback where linking is impossible, with linked-vs-copied counts reported.
Season/episode numbers are immutable once allocated An upstream reorder only updates a diagnostic sourcePosition. Renumbering would move every file in a season and destroy watch state.
A video in two playlists becomes two episodes Each gets an occurrence-scoped uniqueid, or a media server collapses them into one.
Cleanup is ledger-driven, not marker/pattern based A hard-linked media file can carry no in-band marker. Only paths the DB ledger proves MyTube created are deleted; user files in the mirror are preserved and reported as collisions.
Show directory names never change on a channel rename Path stability is a core invariant; the rename updates the NFO title instead.
Promotion carries episode numbers across but retires the old season number SxxExxx-keyed watch state survives the move; reusing the vacated season number under the author show would collide with a future playlist.
Activation does network work before taking the maintenance lock TMDB details and the poster download happen first; the locked section is DB-only, so an interactive request never holds the rename lock across an HTTP round trip.
A collection show is excluded from the author compatibility matcher sourceCollectionId is what prevents two shows that merely share a title from being merged.
media_server_shows.sourceCollectionId is deliberately not a FK SQLite's ALTER TABLE ADD … REFERENCES emits ON DELETE NO ACTION regardless of what drizzle declares, so the FK made collection deletion fail outright.

Commits

Managed TV library (1–4)

  1. feat(media-server) — the feature (schema + migration 0028, identity/metadata resolution, pure planner, materializer, artifact ledger, job/API, incremental hooks, Settings UI, 10 locales, docs).
  2. fix(download) — single-video downloads no longer expand the playlist. Independent bug, cherry-pickable.
  3. fix(db) — self-heal for the catalog when migration 0028 never runs. Depends on 1.
  4. fix(media-server) — route mirror filesystem access through the safe wrappers.

Collection-as-show (5–12)

  1. feat(media-server) — schema + migration 0029 and repository support.
  2. feat(tmdb) — strict match gate and collection-scoped search.
  3. feat(media-server) — reconcile marked collections as their own shows.
  4. feat(media-server) — carry episode numbers across a promotion.
  5. feat(media-server) — poster and TMDB identity in the mirror.
  6. feat(media-server) — activation service and API.
  7. test(media-server) — end-to-end coverage.
  8. feat(media-server) — opt-in UI, i18n, documentation.

Commits 2 and 3 are real production bugs found while testing this on a live NAS deployment; details in their commit messages.

Verification

Backend 230 files, 3124 tests passing
Frontend 182 files, 1844 tests passing
Builds / lint / typecheck Clean

playlistTv.integration.test.ts drives the real reconciler, planner and materializer against a real temp filesystem and a real SQLite catalog, asserting the complete directory listing and parsing every NFO with an XML parser. It caught two bugs unit tests missed (one author producing two shows; season titles falling back to Season NN) — both fixed here. The same fixture was extended rather than replaced for collection-as-show, so author-show behavior is proven unchanged in the same run: an author show and a collection show coexist, a video in both appears once under each sharing one inode, and a second rebuild is idempotent (no relink, no copy, no removal, unchanged mtimes).

Deployed and confirmed working end-to-end on a real Plex 1.43 instance.

Detailed verification notes are kept outside the repository (reports/ is gitignored); ask if you want them pasted here.

Outstanding — why this is a draft

The Jellyfin import test has not been run. The plan's own caution #18 says the issue is not complete without a real media-server import, and only Plex has been exercised so far. Plex 1.43 removed plugin support, so it reads no NFO at all — it validated the directory/SxxExxx contract but nothing about NFO content. Jellyfin is what would prove tvshow.nfo / season.nfo / episode NFOs actually parse and display.

Checklist and procedure are in the verification report.

Known limitations

  • Hard links need one filesystem; split uploads/ across mounts and it silently becomes copies (reported in the summary).
  • Plex ignores season.nfo, so playlist titles never appear there — placement still works via directory + filename tokens.
  • Promotion is not losslessly reversible. Promoting a collection that is already a season removes the old season directory and retires its season number; toggling back off returns it to the author show as a new season with a new number. Renaming the collection is the lossless way to fix only a title.
  • First rebuild is offline, so descriptions stay empty until subscriptions are re-inspected.
  • Only source-backed playlists become seasons; manually created collections do not — unless explicitly marked as their own show.
  • A new locale-parity test records twelve pre-existing untranslated keys (audio format / video resolution, missing from eight locales) as a baseline rather than fixing them here; it blocks new gaps.

🤖 Generated with Claude Code

@codacy-production

codacy-production Bot commented Aug 16, 2026

Copy link
Copy Markdown

Up to standards ✅

🟢 Issues 0 issues

Results:
0 new issues

View in Codacy

🟢 Metrics 1268 complexity

Metric Results
Complexity 1268

View in Codacy

🟢 Coverage 91.07% diff coverage · +0.49% coverage variation

Metric Results
Coverage variation +0.49% coverage variation (-1.00%)
Diff coverage 91.07% diff coverage

View coverage diff in Codacy

Coverage variation details
Coverable lines Covered lines Coverage
Common ancestor commit (54229ef) 30139 25881 85.87%
Head commit (6a07e79) 32117 (+1978) 27737 (+1856) 86.36% (+0.49%)

Coverage variation is the difference between the coverage for the head and common ancestor commits of the pull request branch: <coverage of head commit> - <coverage of common ancestor commit>

Diff coverage details
Coverable lines Covered lines Diff coverage
Pull request (#412) 2060 1876 91.07%

Diff coverage is the percentage of lines that are covered by tests out of the coverable lines that the pull request added or modified: <covered lines added or modified>/<coverable lines added or modified> * 100%

NEW Get contextual insights on your PRs based on Codacy's metrics, along with PR and Jira context, without leaving GitHub. Enable AI reviewer
TIP This summary will be updated as you push new changes.

franklioxygen added a commit that referenced this pull request Aug 16, 2026
…appers

Codacy flagged four security issues in the mirror materializer, all of them
genuine rather than false positives.

Three `fs.*` calls (`readFileSync`, `readdirSync`, `rmdirSync`) bypassed the
audited helpers in `utils/security.ts` and passed computed paths straight to the
filesystem. Those helpers exist precisely so every path is re-validated against an
explicit allowed root before the call, and they carry the reviewed `nosemgrep`
annotations. Use `readFileSafeSync`, `readdirSafeSync` and
`removeEmptyDirSafeSync` against `MEDIA_SERVER_LIBRARY_DIR` instead; the module no
longer imports `fs` at all.

The temp filename also used `Math.random()`. A predictable name in a directory the
user can write to invites a symlink/TOCTOU race between our create and our rename,
so it now uses `crypto.randomBytes`.

Fixes the four Codacy findings on #412; local `codacy_cli_analyze` reports the
file clean.
@franklioxygen franklioxygen changed the title feat(media-server): author / playlist / episode TV hierarchy for media servers (#411) feat(media-server): managed TV library with author shows and collection-as-show (#411) Aug 17, 2026
franklioxygen added a commit that referenced this pull request Aug 18, 2026
…appers

Codacy flagged four security issues in the mirror materializer, all of them
genuine rather than false positives.

Three `fs.*` calls (`readFileSync`, `readdirSync`, `rmdirSync`) bypassed the
audited helpers in `utils/security.ts` and passed computed paths straight to the
filesystem. Those helpers exist precisely so every path is re-validated against an
explicit allowed root before the call, and they carry the reviewed `nosemgrep`
annotations. Use `readFileSafeSync`, `readdirSafeSync` and
`removeEmptyDirSafeSync` against `MEDIA_SERVER_LIBRARY_DIR` instead; the module no
longer imports `fs` at all.

The temp filename also used `Math.random()`. A predictable name in a directory the
user can write to invites a symlink/TOCTOU race between our create and our rename,
so it now uses `crypto.randomBytes`.

Fixes the four Codacy findings on #412; local `codacy_cli_analyze` reports the
file clean.
@franklioxygen
franklioxygen force-pushed the feat/issue411-playlist-tv-export branch 2 times, most recently from 07c579e to 9c77855 Compare August 20, 2026 01:17
franklioxygen added a commit that referenced this pull request Aug 20, 2026
…appers

Codacy flagged four security issues in the mirror materializer, all of them
genuine rather than false positives.

Three `fs.*` calls (`readFileSync`, `readdirSync`, `rmdirSync`) bypassed the
audited helpers in `utils/security.ts` and passed computed paths straight to the
filesystem. Those helpers exist precisely so every path is re-validated against an
explicit allowed root before the call, and they carry the reviewed `nosemgrep`
annotations. Use `readFileSafeSync`, `readdirSafeSync` and
`removeEmptyDirSafeSync` against `MEDIA_SERVER_LIBRARY_DIR` instead; the module no
longer imports `fs` at all.

The temp filename also used `Math.random()`. A predictable name in a directory the
user can write to invites a symlink/TOCTOU race between our create and our rename,
so it now uses `crypto.randomBytes`.

Fixes the four Codacy findings on #412; local `codacy_cli_analyze` reports the
file clean.
@franklioxygen
franklioxygen marked this pull request as ready for review August 20, 2026 01:55

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 9c7785543e

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread backend/src/services/mediaServerExport/playlistTvSync.ts
Comment thread backend/src/services/mediaServerExport/syncService.ts
Comment thread backend/src/services/mediaServerExport/jobService.ts Outdated

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 154700b687

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread backend/src/services/mediaServerExport/catalogRepository.ts Outdated
Comment thread backend/src/services/mediaServerExport/catalogReconciler.ts
Comment thread backend/src/services/mediaServerExport/catalogReconciler.ts

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 240efb1492

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread backend/src/services/mediaServerExport/catalogReconciler.ts Outdated
Comment thread backend/src/services/storageService/videoDeletion.ts Outdated
Comment thread backend/src/services/mediaServerExport/playlistTvSync.ts

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 58cc66f053

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread backend/src/services/mediaServerExport/catalogReconciler.ts Outdated
Comment thread backend/src/controllers/subscriptionController.ts
Comment thread backend/src/services/mediaServerExport/mutationHooks.ts Outdated

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: b4081496b8

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread backend/src/services/mediaServerExport/jobService.ts Outdated
Comment thread backend/src/services/mediaServerExport/scopePreview.ts
Comment thread backend/src/services/mediaServerExport/jobService.ts Outdated

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 9c94f86a1b

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread backend/src/services/mediaServerExport/catalogReconciler.ts
Comment thread backend/src/services/mediaServerExport/playlistTvSync.ts Outdated

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 004ab60478

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread backend/src/services/mediaServerExport/collectionShowActivation.ts
Comment thread backend/src/services/mediaServerExport/catalogReconciler.ts

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 63168e7bf2

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread backend/src/services/mediaServerExport/catalogReconciler.ts
Comment thread backend/src/services/mediaServerExport/artifactRelocation.ts
Comment thread backend/src/services/mediaServerExport/collectionShowActivation.ts

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 12f2478629

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread backend/src/controllers/subscriptionController.ts
Comment thread backend/src/services/subscription/channelPlaylists.ts
Comment thread backend/src/services/mediaServerExport/collectionShowActivation.ts Outdated
Comment thread backend/src/db/schema.ts

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 30b28dc300

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread backend/src/services/mediaServerExport/jobService.ts Outdated
Comment thread backend/src/services/mediaServerExport/mutationHooks.ts Outdated
Comment thread backend/src/services/mediaServerExport/mediaMaterializer.ts Outdated

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 43844ea889

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread backend/src/services/mediaServerExport/catalogReconciler.ts
Comment thread backend/src/services/mediaServerExport/catalogReconciler.ts
…r media servers

Add an opt-in `Author -> playlist seasons` media-server export layout that builds
a separate, MyTube-managed TV library under `uploads/media-library/`: one show per
author, one numbered season per source-backed playlist, and Season 00
(Specials / Unassigned) for videos that belong to no playlist. Each show gets
`tvshow.nfo` and a poster, each season a `season.nfo`, and each episode its own
NFO, thumbnail, subtitles and optional source JSON.

The existing `adjacent` layout remains the default, so nothing changes for
current installations until they opt in.

Durable concepts, all persisted so they never drift:

- Show identity resolves once, in precedence order (channel id, normalized
  channel URL, normalized author name) and is never recomputed from a changed
  display title. A channel rename updates the NFO title, not the directory.
- Season numbers are allocated transactionally on first attachment and never
  reused or renumbered. An upstream playlist reorder only records the new
  `sourcePosition`.
- Episode numbers are seeded from the position observed at first import and are
  immutable afterwards, so a media server never loses watch state to a reshuffle.
- A video in several playlists becomes one episode per playlist, each with its
  own occurrence-scoped `uniqueid` so servers cannot collapse them.

Originals are never moved, renamed, or duplicated: episodes are hard links back
into `uploads/videos/`, so the mirror normally consumes no extra disk. Where hard
links are impossible MyTube copies instead and reports linked-versus-copied
counts; the fallback can be disabled, in which case those episodes report
`hard_link_failed_copy_disabled`.

Cleanup is driven by a database ownership ledger rather than filename patterns or
XML markers, because a hard-linked media file can carry no in-band marker. Only
paths the ledger proves MyTube created are ever deleted; a file the user placed in
the mirror is preserved and reported as a collision instead of overwritten.

Planning is pure and deterministic, materialization is atomic (same-directory
temp write plus rename, ledger row only after the rename succeeds), and rebuilds
are offline, cancellable, idempotent and isolated per show.

Refs #411
Downloading a URL like `watch?v=X&list=Y` made yt-dlp expand the whole playlist:
neither the metadata probe nor the download flags passed `--no-playlist`. A
single-video request therefore extracted full metadata for every entry in the
playlist — minutes of work for one video — and derived the title, author and
filename from the *playlist* envelope rather than the video, which also left the
video without a thumbnail. One private entry anywhere in that playlist failed the
whole download.

`prepareAudioDownloadFlags` had always set `noPlaylist`; only the video path was
missing it, so audio-only downloads were already correct.

Every caller of this path (manual download, subscriptions, continuous-download
tasks, Twitch subscriptions, collision repair) already resolves a single video URL
before calling, and playlists have their own `checkPlaylist` and subscription
flows, so nothing relied on the expansion.
… runs

On a long-lived database the media-server tables and the new `collections`
columns were never created, and every `getCollections()` call then failed with
`no such column: collections.description` — breaking the collections list,
playlist subscriptions and collection-linked downloads, not just the export.

drizzle runs each migration file in a single transaction. On a database whose
schema was partly built by the runtime self-heal, an *earlier* migration can still
abort with a duplicate column; `migrate.ts` deliberately swallows that error, and
drizzle then never reaches any later migration. Migration 0028 was the victim, not
the cause.

Add `ensureMediaServerCatalogTables()` next to the existing
`ensureVisitorUsersTable()` and `ensureFavoritesTables()` self-heals, which exist
for exactly this reason. It is idempotent: `CREATE TABLE/INDEX IF NOT EXISTS` plus
per-column presence checks, since SQLite has no `ADD COLUMN IF NOT EXISTS`.

The existing tests missed this because they replay migrations onto a clean SQLite
file, where no duplicate-column conflict exists. The new test reproduces the
reported state directly.

Refs #411
…appers

Codacy flagged four security issues in the mirror materializer, all of them
genuine rather than false positives.

Three `fs.*` calls (`readFileSync`, `readdirSync`, `rmdirSync`) bypassed the
audited helpers in `utils/security.ts` and passed computed paths straight to the
filesystem. Those helpers exist precisely so every path is re-validated against an
explicit allowed root before the call, and they carry the reviewed `nosemgrep`
annotations. Use `readFileSafeSync`, `readdirSafeSync` and
`removeEmptyDirSafeSync` against `MEDIA_SERVER_LIBRARY_DIR` instead; the module no
longer imports `fs` at all.

The temp filename also used `Math.random()`. A predictable name in a directory the
user can write to invites a symlink/TOCTOU race between our create and our rename,
so it now uses `crypto.randomBytes`.

Fixes the four Codacy findings on #412; local `codacy_cli_analyze` reports the
file clean.
Groundwork for exporting a marked collection as its own media-server show
instead of a season under an author show, with an optional TMDB-resolved
identity.

`collections` gains the opt-in flag, the resolved show metadata (title,
description, poster, source) and the confirmed TMDB identity. `media_server_shows`
gains `sourceCollectionId` plus an offline projection of the TMDB id, media type
and premiere date so the pure planner can build `tvshow.nfo` without reading the
collection row or touching the network.

A partial unique index enforces one show row per collection in the database
rather than only in application code.

The show-export fields are deliberately excluded from `saveCollection`'s metadata
patch, alongside the existing season-allocation columns: they are written only by
the activation transaction, which commits metadata and catalog state together.
A stray save could otherwise allocate an immutable show directory from a title the
user never confirmed.

Migration 0029 ships with a matching idempotent self-heal, per the lesson from
0028 never running on a long-lived database.

Also freezes two behaviors this work must not change: the deliberately loose
scan-flow TMDB gate, and the author matcher merging two identities that share a
platform and normalized title.
Adds the two pieces the collection-as-show flow needs from TMDB, without
disturbing the local-file scan that is the service's only current caller.

`isHighConfidenceTMDBTitleMatch()` sits alongside the existing gate rather than
replacing it. The existing one accepts "two shared tokens", which is right for
scan-imported filenames but wrong for arbitrary user text: `How Many Ants Live On
Earth` shares two tokens with `How To Live On Mars`, and a media-server show
directory is allocated once and never renamed, so accepting that would be
permanent. The strict gate takes exact, collapsed-exact, or a one-directional
full token subset — every query token must appear in the candidate, not the
reverse, since a shorter candidate means the query carries extra meaning
(`The Office` is not `The Office Christmas Special`). Substring containment is
excluded for the same reason.

`collectionSearch.ts` deliberately does not wrap `searchTMDBMultiStrategy()`.
That runs seven fallback strategies including a transformed fuzzy query, filters
with the loose predicate, and returns one winner — all wrong when the user must
see and confirm candidates. This issues one multi-search with the literal query,
drops `person` results, and returns a bounded annotated list. Confidence affects
ranking and labeling only; nothing is ever auto-applied.

Confirmation refetches details by validated `{tmdbId, mediaType}` so a tampered
request cannot write arbitrary text into a show's NFO.
A collection marked `exportAsShow` now exports as its own media-server show with
a single Season 01, instead of a season under an author show. Unmarked
collections and author shows behave exactly as before.

Three things this required, each guarding a way the catalog could go wrong:

`ensureCollectionShow()` is a separate allocator from `ensureMediaServerShow()`.
The author path runs the compatibility matcher, which merges shows sharing a
platform and normalized title — that would collapse two same-named dramas into
one, permanently, since directory names are immutable. A collection-show is keyed
on `collection:<id>` alone, and the matcher now also skips any row carrying
`sourceCollectionId`: the key prefix is not enough, because the matcher compares
platform and title rather than the key.

The marked-collection pass runs before the author pass, so a marked collection is
never also treated as a season and its assignments already exist when the
Season 00 rule decides whether a video still needs a special.

Stale-occurrence detection now compares full occurrence keys
`(collection, show, season, video)` instead of `collection:video`. After a
collection moves between shows, the obsolete row and the desired row reference
the same collection and video, so the old key could not tell them apart and the
mirror kept a duplicate under the previous show. Collections this run did not
evaluate still fall back to the membership check, so a scoped incremental
reconcile never deletes assignments it did not look at.

`sourceCollectionId` is deliberately not a foreign key. SQLite's ALTER TABLE ADD
COLUMN cannot carry an ON DELETE action, so drizzle emitted a plain REFERENCES
defaulting to NO ACTION — which made deleting a collection with a collection-show
fail outright, the opposite of the intended SET NULL and of the design's
requirement that the show row survive. Caught by the new tests.
When a collection that was already a season under an author show is marked as its
own show, its episodes now keep the exact episode number, stem and source
position they had before.

Without this the move re-derived numbers from current collection order, so every
mirror filename changed and a media server would see the whole season vanish and
reappear renamed — losing watch state. Reusing them is not a breach of immutable
numbering: that rule forbids renumbering within a season on an upstream reorder,
not carrying numbers across an explicit user-initiated move.

The carried stem is honored only when its number was also free, so the SxxExxx
token in a filename can never disagree with the episode number it encodes. A
collection with no prior season still allocates fresh numbers from its order.
…irror

Three planner/NFO changes so a collection-show renders correctly.

Poster precedence now branches on `sourceCollectionId`. A collection show goes
persisted poster → episode thumbnail, skipping the author avatar entirely: a
drama uploaded by a third-party channel was otherwise posterised with the
uploader's selfie. Author shows keep persisted → avatar → thumbnail exactly as
before, guarded by its own regression test — this branch must not remove avatar
fallback globally. Collection shows also take their episodes in episode order
rather than by video id, so the representative frame is the first episode.

`tvshow.nfo` gains `<uniqueid type="tmdb">`, emitted only for a confirmed TV id.
TMDB movie and TV ids share a numeric space and an id in `tvshow.nfo` is read as
a TV id, so publishing a movie id would point the server at an unrelated series;
a matched movie still contributes title, overview, date and poster. The MyTube id
remains `default="true"`.

A confirmed TMDB premiere date now outranks the earliest episode upload date,
which is only a stand-in.

The snapshot also emits an explicit season entry for each collection show. Its
collection row carries no `mediaServerShowId` — that column tracks author-season
attachment — so without this the planner fell back to a bare label and a
show-derived season id instead of the collection-scoped one.
Adds the user-facing half of collection-as-show: a read-only TMDB search and a
locked activation that commits the accepted metadata and the flag together.

Ordering is the substance here. A show directory is allocated once and never
moved, so the metadata the user accepted must be persisted before the catalog is
touched — a fallback title would otherwise claim the folder and the confirmed
title could never take it. All network work (TMDB details, poster download)
therefore happens before the maintenance lock is acquired, and the locked section
is database-only, so an interactive request never holds the lock across an HTTP
round trip. Layout and collection are re-read under the lock, since either can
change while the network work is in flight.

A held lock (rebuild or batch rename in progress) returns a typed retryable
error rather than mutating the mirror underneath it.

Poster storage is collection-scoped: a SHA-256 digest of the collection id as the
directory and the validated media type plus numeric TMDB id as the filename, so a
collection id is never a raw path segment and re-resolving writes a new file
rather than overwriting the active poster before the transaction commits. A
poster failure is deliberately non-fatal — the identity is still worth
committing, and the planner falls back to an episode thumbnail.

Confirmation refetches details by validated `{tmdbId, mediaType}`; a title posted
by the browser is ignored, so a tampered request cannot write arbitrary text into
a show's NFO. Manual entry clears every TMDB field rather than leaving a stale
identity attached to a typed title. Deactivation clears only the flag, retaining
the resolved identity for a later re-enable.
Extends the existing playlist-TV fixture rather than starting a new one, so
author-show behavior is proven unchanged in the same run.

Covers an author show and a collection show coexisting in one mirror, episode
numbers and filenames surviving a promotion unchanged, the TMDB unique id and
premiere date reaching tvshow.nfo, the poster branch (collection show takes an
episode thumbnail while the author show keeps its avatar), the duplicate video
appearing under both shows while sharing a single inode with the original,
idempotence after promotion, and the collection-title fallback.

The fixture also now gives every video the channel avatar. Only one video
carried it before, which is not how a real channel looks and made the author
show lose its poster the moment that video moved to a collection show — the
poster-branch test was asserting an artifact of the fixture rather than the
behavior. With the avatar on every video the test proves the real distinction:
the collection show declines an avatar that is available to it.
The previous commit added materializeMediaServerHierarchyAsync so a full
managed-library rebuild would stop holding the event loop, but never wired
it in: syncPlaylistTvLibrary stayed synchronous and kept calling the
synchronous materializer, leaving the new import unused. The job's `await`
therefore received an already-computed value, so the run still occupied the
process from start to finish - the status endpoint went unanswered, and
`cancelRequested` could not become true during the very run it was meant to
stop. A large library, or a copy fallback moving whole video files, blocked
the server for as long as it took.

syncPlaylistTvLibrary is now async and materializes through the yielding
path. The three incremental entry points keep the synchronous materializer:
they touch one video inside a database mutation, where an await would change
transaction semantics for no benefit.

Two regression tests pin the wiring rather than the materializer, which was
already covered: a rebuild must let queued event-loop work run before it
settles, and a cancel flag flipped from a setImmediate while the rebuild is
in flight must stop it short of the full mirror. Both fail against the
synchronous call.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 8af3e9ff0c

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread backend/src/services/mediaServerExport/hierarchyMaterializer.ts Outdated
Comment thread backend/src/services/downloadService.ts
Comment thread backend/src/services/mediaServerExport/catalogReconciler.ts
…ctions, refresh preserved-special show metadata

Addresses the three remaining review threads on #412:

- The async rebuild path only yielded between shows, so one show with many
  episodes (or a large copy fallback) still blocked the event loop and the
  cancel request could never be observed. Show materialization and the stale
  sweep are now generators: the incremental hooks drain them synchronously,
  while the rebuild awaits the event loop between episodes and between swept
  artifacts.

- downloadChannelPlaylists stamps playlist identity onto a reused same-named
  collection but the task processor skips already-downloaded URLs, so existing
  members never got a link hook and sat in Season 00 until a full rebuild. The
  collection is now reconciled via onCollectionMetadataCommitted after its
  task is created, mirroring the channel-playlists watcher.

- The preserved Season 00 early return skipped all show metadata enrichment,
  so a renamed channel's show kept its old title and never gained the newly
  observed channel id - and the next video from that channel failed the
  author/title match and allocated a second show. The owning show's metadata
  is now refreshed when it is unambiguously the same channel, without ever
  reallocating the episode.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: c2d4d40619

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread backend/src/services/mediaServerExport/catalogReconciler.ts
Comment thread backend/src/services/mediaServerExport/catalogRepository.ts
Comment thread backend/src/services/mediaServerExport/catalogRepository.ts Outdated
Comment thread backend/src/services/mediaServerExport/catalogReconciler.ts
Comment thread backend/src/services/mediaServerExport/jobService.ts Outdated
…renamed channel identities

Addresses the five review threads raised on c2d4d40:

- isSourceBackedPlaylistCollection rejected a manual/author_auto collection
  even after downloadChannelPlaylists stamped a real playlist onto it, and a
  continuous playlist task has no subscription row - so the reused-collection
  reconcile added in c2d4d40 had nothing to do and its members stayed in
  Season 00. An explicitly stamped source type plus a durable identifier now
  outranks the origin the collection was born with.

- Promotion carried an episode's stem verbatim, so a collection promoted out
  of Season 02 wrote "Season 01/S02E001 - Title.mp4" and media servers that
  read placement from the SxxExxx token imported it into the wrong season.
  The season token is now rewritten for the season being written; the episode
  number and title portion still carry across untouched.

- updateMediaServerShowMetadata refused to replace a stored channel URL, so a
  handle rename proven by an equal durable channel id left the old URL behind
  and a later offline candidate carrying only the new URL hit the
  conflicting-URL rule and allocated a second show. The refresh is now opt-in
  via allowChannelUrlRefresh, set only on an id-proven match.

- refreshShowForPreservedSpecial rejected the owning show when a redownload
  changed the display name and the handle at once while supplying the
  channel's first durable id. It now also accepts a strictly stronger identity
  the show can adopt without conflict - the assignment is that video's own
  durable placement, and no other show may already claim the id.

- The playlist_tv cleanup confirmation claimed it only touches the managed
  library folder, while the action also sweeps MyTube-owned sidecars beside
  every original. The sweep is deliberate, so the copy now discloses both.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 5b5dfe6185

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread backend/src/services/mediaServerExport/hierarchyPlanner.ts
Comment thread backend/src/services/mediaServerExport/metadataResolver.ts Outdated
Comment thread backend/src/services/mediaServerExport/mediaMaterializer.ts Outdated
…enamed collections, roll back failed fallback publication

Addresses the three review threads raised on 5b5dfe6:

- A planner skip dropped the episode from the plan while its catalog
  assignment stayed valid, and the ledger sweep deletes every path outside
  expectedRelativePaths - so a source that was merely unreachable (unmounted
  drive, NAS blip, file mid-move) cost the user a still-playable mirror, and a
  show whose only episode was skipped lost its tvshow.nfo, poster and
  season.nfo too. Existing ledger paths of skipped-but-still-assigned episodes
  are now protected, along with the show-level artifacts of any show holding a
  skip. Deletion authority stays with the reconciler dropping the assignment
  (which nulls the ledger's assignment_id and severs the protection) and with
  the explicit cleanup action, which plans nothing and protects nothing.

- buildCollectionMetadataPatch kept a persisted channel URL and name merely
  because they were non-empty, so an id-proven rename left them stale. Since
  collection metadata outranks a video's during show resolution, the next
  collection reconcile wrote the old URL back over the show refreshed in
  5b5dfe6 and a later URL-only video allocated a duplicate show. Both fields
  now refresh when the candidate channel id matches the persisted one;
  sourceUrl is excluded because it identifies the playlist, not the channel.

- The fallback publication path (for filesystems whose rename refuses an
  existing destination) unlinked the destination and renamed with no rollback,
  so a failure of that second rename left the artifact missing entirely - the
  caller's finally drops the staged replacement. The old file is now moved
  aside and restored on failure, with the backup path logged if the restore
  itself fails.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 83189d637c

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread backend/src/services/mediaServerExport/jobService.ts Outdated
Comment thread backend/src/services/mediaServerExport/syncService.ts
Comment thread backend/src/services/mediaServerExport/scopePreview.ts Outdated
…ata, count only materializable videos

Addresses the three review threads raised on 83189d6:

- The cleanup action drained cleanupMediaServerMirror synchronously, so the
  event loop could not serve status or cancellation while thousands of
  artifacts were unlinked - the isCancelled callback it was handed could not
  change during the call, and a user trying to stop a destructive sweep was
  only heard once it had finished. Adds cleanupMediaServerMirrorAsync, driving
  the existing per-artifact sweep generator with a yield per artifact, and
  routes the job's cleanup phase through it. The mutation hooks keep the
  synchronous version: they sweep one show inside a database mutation.

- syncMediaServerArtifactsForRecord synthesized a source-JSON envelope even
  when the caller supplied no rawSourceInfo, so an ordinary metadata or
  artwork edit overwrote the rich downloader-generated .info.json and
  permanently discarded extractor-only fields. buildSourceJsonMap now returns
  undefined without fresh extractor output, and materializeEpisode keeps an
  already-published source JSON rather than replacing it with a synthesized
  one. An episode that has none yet is still synthesized, so the planner never
  reserves a path that goes unwritten. This also stops a full offline rebuild
  from flattening every .info.json in the library.

- The rebuild scope preview counted any video with a videoPath, while
  planEpisode deterministically skips cloud:, mount: and http(s) records and
  the reconciler never assigns audio-only media, so the confirmation inflated
  its video and author-show counts. It now applies the planner's own
  managed-local-path predicate. The missing-file probe stays out on purpose:
  it is not deterministic, which is why the copy says "about".

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: f79a55e26f

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread backend/src/services/mediaServerExport/mediaMaterializer.ts
Comment thread backend/src/services/mediaServerExport/collectionShowActivation.ts Outdated
…lection posters

Addresses the two review threads raised on f79a55e:

- The unchanged fast paths in linkMirrorMediaArtifact and copyMirrorImageArtifact
  compared a fingerprint of the SOURCE only, then returned before
  assertDestinationIsReplaceable ran its lstat. A destination swapped for a
  symlink (or a directory, or another assignment's artifact) still matched the
  fingerprint, so the rebuild reported it as unchanged and left the mirror
  serving something the module promises never to create. The destination check
  now runs before the fast path in both, matching what writeMirrorTextArtifact
  already did.

- Collection-show activation downloads its poster before taking the maintenance
  lock, so every later bail-out - lock held by a rebuild, layout changed,
  collection deleted - left a full-size image and its small mirror on disk with
  nothing referencing them. Aborted activations now discard the staged poster,
  except when it is the path the collection already uses, since re-resolving the
  same TMDB match rewrites that exact file. A successful activation also retires
  the poster it replaced: posters are named per TMDB id, so switching matches
  accumulated one image per id. Removal is scoped to the tmdb/collections/
  namespace so a user-supplied image is never deleted.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: c975d4e42a

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread backend/src/services/mediaServerExport/catalogReconciler.ts Outdated
Comment thread backend/src/services/mediaServerExport/collectionShowActivation.ts Outdated
…keep a poster whose refresh failed

Addresses the two review threads raised on c975d4e:

- resolveCollectionShowCandidate copied subscriptions.authorUrl into
  sourceChannelUrl, but subscribePlaylist stores the PLAYLIST url in that
  column and normalizeChannelUrl drops the query string, so every YouTube
  playlist reduced to the identity youtube:channel-url:youtube.com/playlist.
  Every legacy playlist collection (no persisted channel metadata, which is
  exactly the set reaching that fallback) was therefore attached as a season of
  one shared show - permanently, since season numbering is allocated once.
  authorUrl is now only used for a non-playlist subscription; a playlist
  subscription contributes its channel NAME, which subscribePlaylist fills with
  the real uploader, and otherwise resolution falls through to the member
  videos.

- Activation committed a null poster path whenever a download failed, so
  re-confirming the SAME TMDB entry during a network blip dropped the show to
  its thumbnail fallback and, since c975d4e, deleted the still-valid image.
  The previous path is now retained when the TMDB identity is unchanged and no
  replacement was downloaded. Only then: the manual and collection modes clear
  the poster deliberately, and a genuinely different match must not keep the
  old one.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: f8b0d11340

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread backend/src/services/mediaServerExport/jobService.ts Outdated
Comment thread backend/src/services/mediaServerExport/playlistTvSync.ts Outdated
Comment thread backend/src/services/mediaServerExport/hierarchyMaterializer.ts
…arked metadata until a sync succeeds, validate preserved artifacts

Addresses the three review threads raised on f8b0d11. The first and third are
regressions from earlier commits in this PR.

- processAdjacentJob sets status/phase to completed before the managed-mirror
  sweep runs, and that sweep became async in f79a55e - so a status poll landing
  in one of its yields observed "completed" before the sweep count and failures
  were recorded, and a new rebuild admitted by the status === "running" guard
  collided with the maintenance lock this run still held (released only in the
  finally). The adjacent branch now re-opens the job for the secondary phase and
  completes it once, at the end, leaving an already-cancelled run alone.

- syncPlaylistTvForVideo consumed the parked downloader envelope before
  materializing. It is the only copy of the extractor output, so any
  materialization failure lost the extractor-only fields permanently - nothing
  re-fetches them and a later rebuild can only synthesize. It is now peeked, and
  dropped only when the run both owns it and finishes without failures; the
  store bounds itself by age and size, so an unclaimed entry cannot accumulate.

- isArtifactPublished, added in f79a55e for the source-JSON preservation
  branch, checked only the ledger row and existsSync - which follows symlinks
  and accepts directories. A managed .info.json swapped for either was reported
  as published and silently preserved, bypassing the lstat and ownership checks
  the media and image paths perform. It now requires a regular file and a ledger
  row not owned by another assignment, so a swapped destination falls through to
  the write path and is rejected there with the same typed failure.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: e29e27f14a

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread backend/src/services/mediaServerExport/playlistTvSync.ts
Comment thread backend/src/services/mediaServerExport/playlistTvSync.ts Outdated
Comment thread backend/src/services/mediaServerExport/hierarchyPlanner.ts Outdated
Comment thread backend/src/services/mediaServerExport/jobService.ts
…rotection to the season, yield in the adjacent sweeps

Addresses the four review threads raised on e29e27f.

- A collection mutation committed during one of the yielding rebuild's pauses
  could delete an assignment the captured plan still lists. Publishing it then
  wrote the file and failed to record it (the ledger's assignment FK is gone,
  and foreign_keys is ON), stranding an untracked artifact no sweep may remove
  - untracked is exactly how a user's own file is recognized. The materializer
  now re-checks the assignment after each yield and skips it if it is gone.
  Nothing runs between that check and the ledger write, so check and publish
  are effectively atomic; linkVideoToCollection is deliberately left
  unsynchronized rather than made to fail during a rebuild.

- Consuming the parked downloader envelope on "no failures" was too weak: an
  episode the planner skips is never written at all, so it reports no failure
  while its .info.json was never published. The envelope is now dropped only
  when the video was actually planned and published.

- collectSkippedAssignmentPaths protected every assignment-less artifact of a
  show holding any skip, including season.nfo for unrelated seasons - so a
  season the reconciler had just emptied kept its NFO and its directory stayed
  visible. Show-level artifacts are still protected wholesale; season NFOs only
  for the seasons that actually hold a skipped assignment.

- The adjacent sidecar passes deleted one file set per video synchronously, so
  their per-video cancel checks read a flag that could not change. Both now
  yield per video. Making them yield exposed the same premature-completion bug
  in the playlist_tv branch that e29e27f fixed for the adjacent branch, so
  both secondary phases now share one withJobReopenedForSecondaryPhase helper.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 47884935a2

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread backend/src/services/mediaServerExport/hierarchyMaterializer.ts
Comment thread backend/src/services/mediaServerExport/mediaMaterializer.ts
…rtifacts a plan never saw

Addresses the two review threads raised on 4788493.

- Containment inside MEDIA_SERVER_LIBRARY_DIR was checked lexically, and only
  the final component was lstat'd. A show or season directory replaced by a
  symlink is followed by every fs call, so the final component reports an
  ordinary file while it actually lives elsewhere: publication overwrote, and
  cleanup deleted, files outside the mirror entirely. Confirmed by test - the
  sweep really did unlink a file outside the root. New isRealPathInsideDir
  resolves the deepest existing ancestor through symlinks and verifies real
  containment; assertInsideMirror (the choke point for every publish, preserve
  and remove) now enforces it, and the directory prune checks it per level
  because it walks upward and could otherwise rmdir one level beyond the link.

- The rebuild's stale sweep reloaded the ledger but judged it against the
  expected set of a plan captured earlier, so an incremental hook committing
  during one of the rebuild's yields had its freshly published episode and
  season artifacts deleted immediately - the assignment survived, and no later
  hook republishes. The plan now carries the ledger paths that existed when it
  was captured, and the sweep reclaims only those. The explicit cleanup action
  leaves the field undefined and still removes everything the ledger owns.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: b82d8f26df

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread backend/src/services/mediaServerExport/hierarchyMaterializer.ts Outdated
Comment thread backend/src/services/mediaServerExport/collectionShowActivation.ts Outdated
… off the live path

Addresses the two review threads raised on b82d8f2.

- materializeShowSteps built tvshow.nfo (and the episode NFOs' show title) from
  the plan captured before the rebuild started. A rename or TMDB re-match
  committed during one of the rebuild's yields publishes an updated NFO through
  its own hook, and the rebuild then wrote the older copy back over it, leaving
  a stale title on display until some later reconcile. The show row is now
  re-read before writing; identity and directory name are allocated once, so
  only display metadata can differ. A show whose row disappeared entirely is
  skipped rather than written, since recording against a dangling show id would
  fail the ledger FK after the file was already on disk.

- Collection-show activation downloaded its poster straight onto the final
  path, before taking the maintenance lock. For an unchanged TMDB match that
  path IS the collection's live poster, so a request that then returned
  lock_unavailable had still replaced it, and because writeFileSafe is not
  atomic a concurrent rebuild could copy it half-written into the mirror. The
  download now stages to a temporary sibling and is moved into place by a
  same-directory rename only after the lock is held and the collection
  revalidated; aborts discard the staged file and never touch the live one.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 8e2161d227

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread backend/src/services/mediaServerExport/jobService.ts Outdated
Comment thread backend/src/services/mediaServerExport/hierarchyMaterializer.ts Outdated
Comment thread backend/src/services/mediaServerExport/hierarchyMaterializer.ts Outdated
Comment thread backend/src/services/mediaServerExport/jobService.ts Outdated
…metadata, yield in the orphan sweep

CI: CodeQL raised 3 high-severity js/path-injection alerts against
renameSafeSync and unlinkSafeSync in utils/security.ts. The poster staging
added in 8e2161d created new request-reachable flows into both, and neither
carried the inline containment re-check that writeFileSafe, pathExistsSafeSync
and copyFileSafeSync already use precisely because the resolver is invisible to
static analysis. Both now follow that same established pattern.

Review threads raised on 8e2161d:

- The cleanup branch was the last caller still awaiting the mirror sweep
  outside withJobReopenedForSecondaryPhase, so a poll landing in one of its
  per-artifact yields saw "completed" while a destructive full-library delete
  ran on, and a new run was admitted into the lock this one still held.

- materializeEpisode built the episode NFO and source-info envelope from the
  video record the plan captured, so a title or description edit committed
  during a yield was published by its own hook and then reverted by the
  rebuild. The record is now re-read and merged over the captured copy, which
  keeps the plan-time resolved source and artwork paths untouched.

- Season NFOs had the same staleness through their collection: a rename during
  a yield was written by the hook and then overwritten with the captured title
  and plot. Season metadata is now re-resolved from the current collection row.
  Both refreshes are best-effort - a failed read falls back to the captured
  value rather than failing an artifact that would otherwise publish.

- sweepOrphanMediaServerArtifacts scanned all of VIDEOS_DIR and deleted in one
  burst before the first per-video yield, so a cancel arriving during it could
  not be served. Added a yielding variant that checks cancellation between
  deletions, and the adjacent job now abandons the run when it reports one.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 1f7d126371

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread backend/src/services/mediaServerExport/hierarchyMaterializer.ts
Addresses the review thread raised on 1f7d126.

The per-episode assignment check added in 4788493 skips the episodes of a
season whose collection was deleted while the rebuild yields, but the season
NFO was written unconditionally at the top of the loop - putting straight back
what that collection's own hook had just removed. The stale sweep cannot
reclaim it either, since the path is still in the plan's expected set, so an
empty obsolete season stayed visible until another rebuild.

A season now gets no scaffolding at all when none of its planned episodes'
assignments survive. The per-episode check is deliberately kept alongside it:
this one-time test cannot see a deletion that lands later in the run.

A season whose episodes are merely unavailable is untouched - those assignments
still exist, so it keeps its NFO, which is what the skipped-source protection
depends on.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 8a24fcd06a

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread backend/src/services/mediaServerExport/hierarchyMaterializer.ts
Comment thread backend/src/services/tmdbService/poster.ts Outdated
Comment thread backend/src/services/continuousDownload/taskProcessor.ts Outdated
Comment thread backend/src/services/mediaServerExport/scopePreview.ts
…past mirror failures, recover a failed collection link

Addresses the four review threads raised on 8a24fcd. The first two are
regressions from earlier commits in this PR.

- The show ROW outlives its assignments, so the season guard added in 8a24fcd
  was not enough: tvshow.nfo and the poster were still recreated before every
  season was skipped, leaving an empty show directory - the very thing that
  guard prevents one level down. A show with no live assignment is now skipped
  outright. Asked of the catalog rather than the captured plan, so an
  assignment added concurrently still counts.

- publishStagedCollectionPoster (8e2161d) wrapped the rename and the
  small-thumbnail refresh in one try. A mirror failure after a successful
  rename therefore reported a failed publication, and the caller committed a
  null poster path for an image that was on disk - deleting the previous one
  for a newly selected identity. The rename is now the publication; the derived
  mirror is best effort.

- addVideoToCollection returns null rather than throwing when the collection is
  deleted mid-download, so the surrounding try/catch never saw it. The
  downloader had already suppressed its own media-server sync because the task
  carried a collection id, so the video was never exported at all and its
  parked extractor envelope simply expired. The task now exports the video
  directly when the link comes back null.

- The rebuild scope preview counted every marked collection, including ones
  whose members are all audio or non-local and which therefore produce no
  directory. Only collections holding at least one materializable video count.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: a41f53cc65

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment on lines +74 to +75
if (!collectionByVideoId.has(videoId)) {
collectionByVideoId.set(videoId, collection);

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Count all playlist identities for multiply assigned videos

When one local video belongs to multiple unmarked source-backed playlists with different channel identities, this map retains only the first collection. The preview consequently adds only that collection's identity, while catalogReconciler.ts evaluates every qualifying collection independently and materializes the shared video as a season under each channel show. A library containing only that shared video is therefore confirmed as one show even though the rebuild creates two; preserve and evaluate all relevant memberships rather than one arbitrary collection.

Useful? React with 👍 / 👎.

… PR added

CI reported a -1.42% project coverage variation against a -1.0% target. Diff
coverage passes, so the lines this PR adds are covered; the gap is project-wide.
These tests close the part of it this PR is actually responsible for - code it
added that had no direct coverage of its own.

- tmdbService/poster.ts was the largest gap at 59% lines. The activation tests
  mock the poster helpers, so they could never exercise them: a new
  posterStaging suite drives the real implementations. It covers staging beside
  the destination rather than onto it, the SSRF rejection before any request,
  publication surviving a small-mirror failure, a rename failure discarding the
  staged file, removeCollectionPoster refusing paths outside its own namespace,
  and the per-TMDB-id path derivation with its validation rejections.

- isRealPathInsideDir, added for symlinked-ancestor confinement, had untested
  guard branches. Its own suite covers containment, not-yet-existing paths under
  an existing ancestor, escape through a symlinked ancestor, and unusable input.

- sweepOrphanMediaServerArtifactsAsync had no tests: parity with the
  synchronous sweep, stopping partway on cancellation, and doing nothing when
  cancelled up front.

- syncPlaylistTvForCollection was largely uncovered, including the catalogOnly
  branch that backs the promise that a show's directory name is allocated from
  the title the user just confirmed even while the export is off.

- Ownership and pruning guards in mediaMaterializer, and the naming and identity
  contracts, including retokenizeExportStem returning undefined for a stem it
  cannot parse rather than emitting a half-rewritten one.

Project line coverage 87.07% -> 87.37%.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 6a07e7945b

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment on lines +1268 to +1270
pendingCollectionLink:
sub.subscriptionType === "playlist" &&
Boolean(sub.collectionId),

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Export the download when subscription linking fails

For playlist subscriptions this suppresses the downloader's managed-library sync, but the corresponding consumer in checkSubscription (subscriptionService.ts:855-870) ignores the nullable return from addVideoToCollection and catches thrown link errors without invoking any fallback export. If the collection is deleted while the download is running, or linking throws after the download succeeds, the subscription still advances and reports success while the video remains absent from the managed library and its parked extractor metadata eventually expires. Fresh evidence beyond the task-processor fix is this separate subscription-watcher path, which still lacks the same fallback.

Useful? React with 👍 / 👎.

Comment on lines +292 to +296
const published = await publishStagedCollectionPoster(
resolved.posterStagedPath,
resolved.posterFinalPath,
resolved.mediaServerPosterPath as string
);

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Revalidate the collection after poster publication yields

When a user deletes the collection while publishStagedCollectionPoster() is awaiting small-thumbnail generation, the maintenance lock does not prevent that collection mutation. Execution then resumes with the stale current object, publishes a database update that affects zero rows, leaves the just-renamed poster orphaned, and returns an undefined collection that causes the controller response path to fail. Revalidate after this await, or move the yielding derivative work until after the collection metadata has been committed.

Useful? React with 👍 / 👎.

Comment on lines +188 to +192
if (settings.mediaServerExportLayout === "playlist_tv") {
cleanupMediaServerMirror(new Set([show.id]));
}

releaseCollectionShowOwnership(show.id);

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Remove the persisted poster when deleting its collection

When a collection exported from a TMDB match is deleted, this hook cleans its mirror and releases the catalog claim but never removes collection.mediaServerPosterPath or its small-thumbnail mirror. Those paths are unique to the collection, and after the row is deleted nothing retains the reference needed by removeCollectionPoster(), so every such deletion permanently strands both image files.

Useful? React with 👍 / 👎.

Comment on lines +944 to +947
export function releaseCollectionShowOwnership(showId: string): void {
db.update(mediaServerShows)
.set({ sourceCollectionId: null, updatedAt: Date.now() })
.where(eq(mediaServerShows.id, showId))

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Retire detached collection-show rows

Clearing only source_collection_id does not release the deleted collection's directory name: the show row remains, media_server_shows_directory_name_uidx still reserves its directory, and allocateDirectoryName() considers every show row regardless of whether it is detached. Repeatedly creating and deleting collection shows therefore accumulates unreachable catalog rows and forces later unrelated shows with the same title onto unnecessary suffixes, contrary to this cleanup path's stated purpose; delete or explicitly reclaim detached rows after their artifacts are cleaned.

Useful? React with 👍 / 👎.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

nfo入库媒体库问题

1 participant