ombi-mcp/docs/schema/06-verification.md

# 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 requestDate 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.

### Mixed search

Tool: `read_search`.

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

### TMDB TV details

Tool: `read_media`.

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

### Explicit TV episode request

Tool: `write_request_create`.

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

### Whole-season TV request

Tool: `write_request_create`.

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

### Minimal request list

Tool: `read_requests`.

```json
{
  "action": "list"
}
```

### TV child moderation

Tool: `write_request_moderate`.

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

### Movie 4K denial

Tool: `write_request_moderate`.

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

### Album request

Tool: `write_request_create`.

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

### Issue comment

Tool: `write_issue_comment`.

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

### Change a setting

Tool: `write_settings_patch`.

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

### Saved notification test

Tool: `write_integration_test`.

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

## Primary references

- [Local RAML API](../api/raml/api.raml) and [API-key security scheme](../api/raml/securitySchemes/api_key.raml) define the upstream evidence boundary.
- [MCP tools, 2025-11-25](https://modelcontextprotocol.io/specification/2025-11-25/server/tools) defines registration and tool result behaviour.
- [MCP schema reference](https://modelcontextprotocol.io/specification/2025-11-25/schema) defines annotation and message types.
- [MCP authorization](https://modelcontextprotocol.io/specification/2025-11-25/basic/authorization) distinguishes transport authorization from upstream credentials.
- [MCP resources](https://modelcontextprotocol.io/specification/2025-11-25/server/resources) supports optional bounded reference/artwork resources.

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).
- `POST`ing 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) encounter a per-row `NullReferenceException` on non-empty results (observed on Ombi 4.53.10). The adapter surfaces this as `UPSTREAM_REJECTED` rather than silently replacing it with v1 `tvlite` payloads, preserving the expected v2 child-page contract. 
- `#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.