From 6926a91c80a3937749397fb9eead24e95d2b8745 Mon Sep 17 00:00:00 2001 From: Gronod Date: Sat, 19 Sep 2026 18:36:03 +0100 Subject: [PATCH] Fix M7 admin/server wire contracts --- README.md | 2 +- docs/schema/02-tool-mapping.md | 4 +- docs/schema/03-input-schemas.md | 17 +-- docs/schema/05-endpoint-coverage.md | 10 +- docs/schema/06-verification.md | 7 ++ internal/integration_test/contract_test.go | 137 +++++++++++++++++++++ internal/integration_test/live_test.go | 44 +++++++ internal/integration_test/mockombi_test.go | 35 ++++++ internal/tools/args_rest.go | 2 +- internal/tools/logs.go | 50 ++++++-- internal/tools/registry.go | 2 +- internal/tools/schemas.json | 17 +-- internal/tools/server.go | 48 +++----- 13 files changed, 296 insertions(+), 79 deletions(-) diff --git a/README.md b/README.md index 8c4e2c0..319db4a 100644 --- a/README.md +++ b/README.md @@ -115,7 +115,7 @@ Tools are grouped into **bundles** — deployment policy groups, not permission | `read_votes` | Global vote list or votes on a request. | | `read_users` | Self, authorized user lookup, claims, online users, preference read. | | `read_library` | Recent additions, calendar, artwork. | -| `read_server` | Server status, version, features, news, stats, cron validation. | +| `read_server` | Server status, version, features, stats, cron validation. | | `read_integration` | Saved ARR options and authorized media-server metadata. | | `write_request_create` | Create one media request or an explicit collection request. | | `write_request_subscribe` | Subscribe/unsubscribe to a request. | diff --git a/docs/schema/02-tool-mapping.md b/docs/schema/02-tool-mapping.md index d5f1451..61e606e 100644 --- a/docs/schema/02-tool-mapping.md +++ b/docs/schema/02-tool-mapping.md @@ -22,7 +22,7 @@ The [input catalogue](03-input-schemas.md) defines every parameter, required fie | `read_votes` | Global vote list or votes on a request | core | vote page | | `read_users` | Self, authorized user lookup, claims, online users, preference read | core | user/reference page | | `read_library` | Recent additions, calendar, artwork | core | media/calendar/artwork page | -| `read_server` | Status, version, features, news, stats, cron validation | core | metrics/reference page | +| `read_server` | Status, version, features, stats, cron validation | core | metrics/reference page | | `read_integration` | Saved ARR options and authorized media-server metadata | core | reference/user page | | `write_request_create` | One media request or explicit collection request | core | mutation | | `write_request_subscribe` | Subscribe/unsubscribe | core | mutation | @@ -173,7 +173,7 @@ Recent TV grouped and ungrouped routes are separate branches. Calendar has no do Images may be binary, redirects or URLs depending on the route; RAML leaves several response bodies unspecified. Return a safe resource reference only after validating actual content type/shape and destination. Do not invent a signed URL or expose an `ApiKey` query. Random backgrounds are read-only but their outputs are not deterministic; idempotent read annotations describe side effects, not identical results. -Stats passes optional `from` and `to` query values. `update_check` is GET Job/update; `update_info` is GET Update. Running updates is an explicit administrative job. `cron_validate` POSTs `{expression}` to Settings/testcron and is an administrator-gated read calculation, not a scheduled-job mutation. +Stats requires RFC3339 `from` and `to` query values. `update_check` is GET Job/update; `update_info` is GET Update. Running updates is an explicit administrative job. `cron_validate` POSTs `{expression}` to Settings/testcron and is an administrator-gated read calculation, not a scheduled-job mutation. Ombi uses Quartz cron syntax: six or seven fields, with `?` in the unused day-of-month or day-of-week field. `read_integration` prefers saved-settings GETs. Radarr 4K only applies to profiles/root folders, not tags. Sonarr language profiles uses `/v3/LanguageProfiles`. Lidarr Metadata is POST-only, so load saved Lidarr settings privately and construct its request internally. CouchPotato profile is singular and POST-only. Credential acquisition `/CouchPotato/apikey` is never a tool. RPC POST counterparts accepting settings are compatibility adapters to the same read intent; they do not add caller connection overrides. diff --git a/docs/schema/03-input-schemas.md b/docs/schema/03-input-schemas.md index 3f4c5bb..6bc15b7 100644 --- a/docs/schema/03-input-schemas.md +++ b/docs/schema/03-input-schemas.md @@ -1494,18 +1494,6 @@ Read server status, feature availability, version, update information or usage s ], "additionalProperties": false }, - { - "type": "object", - "properties": { - "action": { - "const": "news" - } - }, - "required": [ - "action" - ], - "additionalProperties": false - }, { "type": "object", "properties": { @@ -1546,7 +1534,9 @@ Read server status, feature availability, version, update information or usage s } }, "required": [ - "action" + "action", + "from", + "to" ], "additionalProperties": false }, @@ -1558,6 +1548,7 @@ Read server status, feature availability, version, update information or usage s }, "expression": { "type": "string", + "description": "Quartz cron expression — 6 or 7 fields (seconds minutes hours day-of-month month day-of-week [year]); one of the two day fields must be ?", "minLength": 1, "maxLength": 200 } diff --git a/docs/schema/05-endpoint-coverage.md b/docs/schema/05-endpoint-coverage.md index 1db7b6b..92d1a7c 100644 --- a/docs/schema/05-endpoint-coverage.md +++ b/docs/schema/05-endpoint-coverage.md @@ -298,7 +298,7 @@ The body/response columns describe the **upstream** schema, not a promise to pas | 280 | [GET `/api/v1/Settings/SickRage`](../api/raml/api.raml#L5100) | D | `read_settings / sickrage` | none documented | 200: [SickRageSettings](../api/raml/types/Ombi.Settings.Settings.Models.External.SickRageSettings.raml) | Allowlisted projection; no raw secrets. | | 281 | [GET `/api/v1/Settings/jobs`](../api/raml/api.raml#L5130) | D | `read_settings / jobs` | none documented | 200: [JobSettings](../api/raml/types/Ombi.Settings.Settings.Models.JobSettings.raml) | Allowlisted projection; no raw secrets. | | 282 | [POST `/api/v1/Settings/jobs`](../api/raml/api.raml#L5130) | P | `write_settings_patch / patch:jobs` | body [JobSettings](../api/raml/types/Ombi.Settings.Settings.Models.JobSettings.raml) | 200: [JobSettingsViewModel](../api/raml/types/Ombi.Models.JobSettingsViewModel.raml) | Only typed non-secret patch; preserve omitted/private fields. | -| 283 | [POST `/api/v1/Settings/testcron`](../api/raml/api.raml#L5160) | D | `read_server / cron_validate` | body [CronViewModelBody](../api/raml/types/Ombi.Models.CronViewModelBody.raml) | 200: [CronTestModel](../api/raml/types/Ombi.Models.CronTestModel.raml) | Administrator-gated read calculation. | +| 283 | [POST `/api/v1/Settings/testcron`](../api/raml/api.raml#L5160) | D | `read_server / cron_validate` | body [CronViewModelBody](../api/raml/types/Ombi.Models.CronViewModelBody.raml) | 200: [CronTestModel](../api/raml/types/Ombi.Models.CronTestModel.raml) | Administrator-gated read calculation; Quartz uses 6-7 fields and `?` for the unused day field. | | 284 | [POST `/api/v1/Settings/Issues`](../api/raml/api.raml#L5178) | P | `write_settings_patch / patch:issues` | body [IssueSettings](../api/raml/types/Ombi.Settings.Settings.Models.IssueSettings.raml) | 200: boolean | Only typed non-secret patch; preserve omitted/private fields. | | 285 | [GET `/api/v1/Settings/Issues`](../api/raml/api.raml#L5178) | D | `read_settings / issues` | none documented | 200: [IssueSettings](../api/raml/types/Ombi.Settings.Settings.Models.IssueSettings.raml) | Allowlisted projection; no raw secrets. | | 286 | [GET `/api/v1/Settings/issuesenabled`](../api/raml/api.raml#L5208) | D | `read_settings / issuesenabled` | none documented | 200: boolean | Allowlisted projection; no raw secrets. | @@ -344,12 +344,12 @@ The body/response columns describe the **upstream** schema, not a promise to pas | 326 | [GET `/api/v1/Sonarr/tags`](../api/raml/api.raml#L5783) | D | `read_integration / options:sonarr/tags` | none documented | 200: array<[Tag](../api/raml/types/Ombi.Api.External.ExternalApis.Sonarr.Models.Tag.raml)> | — | | 327 | [GET `/api/v1/Sonarr/enabled`](../api/raml/api.raml#L5815) | D | `read_integration / options:sonarr/enabled` | none documented | 200: boolean | — | | 328 | [GET `/api/v1/Sonarr/version`](../api/raml/api.raml#L5824) | D | `read_integration / options:sonarr/version` | none documented | 200: string | — | -| 329 | [GET `/api/v1/Stats`](../api/raml/api.raml#L5833) | D | `read_server / stats` | query `from`:string optional; query `to`:string optional | 200: [UserStatsSummary](../api/raml/types/Ombi.Core.Engine.UserStatsSummary.raml) | from/to query parameters are supported. | +| 329 | [GET `/api/v1/Stats`](../api/raml/api.raml#L5833) | D | `read_server / stats` | query `from`:string required; query `to`:string required | 200: [UserStatsSummary](../api/raml/types/Ombi.Core.Engine.UserStatsSummary.raml) | from/to are required at the MCP layer; bare calls cause an upstream NRE. | | 330 | [GET `/api/v1/Status`](../api/raml/api.raml#L5849) | D | `read_server / status` | none documented | 200: [HttpStatusCode](../api/raml/types/System.Net.HttpStatusCode.raml) | — | | 331 | [GET `/api/v1/Status/info`](../api/raml/api.raml#L5860) | D | `read_server / status_info` | none documented | 200: string | — | -| 332 | [GET `/api/v2/System/news`](../api/raml/api.raml#L5871) | D | `read_server / news` | none documented | 200: body unspecified | — | -| 333 | [GET `/api/v2/System/logs`](../api/raml/api.raml#L5877) | D | `read_logs / list` | none documented | 200: body unspecified | — | -| 334 | [GET `/api/v2/System/logs/{logFileName}`](../api/raml/api.raml#L5883) | D | `read_logs / read` | path `logFileName`:string required | 200: body unspecified | Vetted opaque ID maps to filename; sanitized bounded local slicing. | +| 332 | [GET `/api/v2/System/news`](../api/raml/api.raml#L5871) | X | action removed | none documented | 200: Markdig HTML text | Not structured data; action removed (#13). | +| 333 | [GET `/api/v2/System/logs`](../api/raml/api.raml#L5877) | D | `read_logs / list` | none documented | 200: string array of file names | Verified on 4.53.10. | +| 334 | [GET `/api/v2/System/logs/{logFileName}`](../api/raml/api.raml#L5883) | D | `read_logs / read` | path `logFileName`:string required | 200: plain text | Vetted opaque ID maps to filename; sanitized bounded local slicing (verified 4.53.10). | | 335 | [GET `/api/v2/System/logs/download/{logFileName}`](../api/raml/api.raml#L5893) | X | `raw diagnostic download` | path `logFileName`:string required | 200: body unspecified | May expose secrets; sanitized logs tool is the supported alternative. | | 336 | [POST `/api/v1/Tester/discord`](../api/raml/api.raml#L5903) | P | `write_integration_test / discord` | body [DiscordNotificationSettings](../api/raml/types/Ombi.Settings.Settings.Models.Notifications.DiscordNotificationSettings.raml) | 200: boolean | Saved authorized profile only; exact tester body differs by service. | | 337 | [POST `/api/v1/Tester/pushbullet`](../api/raml/api.raml#L5923) | P | `write_integration_test / pushbullet` | body [PushbulletSettings](../api/raml/types/Ombi.Settings.Settings.Models.Notifications.PushbulletSettings.raml) | 200: boolean | Saved authorized profile only; exact tester body differs by service. | diff --git a/docs/schema/06-verification.md b/docs/schema/06-verification.md index 3efa99d..20bf2cf 100644 --- a/docs/schema/06-verification.md +++ b/docs/schema/06-verification.md @@ -317,3 +317,10 @@ The gated live suite is committed and ready; run it with a sourced `.env`. It ad ### 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. diff --git a/internal/integration_test/contract_test.go b/internal/integration_test/contract_test.go index a228337..6eacf39 100644 --- a/internal/integration_test/contract_test.go +++ b/internal/integration_test/contract_test.go @@ -3,11 +3,18 @@ package integration_test import ( + "crypto/sha256" + "encoding/hex" "encoding/json" "strings" "testing" ) +func testLogFileID(name string) string { + sum := sha256.Sum256([]byte(name)) + return hex.EncodeToString(sum[:16]) +} + // --- protocol surface --- func TestHandshakeAndToolList(t *testing.T) { @@ -36,6 +43,136 @@ func TestHandshakeAndToolList(t *testing.T) { } } +func TestReadLogsStringArrayList(t *testing.T) { + mock := newMockOmbi(t, "jwt") + c := spawnServer(t, mock.env()) + c.handshake(t) + + out := c.callTool(t, "read_logs", map[string]any{"action": "list"}) + data := requireOK(t, out) + var logs struct { + Kind string `json:"kind"` + Files []struct { + FileID string `json:"file_id"` + Name string `json:"name"` + } `json:"files"` + } + if err := json.Unmarshal(data, &logs); err != nil { + t.Fatalf("logs decode: %v", err) + } + if logs.Kind != "logs" || len(logs.Files) != 2 { + t.Fatalf("unexpected log list: %s", data) + } + for _, f := range logs.Files { + if f.FileID != testLogFileID(f.Name) { + t.Errorf("file ID for %q = %q", f.Name, f.FileID) + } + } +} + +func TestReadLogsReadByFileID(t *testing.T) { + mock := newMockOmbi(t, "jwt") + c := spawnServer(t, mock.env()) + c.handshake(t) + + out := c.callTool(t, "read_logs", map[string]any{ + "action": "read", "file_id": testLogFileID("ombi-20260919.txt"), "limit": 2, + }) + data := requireOK(t, out) + var logs struct { + Kind string `json:"kind"` + Lines []string `json:"lines"` + NextOffset *int `json:"next_offset"` + } + if err := json.Unmarshal(data, &logs); err != nil { + t.Fatalf("logs decode: %v", err) + } + if logs.Kind != "logs" || len(logs.Lines) != 2 || logs.NextOffset != nil { + t.Fatalf("unexpected log read: %s", data) + } + bad := c.callTool(t, "read_logs", map[string]any{ + "action": "read", "file_id": "does-not-exist", + }) + requireErr(t, bad, "NOT_FOUND") +} + +func TestReadServerNewsRemoved(t *testing.T) { + mock := newMockOmbi(t, "jwt") + c := spawnServer(t, mock.env()) + c.handshake(t) + requireErr(t, c.callTool(t, "read_server", map[string]any{"action": "news"}), "INVALID_ARGUMENT") +} + +func TestReadServerStatsRequiresRange(t *testing.T) { + mock := newMockOmbi(t, "jwt") + c := spawnServer(t, mock.env()) + c.handshake(t) + + err := requireErr(t, c.callTool(t, "read_server", map[string]any{"action": "stats"}), "INVALID_ARGUMENT") + if err.Field != "from" || mock.countCalls("GET", "/api/v1/Stats") != 0 { + t.Fatalf("bare stats error/calls = %+v/%d", err, mock.countCalls("GET", "/api/v1/Stats")) + } + err = requireErr(t, c.callTool(t, "read_server", map[string]any{ + "action": "stats", "from": "2026-09-01T00:00:00Z", + }), "INVALID_ARGUMENT") + if err.Field != "to" { + t.Errorf("field = %q, want to", err.Field) + } + requireErr(t, c.callTool(t, "read_server", map[string]any{ + "action": "stats", "from": "2026-09-02T00:00:00Z", "to": "2026-09-01T00:00:00Z", + }), "INVALID_ARGUMENT") + + out := c.callTool(t, "read_server", map[string]any{ + "action": "stats", "from": "2026-09-01T00:00:00Z", "to": "2026-09-02T00:00:00Z", + }) + data := requireOK(t, out) + var metrics struct { + Kind string `json:"kind"` + Values []any `json:"values"` + } + if err := json.Unmarshal(data, &metrics); err != nil { + t.Fatalf("metrics decode: %v", err) + } + if metrics.Kind != "metrics" || len(metrics.Values) != 7 { + t.Fatalf("unexpected stats: %s", data) + } +} + +func TestReadServerCronValidateQuartz(t *testing.T) { + mock := newMockOmbi(t, "jwt") + c := spawnServer(t, mock.env()) + c.handshake(t) + + out := c.callTool(t, "read_server", map[string]any{ + "action": "cron_validate", "expression": "0 0 0 * * ?", + }) + data := requireOK(t, out) + var metrics struct { + Values []struct { + Name string `json:"name"` + Value any `json:"value"` + } `json:"values"` + } + if err := json.Unmarshal(data, &metrics); err != nil || len(metrics.Values) != 1 || + metrics.Values[0].Name != "valid" || metrics.Values[0].Value != true { + t.Fatalf("Quartz-valid expression reported invalid: %s", data) + } + if got := mock.lastBody(t, "POST", "/api/v1/Settings/testcron")["expression"]; got != "0 0 0 * * ?" { + t.Errorf("wire expression = %#v", got) + } + + out = c.callTool(t, "read_server", map[string]any{ + "action": "cron_validate", "expression": "0 0 * * *", + }) + data = requireOK(t, out) + if err := json.Unmarshal(data, &metrics); err != nil || len(metrics.Values) < 1 || metrics.Values[0].Value != false { + t.Fatalf("Quartz-invalid expression reported valid: %s", data) + } + if !strings.Contains(strings.Join(out.Envelope.Warnings, " "), "Quartz cron syntax") { + t.Errorf("missing Quartz warning: %v", out.Envelope.Warnings) + } +} + func TestBundleRestriction(t *testing.T) { mock := newMockOmbi(t, "jwt") env := mock.env() diff --git a/internal/integration_test/live_test.go b/internal/integration_test/live_test.go index 2e0dace..f55cd67 100644 --- a/internal/integration_test/live_test.go +++ b/internal/integration_test/live_test.go @@ -71,6 +71,50 @@ func TestLiveServerStatus(t *testing.T) { assertNoLeak(t, out.Raw) } +func TestLiveServerStats(t *testing.T) { + c := liveServer(t) + out := c.callTool(t, "read_server", map[string]any{ + "action": "stats", "from": "2026-09-01T00:00:00Z", "to": "2026-09-02T00:00:00Z", + }) + data := requireOK(t, out) + var metrics struct { + Kind string `json:"kind"` + } + if err := json.Unmarshal(data, &metrics); err != nil || metrics.Kind != "metrics" { + t.Fatalf("stats result: %v: %s", err, data) + } +} + +func TestLiveLogs(t *testing.T) { + c := liveServer(t) + out := c.callTool(t, "read_logs", map[string]any{"action": "list"}) + data := requireOK(t, out) + var logs struct { + Kind string `json:"kind"` + } + if err := json.Unmarshal(data, &logs); err != nil || logs.Kind != "logs" { + t.Fatalf("logs result: %v: %s", err, data) + } +} + +func TestLiveCronValidateQuartz(t *testing.T) { + c := liveServer(t) + out := c.callTool(t, "read_server", map[string]any{ + "action": "cron_validate", "expression": "0 0 0 * * ?", + }) + data := requireOK(t, out) + if !strings.Contains(string(data), `"name":"valid","value":true`) { + t.Fatalf("Quartz-valid expression reported invalid: %s", data) + } + out = c.callTool(t, "read_server", map[string]any{ + "action": "cron_validate", "expression": "0 0 * * *", + }) + data = requireOK(t, out) + if !strings.Contains(string(data), `"name":"valid","value":false`) { + t.Fatalf("five-field cron reported valid: %s", data) + } +} + // --- read/projection contract against real payloads --- func TestLiveReadRequestsList(t *testing.T) { diff --git a/internal/integration_test/mockombi_test.go b/internal/integration_test/mockombi_test.go index 415ad79..d5cfa5c 100644 --- a/internal/integration_test/mockombi_test.go +++ b/internal/integration_test/mockombi_test.go @@ -62,6 +62,10 @@ func newMockOmbi(t *testing.T, mode string) *mockOmbi { mux.HandleFunc("GET /api/v1/Status", m.wrap(m.fixed(`200`))) 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/Stats", m.wrap(m.stats)) + mux.HandleFunc("POST /api/v1/Settings/testcron", m.wrap(m.testcron)) + mux.HandleFunc("GET /api/v2/System/logs", m.wrap(m.fixed(`["ombi-20260918.txt","ombi-20260919.txt"]`))) + mux.HandleFunc("GET /api/v2/System/logs/{logFileName}", m.wrap(m.logsRead)) 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)) @@ -922,6 +926,37 @@ func (m *mockOmbi) plexLibraries(w http.ResponseWriter, r *http.Request) { w.Write([]byte(`{"successful":true,"data":[{"id":"3","key":"3","type":"show","title":"TV Shows"},{"id":"1","key":"1","type":"movie","title":"Movies"}]}`)) } +func (m *mockOmbi) stats(w http.ResponseWriter, r *http.Request) { + if r.URL.Query().Get("from") == "" || r.URL.Query().Get("to") == "" { + m.jsonErr(w, http.StatusInternalServerError, "Object reference not set to an instance of an object") + return + } + m.fixed(`{"totalRequests":9,"totalMovieRequests":4,"totalTvRequests":5,"totalIssues":0,"completedRequestsMovies":2,"completedRequestsTv":1,"completedRequests":3}`)(w, r) +} + +func (m *mockOmbi) testcron(w http.ResponseWriter, r *http.Request) { + var body struct { + Expression string `json:"expression"` + } + if err := json.NewDecoder(r.Body).Decode(&body); err != nil { + m.jsonErr(w, http.StatusBadRequest, "invalid JSON") + return + } + fields := strings.Fields(body.Expression) + valid := len(fields) >= 6 && len(fields) <= 7 && + (fields[3] == "?") != (fields[5] == "?") + if valid { + m.fixed(`{"success":true}`)(w, r) + return + } + m.fixed(fmt.Sprintf(`{"success":false,"message":%q}`, "CRON Expression "+body.Expression+" is not valid"))(w, r) +} + +func (m *mockOmbi) logsRead(w http.ResponseWriter, r *http.Request) { + w.Header().Set("Content-Type", "text/plain; charset=utf-8") + w.Write([]byte("2026-09-19 10:00:00 [INF] boot\n2026-09-19 10:01:00 [INF] tick\n")) +} + // mockEnv returns the server env pointing at this mock in the // requested mode, plus all three bundles. func (m *mockOmbi) env() map[string]string { diff --git a/internal/tools/args_rest.go b/internal/tools/args_rest.go index ea0b351..323525d 100644 --- a/internal/tools/args_rest.go +++ b/internal/tools/args_rest.go @@ -113,7 +113,7 @@ type LibraryArgs struct { // read_server type ServerArgs struct { - Action string `json:"action"` // status|status_info|about|update_info|update_check|news|landing|features|stats|cron_validate + Action string `json:"action"` // status|status_info|about|update_info|update_check|landing|features|stats|cron_validate From string `json:"from,omitempty"` To string `json:"to,omitempty"` Expression string `json:"expression,omitempty"` diff --git a/internal/tools/logs.go b/internal/tools/logs.go index 7db7907..9be19d5 100644 --- a/internal/tools/logs.go +++ b/internal/tools/logs.go @@ -27,6 +27,34 @@ func logFileName(m map[string]any) string { return jstr(m, "fileName", "filename", "name") } +// logFileNames decodes a listing whose elements are bare file names on Ombi +// 4.53.x. Older deployments that return objects with a name field remain +// supported. Entries with no usable name are ignored. +func (o *op) logFileNames(raw []byte) ([]string, *ToolResult) { + var els []json.RawMessage + if err := json.Unmarshal(raw, &els); err != nil { + return nil, o.fail("UPSTREAM_SCHEMA_MISMATCH", + "upstream response was not a JSON array: "+sanitizeErr(err), false) + } + names := []string{} + for _, el := range els { + var name string + if err := json.Unmarshal(el, &name); err == nil { + if name != "" { + names = append(names, name) + } + continue + } + var record map[string]any + if err := json.Unmarshal(el, &record); err == nil { + if name := logFileName(record); name != "" { + names = append(names, name) + } + } + } + return names, nil +} + // read_logs — list sanitized log file IDs or read a bounded, // sanitized slice of a single log file. func handleLogs(ctx context.Context, env *Env, raw json.RawMessage) *ToolResult { @@ -50,16 +78,10 @@ func (o *op) logsList() *ToolResult { if fail != nil { return fail } - arr, fail := o.decodeArray(raw) + names, fail := o.logFileNames(raw) if fail != nil { return fail } - names := []string{} - for _, m := range arr { - if s := logFileName(m); s != "" { - names = append(names, s) - } - } sort.Strings(names) out := &Logs{Kind: "logs", Files: []LogFile{}} for _, n := range names { @@ -97,13 +119,13 @@ func (o *op) logsRead(a *LogsArgs) *ToolResult { if fail != nil { return fail } - arr, fail := o.decodeArray(raw) + names, fail := o.logFileNames(raw) if fail != nil { return fail } name := "" - for _, m := range arr { - if n := logFileName(m); n != "" && logFileID(n) == a.FileID { + for _, n := range names { + if logFileID(n) == a.FileID { name = n break } @@ -119,11 +141,15 @@ func (o *op) logsRead(a *LogsArgs) *ToolResult { raw = raw[:maxLogBodyBytes] o.truncated = true } - text := sanitizeText(string(raw), maxLogBodyBytes) - lines := strings.Split(text, "\n") + // Sanitize individual lines so control characters and unbounded text do + // not leak while preserving the upstream line boundaries for pagination. + lines := strings.Split(string(raw), "\n") if len(lines) > 0 && lines[len(lines)-1] == "" { lines = lines[:len(lines)-1] } + for i := range lines { + lines[i] = sanitizeText(lines[i], maxLogBodyBytes) + } out := &Logs{Kind: "logs", Lines: []string{}, Offset: &offset} if offset < len(lines) { end := offset + limit diff --git a/internal/tools/registry.go b/internal/tools/registry.go index f45d095..7d8208a 100644 --- a/internal/tools/registry.go +++ b/internal/tools/registry.go @@ -51,7 +51,7 @@ var registry = []ToolDef{ {"read_votes", "core", "Global vote list or votes on a request.", true, false, true, true, []string{"vote_page"}, handleVotes}, {"read_users", "core", "Self, authorized user lookup, claims, online users, preference read.", true, false, true, true, []string{"user_page", "reference_page"}, handleUsers}, {"read_library", "core", "Recent additions, calendar, artwork.", true, false, true, true, []string{"media_page", "calendar_page", "artwork_page"}, handleLibrary}, - {"read_server", "core", "Status, version, features, news, stats, cron validation.", true, false, true, true, []string{"metrics", "reference_page"}, handleServer}, + {"read_server", "core", "Status, version, features, stats, cron validation.", true, false, true, true, []string{"metrics", "reference_page"}, handleServer}, {"read_integration", "core", "Saved ARR options and authorized media-server metadata.", true, false, true, true, []string{"reference_page", "user_page"}, handleIntegration}, {"write_request_create", "core", "One media request or explicit collection request.", false, false, false, true, []string{"mutation"}, handleRequestCreate}, {"write_request_subscribe", "core", "Subscribe/unsubscribe.", false, false, false, true, []string{"mutation"}, handleRequestSubscribe}, diff --git a/internal/tools/schemas.json b/internal/tools/schemas.json index ba7c3f3..3331ffd 100644 --- a/internal/tools/schemas.json +++ b/internal/tools/schemas.json @@ -1796,18 +1796,6 @@ ], "additionalProperties": false }, - { - "type": "object", - "properties": { - "action": { - "const": "news" - } - }, - "required": [ - "action" - ], - "additionalProperties": false - }, { "type": "object", "properties": { @@ -1848,7 +1836,9 @@ } }, "required": [ - "action" + "action", + "from", + "to" ], "additionalProperties": false }, @@ -1860,6 +1850,7 @@ }, "expression": { "type": "string", + "description": "Quartz cron expression — 6 or 7 fields (seconds minutes hours day-of-month month day-of-week [year]); one of the two day fields must be ?", "minLength": 1, "maxLength": 200 } diff --git a/internal/tools/server.go b/internal/tools/server.go index cd6bb2a..5918479 100644 --- a/internal/tools/server.go +++ b/internal/tools/server.go @@ -8,7 +8,7 @@ import ( "ombi-mcp/internal/ombi" ) -// read_server — status, version, features, news, stats and the +// read_server — status, version, features, stats and the // administrator-gated cron validation. Families: metrics, // reference_page. func handleServer(ctx context.Context, env *Env, raw json.RawMessage) *ToolResult { @@ -28,8 +28,6 @@ func handleServer(ctx context.Context, env *Env, raw json.RawMessage) *ToolResul return o.serverUpdateInfo() case "update_check": return o.serverUpdateCheck() - case "news": - return o.serverNews() case "landing": return o.serverLanding() case "features": @@ -137,20 +135,6 @@ func (o *op) serverUpdateCheck() *ToolResult { Values: []Metric{metric("update_available", v, "instance", "")}}) } -func (o *op) serverNews() *ToolResult { - raw, fail := o.call("GET", "/api/v2/System/news", nil, nil) - if fail != nil { - return fail - } - items, fail := o.refArray(raw, - []string{"id"}, []string{"title", "name", "headline"}, "news") - if fail != nil { - return fail - } - return o.ok(&ReferencePage{Kind: "reference_page", - Items: items, Page: singlePage(len(items), "references")}) -} - func (o *op) serverLanding() *ToolResult { raw, fail := o.call("GET", "/api/v1/LandingPage", nil, nil) if fail != nil { @@ -196,24 +180,23 @@ func (o *op) serverFeatures() *ToolResult { } func (o *op) serverStats(a *ServerArgs) *ToolResult { - // from <= to must be enforced by the server per the contract. - if a.From != "" && a.To != "" { - f, ferr := time.Parse(time.RFC3339, a.From) - t, terr := time.Parse(time.RFC3339, a.To) - if ferr != nil || terr != nil { - return o.invalid("from", "from/to must be RFC3339 date-time values") - } - if f.After(t) { - return o.invalid("from", "from must be on or before to") + if !nonempty(a.From) || !nonempty(a.To) { + field := "from" + if nonempty(a.From) { + field = "to" } + return o.invalid(field, "stats requires both from and to as RFC3339 date-time values") } - q := map[string]string{} - if a.From != "" { - q["from"] = a.From + // from <= to is enforced before the upstream call. + f, ferr := time.Parse(time.RFC3339, a.From) + t, terr := time.Parse(time.RFC3339, a.To) + if ferr != nil || terr != nil { + return o.invalid("from", "from/to must be RFC3339 date-time values") } - if a.To != "" { - q["to"] = a.To + if f.After(t) { + return o.invalid("from", "from must be on or before to") } + q := map[string]string{"from": a.From, "to": a.To} raw, fail := o.call("GET", "/api/v1/Stats", q, nil) if fail != nil { return fail @@ -256,5 +239,8 @@ func (o *op) serverCronValidate(a *ServerArgs) *ToolResult { if s := jstr(m, "message"); s != "" { vals = append(vals, metric("message", s, "instance", "")) } + if valid, ok := m["success"].(bool); ok && !valid { + o.warnf("Ombi validates Quartz cron syntax (6-7 fields: seconds minutes hours day-of-month month day-of-week [year]); use ? for the unused day-of-month/day-of-week field") + } return o.ok(&Metrics{Kind: "metrics", Values: vals}) } -- 2.39.5