Files
ombi-mcp/docs/schema/06-verification.md
gronod 80e01253a2
Build and publish / Test and build (linux) (pull_request) Canceled after 0s
Build and publish / Test and build (windows) (pull_request) Canceled after 0s
Build and publish / Build and publish Docker image (pull_request) Canceled after 0s
Build and publish / Test and build (darwin) (pull_request) Canceled after 40s
Build and publish / Test and build (darwin) (push) Successful in 2m9s
Build and publish / Test and build (linux) (push) Successful in 2m34s
Build and publish / Test and build (windows) (push) Successful in 3m11s
Build and publish / Build and publish Docker image (push) Successful in 1m55s
Fix #10, #27 and #28 from the read-tools sweep
v2 request lists sent the RAML example sort field requestDate; Ombi
looks up RequestedDate and NullReferenceException'd every non-empty
page. Browse now streams TV popular/most-watched payloads and skips
the hydrated seasonRequests graph that blew the 8 MiB read budget.
provider_summary treats an empty upstream body as an empty group_page.
2026-09-19 20:59:57 +01:00

27 KiB
Raw Permalink Blame History

Evidence gaps and acceptance criteria

What this review establishes

The review reads all six earlier design documents, inventories every resource/method in api.raml, follows the request/response type references relevant to the tool contracts and inspects the authentication security scheme. The complete inventory is 377 operations, 321 paths. The replacement accounts for 257 direct mappings, 37 alternative routes, 59 partial mappings, 12 internal operations and 12 exclusions. These are design dispositions, not live integration-test results.

The input catalogue contains all 31 proposed tool schemas; it does not stop at the three most complex tools. The output contract binds each of those 31 operations to specific result families. Administrative patch schemas reference 33 writable settings sections; settings reads cover 41 sections. Feature writes are separate actions and not counted as a settings section.

No application code, runtime dependency declaration, credentials, existing draft or RAML file is changed. No live Ombi mutation is performed. Source-specific verification remains necessary before implementing or enabling gated branches.

Checks completed on these documents

  • Parsed the embedded JSON and validated all 31 input schemas plus the common output schema using jsonschema 4.25.1's Draft202012Validator. The validator was installed in a temporary directory, not added to this project.
  • Validated 217 positive branch/result examples, including one minimal input per branch and success/error results for every tool. Rejected 568 negative cases covering missing required fields, unexpected properties and representative invalid media/action combinations. These checks establish schema consistency, not upstream runtime semantics.
  • Compared the ledger's unique method/path set directly with the parsed RAML: exact equality, 377 operations, 321 distinct paths, no omissions or duplicates. Every published tool has an operation owner in the ledger.
  • Checked local Markdown links and all local $ref targets, and confirmed the new documents contain no environment-specific instance URL.
  • Reproduced the earlier drafts' schema issues with a validator: SWE read_requests with only mediaType=movie unexpectedly requires requestId and query; SWE accepts TV with both external IDs and an empty selection; MED accepts movie creation without its body and issue creation without its body.

The schemas deliberately retain runtime checks for semantics JSON Schema cannot establish: upstream identity, permissions, available profiles, provider-ID provenance, duplicate season-number keys, no-op patches, array identity preservation and request outcome reconciliation.

Source defects and semantic gaps

Gap Evidence Required decision / verification
Authentication model JWT-primary per 01-authentication.md; ApiKey scheme is global and Token routes also appear beneath it Login bypasses securedBy remains Verify; Bearer acceptance on all routes remains Verify; api_key mode available via OMBI_AUTH_MODE
API-key principal Security scheme only names a header Determine effective user, quotas, on-behalf rights and required permissions; do not infer admin or anonymous identity
Enum labels RequestType (T1), IssueStatus (T2), NotificationAgent (T3), NotificationType (T4) label maps published (routing rule #4); VoteType, RequestSource, RequestLimitType and legacy orderType/statusType/availabilityType filters remain numeric-only gaps Per-instance verify published maps for version drift and closed→3 (not frontend-confirmed); verify symbolic names for the remaining enums from authoritative controller/enum source; preserve raw *_code codes meanwhile
TV request identity v1 request body has tvDbId; v2 has theMovieDbId Keep provider-specific create branches; never substitute one ID namespace for another
TV search/details id provenance theMovieDbId carries a different namespace per origin route: TVMaze-backed v1 (Search/tv/{term}, Search/tv/info/{tvdbId}) places the TVDB id there and the TVMaze id in seriesId; v2 routes are TMDB-keyed. The v2 Search/tv/{tvdbId} route is a TMDB alias. v2 seriesId echoes the TMDB id. Browse/collection members often omit theMovieDbId and only populate id. Label theMovieDbId (and, when absent, id) per origin (tvdb on v1 TV, tmdb on v2). Emit seriesId as tvmaze only on v1 TVMaze-backed routes. Live-confirm the v1 tv/info/{tvdbId} route shape, the RecentlyAdded TV id namespace, the multi-search TV id namespace, and the by_request externalProviderId namespace
TV result granularity v2 TV list wraps ChildRequests; v1 wraps TvRequests Preserve target kind and parent ID; no silent fallback between units
TV moderation/options/subscriptions/details Several models/routes only say id or requestId Verify each controller's accepted parent/child namespace, independently per operation
On behalf requestOnBehalf is just string Confirm ID versus username and permissions; resolve the public user ID internally if necessary
Whole-season semantics Season/episode properties optional; no empty-list contract season mode exists: adapter expands season_numbers[] to explicit seasons[].episodes as the default construction; whether upstream episodes:[] means whole season remains Verify — never infer empty means all
Request-type filtering Legacy order/status/availability parameters are unlabeled integers Establish finite maps before any legacy filtered adapter; reject unsupported combinations
Path/parameter mismatches IMDb imdbid/imdbId; TV tvdbId/tvdbid; streaming movieDbId/movieDBId Substitute literal path placeholders while preserving parameter meaning
Legacy TV filter mismatch Path names statusFilterType/availabilityFilterType plus separate statusType/availabilityType parameters Verify the actual wire behaviour; do not assume these duplicate-looking parameters are interchangeable
Folded type declarations Some descriptions contain text such as type: integer instead of a YAML type field Treat human text as evidence of intent, not valid machine typing; adapter/public schema must state its choice
Radarr tags POST Body is declared SonarrSettings Prefer existing GET; verify the POST rather than silently correcting RAML
Response omissions Images, user-country list, provider issues, status info, server logs and some integration methods omit schemas Inspect actual supported-version output and use an allowlisted projection; unknown structures fail closed
Integration side effects Some POST credential/setup endpoints lack descriptions Keep outside read tools until behaviour is established; no claim that every POST lookup is harmless
Collection creation No body, single RequestEngineResult response No unsupported modifiers; verify partial success/retry semantics and do not fabricate per-item results
Refresh model token/userename, no refresh-token field Verify typo and semantics; no invented refresh token
Pagination Ambiguous prose and plain arrays mixed with paged wrappers Validate units/offset semantics/total fields per route; no invented totals
Update and test results Boolean, model and unspecified responses differ Interpret per operation, not one universal tester or mutation shape
Request retry GET queue and DELETE entry only Reprocess existing request via v2 when supported; no synthetic POST queue route
Settings replacement POST section bodies, no documented PATCH/ETag Private merge + revision check; report residual race with external writers
Deployment paths RAML contains a fixed baseUri Never use it as a distributable default or public fixture; preserve configured reverse-proxy path prefixes

Acceptance criteria for an implementation

These are future checks, not claims that an implementation was written or tested here.

Schema and routing

  • Validate every published schema with JSON Schema 2020-12 and exercise each branch with both a valid object and common invalid combinations.
  • An empty object must not satisfy a request-creation, issue-creation, moderation or delete schema. Optional defaults must not trigger unrelated conditional requirements.
  • Movie, TVDB TV, TMDB TV, album and collection creation must send the exact wire body and method. TV has no is4kRequest, album has no requestOnBehalf, and collection has no invented body.
  • Denial uses PUT for all media. Similar and actor searches use POST. Lidarr Metadata uses POST. User detail uses Identity/User/{id}. No POST RequestRetry is emitted.
  • Request list rejects album+unavailable, preserves TV child identity, maps request_date to RequestedDate and verifies local versus upstream pagination metadata.
  • Reject empty explicit episode lists, duplicate seasons/episodes, ambiguous providers, irrelevant action properties and overflowing request budgets before upstream calls.
  • Check every method/path pair against the ledger, including spelling/case and request/response types. Generated brace expansion must never add routes.
  • Omitted is_4k/status/sort_direction resolve to documented defaults without triggering conditional requirements.
  • season_numbers requests produce explicit seasons[].episodes wire bodies.
  • Renamed catalogue matches 31 names; no ombi_-prefixed tool advertised.
  • JWT startup path exercised; api_key mode sends UserName only when configured.

Error and output behaviour

  • Handle HTTP 200 business failures, Boolean tester failures, empty-success bodies, missing optional fields and unknown response structures independently.
  • Confirm structuredContent validates, text fallback contains the same bounded projection and MCP isError agrees with ok.
  • Do not infer success from result ID presence or a timeout; expose UNKNOWN_OUTCOME where execution may already have occurred.
  • Verify safe errors for upstream 401/403/404/429/5xx, network errors, invalid JSON and malformed/oversized bodies.
  • Check list pages with zero items, unknown totals, truncated arrays and concurrent insertion/deletion. Do not offer a false continuation offset.
  • Test nested secrets in requests, issues, stats, user records and error bodies, not just settings.

Authorization and effects

  • Apply core/moderation/administration and branch policy both at tools/list and call time. Annotation values cannot grant permissions.
  • Confirm the effective upstream principal and another-user access explicitly. API key mode must not accidentally promise per-client user isolation.
  • Keep one shared HTTP client, encode path/query values, preserve configured prefixes and suppress cross-origin credential forwarding.
  • Ensure authentication recovery cannot replay ambiguous writes; capability probes must never trigger jobs or writes.
  • Preserve settings secrets and omitted values, reject stale revisions, reject unsafe array replacements and disclose external-writer race limits.
  • Verify email recipients, welcome-email target, collection scope, TV parent-delete effects and each enabled job's scope before mutation.
  • Never expose credential acquisition, key rotation, arbitrary HTTP, raw entity replacement or raw logs through an accidental fallback branch.

Review examples

These are illustrative arguments with placeholder IDs, not commands executed against a server. Actual IDs and revision/profile references must come from authorized reads. Numeric examples do not assert real media identities. The TV moderation example requires a verified adapter that establishes ID 456 as a child request.

Tool: read_search.

{
  "action": "multi",
  "query": "Example title",
  "include": [
    "movies",
    "tv_shows"
  ]
}

TMDB TV details

Tool: read_media.

{
  "action": "details",
  "target": {
    "media": "tv",
    "provider": "tmdb",
    "id": 123
  }
}

Explicit TV episode request

Tool: write_request_create.

{
  "action": "tv",
  "provider": "tmdb",
  "id": 123,
  "selection": {
    "mode": "episodes",
    "seasons": [
      {
        "season_number": 1,
        "episodes": [
          1,
          2
        ]
      }
    ]
  }
}

Whole-season TV request

Tool: write_request_create.

{
  "action": "tv",
  "provider": "tmdb",
  "id": 123,
  "selection": {
    "mode": "season",
    "season_numbers": [
      1,
      2
    ]
  }
}

Minimal request list

Tool: read_requests.

{
  "action": "list"
}

TV child moderation

Tool: write_request_moderate.

{
  "action": "approve",
  "media": "tv",
  "request_id": 456
}

Movie 4K denial

Tool: write_request_moderate.

{
  "action": "deny",
  "media": "movie",
  "request_id": 789,
  "is_4k": true,
  "reason": "Not currently accepting 4K requests."
}

Album request

Tool: write_request_create.

{
  "action": "album",
  "musicbrainz_id": "example-release-group-id"
}

Issue comment

Tool: write_issue_comment.

{
  "issue_id": 123,
  "comment": "Playback fails at the same point on a second device."
}

Change a setting

Tool: write_settings_patch.

{
  "action": "patch",
  "section": "ombi",
  "revision": "example-revision-from-settings-read",
  "changes": {
    "hideRequestsUsers": true
  }
}

Saved notification test

Tool: write_integration_test.

{
  "service": "discord",
  "profile_id": "example-authorized-saved-profile"
}

Primary references

The protocol baseline is intentionally pinned; consult the applicable version when choosing a newer transport implementation. RAML corrections or live-server observations should be recorded as versioned adapter evidence rather than silently editing the meaning of this snapshot.

Phase 08 integration verification (2026-09-18)

This section records the runtime verification of the compiled server, performed after Phases 00–07. It complements — does not replace — the design-time checks above.

Harness

internal/integration_test/ (whole package behind //go:build integration; standard go test ./... never compiles it). Run with go test -tags integration ./internal/integration_test/. The suite spawns the compiled binary as a subprocess and speaks raw newline-delimited JSON-RPC 2.0 over stdio (initialize → notifications/initialized → tools/list/tools/call), then decodes the structuredContent envelope. Two upstream targets:

  • Mock Ombi (httptest): enforces both auth modes, records every upstream call for wire-shape assertions, serves realistic fixtures deliberately laced with undocumented fields. Always runs.
  • Live Ombi: the same style of tests gated on OMBI_URL/OMBI_AUTH_MODE/OMBI_USERNAME/OMBI_PASSWORD/OMBI_API_KEY/OMBI_USER_NAME from the parent env (a sourced .env). Skips cleanly when unset. Pending credentials — see "Remaining live Verify items".

Results: mock suite — 32/32 pass

Contract area Evidence
Protocol surface initialize returns serverInfo.name=ombi-mcp; tools/list advertises exactly 31 tools (all bundles) / 18 (core only); a disabled-bundle tool is refused at call time, not just hidden from the list
Read/projection contract read_media (movie + TV details), read_requests (movie/TV lists), read_issues project correctly despite fixtures carrying ~10 undocumented fields each (nested objects, arrays, scalars); allowlist projection confirmed — nothing undocumented passes through; isError mirrors !ok; text content equals the structured envelope
Enum translation (T1–T4) status:1→in_progress, requestType:1→movie twins emitted with *_code preserved; unmapped status:99 keeps code, empty label, and raises a warnings[] entry. Notification template patch translates notification_type/agent labels back to wire ints (T4 issue→1, T3 discord→1); snake_case keys never reach the wire
Expansion logic write_request_create season mode: exactly one private GET /api/v2/Search/tv/moviedb/{id}, exactly one POST /api/v2/Requests/tv; posted seasons[].episodes equals the fixture's explicit episode lists (S1:7 eps, S2:4 eps); requestAll/firstSeason/latestSeason stay false. Flag modes (all/first_season/latest_season) set the matching wire flag with no private GET. episodes mode posts explicit picks to the v1 TVDB route with no details GET. Unknown season number → INVALID_ARGUMENT before any POST
Patch cycle read_settings omits secret/excluded fields into omitted_fields only (values never projected) and emits a revision for writable sections only. write_settings_patch GETs before POSTing, SHA256 revision lock holds (stale → CONFLICT, no POST), null/excluded-field changes → INVALID_ARGUMENT before upstream traffic, no-op patch → INVALID_ARGUMENT, nested objects deep-merge (customPage.content preserved when customPage.enabled patched), secret fields preserved byte-for-byte in the POSTed document
Error boundaries 401 → AUTHENTICATION_FAILED (bad ApiKey; rejected JWT login), 404 → NOT_FOUND (JSON and HTML bodies), 429 → RATE_LIMITED with retry_after_seconds parsed from Retry-After, 500 → retryable UPSTREAM_REJECTED, malformed upstream JSON → UPSTREAM_SCHEMA_MISMATCH, HTTP 200 with isError:true → UPSTREAM_REJECTED with sanitized message, transport failure → retryable UPSTREAM_REJECTED (read) / non-retryable UNKNOWN_OUTCOME (write). Every result is a mapped ToolError envelope; leak scan over the entire raw result confirms no Bearer/Authorization/ApiKey, no HTML, no stack traces — including when the upstream body actively contains them
Auth lifecycle JWT login → cached token reuse → 401 triggers invalidate + single re-login + one retry (verified by rotating the accepted token). api_key mode sends ApiKey and never Authorization, never hits /api/v1/Token
Argument validation {} rejected by write tools; unknown fields rejected (additionalProperties:false semantics); album+unavailable combination rejected — all before upstream calls

Live observations (unauthenticated/invalid-credential probes)

  • GET /api/v1/Status returns HTTP 200 with body 200 and no credentials — the scalar-decode path handles it.
  • GET /api/v1/Settings/clientid and /api/v1/Settings/baseurl are open unauthenticated.
  • Quirk: sending an invalid ApiKey header makes even nominally-open routes return 401 — Ombi validates presented credentials regardless of endpoint auth requirements. Through the server this correctly surfaces as AUTHENTICATION_FAILED (verified against the real instance: TestLiveBadCredentials PASS).
  • POSTing to a dead upstream confirmed read-vs-write error asymmetry (UPSTREAM_REJECTED vs UNKNOWN_OUTCOME).

Handler changes required

None. No discrepancies between the RAML-derived contracts and exercised behaviour were found; no input/output schema change was needed. All defects found during the phase were in the test harness itself (mock body-drain on record, mock token recomputation), fixed there.

Remaining live Verify items (pending credentials)

The gated live suite is committed and ready; run it with a sourced .env. It additionally performs the only state-mutating checks — write_request_create season expansion against a real show (then deletes the created parent) and a self-restoring write_settings_patch cycle — plus read_requests/read_issues/read_media on real payloads and real JWT renewal. Live 429 remains unverifiable by nature (cannot force upstream rate limiting); mock coverage stands in.

Constraint check

  • Input/output schema contracts in docs/schema/ unchanged.
  • go test ./... output is identical to before the phase (package fully behind the tag).
  • No instance URL in committed files; .env guidance points to the gitignored file only.

M5 Findings — Request state and list routes (#7, #10)

  • #7: GET /api/v1/Search/tv/info/{tvdbId} (TVMaze-backed info route) upstream does not set request state flags accurately, returning requested: false regardless of truth. The adapter patches this by performing a bounded GET /api/v1/Request/tv parent scan and overlaying per-episode availability and request states.
  • #10: GET /api/v2/Requests/{movie,tv,album}/... (v2 lists) 500'd on every non-empty page because the adapter sent the RAML example sort field requestDate. Ombi resolves {sort} through TypeDescriptor.GetProperties(...).Find(sortProperty, true) against RequestedDate; a miss leaves prop null and prop.GetValue(x) throws NullReferenceException. Empty pending pages never called GetValue, which is why they appeared to work. The adapter now sends RequestedDate.
  • #10 search fallback: GET /api/v1/Request/tv/search/{term} on 4.53.10 suffers from a LINQ translation bug (upstream Ombi-app/Ombi#5420, fixed by #5421). The adapter gracefully falls back to a bounded v1 parent scan filtering locally on the term if the primary search route fails.

M6 Findings — Integration options and server-id discovery (#16, #17, #18, #20)

  • #20: refOf reference projections defaulted value to the first scalar map entry (firstScalar), which produced nondeterministic results depending on Go map iteration order (e.g. leaking logo paths, booleans, or unrelated weights). The contract is now deterministic: value holds the native-typed identifier matching the resolved ID key, falling back to name string if no ID matched, or omitted otherwise.
  • #20 mojibake handling: language endpoints on Ombi 4.53.10 occasionally return corrupted strings like "??????" or containing \uFFFD. refOf skips corrupted candidates in favor of clean alternates (e.g. name instead of corrupted english_name); if all candidates are corrupted, the string is emitted and a degradation note is recorded in warnings[].
  • #17: Root-folder records (/api/v1/{Sonarr,Radarr,Lidarr}/RootFolders) have no name property on the wire (only path). Category key table refKeySet now maps path to name and preserves id in value (e.g. {id: "21", name: "/media/tv", value: 21}), directly usable as root_folder_id in write operations.
  • #16: GET /api/v1/Plex/servers returns an object wrapper {"success": true, "servers": [...]} rather than a top-level array. refsOrUsers now inspects wrappers, verifies success/failure flags (mapping success: false to UPSTREAM_REJECTED with sanitized upstream messages), and decodes server entries with id=machineId, name=serverName, value=serverId.
  • #16 sibling decode fixes: refsOrScalar was previously intercepting any valid JSON object in its decodeScalar branch and emitting a junk {name: "<cat>"} record, leaving nested-container extraction dead code. Reordering decode passes (refArray → decodeObject → decodeScalar) fixes plex_libraries, media_server info, and media_server libraries.
  • #18: Saved-server identity discovery: flattenSettings previously excluded id, serverId, and machineIdentifier everywhere, making it impossible to discover server IDs for read_integration media_server and plex_libraries. A scoped read-exemption allows server identity leaves under /servers/<digits>/ to appear in read_settings values (top-level section IDs and credentials remain omitted; patches to server identity fields remain rejected with INVALID_ARGUMENT).

Residual Verify items

  • Verify PlexServersAddUserModel and older Ombi instances where servers may be returned as a bare array (now tolerated alongside object wrappers).
  • Verify Emby selectedLibraries[].key ↔ MediaFolders id correspondence across diverse Emby/Jellyfin setups.

M7 Findings — Admin/server wire contracts (#19, #13, #14, #15)

  • #19: GET /api/v2/System/logs returns a JSON string array of log file names on Ombi 4.53.10. The adapter accepts that form and the legacy object form, then reads the selected file as plain text.
  • #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.
  • #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.

Read-tools sweep findings (#10 remainder, #27, #28)

  • #10 remainder: the v2 list 500s were not an upstream per-row serializer bug. Ombi looks up {sort} as a C# property name (RequestedDate); requestDate misses, prop is null, and GetValue throws on the first row. Pending pages were empty so they never threw. The adapter now sends RequestedDate.
  • #27: read_discover browse TV popular/most_watched exceeded the 8 MiB read budget because Ombi hydrates seasonRequests for every show when HideAvailableFromDiscover is on. Browse now streams the JSON array, skips seasonRequests while tokenizing, and uses a 64 MiB safety cap.
  • #28: GET /api/v2/Issues/details/{providerId} returns an empty body when the provider has no issues. provider_summary treats empty/null/[] as an empty group_page instead of a decode failure.