From 7658bc8f39250c87f220271e8cc93537bb3f4c1e Mon Sep 17 00:00:00 2001 From: Gronod Date: Sat, 19 Sep 2026 18:48:47 +0100 Subject: [PATCH 1/2] Fix #8: fall back to search ratings metadata --- docs/schema/02-tool-mapping.md | 2 +- docs/schema/05-endpoint-coverage.md | 4 +- docs/schema/06-verification.md | 4 + internal/integration_test/contract_test.go | 47 ++++++++ internal/integration_test/mockombi_test.go | 21 +++- internal/tools/media.go | 121 ++++++++++++++++++--- 6 files changed, 181 insertions(+), 18 deletions(-) diff --git a/docs/schema/02-tool-mapping.md b/docs/schema/02-tool-mapping.md index 61e606e..94bec38 100644 --- a/docs/schema/02-tool-mapping.md +++ b/docs/schema/02-tool-mapping.md @@ -111,7 +111,7 @@ The [input catalogue](03-input-schemas.md) defines every parameter, required fie `advanced_movie` POSTs the exact DiscoverModel property names: `type:"movie"`, `release_year→releaseYear`, `genre_ids→genreIds`, `keyword_ids→keywordIds`, `watch_provider_ids→watchProviders`, and decade unchanged. Ombi 4.53.x accepts but ignores `companies`, so `company_ids` is not advertised and legacy calls fail explicitly. Do not invent a `query` requirement for discovery. Release year and decade must agree if both are supplied. -`read_media.details` chooses v2 movie TMDB/IMDb, TV TVDB/TMDB, artist, or artist/album routes. TV `tmdb` uses v2 `Search/tv/moviedb/{id}`; TV `tvdb` uses the legacy v1 `Search/tv/info/{tvdbId}` route (the v2 `Search/tv/{tvdbId}` route is a TMDB-keyed alias and must never serve a TVDB lookup). The legacy v1 TVDB info route does not reliably carry request state; for TVDB, the adapter performs a bounded scan of v1 `Request/tv` (matching by `tvDbId` or `imdbId`) to overlay `requested`, `request_targets` (using the real parent request ID), and per-episode availability flags. IMDb path placeholder spelling differs from its parameter declaration. `by_request` chooses movie/request, tv/request or artist/request. `movie_localized` uses POST v1 movie/info. `ratings` uses title and year. `streaming` uses TMDB even for TV. +`read_media.details` chooses v2 movie TMDB/IMDb, TV TVDB/TMDB, artist, or artist/album routes. TV `tmdb` uses v2 `Search/tv/moviedb/{id}`; TV `tvdb` uses the legacy v1 `Search/tv/info/{tvdbId}` route (the v2 `Search/tv/{tvdbId}` route is a TMDB-keyed alias and must never serve a TVDB lookup). The legacy v1 TVDB info route does not reliably carry request state; for TVDB, the adapter performs a bounded scan of v1 `Request/tv` (matching by `tvDbId` or `imdbId`) to overlay `requested`, `request_targets` (using the real parent request ID), and per-episode availability flags. IMDb path placeholder spelling differs from its parameter declaration. `by_request` chooses movie/request, tv/request or artist/request. `movie_localized` uses POST v1 movie/info. `ratings` first uses Ombi's title/year ratings route. Ombi 4.53.x still routes that operation through removed Rotten Tomatoes private endpoints; on a 404/5xx (or a successful empty response), the adapter searches the same media/title and requires an exact case-insensitive title and year match before returning explicitly labelled `tmdb_vote_*` or `tvmaze_site_rating` references. A warning identifies the fallback source; no fuzzy match is substituted. `streaming` uses TMDB even for TV. TV identifier labels follow the origin route, not the upstream field name. The v1 TVMaze-backed routes (`Search/tv/{term}`, `Search/tv/info/{tvdbId}`) place the TVDB id in the misleadingly-named `theMovieDbId` field and the TVMaze id in `seriesId`; emit `tvdb` and `tvmaze` there. The v2 engine routes (browse, `moviedb`, `by_request`, multi-search) are TMDB-keyed; emit `tmdb` and do not treat `seriesId` as TVMaze. When `theMovieDbId` is absent, `id` is labelled with that origin namespace so browse and collection members still carry a usable identifier. Multi-search `mediaType` is case-insensitive (`Artist`→artist). The v1 `RecentlyAdded` TV model's id namespace is unverified — keep `tmdb` pending live confirmation. `read_reference.keywords` passes query `searchTerm`; keyword detail uses its own keyword ID. Watch-provider catalogue search also has optional `searchTerm`; it is distinct from streaming availability for a particular title. Other branches have no request body. Reference values are good optional cached resources, but remain available through tools for tool-only clients. diff --git a/docs/schema/05-endpoint-coverage.md b/docs/schema/05-endpoint-coverage.md index 92d1a7c..7d8f031 100644 --- a/docs/schema/05-endpoint-coverage.md +++ b/docs/schema/05-endpoint-coverage.md @@ -234,8 +234,8 @@ The body/response columns describe the **upstream** schema, not a promise to pas | 216 | [GET `/api/v2/Search/artist/request/{requestId}`](../api/raml/api.raml#L3949) | D | `read_media / by_request` | path `requestId`:integer required | 200: [ArtistInformation](../api/raml/types/Ombi.Core.Models.Search.V2.Music.ArtistInformation.raml) | Request-ID namespace requires verification. MCP `media=album` maps to this literal `artist` route segment. | | 217 | [GET `/api/v2/Search/artist/album/{albumId}`](../api/raml/api.raml#L3976) | D | `read_media / details` | path `albumId`:string required | 200: [ReleaseGroup](../api/raml/types/Ombi.Core.Models.Search.V2.Music.ReleaseGroup.raml) | — | | 218 | [GET `/api/v2/Search/releasegroupart/{musicBrainzId}`](../api/raml/api.raml#L4003) | D | `read_library / album_art` | path `musicBrainzId`:string required | 200: [AlbumArt](../api/raml/types/Ombi.Core.Models.Search.V2.Music.AlbumArt.raml) | — | -| 219 | [GET `/api/v2/Search/ratings/movie/{name}/{year}`](../api/raml/api.raml#L4030) | D | `read_media / ratings` | path `name`:string required; path `year`:integer required | 200: [MovieRatings](../api/raml/types/Ombi.Api.External.ExternalApis.RottenTomatoes.Models.MovieRatings.raml) | — | -| 220 | [GET `/api/v2/Search/ratings/tv/{name}/{year}`](../api/raml/api.raml#L4060) | D | `read_media / ratings` | path `name`:string required; path `year`:integer required | 200: [TvRatings](../api/raml/types/Ombi.Api.External.ExternalApis.RottenTomatoes.Models.TvRatings.raml) | — | +| 219 | [GET `/api/v2/Search/ratings/movie/{name}/{year}`](../api/raml/api.raml#L4030) | D | `read_media / ratings` | path `name`:string required; path `year`:integer required | 200: [MovieRatings](../api/raml/types/Ombi.Api.External.ExternalApis.RottenTomatoes.Models.MovieRatings.raml) | Ombi 4.53.x depends on removed Rotten Tomatoes private endpoints; on 404/5xx/empty results, fall back to an exact title/year v1 movie search and label TMDB rating fields. | +| 220 | [GET `/api/v2/Search/ratings/tv/{name}/{year}`](../api/raml/api.raml#L4060) | D | `read_media / ratings` | path `name`:string required; path `year`:integer required | 200: [TvRatings](../api/raml/types/Ombi.Api.External.ExternalApis.RottenTomatoes.Models.TvRatings.raml) | Ombi 4.53.x depends on removed Rotten Tomatoes private endpoints; on 404/5xx/empty results, fall back to an exact title/year v1 TV search and label TVMaze rating fields. | | 221 | [GET `/api/v2/Search/stream/movie/{movieDbId}`](../api/raml/api.raml#L4090) | D | `read_media / streaming` | path `movieDBId`:integer required | 200: array<[StreamingData](../api/raml/types/Ombi.Core.Models.Search.V2.StreamingData.raml)> | — | | 222 | [GET `/api/v2/Search/stream/tv/{movieDbId}`](../api/raml/api.raml#L4120) | D | `read_media / streaming` | path `movieDbId`:integer required | 200: array<[StreamingData](../api/raml/types/Ombi.Core.Models.Search.V2.StreamingData.raml)> | — | | 223 | [GET `/api/v1/Search/movie/{searchTerm}`](../api/raml/api.raml#L4150) | D | `read_search / text` | path `searchTerm`:unspecified required | 200: array<[SearchMovieViewModel](../api/raml/types/Ombi.Core.Models.Search.SearchMovieViewModel.raml)> | — | diff --git a/docs/schema/06-verification.md b/docs/schema/06-verification.md index 20bf2cf..5cc3de6 100644 --- a/docs/schema/06-verification.md +++ b/docs/schema/06-verification.md @@ -324,3 +324,7 @@ The gated live suite is committed and ready; run it with a sourced `.env`. It ad - `#13`: `GET /api/v2/System/news` returns Markdig-rendered HTML text rather than structured JSON. The `news` action was removed from `read_server`. - `#14`: `GET /api/v1/Stats` binds non-nullable `from` and `to` DateTimes; an empty range triggers an upstream null-reference error. The adapter requires both RFC3339 values before calling Ombi. - `#15`: Ombi validates Quartz.NET cron expressions. They have six or seven fields and require `?` in one of day-of-month or day-of-week; for example, `0 0 0 * * ?` validates while five-field cron and expressions with both day fields as `*` do not. + +## M8 Findings — Upstream ratings and vote failures (#8, #12) + +- `#8`: Ombi 4.53.10 implements both v2 ratings routes by calling `www.rottentomatoes.com/api/private`; both private endpoints now return HTTP 404 and Ombi surfaces the dependency failure as HTTP 500. `read_media ratings` retains the native result when available and otherwise performs an exact title/year lookup through Ombi's normal movie or TV search. Fallback values are source-labelled (`tmdb_vote_average`, `tmdb_vote_count`, or `tvmaze_site_rating`) and the result carries a degradation warning. No fuzzy title or year substitution is allowed. diff --git a/internal/integration_test/contract_test.go b/internal/integration_test/contract_test.go index 6eacf39..54ce6cb 100644 --- a/internal/integration_test/contract_test.go +++ b/internal/integration_test/contract_test.go @@ -273,6 +273,53 @@ func TestReadMediaDetailsProjectsUndocumentedFields(t *testing.T) { assertNoLeak(t, out.Raw) } +func TestReadMediaRatingsFallsBackToSearchMetadata(t *testing.T) { + mock := newMockOmbi(t, "jwt") + c := spawnServer(t, mock.env()) + c.handshake(t) + + for _, tc := range []struct { + media string + name string + year int + want string + }{ + {media: "movie", name: "Alien", year: 1979, want: "tmdb_vote_average"}, + {media: "tv", name: "Breaking Bad", year: 2008, want: "tvmaze_site_rating"}, + } { + out := c.callTool(t, "read_media", map[string]any{ + "action": "ratings", "media": tc.media, "name": tc.name, "year": tc.year, + }) + data := requireOK(t, out) + var page struct { + Items []struct { + Ratings []struct { + Name string `json:"name"` + } `json:"ratings"` + } `json:"items"` + } + if err := json.Unmarshal(data, &page); err != nil { + t.Fatalf("%s ratings decode: %v\n%s", tc.media, err, data) + } + if len(page.Items) != 1 || len(page.Items[0].Ratings) == 0 || + page.Items[0].Ratings[0].Name != tc.want { + t.Fatalf("%s fallback ratings: %s", tc.media, data) + } + if !strings.Contains(strings.Join(out.Envelope.Warnings, " "), "search metadata") { + t.Errorf("%s fallback warning missing: %v", tc.media, out.Envelope.Warnings) + } + } + if calls := mock.countCalls("GET", "/api/v2/Search/ratings/"); calls != 2 { + t.Errorf("ratings endpoint calls = %d, want 2", calls) + } + if calls := mock.countCalls("GET", "/api/v1/Search/movie/Alien"); calls != 1 { + t.Errorf("movie fallback calls = %d, want 1", calls) + } + if calls := mock.countCalls("GET", "/api/v1/Search/tv/Breaking Bad"); calls != 1 { + t.Errorf("TV fallback calls = %d, want 1", calls) + } +} + func TestReadRequestsListProjection(t *testing.T) { mock := newMockOmbi(t, "jwt") c := spawnServer(t, mock.env()) diff --git a/internal/integration_test/mockombi_test.go b/internal/integration_test/mockombi_test.go index d5cfa5c..8d3e79f 100644 --- a/internal/integration_test/mockombi_test.go +++ b/internal/integration_test/mockombi_test.go @@ -83,9 +83,11 @@ func newMockOmbi(t *testing.T, mode string) *mockOmbi { mux.HandleFunc("GET /api/v2/Search/actor/{id}/tv", m.wrap(m.actorTVCredits)) mux.HandleFunc("POST /api/v2/Search/advancedSearch/movie/{pos}/{amt}", m.wrap(m.advancedMovie)) mux.HandleFunc("POST /api/v2/Search/multi/{term}", m.wrap(m.multiSearch)) + mux.HandleFunc("GET /api/v2/Search/ratings/{media}/{name}/{year}", m.wrap(m.ratingsUnavailable)) mux.HandleFunc("GET /api/v2/Requests/recentlyRequested", m.wrap(m.recentlyRequested)) mux.HandleFunc("GET /api/v1/Search/tv/info/{id}", m.wrap(m.tvInfoTVDB)) mux.HandleFunc("GET /api/v1/Search/tv/{term}", m.wrap(m.tvSearch)) + mux.HandleFunc("GET /api/v1/Search/movie/{term}", m.wrap(m.movieSearch)) mux.HandleFunc("GET /api/v1/Request/movie/info/{id}", m.wrap(m.movieInfo)) mux.HandleFunc("POST /api/v2/Requests/tv", m.wrap(m.createTV)) mux.HandleFunc("POST /api/v1/Request/tv", m.wrap(m.createTV)) @@ -460,14 +462,21 @@ func mockTVSearchHit() map[string]any { "imdbId": "tt0903747", "title": "Breaking Bad", "name": "Breaking Bad", "overview": "A chemistry teacher turns to cooking meth.", - "firstAired": "2008-01-20T00:00:00", - "available": false, "requested": false, + "firstAired": "2008-01-20T00:00:00", "siteRating": 9, + "available": false, "requested": false, "poster": "/ggFHVNu6YYI5L9pCfOacjizRGt.jpg", "genre": []any{"Crime", "Drama"}, "extraSearchField": []any{"z"}, } } +func mockMovieSearchHit() map[string]any { + return map[string]any{ + "id": 348, "title": "Alien", "releaseDate": "1979-05-25T00:00:00", + "voteAverage": 8.2, "voteCount": 15400, + } +} + func mockMovieRequest() map[string]any { return map[string]any{ "id": 10, "requestId": 10, "theMovieDbId": 27205, "imdbId": "tt1375666", @@ -730,6 +739,14 @@ func (m *mockOmbi) tvSearch(w http.ResponseWriter, r *http.Request) { m.json([]any{mockTVSearchHit()})(w, r) } +func (m *mockOmbi) movieSearch(w http.ResponseWriter, r *http.Request) { + m.json([]any{mockMovieSearchHit()})(w, r) +} + +func (m *mockOmbi) ratingsUnavailable(w http.ResponseWriter, r *http.Request) { + m.jsonErr(w, http.StatusInternalServerError, "external ratings provider failed") +} + // tvBrowse is the v2 TMDB-keyed popular/anticipated/trending list — // members carry the provider id only in `id`. func (m *mockOmbi) tvBrowse(w http.ResponseWriter, r *http.Request) { diff --git a/internal/tools/media.go b/internal/tools/media.go index 8ab5c1e..4abb5a1 100644 --- a/internal/tools/media.go +++ b/internal/tools/media.go @@ -4,6 +4,8 @@ import ( "context" "encoding/json" "fmt" + "sort" + "strings" "ombi-mcp/internal/ombi" ) @@ -219,23 +221,116 @@ func (o *op) mediaRatings(a *mediaCallArgs) *ToolResult { raw, fail := o.call("GET", fmt.Sprintf("/api/v2/Search/ratings/%s/%s/%d", media, seg(a.Name), *a.Year), nil, nil) - if fail != nil { + if fail != nil && !ratingsFallbackEligible(fail) { return fail } - m, fail := o.decodeObject(raw) - if fail != nil { - return fail - } - it := Media{Media: media, Identifiers: []Identifier{}, Title: a.Name, Year: a.Year} - for k, v := range m { - if i, ok := toInt(v); ok { - it.Ratings = append(it.Ratings, Reference{Name: k, Value: i}) - } else if s, ok := v.(string); ok && s != "" { - it.Ratings = append(it.Ratings, Reference{Name: k, Value: s}) + if fail == nil { + var m map[string]any + if err := json.Unmarshal(raw, &m); err != nil { + return o.fail("UPSTREAM_SCHEMA_MISMATCH", + fmt.Sprintf("upstream response was not a JSON object: %s", sanitizeErr(err)), false) + } + if m != nil { + if ratings := ratingReferences(m); len(ratings) > 0 { + return o.ratingPage(media, a.Name, *a.Year, ratings) + } } } - return o.ok(&MediaPage{Kind: "media_page", - Items: []Media{it}, Page: singlePage(1, "media")}) + + // Ombi 4.53.x still exposes the Rotten Tomatoes routes above, but their + // private upstream endpoints have been removed. Fall back to rating + // metadata already returned by Ombi's normal search providers rather than + // turning that dependency failure into an unusable MCP action. + page, fallbackFail := o.mediaRatingsFromSearch(media, a.Name, *a.Year) + if fallbackFail != nil { + if fail != nil { + return fail + } + return fallbackFail + } + return o.ok(page) +} + +func ratingsFallbackEligible(fail *ToolResult) bool { + if fail == nil || fail.Error == nil || fail.Error.HTTPStatus == nil { + return false + } + status := *fail.Error.HTTPStatus + return status == 404 || status >= 500 +} + +func ratingReferences(m map[string]any) []Reference { + keys := make([]string, 0, len(m)) + for k := range m { + keys = append(keys, k) + } + sort.Strings(keys) + + refs := []Reference{} + for _, k := range keys { + v := m[k] + if i, ok := toInt(v); ok { + refs = append(refs, Reference{Name: k, Value: i}) + } else if s, ok := v.(string); ok && s != "" { + refs = append(refs, Reference{Name: k, Value: s}) + } + } + return refs +} + +func (o *op) mediaRatingsFromSearch(media, name string, year int) (*MediaPage, *ToolResult) { + raw, fail := o.call("GET", "/api/v1/Search/"+media+"/"+seg(name), nil, nil) + if fail != nil { + return nil, fail + } + arr, fail := o.decodeArray(raw) + if fail != nil { + return nil, fail + } + + for _, m := range arr { + title := jstr(m, "title", "name") + resultYear := yearOf(jstr(m, "releaseDate", "firstAired", "releaseYear")) + if !strings.EqualFold(strings.TrimSpace(title), strings.TrimSpace(name)) || + resultYear == nil || *resultYear != year { + continue + } + + refs := []Reference{} + source := "" + if media == "movie" { + source = "TMDB" + if v, ok := m["voteAverage"].(float64); ok { + refs = append(refs, Reference{Name: "tmdb_vote_average", Value: v}) + } + if v, ok := toInt(m["voteCount"]); ok { + refs = append(refs, Reference{Name: "tmdb_vote_count", Value: v}) + } + } else { + source = "TVMaze" + if v, ok := toInt(m["siteRating"]); ok { + refs = append(refs, Reference{Name: "tvmaze_site_rating", Value: v}) + } + } + o.warnf("Ombi's Rotten Tomatoes ratings endpoint was unavailable; ratings were read from %s search metadata", source) + if len(refs) == 0 { + o.warnf("the matching %s search result did not include rating metadata", media) + } + return &MediaPage{Kind: "media_page", Items: []Media{{ + Media: media, Identifiers: []Identifier{}, Title: title, Year: resultYear, Ratings: refs, + }}, Page: singlePage(1, "media")}, nil + } + + o.warnf("Ombi's Rotten Tomatoes ratings endpoint was unavailable and search returned no exact title/year match") + return &MediaPage{Kind: "media_page", Items: []Media{{ + Media: media, Identifiers: []Identifier{}, Title: name, Year: &year, Ratings: []Reference{}, + }}, Page: singlePage(1, "media")}, nil +} + +func (o *op) ratingPage(media, title string, year int, ratings []Reference) *ToolResult { + return o.ok(&MediaPage{Kind: "media_page", Items: []Media{{ + Media: media, Identifiers: []Identifier{}, Title: title, Year: &year, Ratings: ratings, + }}, Page: singlePage(1, "media")}) } func (o *op) mediaStreaming(a *mediaCallArgs) *ToolResult { -- 2.39.5 From 958006cff264e058e2dc0637c01c091c93fef6d1 Mon Sep 17 00:00:00 2001 From: Gronod Date: Sat, 19 Sep 2026 18:50:24 +0100 Subject: [PATCH 2/2] Document #12 vote read upstream failures --- docs/schema/02-tool-mapping.md | 2 +- docs/schema/03-input-schemas.md | 2 +- docs/schema/05-endpoint-coverage.md | 4 +-- docs/schema/06-verification.md | 1 + internal/integration_test/contract_test.go | 34 ++++++++++++++++++++++ internal/integration_test/mockombi_test.go | 14 +++++++++ 6 files changed, 53 insertions(+), 4 deletions(-) diff --git a/docs/schema/02-tool-mapping.md b/docs/schema/02-tool-mapping.md index 94bec38..b69cd6f 100644 --- a/docs/schema/02-tool-mapping.md +++ b/docs/schema/02-tool-mapping.md @@ -159,7 +159,7 @@ Issue `list` uses v1 paged issues and returns individual records. `summary` uses Issue creation maps only title, subject, description, `issueCategoryId`, numeric `requestType`, and optional `requestId`/`providerId`. Require at least one association by design; RAML marks these properties optional and does not establish a server requirement. If both are provided, verify they refer to the same media. The server supplies author, timestamps and state; never accept `userReported`, comments, resolved date or persistence ID from the model. Comment POST uses `{comment, issueId}`. Status POST uses `{issueId, status}`. Category POST uses `{value}`. Delete IDs are in the path; no speculative update-category endpoint exists. -Vote reads use `Vote/music/{requestId}` for albums; writes use `Vote/{up|down}/album/{requestId}`. Do not derive both directions from one generic segment rule. Vote toggling/repeated-call behaviour is unverified, hence no idempotence promise. +Vote reads use `Vote/music/{requestId}` for albums; writes use `Vote/{up|down}/album/{requestId}`. Do not derive both directions from one generic segment rule. Vote toggling/repeated-call behaviour is unverified, hence no idempotence promise. On Ombi 4.53.10, the global vote view and movie vote-record route can return HTTP 500 for existing instance data while the corresponding empty TV route succeeds. Ombi exposes no equivalent read route from which the adapter can reconstruct vote ownership or totals. Preserve `UPSTREAM_REJECTED` in that case; never convert the failure into a fabricated empty `vote_page`. User GET-by-ID is `/Identity/User/{id}`. `self`, all users, dropdown, claims, online and notification preferences each have their exact separate routes. Respect hide-user settings and upstream visibility; do not disclose hidden requester IDs simply to populate a normalized field. diff --git a/docs/schema/03-input-schemas.md b/docs/schema/03-input-schemas.md index 6bc15b7..03025cd 100644 --- a/docs/schema/03-input-schemas.md +++ b/docs/schema/03-input-schemas.md @@ -987,7 +987,7 @@ Read issue records, provider-grouped summaries, comments or counts. ## read_votes -Read all voteable requests or votes for a movie, TV or album request. +Read all voteable requests or votes for a movie, TV or album request. Some Ombi 4.53.x data sets make the global or movie read routes fail upstream; the tool reports `UPSTREAM_REJECTED` rather than treating unknown votes as an empty page. ```json { diff --git a/docs/schema/05-endpoint-coverage.md b/docs/schema/05-endpoint-coverage.md index 7d8f031..a874c34 100644 --- a/docs/schema/05-endpoint-coverage.md +++ b/docs/schema/05-endpoint-coverage.md @@ -383,13 +383,13 @@ The body/response columns describe the **upstream** schema, not a promise to pas | 365 | [POST `/api/v1/Token/requirePassword`](../api/raml/api.raml#L6464) | I | `upstream authentication adapter` | body [UserAuthModel](../api/raml/types/Ombi.Models.UserAuthModel.raml) | 200: boolean | Not model-callable; JWT-primary auth adapter (`OMBI_AUTH_MODE=jwt`); login/refresh semantics per `01-authentication.md`; Bearer-on-all-routes remains Verify. | | 366 | [POST `/api/v1/Token/header_auth`](../api/raml/api.raml#L6482) | I | `upstream authentication adapter` | none documented | 200: body unspecified | Not model-callable; JWT-primary auth adapter (`OMBI_AUTH_MODE=jwt`); login/refresh semantics per `01-authentication.md`; Bearer-on-all-routes remains Verify. | | 367 | [GET `/api/v1/Update`](../api/raml/api.raml#L6494) | D | `read_server / update_info` | none documented | 200: [UpdateModel](../api/raml/types/Ombi.Core.Processor.UpdateModel.raml) | — | -| 368 | [GET `/api/v1/Vote`](../api/raml/api.raml#L6503) | D | `read_votes / list` | none documented | 200: array<[VoteViewModel](../api/raml/types/Ombi.Core.Models.UI.VoteViewModel.raml)> | — | +| 368 | [GET `/api/v1/Vote`](../api/raml/api.raml#L6503) | D | `read_votes / list` | none documented | 200: array<[VoteViewModel](../api/raml/types/Ombi.Core.Models.UI.VoteViewModel.raml)> | Ombi 4.53.10 can return HTTP 500 for existing instance data; no lossless alternate read route exists, so preserve `UPSTREAM_REJECTED`. | | 369 | [POST `/api/v1/Vote/up/movie/{requestId}`](../api/raml/api.raml#L6515) | D | `write_vote` | path `requestId`:integer required | 200: [VoteEngineResult](../api/raml/types/Ombi.Core.Models.VoteEngineResult.raml) | — | | 370 | [POST `/api/v1/Vote/up/tv/{requestId}`](../api/raml/api.raml#L6530) | D | `write_vote` | path `requestId`:integer required | 200: [VoteEngineResult](../api/raml/types/Ombi.Core.Models.VoteEngineResult.raml) | — | | 371 | [POST `/api/v1/Vote/up/album/{requestId}`](../api/raml/api.raml#L6545) | D | `write_vote` | path `requestId`:integer required | 200: [VoteEngineResult](../api/raml/types/Ombi.Core.Models.VoteEngineResult.raml) | — | | 372 | [POST `/api/v1/Vote/down/movie/{requestId}`](../api/raml/api.raml#L6560) | D | `write_vote` | path `requestId`:integer required | 200: [VoteEngineResult](../api/raml/types/Ombi.Core.Models.VoteEngineResult.raml) | — | | 373 | [POST `/api/v1/Vote/down/tv/{requestId}`](../api/raml/api.raml#L6575) | D | `write_vote` | path `requestId`:integer required | 200: [VoteEngineResult](../api/raml/types/Ombi.Core.Models.VoteEngineResult.raml) | — | | 374 | [POST `/api/v1/Vote/down/album/{requestId}`](../api/raml/api.raml#L6590) | D | `write_vote` | path `requestId`:integer required | 200: [VoteEngineResult](../api/raml/types/Ombi.Core.Models.VoteEngineResult.raml) | — | -| 375 | [GET `/api/v1/Vote/movie/{requestId}`](../api/raml/api.raml#L6605) | D | `read_votes / get` | path `requestId`:integer required | 200: array<[Votes](../api/raml/types/Ombi.Store.Entities.Votes.raml)> | — | +| 375 | [GET `/api/v1/Vote/movie/{requestId}`](../api/raml/api.raml#L6605) | D | `read_votes / get` | path `requestId`:integer required | 200: array<[Votes](../api/raml/types/Ombi.Store.Entities.Votes.raml)> | Ombi 4.53.10 can return HTTP 500 for existing instance data; no lossless alternate read route exists, so preserve `UPSTREAM_REJECTED`. | | 376 | [GET `/api/v1/Vote/music/{requestId}`](../api/raml/api.raml#L6621) | D | `read_votes / get` | path `requestId`:integer required | 200: array<[Votes](../api/raml/types/Ombi.Store.Entities.Votes.raml)> | — | | 377 | [GET `/api/v1/Vote/tv/{requestId}`](../api/raml/api.raml#L6637) | D | `read_votes / get` | path `requestId`:integer required | 200: array<[Votes](../api/raml/types/Ombi.Store.Entities.Votes.raml)> | — | diff --git a/docs/schema/06-verification.md b/docs/schema/06-verification.md index 5cc3de6..56c18e0 100644 --- a/docs/schema/06-verification.md +++ b/docs/schema/06-verification.md @@ -328,3 +328,4 @@ The gated live suite is committed and ready; run it with a sourced `.env`. It ad ## M8 Findings — Upstream ratings and vote failures (#8, #12) - `#8`: Ombi 4.53.10 implements both v2 ratings routes by calling `www.rottentomatoes.com/api/private`; both private endpoints now return HTTP 404 and Ombi surfaces the dependency failure as HTTP 500. `read_media ratings` retains the native result when available and otherwise performs an exact title/year lookup through Ombi's normal movie or TV search. Fallback values are source-labelled (`tmdb_vote_average`, `tmdb_vote_count`, or `tvmaze_site_rating`) and the result carries a degradation warning. No fuzzy title or year substitution is allowed. +- `#12`: `GET /api/v1/Vote` and `GET /api/v1/Vote/movie/{requestId}` return HTTP 500 on the verified Ombi 4.53.10 data set, while an empty TV request returns HTTP 200 with `[]`. The global controller builds derived per-request summaries and the per-media controllers are the only raw vote-record reads; there is no second lossless API from which the MCP can recover user vote identity and counts. The adapter therefore preserves the sanitized, retryable `UPSTREAM_REJECTED` error and never substitutes an empty page. Mock coverage fixes this error boundary as part of the public contract. diff --git a/internal/integration_test/contract_test.go b/internal/integration_test/contract_test.go index 54ce6cb..56d2d29 100644 --- a/internal/integration_test/contract_test.go +++ b/internal/integration_test/contract_test.go @@ -6,6 +6,7 @@ import ( "crypto/sha256" "encoding/hex" "encoding/json" + "net/http" "strings" "testing" ) @@ -320,6 +321,39 @@ func TestReadMediaRatingsFallsBackToSearchMetadata(t *testing.T) { } } +func TestReadVotesPreservesUpstreamFailures(t *testing.T) { + mock := newMockOmbi(t, "jwt") + c := spawnServer(t, mock.env()) + c.handshake(t) + + cases := []map[string]any{ + {"action": "list"}, + {"action": "get", "media": "movie", "request_id": 2207}, + } + for _, args := range cases { + out := c.callTool(t, "read_votes", args) + err := requireErr(t, out, "UPSTREAM_REJECTED") + if err.HTTPStatus == nil || *err.HTTPStatus != http.StatusInternalServerError { + t.Errorf("vote read HTTP status = %v, want 500", err.HTTPStatus) + } + if !err.Retryable { + t.Errorf("vote read 500 should retain generic retryable mapping") + } + if len(out.Envelope.Data) != 0 && string(out.Envelope.Data) != "null" { + t.Errorf("vote read fabricated data after upstream failure: %s", out.Envelope.Data) + } + } + + out := c.callTool(t, "read_votes", map[string]any{ + "action": "get", "media": "tv", "request_id": 759, + }) + data := requireOK(t, out) + if !strings.Contains(string(data), `"kind":"vote_page"`) || + !strings.Contains(string(data), `"items":[]`) { + t.Fatalf("empty TV votes should remain a successful empty page: %s", data) + } +} + func TestReadRequestsListProjection(t *testing.T) { mock := newMockOmbi(t, "jwt") c := spawnServer(t, mock.env()) diff --git a/internal/integration_test/mockombi_test.go b/internal/integration_test/mockombi_test.go index 8d3e79f..44f9d4a 100644 --- a/internal/integration_test/mockombi_test.go +++ b/internal/integration_test/mockombi_test.go @@ -117,6 +117,8 @@ func newMockOmbi(t *testing.T, mode string) *mockOmbi { mockUser("u-1", "alice"), mockUser("u-2", "bob"), }))) mux.HandleFunc("GET /api/v1/Identity/User/{id}", m.wrap(m.userGet)) + mux.HandleFunc("GET /api/v1/Vote", m.wrap(m.voteListFailure)) + mux.HandleFunc("GET /api/v1/Vote/{media}/{id}", m.wrap(m.voteGet)) mux.HandleFunc("GET /api/v1/Radarr/Profiles", m.wrap(m.fixed(`[{"id":6,"name":"HD","weight":7,"enabled":false}]`))) mux.HandleFunc("GET /api/v2/Search/Genres/{media}", m.wrap(m.fixed(`[{"id":53,"name":"Thriller"}]`))) mux.HandleFunc("GET /api/v2/Search/Languages", m.wrap(m.fixed(`[{"iso_639_1":"ky","english_name":"??????","name":"Кыргызча"},{"iso_639_1":"zz","english_name":"??????","name":"??????"}]`))) @@ -747,6 +749,18 @@ func (m *mockOmbi) ratingsUnavailable(w http.ResponseWriter, r *http.Request) { m.jsonErr(w, http.StatusInternalServerError, "external ratings provider failed") } +func (m *mockOmbi) voteListFailure(w http.ResponseWriter, r *http.Request) { + m.jsonErr(w, http.StatusInternalServerError, "vote view could not be generated") +} + +func (m *mockOmbi) voteGet(w http.ResponseWriter, r *http.Request) { + if r.PathValue("media") == "tv" { + m.json([]any{})(w, r) + return + } + m.jsonErr(w, http.StatusInternalServerError, "vote records could not be read") +} + // tvBrowse is the v2 TMDB-keyed popular/anticipated/trending list — // members carry the provider id only in `id`. func (m *mockOmbi) tvBrowse(w http.ResponseWriter, r *http.Request) { -- 2.39.5