feat(requestrouter): carry requested seasons to request routers - #25
Conversation
Series requests can name seasons, but RequestDescriptor had no field for them, so a request router could only add the whole series. The host therefore refuses to request the missing seasons of a series it already has whenever a download server takes series. Add, both appended without renumbering: - RequestDescriptor.seasons = 10. Empty means the whole series; 0 is Specials. - CapabilityDescriptor.request_router = 12, a RequestRouterDescriptor with supports_seasons = 1. Plugins that honour seasons set it; an absent descriptor, as in every plugin built on an older SDK, means false, so the host can tell the two apart from the manifest. The manifest validator keeps the descriptor optional and rejects it on other capability types. convert round-trips it so the host can read the flag from stored capability metadata. Closes #24 Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
|
Navigate logical layers of code changes, visualize relationships, and explore their blast radius. No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Advanced Run ID: ⛔ Files ignored due to path filters (2)
📒 Files selected for processing (9)
Included review availability: This review used your included allowance. Your plan provides up to 1 included review per hour; 0 remain after this review. 📝 WalkthroughWalkthroughThe protocol adds season-scoped requests and a request-router capability descriptor. Manifest validation and capability conversion handle the descriptor. Documentation and tests describe and check the new fields and season delivery. ChangesSeason-aware requests
Priority: ⬇️ Low Estimated code review effort: 2 (Simple) | ~12 minutes Change: Feature Merge Risk: ⚪ Minimal · up to No identified issue blocks merging the season-aware request router changes after normal checks. Security Architecture ReviewSecurity architecture risk: 🟡 Moderate · up to Season-scoped requests require coordinated host and plugin behavior. Older plugins treat them as whole-series requests, so a rollout mistake could acquire more seasons than requested. The reviewed SDK does not establish that this happens in production. Retained concerns
Security review detailsSecurity Blast Radius
Trust Boundaries and Controls
Resilience and Maintainability Implications
Hardening Proposals
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Full details: Docstring CoverageExplanation Docstring coverage is 33.33% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 12 functions across 5 files. (4 skipped: 4 unsupported.)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
1 participant
Problem
Related issue: #24
Used by: Silo-Server/silo-server#1554 (host), Silo-Community/silo-plugins-requests-arr#11 (Sonarr).
Silo is adding season picking to series requests, but
RequestDescriptorhas no field for seasons, so a request router can only add the whole series. The host therefore refuses to request the missing seasons of a series already in the library whenever a download server takes series (missing_seasons_requestablereportsfalse); otherwise the plugin would add the whole series again.Approach
Both additions are appended; nothing is renumbered, and
buf breakingagainstmainreports nothing.RequestDescriptorseasons = 10(repeated int32). Empty means the whole series, as every request means today;0is Specials.CapabilityDescriptorrequest_router = 12, a newRequestRouterDescriptor { bool supports_seasons = 1; }A plugin declares season support in its manifest:
{ "type": "request_router.v1", "id": "arr", "request_router": { "supports_seasons": true } }A declaring plugin acquires only the requested seasons. When the series already exists upstream, it adds those seasons to what it already tracks and leaves the others alone, and repeating a request converges. The host sends a request for only the missing seasons of a series it already has only to declaring plugins.
Why a typed descriptor. It follows the existing
watch_sync_providerandnetwork_access_providerblocks, so the host reads it from the manifest without launching the plugin. An absent descriptor meansfalse, which is what every plugin built on an older SDK reports; that is how the host tells the two apart. The alternative, a free-form feature string list, would be the first of its kind in the manifest and unvalidated.Validator. The descriptor is optional, so existing request routers stay valid; it is rejected on any other capability type.
Stored metadata.
convertround-tripsrequest_router, because the host persists capability metadata withCapabilityRecordsFromManifestand reads it back withDecodeCapability. Without this the host would never see the flag.Docs: a "Request routers" section in
README.mdand a presence note indocs/compatibility.md.After merge this should be tagged v0.18.0 (additive, minor).
Validation
Fulfillcall over gRPC (bufconn) deliversseasons[0, 4]to the plugin;supports_seasonsloads, and one without the descriptor loads as declaring no support;CapabilityRecordsFromManifest→DecodeCapability, stored metadata without it decodes as absent, and unknown descriptor fields are discarded.go test ./...,go vet ./..., and thehello-scheduled-task,hello-runtime-host, andhello-network-accessbuilds pass.protocis not installed on the build host, somake protostops at itsprotoccheck. I ran the step it wraps,buf generate, with protoc-gen-go v1.36.11 and protoc-gen-go-grpc v1.6.1. On a clean tree it produced no diff; after the change it touched onlycommon.pb.goandrequest_router.pb.go, and a second run changed nothing.gofmt -l .lists onlypkg/pluginsdk/runtime/scan_source_test.go, which predates this change and is not touched by it.Risks
None identified for mixed versions. Manifest loading has ignored unknown fields since v0.4.0, and
DecodeCapabilityreads only the keys it knows, so a host built on an older SDK installs a plugin that sets the flag, drops the flag, and keeps whole-series behaviour. A plugin built on an older SDK ignoresseasonsas an unknown field and adds the whole series, which is why the host gates season-only requests on the manifest flag.Checklist
AI Disclosure
convertround-trip, validation, the documented contract against the code, and whether the tests would fail without the change. It found no issues. It could not run commands, so the validation above was run separately.🤖 Generated with Claude Code
Note
Add requested seasons to request-router proto and manifest contract
seasonsfield (field 10) topluginv1.RequestDescriptor; an empty list means the whole series and zero means Specials in request_router.protopluginv1.RequestRouterDescriptorwith asupports_seasonsflag, attached toCapabilityDescriptorat field 12 in common.protorequest_router.v1capabilitiesseasonsis a repeated field with no presence semantics; older plugins ignore it and fulfil whole-series requestsMacroscope summarized cd4c295.