From 9ead29bfcced0fb62ce68de58c5aea72794bfe76 Mon Sep 17 00:00:00 2001 From: Quick <31828688+Quick104@users.noreply.github.com> Date: Sun, 5 Jul 2026 13:11:12 -0400 Subject: [PATCH] fix(historyimport): make Plex watchlist import actually work The browser Plex OAuth flow only sent the PMS server access token, which the discover API rejects (401), so the watchlist step always failed with a buried warning. The web client now forwards the plex.tv account token via a new additive plex_account_token field. The discover watchlist listing also ignores includeGuids, so items arrived without external ids and could only exact-title/year match. FetchWatchlist now resolves ids per item from the discover metadata endpoint, decoding both Metadata- and Video-keyed containers, and degrades to a title/year fallback warning instead of dropping items. Part of #245 Co-Authored-By: Claude Fable 5 --- internal/historyimport/plex_client.go | 75 ++++++++++++-- internal/historyimport/plex_provider.go | 3 +- internal/historyimport/plex_watchlist_test.go | 98 ++++++++++++++++++- internal/historyimport/service.go | 8 +- internal/historyimport/types.go | 5 + web/src/api/types.ts | 1 + web/src/lib/plexAuth.ts | 11 ++- .../pages/settings/HistoryImportSettings.tsx | 9 +- .../pages/settings/WebhookSyncSettings.tsx | 2 +- 9 files changed, 195 insertions(+), 17 deletions(-) diff --git a/internal/historyimport/plex_client.go b/internal/historyimport/plex_client.go index 9e715f33..43b339a3 100644 --- a/internal/historyimport/plex_client.go +++ b/internal/historyimport/plex_client.go @@ -151,6 +151,10 @@ type plexMediaContainer struct { TotalSize int `json:"totalSize"` Offset int `json:"offset"` Metadata []PlexItem `json:"Metadata"` + // Video mirrors Metadata: the discover API inconsistently keys some + // responses on "Video" instead of "Metadata" (movie items in + // particular), so both must be decoded. + Video []PlexItem `json:"Video"` Directory []struct { Key string `json:"key"` Type string `json:"type"` @@ -159,6 +163,15 @@ type plexMediaContainer struct { } `json:"MediaContainer"` } +// items returns the container's media entries regardless of whether the +// upstream keyed them on "Metadata" or "Video". +func (c *plexMediaContainer) items() []PlexItem { + if len(c.MediaContainer.Metadata) > 0 { + return c.MediaContainer.Metadata + } + return c.MediaContainer.Video +} + type PlexItem struct { RatingKey string `json:"ratingKey"` Key string `json:"key"` @@ -268,7 +281,12 @@ func (c *PlexClient) fetchSectionItems(ctx context.Context, baseURL, token, sect // Plex discover API. It authenticates with the plex.tv ACCOUNT token (from // the PIN/OAuth session), not a server access token: the watchlist belongs // to the account, not to any PMS. -func (c *PlexClient) FetchWatchlist(ctx context.Context, accountToken string) ([]PlexItem, error) { +// +// The discover listing does not honor includeGuids, so items usually arrive +// without external ids. Each id-less item gets a follow-up per-item metadata +// fetch to resolve its Guid array; failures there degrade to warnings so the +// rest of the watchlist still imports (matching falls back to title/year). +func (c *PlexClient) FetchWatchlist(ctx context.Context, accountToken string) ([]PlexItem, []string, error) { base := c.discoverBaseURL if base == "" { base = plexDiscoverBaseURL @@ -283,20 +301,63 @@ func (c *PlexClient) FetchWatchlist(ctx context.Context, accountToken string) ([ reqURL := fmt.Sprintf("%s/library/sections/watchlist/all?%s", base, query.Encode()) req, err := http.NewRequestWithContext(ctx, http.MethodGet, reqURL, nil) if err != nil { - return nil, err + return nil, nil, err } c.setPlexHeaders(req, accountToken) var container plexMediaContainer if err := c.doJSON(req, &container); err != nil { - return nil, fmt.Errorf("fetching Plex watchlist (offset %d): %w", offset, err) + return nil, nil, fmt.Errorf("fetching Plex watchlist (offset %d): %w", offset, err) } - allItems = append(allItems, container.MediaContainer.Metadata...) - offset += len(container.MediaContainer.Metadata) - if offset >= container.MediaContainer.TotalSize || len(container.MediaContainer.Metadata) == 0 { + items := container.items() + allItems = append(allItems, items...) + offset += len(items) + if offset >= container.MediaContainer.TotalSize || len(items) == 0 { break } } - return allItems, nil + + var warnings []string + unresolved := 0 + for i := range allItems { + if len(allItems[i].Guid) > 0 { + continue + } + detail, err := c.fetchWatchlistItemMetadata(ctx, base, accountToken, allItems[i].RatingKey) + if err != nil || detail == nil { + unresolved++ + continue + } + allItems[i].Guid = detail.Guid + if allItems[i].Year == 0 { + allItems[i].Year = detail.Year + } + } + if unresolved > 0 { + warnings = append(warnings, fmt.Sprintf( + "watchlist: could not resolve external ids for %d of %d items; those fall back to exact title/year matching", + unresolved, len(allItems))) + } + return allItems, warnings, nil +} + +// fetchWatchlistItemMetadata resolves one watchlist entry's full metadata +// (including its external-id Guid array) from the discover API. +func (c *PlexClient) fetchWatchlistItemMetadata(ctx context.Context, base, accountToken, ratingKey string) (*PlexItem, error) { + reqURL := fmt.Sprintf("%s/library/metadata/%s", base, url.PathEscape(ratingKey)) + req, err := http.NewRequestWithContext(ctx, http.MethodGet, reqURL, nil) + if err != nil { + return nil, err + } + c.setPlexHeaders(req, accountToken) + var container plexMediaContainer + if err := c.doJSON(req, &container); err != nil { + return nil, fmt.Errorf("fetching Plex watchlist metadata for %s: %w", ratingKey, err) + } + items := container.items() + if len(items) == 0 { + return nil, nil + } + return &items[0], nil } func (c *PlexClient) FetchOnDeck(ctx context.Context, baseURL, token string) ([]PlexItem, error) { diff --git a/internal/historyimport/plex_provider.go b/internal/historyimport/plex_provider.go index d3354af0..941fb315 100644 --- a/internal/historyimport/plex_provider.go +++ b/internal/historyimport/plex_provider.go @@ -80,10 +80,11 @@ func (p *PlexServerProvider) Fetch(ctx context.Context) ([]Record, []string, err // effort: a watchlist fetch failure downgrades to a warning so the // watch-history import still completes (issue #245). if p.accountToken != "" { - items, err := p.client.FetchWatchlist(ctx, p.accountToken) + items, watchlistWarnings, err := p.client.FetchWatchlist(ctx, p.accountToken) if err != nil { warnings = append(warnings, fmt.Sprintf("watchlist fetch failed: %v", err)) } else { + warnings = append(warnings, watchlistWarnings...) for _, item := range items { records = append(records, NormalizePlexWatchlistItem(item)) } diff --git a/internal/historyimport/plex_watchlist_test.go b/internal/historyimport/plex_watchlist_test.go index 4a909390..ea8d757d 100644 --- a/internal/historyimport/plex_watchlist_test.go +++ b/internal/historyimport/plex_watchlist_test.go @@ -88,10 +88,13 @@ func TestFetchWatchlistPaginatesDiscoverAPI(t *testing.T) { client := NewPlexClient() client.discoverBaseURL = server.URL - items, err := client.FetchWatchlist(context.Background(), "account-token-1") + items, warnings, err := client.FetchWatchlist(context.Background(), "account-token-1") if err != nil { t.Fatalf("FetchWatchlist: %v", err) } + if len(warnings) != 0 { + t.Fatalf("warnings = %v, want none (all items carried guids)", warnings) + } if gotToken != "account-token-1" { t.Fatalf("token header = %q, want account token", gotToken) } @@ -103,19 +106,108 @@ func TestFetchWatchlistPaginatesDiscoverAPI(t *testing.T) { } } +// The discover listing does not honor includeGuids in practice: items arrive +// without external ids, and some detail responses key their payload on +// "Video" instead of "Metadata". Both must be handled or matching silently +// degrades to exact title/year. +func TestFetchWatchlistResolvesGuidsViaItemMetadata(t *testing.T) { + detailCalls := map[string]int{} + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.Header().Set("Content-Type", "application/json") + switch r.URL.Path { + case "/library/sections/watchlist/all": + _, _ = fmt.Fprint(w, `{"MediaContainer":{"totalSize":2,"Metadata":[ + {"ratingKey":"wl-movie","type":"movie","title":"Dune: Part Two","year":2024}, + {"ratingKey":"wl-show","type":"show","title":"Severance","year":2022} + ]}}`) + case "/library/metadata/wl-movie": + detailCalls["wl-movie"]++ + // Movie detail keyed on "Video" (discover inconsistency). + _, _ = fmt.Fprint(w, `{"MediaContainer":{"Video":[ + {"ratingKey":"wl-movie","type":"movie","title":"Dune: Part Two","year":2024, + "Guid":[{"id":"imdb://tt15239678"},{"id":"tmdb://693134"}]} + ]}}`) + case "/library/metadata/wl-show": + detailCalls["wl-show"]++ + _, _ = fmt.Fprint(w, `{"MediaContainer":{"Metadata":[ + {"ratingKey":"wl-show","type":"show","title":"Severance","year":2022, + "Guid":[{"id":"tvdb://371980"}]} + ]}}`) + default: + t.Errorf("unexpected path %q", r.URL.Path) + w.WriteHeader(http.StatusNotFound) + } + })) + defer server.Close() + + client := NewPlexClient() + client.discoverBaseURL = server.URL + items, warnings, err := client.FetchWatchlist(context.Background(), "tok") + if err != nil { + t.Fatalf("FetchWatchlist: %v", err) + } + if len(warnings) != 0 { + t.Fatalf("warnings = %v, want none (all ids resolved)", warnings) + } + if len(items) != 2 { + t.Fatalf("items = %d, want 2", len(items)) + } + if detailCalls["wl-movie"] != 1 || detailCalls["wl-show"] != 1 { + t.Fatalf("detail fetches = %v, want one per id-less item", detailCalls) + } + movie := NormalizePlexWatchlistItem(items[0]) + if movie.IMDbID != "tt15239678" || movie.TMDBID != "693134" { + t.Fatalf("movie ids = imdb %q tmdb %q, want resolved from detail fetch", movie.IMDbID, movie.TMDBID) + } + show := NormalizePlexWatchlistItem(items[1]) + if show.TVDBID != "371980" { + t.Fatalf("show tvdb id = %q, want resolved from detail fetch", show.TVDBID) + } +} + +// A failed per-item metadata fetch must not sink the watchlist: the item +// falls back to title/year matching and the fetch reports one warning. +func TestFetchWatchlistWarnsWhenGuidResolutionFails(t *testing.T) { + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.Header().Set("Content-Type", "application/json") + switch r.URL.Path { + case "/library/sections/watchlist/all": + _, _ = fmt.Fprint(w, `{"MediaContainer":{"totalSize":1,"Metadata":[ + {"ratingKey":"wl-1","type":"movie","title":"Dune: Part Two","year":2024} + ]}}`) + default: + w.WriteHeader(http.StatusInternalServerError) + } + })) + defer server.Close() + + client := NewPlexClient() + client.discoverBaseURL = server.URL + items, warnings, err := client.FetchWatchlist(context.Background(), "tok") + if err != nil { + t.Fatalf("FetchWatchlist: %v", err) + } + if len(items) != 1 || items[0].Title != "Dune: Part Two" { + t.Fatalf("items = %+v, want the listing entry kept", items) + } + if len(warnings) != 1 { + t.Fatalf("warnings = %v, want exactly one unresolved-ids warning", warnings) + } +} + // Guard against page-size regressions: a server reporting a huge total but // returning empty pages must not loop forever. func TestFetchWatchlistStopsOnEmptyPage(t *testing.T) { calls := 0 server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { calls++ - fmt.Fprint(w, `{"MediaContainer":{"totalSize":999,"Metadata":[]}}`) + _, _ = fmt.Fprint(w, `{"MediaContainer":{"totalSize":999,"Metadata":[]}}`) })) defer server.Close() client := NewPlexClient() client.discoverBaseURL = server.URL - items, err := client.FetchWatchlist(context.Background(), "tok") + items, _, err := client.FetchWatchlist(context.Background(), "tok") if err != nil { t.Fatalf("FetchWatchlist: %v", err) } diff --git a/internal/historyimport/service.go b/internal/historyimport/service.go index 632f2b08..6d59df41 100644 --- a/internal/historyimport/service.go +++ b/internal/historyimport/service.go @@ -374,7 +374,11 @@ func (s *Service) resolvePlexAuth(ctx context.Context, userID int, input CreateR if input.PlexToken == "" { return nil, "", fmt.Errorf("plex_token is required for browser Plex imports") } - return &plexAuth{BaseURL: input.PlexBaseURL, Token: input.PlexToken, AccountToken: input.PlexToken}, ConnectionModePlexOAuth, nil + // Prefer the explicit account token: in the browser OAuth flow + // PlexToken is a PMS access token, which account-level APIs (the + // watchlist) reject. Falling back to PlexToken keeps manually pasted + // plex.tv account tokens working. + return &plexAuth{BaseURL: input.PlexBaseURL, Token: input.PlexToken, AccountToken: firstNonEmpty(input.PlexAccountToken, input.PlexToken)}, ConnectionModePlexOAuth, nil } if input.PlexToken != "" { return nil, "", fmt.Errorf("plex_base_url is required for browser Plex imports") @@ -394,7 +398,7 @@ func (s *Service) resolvePlexAuth(ctx context.Context, userID int, input CreateR if input.PlexToken == "" { return nil, "", fmt.Errorf("plex_token is required for predefined Plex sources") } - return &plexAuth{BaseURL: source.BaseURL, Token: input.PlexToken, AccountToken: input.PlexToken}, ConnectionModePredefined, nil + return &plexAuth{BaseURL: source.BaseURL, Token: input.PlexToken, AccountToken: firstNonEmpty(input.PlexAccountToken, input.PlexToken)}, ConnectionModePredefined, nil } return nil, "", fmt.Errorf("plex_session_id, plex_base_url, or source_id is required for Plex imports") diff --git a/internal/historyimport/types.go b/internal/historyimport/types.go index 7f7bae2e..3262177c 100644 --- a/internal/historyimport/types.go +++ b/internal/historyimport/types.go @@ -212,6 +212,11 @@ type CreateRunInput struct { PlexServerID string `json:"plex_server_id,omitempty"` PlexBaseURL string `json:"plex_base_url,omitempty"` PlexToken string `json:"plex_token,omitempty"` + // PlexAccountToken is the plex.tv account token from a browser-side + // PIN/OAuth flow. PlexToken is a PMS access token in that flow and is + // rejected by account-level APIs (the watchlist), so clients that hold + // both must send both. + PlexAccountToken string `json:"plex_account_token,omitempty"` } type LoginConnectInput struct { diff --git a/web/src/api/types.ts b/web/src/api/types.ts index 11a8ecec..0134d648 100644 --- a/web/src/api/types.ts +++ b/web/src/api/types.ts @@ -480,6 +480,7 @@ export interface CreateHistoryImportRunRequest { plex_server_id?: string; plex_base_url?: string; plex_token?: string; + plex_account_token?: string; } export interface CreateHistoryImportSourceRequest { diff --git a/web/src/lib/plexAuth.ts b/web/src/lib/plexAuth.ts index 7f8ad588..a8e32781 100644 --- a/web/src/lib/plexAuth.ts +++ b/web/src/lib/plexAuth.ts @@ -169,15 +169,22 @@ export async function listPlexResources(token: string): Promise { +): Promise { const authToken = await checkPlexPin(pinID, pinCode); if (!authToken) { throw new Error("Plex sign-in was not completed. Please try again."); } - return listPlexResources(authToken); + const servers = await listPlexResources(authToken); + return { accountToken: authToken, servers }; } export function getPreferredPlexServerURL(server: BrowserPlexServer): string { diff --git a/web/src/pages/settings/HistoryImportSettings.tsx b/web/src/pages/settings/HistoryImportSettings.tsx index 6f770afd..e2ff6566 100644 --- a/web/src/pages/settings/HistoryImportSettings.tsx +++ b/web/src/pages/settings/HistoryImportSettings.tsx @@ -102,6 +102,7 @@ export default function HistoryImportSettings() { // Plex state const [plexServers, setPlexServers] = useState([]); + const [plexAccountToken, setPlexAccountToken] = useState(""); const [plexServerId, setPlexServerId] = useState(""); const [plexSavedSourceId, setPlexSavedSourceId] = useState(""); const [plexToken, setPlexToken] = useState(""); @@ -169,16 +170,21 @@ export default function HistoryImportSettings() { void (async () => { try { - const servers = await completePlexAuthentication(pinID, returnedPlexPinCode); + const { accountToken, servers } = await completePlexAuthentication( + pinID, + returnedPlexPinCode, + ); if (cancelled) { return; } + setPlexAccountToken(accountToken); setPlexServers(servers); setPlexServerId(servers[0]?.clientIdentifier ?? ""); } catch (error) { if (cancelled) { return; } + setPlexAccountToken(""); setPlexServers([]); setPlexServerId(""); setPlexAuthError(error instanceof Error ? error.message : "Failed to finish Plex sign-in"); @@ -255,6 +261,7 @@ export default function HistoryImportSettings() { source: "plex", plex_base_url: selectedPlexOAuthServerURL, plex_token: selectedPlexOAuthServer.accessToken, + plex_account_token: plexAccountToken || undefined, }); setActiveRunId(run.id); } else if (plexMode === "saved" && selectedPlexSavedSource) { diff --git a/web/src/pages/settings/WebhookSyncSettings.tsx b/web/src/pages/settings/WebhookSyncSettings.tsx index b34fda51..be6fafa6 100644 --- a/web/src/pages/settings/WebhookSyncSettings.tsx +++ b/web/src/pages/settings/WebhookSyncSettings.tsx @@ -421,7 +421,7 @@ export default function WebhookSyncSettings() { void (async () => { try { - const servers = await completePlexAuthentication(pinID, returnedPlexPinCode); + const { servers } = await completePlexAuthentication(pinID, returnedPlexPinCode); if (cancelled) { return; }