From 012f6c0852a0530fc2d334175dcd7a325a59dbd8 Mon Sep 17 00:00:00 2001 From: Gronod Date: Sat, 19 Sep 2026 16:04:33 +0100 Subject: [PATCH 1/6] Decode the v1 TV parent list's RequestsViewModel wrapper MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Live Ombi serves GET /api/v1/Request/tv/{count}/{pos}/1/0/0 as {"collection":[...],"total":N}, not a bare array, so decodeArray made every bounded parent scan fail on real instances — the #7 details overlay and the #10 search fallback only worked against the mock's unrealistic bare-array fixture. Decode the documented wrapper shape (tolerating bare arrays) and fix the mock to serve the real shape so tests exercise what production sees. --- internal/integration_test/mockombi_test.go | 35 ++++++++++++---------- internal/tools/requestscan.go | 30 ++++++++++++++++++- 2 files changed, 49 insertions(+), 16 deletions(-) diff --git a/internal/integration_test/mockombi_test.go b/internal/integration_test/mockombi_test.go index a140376..e7b3f71 100644 --- a/internal/integration_test/mockombi_test.go +++ b/internal/integration_test/mockombi_test.go @@ -575,29 +575,34 @@ func (m *mockOmbi) movieList(w http.ResponseWriter, r *http.Request) { func (m *mockOmbi) tvParentList(w http.ResponseWriter, r *http.Request) { pos := r.PathValue("pos") if pos != "0" { - m.json([]any{})(w, r) + m.json(map[string]any{"collection": []any{}, "total": 1})(w, r) return } - m.json([]any{ - map[string]any{ - "id": 42, - "tvDbId": 81189, - "title": "trigger-500 and something", // For search fallback test - "childRequests": []any{ - map[string]any{ - "seasonRequests": []any{ - map[string]any{ - "seasonNumber": 1, - "episodes": []any{ - map[string]any{"episodeNumber": 1, "requested": true}, - map[string]any{"episodeNumber": 2, "requested": true}, - map[string]any{"episodeNumber": 3, "requested": true}, + // RequestsViewModel: the v1 paged route wraps rows in + // {collection,total} — scans that decode a bare array break on live. + m.json(map[string]any{ + "collection": []any{ + map[string]any{ + "id": 42, + "tvDbId": 81189, + "title": "trigger-500 and something", // For search fallback test + "childRequests": []any{ + map[string]any{ + "seasonRequests": []any{ + map[string]any{ + "seasonNumber": 1, + "episodes": []any{ + map[string]any{"episodeNumber": 1, "requested": true}, + map[string]any{"episodeNumber": 2, "requested": true}, + map[string]any{"episodeNumber": 3, "requested": true}, + }, }, }, }, }, }, }, + "total": 1, })(w, r) } diff --git a/internal/tools/requestscan.go b/internal/tools/requestscan.go index 963b0d2..0d827c8 100644 --- a/internal/tools/requestscan.go +++ b/internal/tools/requestscan.go @@ -1,6 +1,7 @@ package tools import ( + "bytes" "fmt" ) @@ -18,7 +19,7 @@ func (o *op) eachTVRequestParent(fn func(map[string]any) bool) (truncated bool, if fail != nil { return false, fail } - arr, fail := o.decodeArray(raw) + arr, fail := o.decodeTVParentPage(raw) if fail != nil { return false, fail } @@ -38,6 +39,33 @@ func (o *op) eachTVRequestParent(fn func(map[string]any) bool) (truncated bool, return true, nil // hit cap } +// decodeTVParentPage decodes one page of the v1 TV parent list. The +// documented shape is RequestsViewModel — a +// {"collection":[...],"total":N} object — but a bare array is tolerated +// for versions or proxies that unwrap it. +func (o *op) decodeTVParentPage(raw []byte) ([]map[string]any, *ToolResult) { + trim := bytes.TrimSpace(raw) + if len(trim) > 0 && trim[0] == '[' { + return o.decodeArray(raw) + } + vm, fail := o.decodeObject(raw) + if fail != nil { + return nil, fail + } + if _, present := vm["collection"]; !present { + return nil, o.fail("UPSTREAM_SCHEMA_MISMATCH", + "tv parent page object lacked a collection", false) + } + arr := jarr(vm, "collection") + out := make([]map[string]any, 0, len(arr)) + for _, v := range arr { + if m, ok := v.(map[string]any); ok { + out = append(out, m) + } + } + return out, nil +} + // mergeTVRequestState attempts to match and apply request state from a parent record p. // Returns true if the parent matched (stopping the scan). func mergeTVRequestState(it *Media, p map[string]any, tvdbID int, imdbID string) bool { -- 2.39.5 From 40155b5981fedbc40ad5192f69065740b0b239bc Mon Sep 17 00:00:00 2001 From: Gronod Date: Sat, 19 Sep 2026 16:15:18 +0100 Subject: [PATCH 2/6] Resolve recent TV rows to real tv_parent request ids MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Closes #11. Ombi builds recentlyRequested TV rows from child requests, so requestId is a child id — and upstream persists the provider id as the child PK for new-request children, so the value is provider-shaped (TVDB/TMDB) while the parent request id never appears in the payload. Consumers following target into read_requests get hit upstream 500s. TV rows now resolve through one bounded v1 parent scan: an exact child-id match against embedded childRequests (authoritative), then a provider-id fallback against tvDbId/externalProviderId covering versions that put the provider id in requestId directly. The child id is preserved as an ombi_tv_child identifier, provider ids land under tvdb/tmdb/imdb, and unresolvable rows emit target.id 0 with a warning rather than a fabricated or provider-shaped target. --- internal/integration_test/contract_test.go | 49 ++++++--- internal/integration_test/mockombi_test.go | 59 +++++++++-- internal/tools/project_test.go | 40 ++++++++ internal/tools/requests.go | 54 ++++++++-- internal/tools/requestscan.go | 114 +++++++++++++++++++++ 5 files changed, 283 insertions(+), 33 deletions(-) diff --git a/internal/integration_test/contract_test.go b/internal/integration_test/contract_test.go index ad178eb..d7efd56 100644 --- a/internal/integration_test/contract_test.go +++ b/internal/integration_test/contract_test.go @@ -646,7 +646,7 @@ func TestReadRequestsRecentTVParentUsesRequestID(t *testing.T) { if err := json.Unmarshal(data, &page); err != nil { t.Fatalf("decode: %v\n%s", err, data) } - var movie, tv *struct { + items := map[string]*struct { Target struct { Kind string `json:"kind"` ID int `json:"id"` @@ -656,35 +656,50 @@ func TestReadRequestsRecentTVParentUsesRequestID(t *testing.T) { Namespace string `json:"namespace"` Value string `json:"value"` } `json:"identifiers"` - } + }{} for i := range page.Items { - switch page.Items[i].Title { - case "Yummy": - movie = &page.Items[i] - case "Seven Up!": - tv = &page.Items[i] - } + it := page.Items[i] + items[it.Title] = &it } - if movie == nil || tv == nil { + movie, tv, tvChildID, orphan := items["Yummy"], items["Seven Up!"], + items["trigger-500 and something"], items["Provider Only"] + if movie == nil || tv == nil || tvChildID == nil || orphan == nil { t.Fatalf("missing recent items: %s", data) } if movie.Target.Kind != "movie" || movie.Target.ID != 2207 { t.Errorf("movie target = %+v", movie.Target) } - if tv.Target.Kind != "tv_parent" || tv.Target.ID != 88 { - t.Errorf("tv_parent target = %+v, want request id 88 not provider 259032", tv.Target) + // TV requestId 259032 is a provider-shaped child PK upstream; the + // parent scan must resolve it to the real parent request id 909. + if tv.Target.Kind != "tv_parent" || tv.Target.ID != 909 { + t.Errorf("tv_parent target = %+v, want resolved parent id 909 not child/provider 259032", tv.Target) } tvIDs := map[string]string{} for _, id := range tv.Identifiers { tvIDs[id.Namespace] = id.Value } - if tvIDs["tmdb"] != "259032" { - t.Errorf("tv provider id missing from identifiers: %v", tvIDs) + if tvIDs["tvdb"] != "259032" || tvIDs["tmdb"] != "118680" || tvIDs["imdb"] != "tt1439629" { + t.Errorf("tv identifiers missing parent provider ids: %v", tvIDs) } - for _, it := range page.Items { - if it.Title == "Provider Only" && it.Target.ID == 111 { - t.Errorf("provider id occupied target.id on requestId-less recent item: %+v", it.Target) - } + if tvIDs["ombi_tv_child"] != "259032" { + t.Errorf("child request id missing from identifiers: %v", tvIDs) + } + // A real (non-provider-shaped) child id resolves via the parent's + // embedded childRequests[].id alone. + if tvChildID.Target.Kind != "tv_parent" || tvChildID.Target.ID != 42 { + t.Errorf("child-id recent item target = %+v, want parent id 42", tvChildID.Target) + } + // Unresolvable rows keep id 0 and a provider-shaped `id` must + // never occupy the target. + if orphan.Target.Kind != "tv_parent" || orphan.Target.ID != 0 { + t.Errorf("unresolvable tv item target = %+v, want tv_parent id 0", orphan.Target) + } + if len(out.Envelope.Warnings) == 0 { + t.Errorf("expected a warning for the unresolvable tv item: %s", out.Raw) + } + // All TV rows resolve in a single bounded parent scan. + if n := mock.countCalls("GET", "/api/v1/Request/tv/"); n != 1 { + t.Errorf("tv parent scan calls = %d, want 1", n) } assertNoLeak(t, out.Raw) } diff --git a/internal/integration_test/mockombi_test.go b/internal/integration_test/mockombi_test.go index e7b3f71..6ff2e69 100644 --- a/internal/integration_test/mockombi_test.go +++ b/internal/integration_test/mockombi_test.go @@ -583,14 +583,20 @@ func (m *mockOmbi) tvParentList(w http.ResponseWriter, r *http.Request) { m.json(map[string]any{ "collection": []any{ map[string]any{ - "id": 42, - "tvDbId": 81189, - "title": "trigger-500 and something", // For search fallback test + "id": 42, + "tvDbId": 81189, + "externalProviderId": 1396, + "imdbId": "tt0903747", + "title": "trigger-500 and something", // For search fallback test "childRequests": []any{ map[string]any{ + // A real (non-provider-shaped) child request + // id — recent rows reference this in requestId. + "id": 88, "seasonRequests": []any{ map[string]any{ - "seasonNumber": 1, + "childRequestId": 88, + "seasonNumber": 1, "episodes": []any{ map[string]any{"episodeNumber": 1, "requested": true}, map[string]any{"episodeNumber": 2, "requested": true}, @@ -601,8 +607,30 @@ func (m *mockOmbi) tvParentList(w http.ResponseWriter, r *http.Request) { }, }, }, + map[string]any{ + // Upstream persists the provider id as the child PK + // for new-request children — this child's id is the + // parent's tvDbId, exactly like live rows. + "id": 909, + "tvDbId": 259032, + "externalProviderId": 118680, + "imdbId": "tt1439629", + "title": "Seven Up!", + "childRequests": []any{ + map[string]any{ + "id": 259032, + "seasonRequests": []any{ + map[string]any{ + "childRequestId": 259032, + "seasonNumber": 1, + "episodes": []any{}, + }, + }, + }, + }, + }, }, - "total": 1, + "total": 2, })(w, r) } @@ -757,16 +785,27 @@ func (m *mockOmbi) recentlyRequested(w http.ResponseWriter, r *http.Request) { "requestDate": "2026-09-01T10:00:00", }, map[string]any{ - // Live TV recent items put the provider id in `id` and - // may omit requestId; target.id must not become 259032. - "id": 259032, "requestId": 88, "type": 0, - "title": "Seven Up!", "mediaId": "259032", + // Live TV rows carry the CHILD request id in requestId — + // for new-request children upstream persists the provider + // id as the child PK — and the parent's externalProviderId + // in mediaId. The parent request id is absent entirely. + "requestId": 259032, "type": 0, + "title": "Seven Up!", "mediaId": "118680", "userId": "u-1", "requestDate": "2026-09-02T10:00:00", }, map[string]any{ + // A real (non-provider-shaped) child id — resolvable only + // via the parent's embedded childRequests[].id. + "requestId": 88, "type": 0, + "title": "trigger-500 and something", "mediaId": "1396", + "userId": "u-2", "requestDate": "2026-09-03T10:00:00", + }, + map[string]any{ + // Unresolvable: no requestId, no parent match; a stray + // provider-shaped `id` must never occupy target.id. "id": 111, "type": 0, "title": "Provider Only", "mediaId": "111", "userId": "u-1", - "requestDate": "2026-09-03T10:00:00", + "requestDate": "2026-09-04T10:00:00", }, })(w, r) } diff --git a/internal/tools/project_test.go b/internal/tools/project_test.go index 7adcc87..3de9210 100644 --- a/internal/tools/project_test.go +++ b/internal/tools/project_test.go @@ -208,6 +208,46 @@ func TestMergeTVRequestStateNoMatch(t *testing.T) { } } +func TestTVParentMatchChildIDBeatsProviderID(t *testing.T) { + parent := map[string]any{ + "id": 909, "tvDbId": 259032, "externalProviderId": 118680, + "childRequests": []any{ + map[string]any{ + "id": 88, + "seasonRequests": []any{ + map[string]any{"childRequestId": 88, "seasonNumber": 1}, + }, + }, + }, + } + if got := tvParentMatch(parent, tvRecentKey{requestID: 88, mediaID: "1396"}); got != tvMatchChild { + t.Errorf("real child id must match tier 1, got %d", got) + } + if got := tvParentMatch(parent, tvRecentKey{requestID: 259032}); got != tvMatchProvider { + t.Errorf("provider-shaped child pk falls back to tier 2 without embedded child, got %d", got) + } + if got := tvParentMatch(parent, tvRecentKey{requestID: 0, mediaID: "118680"}); got != tvMatchProvider { + t.Errorf("mediaId provider match = %d, want %d", got, tvMatchProvider) + } + if got := tvParentMatch(parent, tvRecentKey{requestID: 777, mediaID: "999"}); got != tvMatchNone { + t.Errorf("unrelated key matched: %d", got) + } +} + +func TestTVParentMatchProviderShapedChildPK(t *testing.T) { + // Live rows: upstream persists the provider id as the child PK, so + // childRequests[].id equals tvDbId/externalProviderId. + parent := map[string]any{ + "id": 909, "tvDbId": 259032, + "childRequests": []any{ + map[string]any{"id": 259032}, + }, + } + if got := tvParentMatch(parent, tvRecentKey{requestID: 259032}); got != tvMatchChild { + t.Errorf("provider-shaped child id is still an exact tier-1 match, got %d", got) + } +} + func TestRefOfDeterministicValue(t *testing.T) { o := &op{} m := map[string]any{ diff --git a/internal/tools/requests.go b/internal/tools/requests.go index d04ca41..d6dc766 100644 --- a/internal/tools/requests.go +++ b/internal/tools/requests.go @@ -208,6 +208,11 @@ func (o *op) requestsRecent(a *RequestsListArgs) *ToolResult { return fail } items := []Request{} + type pendingTV struct { + idx int + key tvRecentKey + } + pending := []pendingTV{} for _, m := range arr { code, label := o.requestTypeTwin(m["type"]) kind := "movie" @@ -223,17 +228,54 @@ func (o *op) requestsRecent(a *RequestsListArgs) *ToolResult { kind = "movie" } r := o.projectRequest(m, kind) - // RecentlyRequestedModel.RequestId is the Ombi request id. - // Extra `id` on a TV payload is a provider id and must not - // occupy target.id even when requestId is absent. - if id, ok := toInt(m["requestId"]); ok && id > 0 { - r.Target.ID = id - } else { + if kind == "tv_parent" { + // On TV rows RecentlyRequestedModel.requestId is the CHILD + // request id (upstream persists the provider id as the + // child PK for new-request children), never the parent id + // `get` needs. Zero whatever requestId/`id` projected and + // resolve the parent through the v1 parent list; the child + // id stays available as an ombi_tv_child identifier. r.Target.ID = 0 + key := tvRecentKey{mediaID: jstr(m, "mediaId")} + if id, ok := toInt(m["requestId"]); ok && id > 0 { + key.requestID = id + r.Identifiers = addID(r.Identifiers, "ombi_tv_child", id) + } + pending = append(pending, pendingTV{len(items), key}) + } else { + // RecentlyRequestedModel.requestId is the request id for + // movie/album rows; a stray provider `id` must not occupy + // the target even when requestId is absent. + if id, ok := toInt(m["requestId"]); ok && id > 0 { + r.Target.ID = id + } else { + r.Target.ID = 0 + } } r.RequestedUserID = jstr(m, "userId") items = append(items, r) } + if len(pending) > 0 { + keys := make([]tvRecentKey, len(pending)) + for i, p := range pending { + keys[i] = p.key + } + parents := o.resolveRecentTVParents(keys) + for i, p := range pending { + r := &items[p.idx] + parent, ok := parents[i] + pid := jint(parent, "id") + if !ok || pid == nil || *pid < 1 { + o.warnf("recent tv item %q has no resolvable parent request; target.id left unknown", + r.Title) + continue + } + r.Target.ID = *pid + r.Identifiers = addID(r.Identifiers, "tvdb", parent["tvDbId"]) + r.Identifiers = addID(r.Identifiers, "tmdb", parent["externalProviderId"]) + r.Identifiers = addID(r.Identifiers, "imdb", parent["imdbId"]) + } + } win, pg := localWindow(o, items, a.Page, "requests") return o.ok(&RequestPage{Kind: "request_page", Items: win, Page: pg}) } diff --git a/internal/tools/requestscan.go b/internal/tools/requestscan.go index 0d827c8..d9c73d0 100644 --- a/internal/tools/requestscan.go +++ b/internal/tools/requestscan.go @@ -3,6 +3,7 @@ package tools import ( "bytes" "fmt" + "strconv" ) // eachTVRequestParent iterates GET /api/v1/Request/tv/{count}/{pos}/1/0/0 @@ -66,6 +67,119 @@ func (o *op) decodeTVParentPage(raw []byte) ([]map[string]any, *ToolResult) { return out, nil } +// tvRecentKey carries the identity hints one recentlyRequested TV row +// provides: requestId is the CHILD request id (Ombi builds recent TV +// rows from child requests and persists the provider id as the child +// PK for new-request children), mediaId the parent's +// externalProviderId. The parent request id itself is never sent. +type tvRecentKey struct { + requestID int + mediaID string +} + +const ( + tvMatchNone = iota + tvMatchChild + tvMatchProvider +) + +// tvParentChildIDs collects the real child request ids a TvRequests +// parent record embeds: childRequests[].id plus +// seasonRequests[].childRequestId. +func tvParentChildIDs(p map[string]any) map[int]bool { + ids := map[int]bool{} + for _, cr := range jarr(p, "childRequests") { + crm, ok := cr.(map[string]any) + if !ok { + continue + } + if id := jint(crm, "id"); id != nil && *id > 0 { + ids[*id] = true + } + for _, sr := range jarr(crm, "seasonRequests") { + srm, ok := sr.(map[string]any) + if !ok { + continue + } + if id := jint(srm, "childRequestId"); id != nil && *id > 0 { + ids[*id] = true + } + } + } + return ids +} + +// tvParentProviderIDs collects a parent record's provider ids as a +// string set so both int requestId and string mediaId keys compare. +func tvParentProviderIDs(p map[string]any) map[string]bool { + ids := map[string]bool{} + for _, k := range []string{"tvDbId", "externalProviderId"} { + if s, ok := toStr(p[k]); ok && s != "" && s != "0" { + ids[s] = true + } + } + return ids +} + +// tvParentMatch reports how parent record p matches key: tvMatchChild +// when key.requestID is one of the parent's embedded child request ids +// (authoritative), tvMatchProvider when requestID/mediaID equals a +// parent provider id (fallback — covers versions that put the provider +// id in requestId directly or omit embedded children). +func tvParentMatch(p map[string]any, k tvRecentKey) int { + if k.requestID > 0 && tvParentChildIDs(p)[k.requestID] { + return tvMatchChild + } + provs := tvParentProviderIDs(p) + if k.requestID > 0 && provs[strconv.Itoa(k.requestID)] { + return tvMatchProvider + } + if k.mediaID != "" && provs[k.mediaID] { + return tvMatchProvider + } + return tvMatchNone +} + +// resolveRecentTVParents maps recentlyRequested TV rows onto their +// parent request records via one bounded parent scan. Child-id matches +// win over provider-id candidates so an unrelated parent's provider id +// can never shadow a real child id found later in the scan. Returns +// row-index → parent record; scan failures degrade to a warning, never +// an error, since the recent payload itself succeeded. +func (o *op) resolveRecentTVParents(keys []tvRecentKey) map[int]map[string]any { + resolved := map[int]map[string]any{} + candidate := map[int]map[string]any{} + truncated, fail := o.eachTVRequestParent(func(p map[string]any) bool { + for i, k := range keys { + if _, done := resolved[i]; done { + continue + } + switch tvParentMatch(p, k) { + case tvMatchChild: + resolved[i] = p + delete(candidate, i) + case tvMatchProvider: + if _, ok := candidate[i]; !ok { + candidate[i] = p + } + } + } + return len(resolved) == len(keys) + }) + for i, p := range candidate { + if _, ok := resolved[i]; !ok { + resolved[i] = p + } + } + switch { + case fail != nil: + o.warnf("tv parent lookup failed; recent tv targets left unresolved") + case truncated: + o.warnf("tv parent lookup truncated; some recent tv targets may be unresolved") + } + return resolved +} + // mergeTVRequestState attempts to match and apply request state from a parent record p. // Returns true if the parent matched (stopping the scan). func mergeTVRequestState(it *Media, p map[string]any, tvdbID int, imdbID string) bool { -- 2.39.5 From 0ce9e2626b1e24716ceb91f821dcb2bbcfba55e7 Mon Sep 17 00:00:00 2001 From: Gronod Date: Sat, 19 Sep 2026 16:16:54 +0100 Subject: [PATCH 3/6] Surface parent provider ids on tv_child projections MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ChildRequests carries no top-level provider ids upstream — they live on the embedded parentRequest navigation property — so children/list items emitted empty identifiers. Read them from the embedded record; the child target id itself stays the true child PK (provider-shaped for first-request children by upstream design, and callable for child-scoped operations). Mock child rows and the new /Request/tv/{id}/child fixture now match the real wire shape. --- internal/integration_test/contract_test.go | 47 ++++++++++++++++++++++ internal/integration_test/mockombi_test.go | 16 +++++++- internal/tools/project.go | 7 ++++ 3 files changed, 69 insertions(+), 1 deletion(-) diff --git a/internal/integration_test/contract_test.go b/internal/integration_test/contract_test.go index d7efd56..41a3a85 100644 --- a/internal/integration_test/contract_test.go +++ b/internal/integration_test/contract_test.go @@ -218,6 +218,53 @@ func TestReadRequestsTVChildrenAreChildKind(t *testing.T) { } } +func TestReadRequestsChildrenSurfacesParentProviderIDs(t *testing.T) { + mock := newMockOmbi(t, "jwt") + c := spawnServer(t, mock.env()) + c.handshake(t) + + out := c.callTool(t, "read_requests", map[string]any{ + "action": "children", "parent_request_id": 12, + }) + data := requireOK(t, out) + var page struct { + Items []struct { + Target struct { + Kind string `json:"kind"` + ID int `json:"id"` + } `json:"target"` + ParentRequestID *int `json:"parent_request_id"` + Identifiers []struct { + Namespace string `json:"namespace"` + Value string `json:"value"` + } `json:"identifiers"` + } `json:"items"` + } + if err := json.Unmarshal(data, &page); err != nil { + t.Fatalf("decode: %v\n%s", err, data) + } + if len(page.Items) != 1 { + t.Fatalf("expected 1 child, got %d: %s", len(page.Items), data) + } + it := page.Items[0] + // The child id is the real upstream child PK — provider-shaped for + // first-request children by design — and stays callable. + if it.Target.Kind != "tv_child" || it.Target.ID != 77 { + t.Errorf("child target = %+v, want tv_child 77", it.Target) + } + if it.ParentRequestID == nil || *it.ParentRequestID != 12 { + t.Errorf("parent_request_id = %v, want 12", it.ParentRequestID) + } + ids := map[string]string{} + for _, id := range it.Identifiers { + ids[id.Namespace] = id.Value + } + if ids["tvdb"] != "81189" || ids["tmdb"] != "1396" || ids["imdb"] != "tt0903747" { + t.Errorf("parentRequest provider ids not surfaced: %v", ids) + } + assertNoLeak(t, out.Raw) +} + // --- TV identifier namespaces (Gitea issue #1) --- func TestReadMediaDetailsTVDBUsesV1InfoRoute(t *testing.T) { diff --git a/internal/integration_test/mockombi_test.go b/internal/integration_test/mockombi_test.go index 6ff2e69..e943010 100644 --- a/internal/integration_test/mockombi_test.go +++ b/internal/integration_test/mockombi_test.go @@ -63,6 +63,7 @@ func newMockOmbi(t *testing.T, mode string) *mockOmbi { mux.HandleFunc("GET /api/v1/Status/info", m.wrap(m.fixed(`"mock-status-info"`))) mux.HandleFunc("GET /api/v1/Settings/about", m.wrap(m.json(mockAbout()))) mux.HandleFunc("GET /api/v1/Request/tv/{count}/{pos}/{o}/{s}/{a}", m.wrap(m.tvParentList)) + mux.HandleFunc("GET /api/v1/Request/tv/{id}/child", m.wrap(m.tvChildren)) mux.HandleFunc("GET /api/v2/Requests/movie/{amt}/{pos}/requestDate/{order}", m.wrap(m.movieList)) mux.HandleFunc("GET /api/v2/Requests/tv/{amt}/{pos}/requestDate/{order}", m.wrap(m.tvList)) mux.HandleFunc("GET /api/v2/Search/movie/{id}", m.wrap(m.movieDetails)) @@ -494,7 +495,12 @@ func mockTVChild() map[string]any { }, }, }, - "tvDbId": 81189, "externalProviderId": 1396, "imdbId": "tt0903747", + // ChildRequests carries no top-level provider ids upstream — + // they live on the embedded parent navigation property. + "parentRequest": map[string]any{ + "id": 12, "tvDbId": 81189, + "externalProviderId": 1396, "imdbId": "tt0903747", + }, "childFieldFuture": []any{1, 2}, } } @@ -634,6 +640,14 @@ func (m *mockOmbi) tvParentList(w http.ResponseWriter, r *http.Request) { })(w, r) } +func (m *mockOmbi) tvChildren(w http.ResponseWriter, r *http.Request) { + if r.PathValue("id") != "12" { + m.jsonErr(w, http.StatusNotFound, "Parent request not found") + return + } + m.json([]any{mockTVChild()})(w, r) +} + func (m *mockOmbi) tvList(w http.ResponseWriter, r *http.Request) { w.Header().Set("Content-Type", "application/json") json.NewEncoder(w).Encode(map[string]any{ diff --git a/internal/tools/project.go b/internal/tools/project.go index 074e030..0fe2986 100644 --- a/internal/tools/project.go +++ b/internal/tools/project.go @@ -448,6 +448,13 @@ func (o *op) projectRequest(m map[string]any, kind string) Request { r.Identifiers = addID(r.Identifiers, "tmdb", m["externalProviderId"]) r.Identifiers = addID(r.Identifiers, "tmdb", m["mediaId"]) r.Identifiers = addID(r.Identifiers, "imdb", m["imdbId"]) + // ChildRequests carries no top-level provider ids — they live + // on the embedded parent record. + if pr := jobj(m, "parentRequest"); pr != nil { + r.Identifiers = addID(r.Identifiers, "tvdb", pr["tvDbId"]) + r.Identifiers = addID(r.Identifiers, "tmdb", pr["externalProviderId"]) + r.Identifiers = addID(r.Identifiers, "imdb", pr["imdbId"]) + } case "album": r.Identifiers = addID(r.Identifiers, "musicbrainz", m["foreignAlbumId"]) r.Identifiers = addID(r.Identifiers, "musicbrainz", m["mediaId"]) -- 2.39.5 From ff18e60d5a2b1f4194a15abfb74a754977c65bcd Mon Sep 17 00:00:00 2001 From: Gronod Date: Sat, 19 Sep 2026 16:18:04 +0100 Subject: [PATCH 4/6] Resolve discover requested-fallback TV rows to tv_parent targets MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The recentlyRequested fallback emitted tv_child targets for TV rows while the primary requested-browse path emits tv_parent — and a tv_child target is a dead end for read_requests get/children. Resolve the child request id to the parent via the same bounded scan so both paths agree; rows that fail resolution keep the truthful tv_child target. --- internal/integration_test/contract_test.go | 12 ++++++++++++ internal/tools/discover.go | 22 ++++++++++++++++++++++ 2 files changed, 34 insertions(+) diff --git a/internal/integration_test/contract_test.go b/internal/integration_test/contract_test.go index 41a3a85..a228337 100644 --- a/internal/integration_test/contract_test.go +++ b/internal/integration_test/contract_test.go @@ -559,6 +559,10 @@ func TestReadDiscoverRequestedFallsBackToRecentFeed(t *testing.T) { Namespace string `json:"namespace"` Value string `json:"value"` } `json:"identifiers"` + RequestTargets []struct { + Kind string `json:"kind"` + ID int `json:"id"` + } `json:"request_targets"` } `json:"items"` Page struct { Mode string `json:"mode"` @@ -582,6 +586,14 @@ func TestReadDiscoverRequestedFallsBackToRecentFeed(t *testing.T) { if ids["tmdb"] == "" { t.Errorf("%s fallback missing tmdb identifier: %s", media, data) } + if media == "tv" { + // Recent TV rows carry child request ids; the fallback must + // resolve them to the parent target like the primary path. + rt := page.Items[0].RequestTargets + if len(rt) != 1 || rt[0].Kind != "tv_parent" || rt[0].ID != 909 { + t.Errorf("tv fallback request_targets = %+v, want [{tv_parent 909}]", rt) + } + } assertNoLeak(t, out.Raw) } if n := mock.countCalls("GET", "/api/v2/Requests/recentlyRequested"); n != 2 { diff --git a/internal/tools/discover.go b/internal/tools/discover.go index 553bc4c..7ee7c66 100644 --- a/internal/tools/discover.go +++ b/internal/tools/discover.go @@ -111,6 +111,11 @@ func (o *op) discoverRequestedFallback(a *DiscoverArgs, media string) *ToolResul wantType = 0 } items := make([]Media, 0, len(arr)) + type pendingTV struct { + idx int + key tvRecentKey + } + pending := []pendingTV{} for _, m := range arr { kind, ok := toInt(m["type"]) if !ok || kind != wantType { @@ -132,7 +137,12 @@ func (o *op) discoverRequestedFallback(a *DiscoverArgs, media string) *ToolResul if id, ok := toInt(m["requestId"]); ok && id > 0 { kind := "movie" if media == "tv" { + // Recent TV requestId is a child request id — resolve + // to the parent below; tv_child stays as the truthful + // fallback when no parent can be found. kind = "tv_child" + pending = append(pending, pendingTV{ + len(items), tvRecentKey{requestID: id, mediaID: jstr(m, "mediaId")}}) } it.RequestTargets = []OutTarget{{Kind: kind, ID: id}} } @@ -143,6 +153,18 @@ func (o *op) discoverRequestedFallback(a *DiscoverArgs, media string) *ToolResul } items = append(items, it) } + if len(pending) > 0 { + keys := make([]tvRecentKey, len(pending)) + for i, p := range pending { + keys[i] = p.key + } + parents := o.resolveRecentTVParents(keys) + for i, p := range pending { + if pid := jint(parents[i], "id"); pid != nil && *pid > 0 { + items[p.idx].RequestTargets = []OutTarget{{Kind: "tv_parent", ID: *pid}} + } + } + } win, _ := localWindow(o, items, a.Page, "media") o.truncated = true o.warnf("requested browse fell back to Ombi's bounded recently-requested feed") -- 2.39.5 From d07ef4f818e8557c33db19ef20e9d4560693ff03 Mon Sep 17 00:00:00 2001 From: Gronod Date: Sat, 19 Sep 2026 16:19:02 +0100 Subject: [PATCH 5/6] Treat requestId 0 as absent in the TV details overlay gate MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Upstream Search/tv/info responses send requestId 0 (not a missing field) for shows without a linked request, so the #7 request-state overlay never fired on live instances — requested shows reported requested:false. Now that the parent scan decodes the real collection wrapper, gating on requestId nil-or-0 lets the overlay actually run. --- internal/integration_test/mockombi_test.go | 3 ++- internal/tools/media.go | 5 ++++- 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/internal/integration_test/mockombi_test.go b/internal/integration_test/mockombi_test.go index e943010..415ad79 100644 --- a/internal/integration_test/mockombi_test.go +++ b/internal/integration_test/mockombi_test.go @@ -420,7 +420,8 @@ func mockTVInfoV1() map[string]any { "title": "Breaking Bad", "name": "Breaking Bad", "overview": "A chemistry teacher turns to cooking meth.", "firstAired": "2008-01-20T00:00:00", "status": "Ended", - "available": false, "requested": false, "fullyAvailable": false, + // Live sends requestId 0 (not absent) on unrequested shows. + "available": false, "requested": false, "requestId": 0, "fullyAvailable": false, "poster": "/ggFHVNu6YYI5L9pCfOacjizRGt.jpg", "banner": "/tsRy63Mu5cu8etL1X7ZLyf7UP1M.jpg", "genre": []any{"Crime", "Drama", "Thriller"}, diff --git a/internal/tools/media.go b/internal/tools/media.go index 10d6fb7..8ab5c1e 100644 --- a/internal/tools/media.go +++ b/internal/tools/media.go @@ -133,7 +133,10 @@ func (o *op) mediaDetails(a *mediaCallArgs) *ToolResult { // #7 request state overlay for tvdb if t.Media == "tv" && t.Provider == "tvdb" { reqVal := jbool(m, "requested") - if (reqVal == nil || !*reqVal) && jint(m, "requestId") == nil { + // Upstream sends requestId 0 (not absent) on unrequested shows — + // a nil-only check keeps the overlay from ever firing on live. + rid := jint(m, "requestId") + if (reqVal == nil || !*reqVal) && (rid == nil || *rid == 0) { id, _ := t.idInt() o.overlayTVRequestState(&it, id, jstr(m, "imdbId")) } -- 2.39.5 From 5b1be698d601b3e139f360ce0fae9c994cc84427 Mon Sep 17 00:00:00 2001 From: Gronod Date: Sat, 19 Sep 2026 16:20:03 +0100 Subject: [PATCH 6/6] Document recent TV identity resolution and add a live callable check MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The schema contract claimed recent target.id is requestId for every kind — upstream TV rows actually carry a child request id there. The docs now describe the bounded parent-scan resolution, the ombi_tv_child identifier, and the id-0-with-warning degradation. A live test proves a recent tv_parent target is callable via get. --- docs/schema/02-tool-mapping.md | 2 +- docs/schema/04-results.md | 2 +- internal/integration_test/live_test.go | 48 ++++++++++++++++++++++++++ 3 files changed, 50 insertions(+), 2 deletions(-) diff --git a/docs/schema/02-tool-mapping.md b/docs/schema/02-tool-mapping.md index d49c9ab..d5f1451 100644 --- a/docs/schema/02-tool-mapping.md +++ b/docs/schema/02-tool-mapping.md @@ -120,7 +120,7 @@ TV identifier labels follow the origin route, not the upstream field name. The v Use v2 list/status routes, always with sort and page segments. `status` defaults `all`; `sort_direction` maps to the `{sortOrder}` segment (`asc`/`desc`). Public `sort.field=request_date` maps to the documented example `requestDate`; no speculative sort fields are published. `all` means the base route, not an `/all/` segment. Album lacks an unavailable-status route, so that combination fails schema validation. The misspelled movie `availble` route is an explicitly gated compatibility alias, not the primary path. -`get` supports movie and TV parent only; no album single-request endpoint is advertised. `children` returns children for a parent. Request `search` uses the appropriate v1 route and rejects list-only status/sort arguments. `recent` uses v2 recentlyRequested; `target.id` is `requestId` (never a provider id left in `id`). `retry_queue` is a privileged GET and returns queue IDs separately from underlying request IDs. +`get` supports movie and TV parent only; no album single-request endpoint is advertised. `children` returns children for a parent; child ids are the real upstream child PKs (provider-shaped for first-request children by design), with provider ids surfaced from the embedded `parentRequest`. Request `search` uses the appropriate v1 route and rejects list-only status/sort arguments. `recent` uses v2 recentlyRequested; for movie/album `target.id` is `requestId`, while TV `requestId` is a child request id (often provider-shaped) that is resolved to the parent request id through a bounded v1 parent scan — unresolvable rows emit `target.id` 0 with a warning rather than a provider-shaped target. `retry_queue` is a privileged GET and returns queue IDs separately from underlying request IDs. `read_request_stats.counts` uses Request/count; `total` uses the media's total endpoint; `quota` uses its remaining endpoint. Quota belongs to the actual upstream principal. `has_requests` requires an explicit `user_id` by MCP policy and sends it as the optional upstream `userId` query; viewing another user is subject to authorization. Do not combine instance totals with a per-user quota under an unlabeled “total”. diff --git a/docs/schema/04-results.md b/docs/schema/04-results.md index ce30ec2..568104e 100644 --- a/docs/schema/04-results.md +++ b/docs/schema/04-results.md @@ -17,7 +17,7 @@ Each per-tool output schema is intentionally a bounded projection, not the recur | Family | Projection rules | |---|---| | media_page | Search/discovery/details return zero or more normalized media records. Details usually has one item. Keep every known ID namespace; never emit the same namespace+value twice. On TV the upstream `theMovieDbId` field name lies: TVMaze-backed v1 routes (`Search/tv/{term}`, `Search/tv/info/{tvdbId}`) carry the TVDB id there (emit `tvdb`, plus `seriesId`→`tvmaze`); TMDB-keyed v2 routes carry the TMDB id (emit `tmdb`) and must not label `seriesId` as `tvmaze` — on those routes it echoes the TMDB id. When `theMovieDbId` is absent, `id` is labelled with the same origin namespace (v2 browse/collection members and MovieFullInfoViewModel). `belongsToCollection.id` is the collection's TMDB id, not the movie's. Multi-search `mediaType` is matched case-insensitively (`Artist`→`artist`/`musicbrainz`). The namespace label is per origin route, never per field name. Credit calls require the caller-supplied person name because Ombi returns only the person ID; TV credit titles are enriched from their TMDB detail records. If requested browse falls back to Ombi's bounded recently-requested feed, mark it truncated and leave total/continuation unknown. Map cast/crew into credits; title-specific streaming into providers; rating fields into named rating references. Never claim a global provider catalogue is a title's availability. Collections keep their own collection identity and returned members. | -| request_page | Map the Ombi request id to target kind and ID: prefer `requestId` over `id`. v2 TV list items are children and `parentRequestId` is preserved; v1 parent records stay parents. On `recent`, `RecentlyRequestedModel.requestId` is the target id for every kind; a provider-shaped `id` (common on TV) is never the target — provider values go in `identifiers` (`mediaId`, `tvDbId`, `externalProviderId`). Include standard and 4K state separately. Never infer one combined lifecycle status when booleans disagree. | +| request_page | Map the Ombi request id to target kind and ID: prefer `requestId` over `id`. v2 TV list items are children and `parentRequestId` is preserved; v1 parent records stay parents; child provider ids live on the embedded `parentRequest` record. On `recent`, `RecentlyRequestedModel.requestId` is the request id for movie/album, but on TV rows it is a *child* request id (upstream builds recent TV rows from child requests and persists the provider id as the child PK for new-request children). The `tv_parent` target id is therefore resolved through a bounded v1 parent scan — child-id match first, then `tvDbId`/`externalProviderId` fallback — the child id is emitted as an `ombi_tv_child` identifier, provider values land in `identifiers` (`mediaId`, `tvDbId`, `externalProviderId`), and unresolvable rows emit `target.id` 0 with a warning. A provider-shaped value is never the target. Include standard and 4K state separately. Never infer one combined lifecycle status when booleans disagree. | | issue_page | Project writable/display fields plus IDs/timestamps. Wire `resovledDate` maps to `resolved_date` without changing upstream spelling. Omit nested user objects and comments unless requested separately. | | group_page | v2 issue summaries are provider groups. Count and page units describe groups, not individual issues. Truncate nested issues with a warning. | | comment_page | Preserve comment text and authorized author identifier, omit full user graph. | diff --git a/internal/integration_test/live_test.go b/internal/integration_test/live_test.go index c1c4b04..2e0dace 100644 --- a/internal/integration_test/live_test.go +++ b/internal/integration_test/live_test.go @@ -477,6 +477,54 @@ func TestLiveDiscoverTVBrowseHasIdentifiers(t *testing.T) { assertNoLeak(t, out.Raw) } +func TestLiveRecentTVParentTargetIsCallable(t *testing.T) { + c := liveServer(t) + out := c.callTool(t, "read_requests", map[string]any{"action": "recent"}) + data := requireOK(t, out) + var page struct { + Items []struct { + Target struct { + Kind string `json:"kind"` + ID int `json:"id"` + } `json:"target"` + Title string `json:"title"` + } `json:"items"` + } + if err := json.Unmarshal(data, &page); err != nil { + t.Fatalf("decode: %v\n%s", err, data) + } + var tv *struct { + Target struct { + Kind string `json:"kind"` + ID int `json:"id"` + } `json:"target"` + Title string `json:"title"` + } + for i := range page.Items { + if page.Items[i].Target.Kind == "tv_parent" { + tv = &page.Items[i] + break + } + } + if tv == nil { + t.Skip("live recent feed has no TV rows") + } + // Issue #11: upstream emits a provider-shaped child request id + // here; the resolved tv_parent id must be callable via get. + if tv.Target.ID < 1 { + t.Skipf("recent tv row %q left unresolved (id 0)", tv.Title) + } + out = c.callTool(t, "read_requests", map[string]any{ + "action": "get", + "target": map[string]any{"kind": "tv_parent", "id": tv.Target.ID}, + }) + if out.IsError { + t.Fatalf("recent tv_parent target %d not callable via get (issue #11): %s", + tv.Target.ID, out.Raw) + } + assertNoLeak(t, out.Raw) +} + func TestLiveSearchMultiArtistMapped(t *testing.T) { c := liveServer(t) out := c.callTool(t, "read_search", map[string]any{ -- 2.39.5