feat: add watch_sync_provider.v1 capability - #10
Conversation
|
Warning Review limit reached
Next review available in: 51 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (3)
📝 WalkthroughWalkthroughAdds the ChangesWatch sync provider capability
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant Host
participant GRPCPlugin
participant WatchSyncProvider
Host->>GRPCPlugin: Register WatchSyncProvider service
Host->>WatchSyncProvider: ApplyEvents ordered desired-state events
WatchSyncProvider-->>Host: Return apply results and updated credentials
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@pkg/pluginsdk/manifest/manifest.go`:
- Around line 137-172: Update validateWatchSyncCapability to reject
WATCH_SYNC_AUTH_METHOD_UNSPECIFIED entries after confirming auth methods are
present, and require supported_media_types to be non-empty before validating
each value as movie or episode. Preserve the existing validation errors and
checks for valid configurations.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: bec2cf59-6e85-40ca-8081-90df491a5139
⛔ Files ignored due to path filters (3)
pkg/pluginproto/silo/plugin/v1/common.pb.gois excluded by!**/*.pb.gopkg/pluginproto/silo/plugin/v1/watch_sync_provider.pb.gois excluded by!**/*.pb.gopkg/pluginproto/silo/plugin/v1/watch_sync_provider_grpc.pb.gois excluded by!**/*.pb.go
📒 Files selected for processing (10)
README.mdpkg/pluginsdk/capability/capability.gopkg/pluginsdk/convert/convert.gopkg/pluginsdk/convert/watch_sync_provider_test.gopkg/pluginsdk/manifest/manifest.gopkg/pluginsdk/manifest/watch_sync_provider_test.gopkg/pluginsdk/runtime/runtime.gopkg/pluginsdk/runtime/watch_sync_provider_test.goproto/silo/plugin/v1/common.protoproto/silo/plugin/v1/watch_sync_provider.proto
Quick104
left a comment
Quick104
left a comment
There was a problem hiding this comment.
Validated the final provider-neutral contract, generated protobufs, manifest conversion/validation, full race tests, vet, and disposable host-adapter integration proof.
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
proto/silo/plugin/v1/watch_sync_provider.proto (1)
155-163: 🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy liftDefine how credential updates interact with response faults.
updated_credentialsmay be returned alongsidefault, but the contract only defines discarding results/pages—not whether credential updates are persisted, rejected, or discarded. Different implementations could lose token rotation or apply credentials from a failed operation. Specify and validate one deterministic rule across credential, apply, and remote-state responses.Also applies to: 224-230, 270-285
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@proto/silo/plugin/v1/watch_sync_provider.proto` around lines 155 - 163, Define a single deterministic policy for handling updated credentials when a response also contains a fault, and document it in the WatchSync response contracts. Apply the same policy consistently to WatchSyncCredentialResponse and the related credential, apply, and remote-state response messages, then update validation or implementations to enforce it so credentials are never ambiguously persisted or discarded.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@README.md`:
- Around line 150-155: Update the README pagination description to say the host
“commits each successful page” instead of “commits every page,” preserving the
surrounding behavior and terminology.
---
Outside diff comments:
In `@proto/silo/plugin/v1/watch_sync_provider.proto`:
- Around line 155-163: Define a single deterministic policy for handling updated
credentials when a response also contains a fault, and document it in the
WatchSync response contracts. Apply the same policy consistently to
WatchSyncCredentialResponse and the related credential, apply, and remote-state
response messages, then update validation or implementations to enforce it so
credentials are never ambiguously persisted or discarded.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 5abbf985-ec06-4bfe-97db-b53817c30b48
⛔ Files ignored due to path filters (1)
pkg/pluginproto/silo/plugin/v1/watch_sync_provider.pb.gois excluded by!**/*.pb.go
📒 Files selected for processing (6)
README.mdpkg/pluginproto/silo/plugin/v1/watch_sync_provider_test.gopkg/pluginsdk/convert/watch_sync_provider_test.gopkg/pluginsdk/manifest/manifest.gopkg/pluginsdk/manifest/watch_sync_provider_test.goproto/silo/plugin/v1/watch_sync_provider.proto
🚧 Files skipped from review as they are similar to previous changes (2)
- pkg/pluginsdk/convert/watch_sync_provider_test.go
- pkg/pluginsdk/manifest/manifest.go
Quick104
left a comment
Quick104
left a comment
There was a problem hiding this comment.
Approved at cfabc3d after resolving all CodeRabbit feedback. Verified deterministic credential/fault commit ordering, protobuf regeneration, go test -race ./..., go vet ./..., focused host-adapter proof, and clean autoreview.
2 participants
Summary
watch_sync_provider.v1gRPC capabilityThe contract keeps encrypted credentials, OAuth state, durable delivery, retries, and reconciliation host-owned. Plugins receive secret material only for individual RPC calls and are explicitly required not to persist or log it.
Why
I was working on adding a plugin for AniList sync and noticed that the existing
event_consumer.v1path cannot reliably sync playback completion: it has no completion event, per-profile encrypted credentials, durable event IDs, retry/replay, or rich media identity. Adding those concerns to the generic event-consumer path would duplicate infrastructure Silo already has for built-in watch providers.This capability provides the narrow plugin boundary needed to adapt external trackers to Silo's existing watchsync pipeline, keeping encryption, connection ownership, retries, scrobble sessions, and reconciliation host-owned. The contract is provider-neutral rather than AniList-specific so other watch-tracking plugins can use the same integration.
A corresponding server adapter has been implemented and tested on a branch in the contributor's fork to validate the SDK contract against the existing watchsync interfaces. If this SDK capability is merged and released, that server work can be updated to the tagged SDK version and proposed separately for review; it is not part of this PR.
Validation
go test -race ./...go vet ./...AI use disclosure
AI assistance was used for repository research and Go/protobuf syntax suggestions. The implementation, API contract, generated code, tests, and review fixes were manually reviewed and validated by the contributor.
Summary by CodeRabbit
Summary by CodeRabbit
New Features
watch_sync_provider.v1, including authorization/credential exchange, account retrieval, applying ordered watch events, and listing remote watch state.Bug Fixes
Validation & SDK Updates
Documentation
Tests