feat(watchlist): add titles that aren't in the library yet - #1711
Conversation
Profiles can add a movie or series from Discover to their watchlist before the server has it. The entry is keyed by the title's provider IDs, kept in its own tables, and moves onto the library watchlist when the title arrives. Adding can also request the title, so the existing fulfilled notification tells the profile when it lands. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
|
Important Review skippedToo many files! This PR contains 132 files, which is 32 over the limit of 100. To get a review, reduce the PR to 100 files or fewer by splitting it into smaller PRs or changing its base branch. Upgrade to a paid plan to raise the limit. This review couldn't start because sufficient usage credits or metered capacity aren't available. Add credits or update usage-based reviews in the billing tab, then retry. ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Advanced Run ID: 📒 Files selected for processing (132)
You can disable this status message by setting the Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
Code Review Completed! 🔥The code review was successfully completed based on your current configurations. Kody Guide: Usage and ConfigurationInteracting with Kody
Providing Context (Files & MCPs)Add these hints in your PR description (or a comment) to unlock deeper checks:
Current Kody ConfigurationReview OptionsThe following review options are enabled or disabled:
|
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d50c67f974
ℹ️ 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".
A request keeps the TMDB ID it was made under, but the watchlist looked up requests only by the title's current ID. After TMDB replaced the ID, the card showed no request, a repeat add could file a duplicate, and removing the title left the watchlist-made request active. Request lookups, follows and withdrawals now use every TMDB ID the title has had. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1b1bf11f1d
ℹ️ 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".
![kody-ai[bot]](https://avatars.githubusercontent.com/in/413034?s=60&v=4){kind=small}
Promotion holds the title lock in a transaction while the library watchlist write takes a second pool connection. With database.max_connections set to 1 that write waited for the request to time out, so watchlist and item reads hung whenever a title was ready to move. A one-connection pool now writes to the library watchlist first and locks afterwards. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Code Review Completed! 🔥The code review was successfully completed based on your current configurations. Kody Guide: Usage and ConfigurationInteracting with Kody
Providing Context (Files & MCPs)Add these hints in your PR description (or a comment) to unlock deeper checks:
Current Kody ConfigurationReview OptionsThe following review options are enabled or disabled:
|
|
The
All result cells in these listed tasks are currently Automated check: Macroscope check run agent (gpt-6-luna). No validation was performed. Posted via Macroscope — v1 validation impact |
|
Updated the |
|
The Please update the line to:
Automated check: Macroscope check run agent (gpt-6-luna). No validation was performed. Posted via Macroscope — v1 validation impact |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1dd0fcc6c3
ℹ️ 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".
Deleting a title removed its last entry, and with it the title's former TMDB IDs, before withdrawing the watchlist's requests. A withdrawal that failed part way could then never be retried under the former ID. The withdrawal now runs first. Library matching on add and remove also passes every TMDB ID the title has had, so a library copy known only by a replaced ID is found. The one-connection-pool promotion checks that the entry still exists before it writes to the library watchlist. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Code Review Completed! 🔥The code review was successfully completed based on your current configurations. Kody Guide: Usage and ConfigurationInteracting with Kody
Providing Context (Files & MCPs)Add these hints in your PR description (or a comment) to unlock deeper checks:
Current Kody ConfigurationReview OptionsThe following review options are enabled or disabled:
|
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ecce9887b4
ℹ️ 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".
Deleting a profile left the unsent requests its watchlist made active: the profile's entries are gone by the time the purge runs, and requests only reference the account. The profile delete now withdraws them by requester profile and source. An add and a delete of the same title could interleave so that the add's request landed after the delete had withdrawn and removed the entry. The delete now withdraws again after removing the entry, and the add rechecks its entry after requesting and withdraws if it is gone. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Code Review Completed! 🔥The code review was successfully completed based on your current configurations. Kody Guide: Usage and ConfigurationInteracting with Kody
Providing Context (Files & MCPs)Add these hints in your PR description (or a comment) to unlock deeper checks:
Current Kody ConfigurationReview OptionsThe following review options are enabled or disabled:
|
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c43543c03e
ℹ️ 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".
| if _, err := reg.deps.WatchlistTitles.RemoveWatchlistTitle(ctx, viewer, in.MediaType, in.TMDBID); err != nil { | ||
| return nil, serviceProblem(err) | ||
| } | ||
| if err := reg.withdrawWatchlistRequests(ctx, rv, in.MediaType, ids); err != nil { |
There was a problem hiding this comment.
When a DELETE pauses here after removing the entry and a concurrent PUT then re-creates both the entry and its watchlist-sourced request, this second withdrawal cancels the newer PUT's request even though its entry remains. The final state therefore depends on timing and can leave a successfully re-added title without the automatic request it just created; condition the post-removal withdrawal on the entry still being absent or serialize it with re-adds.
AGENTS.md reference: AGENTS.md:L73-L75
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
This is the same remaining race as the thread on internal/apiv2/watchlist_titles.go further up. WatchlistTitleOff narrows it, but the absence check and the second withdrawal are still separate steps, so a re-add that finishes between them can lose its request. Making the withdrawal conditional and atomic, keyed to the entry that made the request, needs a change on the requests side. I've asked the maintainer to choose between that and the documented trade-off, and I'm leaving this open until they decide.
The add's recheck and the delete's second withdrawal treated a missing external entry as "removed". An entry that promotion moved to the library watchlist while the add was requesting, or that an overlapping re-add put back, would then lose the request it should keep. Both now withdraw only when the title is off the watchlist in both forms: no external entry and no library watchlist entry for its library copy. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Code Review Completed! 🔥The code review was successfully completed based on your current configurations. Kody Guide: Usage and ConfigurationInteracting with Kody
Providing Context (Files & MCPs)Add these hints in your PR description (or a comment) to unlock deeper checks:
Current Kody ConfigurationReview OptionsThe following review options are enabled or disabled:
|
| off, err := reg.deps.WatchlistTitles.WatchlistTitleOff(ctx, viewer, snap) | ||
| if err != nil { | ||
| return nil, serviceProblem(err) | ||
| } | ||
| if off { | ||
| if err := reg.withdrawWatchlistRequests(ctx, rv, in.MediaType, ids); err != nil { |
There was a problem hiding this comment.
Race condition identified between WatchlistTitleOff and the second withdrawal: a delete can read off=true, then an overlapping add can save its entry, create its request, and pass its membership check before the delete resumes, allowing WithdrawWatchlistRequest to cancel the re-add's unsent request or remove its follows despite the title being watchlisted. Serialize the membership/request lifecycle per profile and title rather than relying on a separate membership check.
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
There was a problem hiding this comment.
This is the same remaining race as the thread on internal/apiv2/watchlist_titles.go further up. WatchlistTitleOff narrows it, but the absence check and the second withdrawal are still separate steps, so a re-add that finishes between them can lose its request. Making the withdrawal conditional and atomic, keyed to the entry that made the request, needs a change on the requests side. I've asked the maintainer to choose between that and the documented trade-off, and I'm leaving this open until they decide.
There was a problem hiding this comment.
Agreed—this is the same unresolved race, not a separate finding. A re-add can complete after WatchlistTitleOff returns true but before the second withdrawal, leaving the title watchlisted while its request is canceled or its follows are removed.
An atomic withdrawal tied to the entry that created the request would address that gap; another membership check would only narrow it. Pending the maintainer’s decision to fix it or explicitly accept the documented trade-off, the finding should remain open.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 79d8f1c64c
ℹ️ 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".
| if err := addToLibrary(ctx); err != nil { | ||
| return false, false, err | ||
| } | ||
| moved, err = s.repo.promoteEntry(ctx, v.UserID, v.ProfileID, titleID, func(context.Context) error { return nil }) |
There was a problem hiding this comment.
When database.max_connections is set to the supported minimum of 1, this path writes the library entry before acquiring the title lock. If a concurrent DELETE removes the title entry and finishes its library lookup before addToLibrary runs, this call then adds the library entry and promoteEntry finds no title to move, leaving the successfully removed title on the watchlist. The one-connection fallback needs coordination with removal rather than this check-then-write sequence.
AGENTS.md reference: AGENTS.md:L73-L75
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
This is the one-connection-pool trade-off already discussed and documented in docs/architecture/external-watchlist.md. With a single pool connection, the library write can't run while the title lock's transaction holds that connection, so this path writes first after checking the entry still exists. A delete landing between that check and the lock can still leave the item on the library watchlist. Pools of two or more connections use the locked path, where this can't happen. I'm leaving this open for the maintainer, together with the related re-add race.
| off, err := reg.deps.WatchlistTitles.WatchlistTitleOff(ctx, viewer, snap) | ||
| if err != nil { | ||
| return nil, serviceProblem(err) | ||
| } | ||
| if off { | ||
| if err := reg.withdrawWatchlistRequests(ctx, rv, in.MediaType, ids); err != nil { |
There was a problem hiding this comment.
When a PUT re-adds the title after WatchlistTitleOff returns true but before the second withdrawal, the DELETE cancels the new PUT's watchlist-created request even though its entry remains. Fresh evidence in this revision is that the attempted fix still leaves the absence check and withdrawal as separate service calls, so the same cross-node interleaving remains possible; serialize the recheck with withdrawal or make withdrawal conditional in one atomic operation.
AGENTS.md reference: AGENTS.md:L73-L75
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
This is the same remaining race as the thread on internal/apiv2/watchlist_titles.go further up. WatchlistTitleOff narrows it, but the absence check and the second withdrawal are still separate steps, so a re-add that finishes between them can lose its request. Making the withdrawal conditional and atomic, keyed to the entry that made the request, needs a change on the requests side. I've asked the maintainer to choose between that and the documented trade-off, and I'm leaving this open until they decide.
A delete removed the title's last entry, and with it the title's former TMDB IDs, before removing the library watchlist entry. If that library step failed, a retry could no longer find a library copy known only by a former ID. The library entry now goes first, matched by every ID while the entry stands, and is checked again after the entry is removed. Reading the watchlist titles tab also moves titles the library now has onto the library watchlist, so the web now refreshes the library tab after every titles read, and the home row and item states when a listed title is gone. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Code Review Completed! 🔥The code review was successfully completed based on your current configurations. Kody Guide: Usage and ConfigurationInteracting with Kody
Providing Context (Files & MCPs)Add these hints in your PR description (or a comment) to unlock deeper checks:
Current Kody ConfigurationReview OptionsThe following review options are enabled or disabled:
|
| title, _, err := titles.Remove(ctx, viewer.watchlistViewer(), mediaType, tmdbID) | ||
| if err != nil { | ||
| return nil, apiError(http.StatusInternalServerError, "internal_error", "Failed to remove from watchlist") | ||
| } | ||
| if err := h.removeLibraryWatchlistItem(ctx, viewer, itemID); err != nil { | ||
| return nil, err |
There was a problem hiding this comment.
Retry information loss occurs when promotion recreates the library entry between the first removal and titles.Remove, then the second library removal fails: promotion/removal deletes the last title and its former IDs before the failing operation, so retrying DELETE with the current TMDB ID cannot find a library item carrying only a former ID. Preserve durable alias information until the final library cleanup succeeds.
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
There was a problem hiding this comment.
This case needs three things together:
- a promotion runs in the gap between the delete's first library removal and its entry removal;
- the second library removal then fails;
- the library copy carries only a former TMDB ID.
A promotion moving the last entry deletes the title and its aliases by design, because the title is then a normal library item. So after (1), nothing in the watchlist tables can map the current TMDB ID back to that item. What's left is not hidden or lost: the item sits on the profile's library watchlist, shows on the watchlist page, and can be removed there through the normal DELETE /api/v2/watchlist/{item_id}. Keeping the aliases alive past promotion only for this retry would add a second lifetime rule to the title records. I'd rather not do that, and I'm leaving the thread open for the maintainer to weigh in.
There was a problem hiding this comment.
Agreed—the entry remains visible and removable through DELETE /api/v2/watchlist/{item_id}. This is a retryability gap, not loss of the watchlist entry, and the original high-severity classification overstates the impact.
The narrower concern still holds: after that race and failure, retrying the title-based DELETE with the current TMDB ID cannot finish the removal because the alias mapping is gone. Recovery requires switching to the library-item endpoint.
Given the three conditions required and the existing recovery path, retaining aliases beyond promotion is not clearly justified solely for this case. I’d leave this as a low-severity edge case for the maintainer to accept or address, rather than insist on the proposed alias-lifetime change. A focused regression test could document the limitation and confirm that the surviving library entry remains removable.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4a7b598b5f
ℹ️ 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".
| // AddSnapshot is Add for callers that already hold a snapshot, such as an | ||
| // import that keeps the source's added_at. | ||
| func (s *Titles) AddSnapshot(ctx context.Context, v Viewer, snap Snapshot, addedAt time.Time) (Entry, bool, error) { |
There was a problem hiding this comment.
When a Trakt, Simkl, or MDBList watchlist row has no catalog match, watchsync.Service.importList still takes the unchanged match == nil branch at internal/watchsync/lists.go:269-278, increments Unmatched, and discards the row. Repository-wide search shows AddSnapshot is called only by the HTTP handler and tests, so this import-capable entry point is never wired into watch-provider sync; those users continue losing every outside-library title the feature is intended to retain. Route unmatched watchlist rows, but not favorites, through the new title store while retaining their provider timestamp and sync state.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
This is deliberately out of scope for this PR. The design splits import and export of outside-library titles for Trakt, Simkl, MDBList and Plex into a follow-up phase, with its own review. The shadow table watch_provider_list_items is keyed by media_item_id, and removal sync needs a stand-in key and a way to move it on promotion; those need their own design. I've corrected the PR description, which read as if this PR fixed the sync drop. I'm leaving this thread open for the maintainer, who owns that phase.
Requests a watchlist add creates carried no overview or backdrop: the add keeps only its own snapshot. CreateRequest now fills empty display fields from the TMDB detail it already reads, for any caller. The web read at most 25 pages of watchlist titles and showed the rest as the whole list. It now follows the cursor until the server has no more pages and fails on a repeated cursor instead of truncating. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Code Review Completed! 🔥The code review was successfully completed based on your current configurations. Kody Guide: Usage and ConfigurationInteracting with Kody
Providing Context (Files & MCPs)Add these hints in your PR description (or a comment) to unlock deeper checks:
Current Kody ConfigurationReview OptionsThe following review options are enabled or disabled:
|
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 0e8b402247
ℹ️ 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".
When TMDB reported an IMDb or TVDB ID another watchlist title already owned, the alias was skipped but the ID still landed in the title's own fields, which library matching and request presence read directly. The title could then match the other title's library copy. Those fields now hold only IDs the title owns as aliases: a refresh keeps the ID it already had, a new title leaves the field empty. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Code Review Completed! 🔥The code review was successfully completed based on your current configurations. Kody Guide: Usage and ConfigurationInteracting with Kody
Providing Context (Files & MCPs)Add these hints in your PR description (or a comment) to unlock deeper checks:
Current Kody ConfigurationReview OptionsThe following review options are enabled or disabled:
|
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e320cf4793
ℹ️ 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".
When a title matched several catalog items but the viewer could see only one, promotion and the add/remove library lookup gave up as ambiguous before checking access, so the title stayed outside the library for that viewer and a delete could miss its library watchlist entry. Matches are now filtered by the viewer's access first, and exactly one visible copy is required. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Code Review Completed! 🔥The code review was successfully completed based on your current configurations. Kody Guide: Usage and ConfigurationInteracting with Kody
Providing Context (Files & MCPs)Add these hints in your PR description (or a comment) to unlock deeper checks:
Current Kody ConfigurationReview OptionsThe following review options are enabled or disabled:
|
Filtering each title's library matches by access ran one EnsureAccessible query per matched copy on every watchlist read. Promotion now checks every matched copy with one EnsureAccessibleIDs call and filters from that. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Code Review Completed! 🔥The code review was successfully completed based on your current configurations. Kody Guide: Usage and ConfigurationInteracting with Kody
Providing Context (Files & MCPs)Add these hints in your PR description (or a comment) to unlock deeper checks:
Current Kody ConfigurationReview OptionsThe following review options are enabled or disabled:
|
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 02590f298b
ℹ️ 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".
| if err != nil { | ||
| return nil, apiError(http.StatusInternalServerError, "internal_error", "Failed to look up the watchlist title") | ||
| } | ||
| tmdbIDs, imdbID, tvdbID := []int{tmdbID}, "", 0 |
There was a problem hiding this comment.
When a repointed title is promoted through a former TMDB/IMDb/TVDB alias and no other profile keeps the title, promotion deletes its title and alias rows before a later DELETE can begin. This fallback then searches the catalog using only the URL's TMDB ID; if the catalog item is identifiable only by a discarded alias, a stale client receives 204 but the promoted library-watchlist entry remains. Fresh evidence beyond the earlier lookup-order fix is that promotion can remove the snapshot before this handler reaches Find, so resolving the item before this handler's own removal does not cover the race.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
This is the same edge as the delete-retry thread on this file further up. Once promotion moves the last entry, the title and its aliases are deleted by design, because the title is then an ordinary library item. A stale client deleting by a TMDB ID the catalog item doesn't carry gets a 204, while the item stays on the library watchlist. It remains visible there and is removed through DELETE /api/v2/watchlist/{item_id}, which is what a client refreshing after promotion uses. Kody rated that thread low severity with a working recovery path. Keeping the aliases alive past promotion would give the title records a second lifetime rule. I'm leaving this open alongside that one for the maintainer to decide.
Every page of a watchlist or watchlist-titles read ran the profile-wide promotion, so reading a long list scanned the whole profile once per page. Promotion now runs on the first page only. The watchlist titles tab also refreshes every 30 seconds while any title is downloading, as the other request surfaces do, so its progress doesn't freeze. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Code Review Completed! 🔥The code review was successfully completed based on your current configurations. Kody Guide: Usage and ConfigurationInteracting with Kody
Providing Context (Files & MCPs)Add these hints in your PR description (or a comment) to unlock deeper checks:
Current Kody ConfigurationReview OptionsThe following review options are enabled or disabled:
|
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 9063529555
ℹ️ 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".
| if h.WatchlistTitlesPurger != nil { | ||
| purgeCtx, cancel := context.WithTimeout(context.WithoutCancel(ctx), 30*time.Second) | ||
| defer cancel() | ||
| if purgeErr := h.WatchlistTitlesPurger.PurgeProfile(purgeCtx, userID, profileID); purgeErr != nil { | ||
| slog.WarnContext(ctx, "profile watchlist-title purge failed after delete", "component", "api", "user_id", userID, "profile_id", profileID, "error", purgeErr) |
There was a problem hiding this comment.
When a watchlist-title PUT passes profile authentication before a concurrent profile deletion but reaches AddSnapshot after this purge finishes, it can recreate user_watchlist_titles for the deleted profile because that table deliberately has no profile foreign key; the subsequent auto-request can likewise persist with the deleted requested_by_profile_id. No later cleanup targets that profile, so a cross-node race leaves an unreachable entry and potentially an active download request. Coordinate title mutations with profile deletion or leave a durable tombstone that rejects/cleans late writes.
AGENTS.md reference: AGENTS.md:L13-L18
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Agreed that the window exists: a PUT that authenticated before the profile was deleted and writes after the purge can leave an orphaned entry. And, with auto-request on, it can leave a request naming the deleted profile. This isn't specific to watchlist titles, though. Every shared Postgres table keyed by profile (for example user_dropped_series and the device-library and notification rows) has the same window, because profiles can live in the SQLite user store and those tables deliberately have no profile foreign key. The migration comment and docs/architecture/external-watchlist.md say this table follows that pattern. Requests have never had a profile foreign key either: a direct request made just before a profile delete outlives it the same way, and it stays visible in the admin queue. Closing this properly means a deletion marker or cleanup covering all of those tables, which is a cross-cutting change I'd keep out of this PR. I'm leaving this open for the maintainer to decide whether that belongs in a follow-up.
1 participant
Problem
Related issue: N/A
Validation tasks: changes #1183 C1, C2; changes #1184 C1, C2, C3; changes #1202 C1, C2, C3, C4; changes #1203 C1; changes #1204 C1, C2, C3
A profile can only put titles that are already in the library on its
watchlist. Someone who finds a movie or series in Discover can request it but
can't save it for later. (Watch-provider sync drops Trakt, Simkl and MDBList
watchlist items that aren't in the library too. Importing those into the new
storage is planned follow-up work and not part of this PR.)
This change lets a profile add a TMDB movie or series to its watchlist before
the server has it. The watchlist page gets a "Not in your library yet" tab for
these titles. When a title arrives in the library, its entry moves to the
normal watchlist with its original date. Adding can also request the title,
so the existing "request fulfilled" notification tells the profile when it's
ready.
Approach
Storage. Titles outside the library live in their own tables:
watchlist_titles: one row per title, shared by every profile that addsit, with the current IDs and a display snapshot;
watchlist_title_aliases: every TMDB, IMDb or TVDB ID the title has had;user_watchlist_titles: one row per profile entry.Nothing is written to
media_items, and no catalogcontent_idis stored,so catalog queries and the content-ID rename and merge paths are unchanged.
Placeholder catalog rows were ruled out because every catalog query would
need to filter them out.
Arrival. Whether a title is in the library is worked out when it's
needed, by matching its aliases against
media_items,media_item_provider_idsandstale_media_idsin enabled libraries. Theentry moves onto
user_watchlistwhen the profile reads the watchlist (v1and v2), the catalog with
source=watchlist, the home watchlist row, or theitem's detail page. The move keeps the original
added_atand fires thenormal watchlist side effects once. There is no scheduled task. The item
list, people and jellycompat reads don't move entries.
Changing IDs. Viewing a title's Discover page refreshes its stored IDs
from the TMDB response the page already fetches. Reading the list checks up
to three overdue titles in the background, with a one-hour claim so only one
node checks each title. A TMDB 404 is only acted on after a second 404 at
least 24 hours later. The title is then found again through its IMDb or TVDB
ID (TMDB
/find). If that finds another stored title, the two merge. TitlesTMDB lists twice are marked as needing attention, and titles TMDB no longer
lists are marked as such. Entries are never removed automatically.
Requests. Adding requests the title, or follows someone else's open
request for it, when all of these hold:
request_settings.watchlist_requestsis on (default on);requests.watchlist_auto_requestis on (default on);Limits, approval and routing work as they do for the Request button.
Requests created this way are recorded with
media_requests.source = 'watchlist'. Removing a title cancels its request only if the watchlistcreated it and nothing has been sent to a download server yet.
API.
/api/v2additions only;/api/v1and jellycompat are unchanged:/api/v2/watchlist/titles;in_watchliston Discover results and detail;watchlist_titles_supportedandwatchlist_requestson the requestsstatus;
sourceon requests.With requests disabled, the new operations answer 409 and the web shows no
entry points.
Web.
library yet". The second explains that its titles can't be played yet.
request_statuscard overlay badge, so itfollows the profile's overlay style, corner and on/off settings. Downloads
also get the Continue Watching progress bar.
Settings contract. Revision 15 adds
requests.watchlist_auto_requestandthe
request_statusoverlay id.Related fix. The notification interest hook now wraps
AddToWatchlistAt.Before, series added to a watchlist by watch-provider and Plex imports never
queued an interest recompute.
Validation
make lint-changedreports 0issues.
18 for watchlist (with
-race), requests, apiv2, catalog, handlers,notifications, pgstore and sections.
timezone=UTCset on the test databaseURL.
make migrate-validatepasses. Before rebasing onto current main, thethree new migrations were rolled down and back up, and
make test-db-pinspassed.
verify-apiv2-openapi,-web-types,-contractand-fixtures,verify-settings-bindings-allandverify-local-pathspass. The contractdiff reports 25 additive changes and none breaking.
make test-webpasses (703 files, 6073 tests), and so does the bundle budgetcheck.
/findresponses and TMDB's behavior for deleted IDs. The testsuse stubs.
scale. There is no
EXPLAIN ANALYZEagainst a large fixture.out.
TestOrphanCleanupAfterPlexTableDropPostgres,TestAdminSubtitleListPageDBand
TestCatalogTransferPersistsJobsAndSignedLinkalso fail on main.internal/librarymonitorTestFailedBackendIsReplacedis flaky underfull-suite load.
Risks
stale_media_ids (provider, provider_id)index is builtCONCURRENTLYin its own non-transactionalmigration.
requests. They go through the usual limits and approval, and an admin can
turn it off in Settings › Requests.
detail each run one extra indexed query.
database.max_connections: 1, moving anarrived title writes the library watchlist entry before it locks the title,
because the lock's transaction would hold the pool's only connection. A
remove that races the move can leave the item on the library watchlist.
web/perf-budget.jsongoes up by about 1.2 KB brotli. Thatcovers the overlay entry, three icons and the regenerated contract types.
tab, the Discover button, the profile setting and the
request_statusoverlay id: Watchlist titles that aren't in the library yet silo-apple#560 and Watchlist titles that aren't in the library yet silo-android#428.
The user manual needs the new tab, button, settings and badge:
Manual: watchlist titles that aren't in the library yet siloserver.org#63.
Checklist
AI Disclosure
approved the spec and the UI mockups. The code has not yet been verified by
a human.
against the design spec, covering backend correctness and concurrency, API
contract and access control, the web client, and spec coverage. They
reported 11 findings, 2 of them major: the tabs and the Discover button
stayed visible with requests disabled. All 11 were fixed with tests.
🤖 Generated with Claude Code