feat(watchsync): add plugin-backed providers - #475
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds plugin-backed watch-sync providers with RPC support, lifecycle discovery, atomic registry replacement, authentication, state synchronization, encrypted credentials, scrobble completion handling, and durable export status transitions. ChangesWatch-sync plugin integration
Estimated code review effort: 5 (Critical) | ~120 minutes Sequence Diagram(s)sequenceDiagram
participant PluginLifecycle
participant SiloServer
participant PluginService
participant WatchSyncProvider
participant WatchSyncRepository
PluginLifecycle->>SiloServer: trigger provider reload
SiloServer->>PluginService: resolve watch-sync capability
PluginService-->>SiloServer: return RPC client
SiloServer->>WatchSyncProvider: construct and register provider
WatchSyncProvider->>PluginService: apply authentication or sync events
PluginService-->>WatchSyncProvider: return credentials, state, or event results
SiloServer->>WatchSyncRepository: persist credentials and export state
Possibly related PRs
Suggested labels: Suggested reviewers: 🚥 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: 3
🤖 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 `@internal/watchsync/plugin_provider.go`:
- Around line 50-61: Update NewPluginProvider and the ExportHistory and scrobble
export paths to honor WatchSyncProviderDescriptor.supported_media_types.
Validate or retain the advertised media types during provider construction, then
reject or skip events whose Media.MediaType is not supported before applying or
exporting them; preserve existing behavior for compatible media types.
In `@internal/watchsync/service_test.go`:
- Around line 329-336: Update
serviceFakeRepo.MarkHistoryExportSatisfiedByScrobble to skip records whose
status is already sent, matching the Postgres implementation. Only set
historyExportStatusSatisfiedByScrobble for matching unsent exports, preserving
existing error and lookup behavior.
In `@internal/watchsync/service.go`:
- Around line 1372-1380: Propagate failures from MarkHistoryExportStatus instead
of discarding them in both non-retryable paths: ExportWatched at
internal/watchsync/service.go lines 1372-1380 and exportLocalPlays at lines
1487-1495. Join each status-write error with the original export error and
return the combined error while preserving the existing failed-count updates.
🪄 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: 59e79505-cbfd-48e7-97d1-027609dd55db
📒 Files selected for processing (15)
cmd/silo/main.gocmd/silo/main_test.godocs/superpowers/specs/2026-07-25-plugin-watch-sync-provider-design.mdinternal/pluginhost/client.gointernal/pluginhost/handshake.gointernal/plugins/service.gointernal/plugins/service_hot_reload_test.gointernal/watchsync/plugin_provider.gointernal/watchsync/plugin_provider_test.gointernal/watchsync/registry.gointernal/watchsync/registry_test.gointernal/watchsync/repository.gointernal/watchsync/service.gointernal/watchsync/service_test.gointernal/watchsync/types.go
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
internal/watchsync/service.go (1)
2046-2054: 🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy liftDo not make scrobble reconciliation best-effort.
If this update fails, the pre-created export remains
pending; a later export can send an ApplyEvents watched event even though the completed stop already succeeded. Persist or durably retry this transition without re-dispatching the stop.As per coding guidelines, “Keep Go backend behavior predictable under load and during failures.”
🤖 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 `@internal/watchsync/service.go` around lines 2046 - 2054, Update the scrobble reconciliation flow around MarkHistoryExportSatisfiedByScrobble so a failure is not merely logged and ignored. Ensure the transition from pending to satisfied is persisted or durably retried before completing reconciliation, without dispatching the stop event again; preserve the existing connection, provider, and history identifiers.Source: Coding guidelines
🤖 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 `@internal/watchsync/service.go`:
- Around line 1513-1515: Update the invalid-credential branch in the surrounding
watch-sync operation to persist the failure on the connection’s LastError before
returning it, matching ExportWatched’s behavior. Keep the existing
isWatchSyncInvalidCredentialError check and direct error return after saving the
error.
---
Outside diff comments:
In `@internal/watchsync/service.go`:
- Around line 2046-2054: Update the scrobble reconciliation flow around
MarkHistoryExportSatisfiedByScrobble so a failure is not merely logged and
ignored. Ensure the transition from pending to satisfied is persisted or durably
retried before completing reconciliation, without dispatching the stop event
again; preserve the existing connection, provider, and history identifiers.
🪄 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: c6936101-d5ed-49ce-9f26-e4950ec2a001
📒 Files selected for processing (6)
internal/watchsync/plugin_provider.gointernal/watchsync/plugin_provider_test.gointernal/watchsync/repository.gointernal/watchsync/service.gointernal/watchsync/service_test.gointernal/watchsync/types.go
🚧 Files skipped from review as they are similar to previous changes (2)
- internal/watchsync/service_test.go
- internal/watchsync/plugin_provider.go
0f35fd7 to
bcb31bc
Compare
|
Expanded host support is now on This supersedes the original “first slice” scope. The adapter now covers device/API auth, complete credential/config propagation, watched/progress/favorite/watchlist import and export operations, ordered lists, unwatch, and live start/pause/stop scrobbles. It also resolves the outstanding completed-scrobble review finding: after a remote stop succeeds, a failed local history transition is recorded for durable database-only retry, so reconciliation does not dispatch the stop again. Dependencies and evidence are in the updated PR body. The SDK contract is Silo-Server/silo-plugin-sdk#13; its CI is green. The exact server head passes focused and race tests, changed-line lint with zero issues, local-path verification, and diff checks. |
There was a problem hiding this comment.
Actionable comments posted: 7
🧹 Nitpick comments (6)
internal/plugins/watch_sync_config_test.go (1)
9-36: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winConsider adding two edge cases.
The test covers the fail-closed classification path well. Two additions would guard the branches that production code already handles:
- A
nilentry in the[]*RuntimeConfigslice, whichwatchSyncProviderConfigskips at line 52.- A field name with surrounding whitespace, which currently routes a declared public field into
secret_values. See the related comment oninternal/plugins/watch_sync_config.go.🤖 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 `@internal/plugins/watch_sync_config_test.go` around lines 9 - 36, Extend TestWatchSyncProviderConfigClassifiesManifestFields with cases for a nil RuntimeConfig entry and a declared public field whose name has surrounding whitespace. Verify watchSyncProviderConfig skips the nil entry without error, and assert the whitespace-normalized public field is classified in GetSecretValues rather than GetValues as required by the existing production behavior.internal/watchsync/plugin_provider.go (1)
634-648: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚖️ Poor tradeoffA descriptor advertising both auth methods loses the device-code flow.
supportedWatchSyncAuthMethodreturns a single method. It returnsAuthMethodAPIKeyas soon as it sees that method, so a descriptor advertising both API key and device code exposes only the API-key route.StartDeviceAuththen rejects every call at line 183.The single
authMethodstring field is the constraint, not this function. If dual-method plugins are expected, the provider needs a set of supported methods and the connection routes need to select one. If they are not expected, add a short comment recording that API key takes precedence deliberately.🤖 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 `@internal/watchsync/plugin_provider.go` around lines 634 - 648, Update supportedWatchSyncAuthMethod and the provider’s single authMethod representation to preserve both API-key and device-code capabilities for descriptors advertising both methods; adjust the connection/auth routes, including StartDeviceAuth, to select the appropriate supported method instead of rejecting device-code authentication. If dual-method support is intentionally out of scope, retain the current precedence and add a concise comment documenting that API key deliberately takes precedence.migrations/sql/20260805221735_add_watch_provider_plugin_credentials.sql (1)
9-13: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueDocument that the rollback discards plugin credential data.
Both
ADD COLUMNstatements are metadata-only on PostgreSQL 11 and later, so the Up path is safe on a large table. The Down path is the correct inverse.One consequence is worth recording:
plugin_credentialsis the only store forTokenType,Scopes, andSecretAttributeson plugin-backed connections. A rollback drops that data permanently, and affected users must reconnect their plugin providers. Add a short comment in the migration or a note in the release notes so an operator knows this before rolling back.The Squawk
ban-drop-columnwarnings on Lines 10 and 13 apply to the Down section and are expected for a reversible column addition.As per coding guidelines: "Create database changes as timestamped Goose SQL migrations using
make migrate-create NAME=...; never rungoose fix, never create paired.up.sqland.down.sqlfiles" — this file follows that pattern.🤖 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 `@migrations/sql/20260805221735_add_watch_provider_plugin_credentials.sql` around lines 9 - 13, Add a concise comment near the Down-section DROP COLUMN for plugin_credentials documenting that rollback permanently discards TokenType, Scopes, and SecretAttributes and requires affected users to reconnect plugin providers. Leave the migration structure and expected drop statements unchanged.Source: Linters/SAST tools
internal/watchsync/service_test.go (1)
592-594: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winGuard
ListOpenScrobbleSessionslike the other scrobble accessors.This change puts every other reader and writer of the fake's scrobble state behind
scrobbleMu, and addsscrobbleUpdatesSnapshotfor safe reads.ListOpenScrobbleSessionsis now the only accessor that takes no lock, and it returns the shared slice itself rather than a copy. Current tests setscrobbleSessionsbefore the sweep, so nothing races today. A future test that mutates sessions during a sweep would trip-race.♻️ Proposed fix
func (r *serviceFakeRepo) ListOpenScrobbleSessions(_ context.Context) ([]ScrobbleSession, error) { - return r.scrobbleSessions, nil + r.scrobbleMu.Lock() + defer r.scrobbleMu.Unlock() + return append([]ScrobbleSession(nil), r.scrobbleSessions...), nil }🤖 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 `@internal/watchsync/service_test.go` around lines 592 - 594, Update serviceFakeRepo.ListOpenScrobbleSessions to read scrobbleSessions under scrobbleMu and return a copy of the slice, matching the locking and snapshot behavior of the other scrobble accessors. Preserve the existing session contents and error result while preventing callers from sharing the backing slice.internal/watchsync/repository.go (1)
1191-1201: 🚀 Performance & Scalability | 🔵 TrivialBound the reconciliation query and add a matching partial index.
ListPendingScrobbleReconciliationsselects every unreconciled row with noLIMIT. The sweeper calls it on each pass. If reconciliation keeps failing for some sessions, the result set grows without bound and each sweep loads all of them into memory.Consider a
LIMITwithORDER BY stop_sent_at ASC(already present) so each sweep drains a bounded batch. A partial index also keeps the scan cheap as the session table grows:CREATE INDEX CONCURRENTLY IF NOT EXISTS watch_provider_scrobble_sessions_pending_reconciliation_idx ON watch_provider_scrobble_sessions (stop_sent_at) WHERE stop_sent_at IS NOT NULL AND completed = true AND history_reconciled_at IS NULL;🤖 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 `@internal/watchsync/repository.go` around lines 1191 - 1201, Update ListPendingScrobbleReconciliations to add a bounded LIMIT to the existing stop_sent_at ASC query so each sweep returns only a fixed batch of pending reconciliations. Add the matching partial index on watch_provider_scrobble_sessions(stop_sent_at) for rows with stop_sent_at set, completed true, and history_reconciled_at unset, using the project’s migration mechanism.internal/watchsync/repository_test.go (1)
41-49: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd a negative case for the AAD binding.
The test proves the happy-path round trip. It does not prove the property the AAD exists for: a bundle encrypted for one connection must not decrypt into a different connection. Add a second decode with a changed
UserIDorProviderand assert that it returns an error. This locks in the cross-connection substitution guarantee.♻️ Proposed addition
if output.AccessToken != input.AccessToken || output.RefreshToken != input.RefreshToken || output.TokenType != input.TokenType || !output.TokenExpiresAt.Equal(expiresAt) || !reflect.DeepEqual(output.Scopes, input.Scopes) || !reflect.DeepEqual(output.SecretAttributes, input.SecretAttributes) { t.Fatalf("decoded credentials = %#v", output) } + foreign := Connection{Provider: input.Provider, UserID: input.UserID + 1, ProfileID: input.ProfileID} + if err := repository.decodePluginCredentials(&foreign, encoded); err == nil { + t.Fatalf("credentials decrypted for a different connection: %#v", foreign) + } }🤖 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 `@internal/watchsync/repository_test.go` around lines 41 - 49, Extend the credential round-trip test around decodePluginCredentials with a negative AAD-binding case: create a second Connection differing in UserID or Provider, decode the same encoded bundle into it, and assert that decodePluginCredentials returns an error. Keep the existing matching-connection assertions unchanged.
🤖 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 `@go.mod`:
- Line 112: Update the github.com/Silo-Server/silo-plugin-sdk dependency in
go.mod to use an approved released version or a silo-server feature-branch
reference, rather than the pseudo-version sourced from the unmerged
codex/watch-sync-provider-v2 branch; document the follow-up plan if a release is
still required.
In `@internal/plugins/watch_sync_config.go`:
- Around line 55-72: Update the config-field loop around watchSyncConfigString
to trim config.Key and field once, skip entries when either trimmed component is
empty, and build the key from those trimmed values. Use the trimmed field name
for the public lookup so it matches the normalized keys produced by
GlobalConfigFieldSets, while preserving the existing public and protected map
assignment behavior.
In `@internal/watchsync/plugin_provider_state.go`:
- Around line 246-256: The event-building loops in
internal/watchsync/plugin_provider_state.go at lines 246-256 and 284-300 must
record unsupported media as terminal failures instead of silently skipping
items. In both loops, collect the affected play.HistoryID or item.MediaItemID
with unsupportedWatchSyncMediaMessage(...) and merge those entries into
result.Failed after applyPluginEvents returns; in lines 284-300, derive
ListPosition from len(events) so skipped items do not create ordering gaps.
- Around line 188-235: Bound accumulated remote-state items in the traversal
loop around result.items and ListRemoteState by adding a total-item cap
independent of maxRemoteStatePages. Before appending each response’s items,
detect whether the cap would be exceeded and return an error without advancing
or returning a cursor; preserve normal pagination behavior when within the cap
so the next run restarts the family after rejection.
In `@internal/watchsync/plugin_provider.go`:
- Around line 434-447: Update applyScrobble’s handling of applyPluginEvents
results to preserve retry classification: return retryableProviderError for
WATCH_SYNC_APPLY_STATUS_RETRY when the fault is not rate-limited, and return a
terminal error for WATCH_SYNC_APPLY_STATUS_REJECTED. Follow the existing
ExportHistory pattern so SweepOpenScrobbles does not redispatch terminal
failures indefinitely.
- Around line 204-222: Update the device-authorization conversion before
constructing DeviceAuthSession: call response.GetExpiresAt().CheckValid() and
return an incomplete/invalid authorization error when validation fails, then
parse verificationURL with url.Parse and require an allowed scheme before
assigning it. Preserve the existing complete-URL preference and only persist the
session after both validations succeed.
In `@internal/watchsync/repository.go`:
- Around line 1375-1418: Ensure every token write path, including
UpsertConnection and traktCollectionTokenResolver.updateTokens, persists
plugin_credentials consistently by re-encoding credentials through
encodePluginCredentials after Trakt refresh or any
watch_provider_connections.update_tokens SQL update. Alternatively, remove
access-token fields from storedPluginCredentials and treat the connection
columns as authoritative; choose one source of truth and apply it consistently
across all writers.
---
Nitpick comments:
In `@internal/plugins/watch_sync_config_test.go`:
- Around line 9-36: Extend TestWatchSyncProviderConfigClassifiesManifestFields
with cases for a nil RuntimeConfig entry and a declared public field whose name
has surrounding whitespace. Verify watchSyncProviderConfig skips the nil entry
without error, and assert the whitespace-normalized public field is classified
in GetSecretValues rather than GetValues as required by the existing production
behavior.
In `@internal/watchsync/plugin_provider.go`:
- Around line 634-648: Update supportedWatchSyncAuthMethod and the provider’s
single authMethod representation to preserve both API-key and device-code
capabilities for descriptors advertising both methods; adjust the
connection/auth routes, including StartDeviceAuth, to select the appropriate
supported method instead of rejecting device-code authentication. If dual-method
support is intentionally out of scope, retain the current precedence and add a
concise comment documenting that API key deliberately takes precedence.
In `@internal/watchsync/repository_test.go`:
- Around line 41-49: Extend the credential round-trip test around
decodePluginCredentials with a negative AAD-binding case: create a second
Connection differing in UserID or Provider, decode the same encoded bundle into
it, and assert that decodePluginCredentials returns an error. Keep the existing
matching-connection assertions unchanged.
In `@internal/watchsync/repository.go`:
- Around line 1191-1201: Update ListPendingScrobbleReconciliations to add a
bounded LIMIT to the existing stop_sent_at ASC query so each sweep returns only
a fixed batch of pending reconciliations. Add the matching partial index on
watch_provider_scrobble_sessions(stop_sent_at) for rows with stop_sent_at set,
completed true, and history_reconciled_at unset, using the project’s migration
mechanism.
In `@internal/watchsync/service_test.go`:
- Around line 592-594: Update serviceFakeRepo.ListOpenScrobbleSessions to read
scrobbleSessions under scrobbleMu and return a copy of the slice, matching the
locking and snapshot behavior of the other scrobble accessors. Preserve the
existing session contents and error result while preventing callers from sharing
the backing slice.
In `@migrations/sql/20260805221735_add_watch_provider_plugin_credentials.sql`:
- Around line 9-13: Add a concise comment near the Down-section DROP COLUMN for
plugin_credentials documenting that rollback permanently discards TokenType,
Scopes, and SecretAttributes and requires affected users to reconnect plugin
providers. Leave the migration structure and expected drop statements unchanged.
🪄 Autofix
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: cfa87662-020c-493b-8311-1c5cb05853bb
⛔ Files ignored due to path filters (1)
go.sumis excluded by!**/*.sum
📒 Files selected for processing (17)
cmd/silo/main.gocmd/silo/main_test.godocs/superpowers/specs/2026-07-25-plugin-watch-sync-provider-design.mdgo.modinternal/pluginhost/client.gointernal/plugins/watch_sync_config.gointernal/plugins/watch_sync_config_test.gointernal/watchsync/lists.gointernal/watchsync/plugin_provider.gointernal/watchsync/plugin_provider_state.gointernal/watchsync/plugin_provider_test.gointernal/watchsync/repository.gointernal/watchsync/repository_test.gointernal/watchsync/service.gointernal/watchsync/service_test.gointernal/watchsync/types.gomigrations/sql/20260805221735_add_watch_provider_plugin_credentials.sql
🚧 Files skipped from review as they are similar to previous changes (3)
- cmd/silo/main_test.go
- cmd/silo/main.go
- internal/watchsync/service.go
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 `@migrations/sql/20260805221735_add_watch_provider_plugin_credentials.sql`:
- Around line 8-13: Move watch_provider_scrobble_reconcile_pending_idx out of
the current migration into a new timestamped Goose migration, created through
the standard migrate-create workflow. Define the Up migration with a NO
TRANSACTION header, cleanup of any invalid prior index before retrying, and
CREATE INDEX CONCURRENTLY IF NOT EXISTS; define Down with DROP INDEX
CONCURRENTLY IF EXISTS. Remove the normal index creation and rollback statements
from the original migration.
🪄 Autofix
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: 0a447e62-3442-44bb-b2c9-2641df82f490
⛔ Files ignored due to path filters (1)
go.sumis excluded by!**/*.sum
📒 Files selected for processing (15)
docs/superpowers/specs/2026-07-25-plugin-watch-sync-provider-design.mdgo.modinternal/pluginhost/client.gointernal/plugins/watch_sync_config.gointernal/plugins/watch_sync_config_test.gointernal/watchsync/lists.gointernal/watchsync/plugin_provider.gointernal/watchsync/plugin_provider_state.gointernal/watchsync/plugin_provider_test.gointernal/watchsync/repository.gointernal/watchsync/repository_test.gointernal/watchsync/service.gointernal/watchsync/service_test.gointernal/watchsync/types.gomigrations/sql/20260805221735_add_watch_provider_plugin_credentials.sql
🚧 Files skipped from review as they are similar to previous changes (7)
- internal/plugins/watch_sync_config_test.go
- go.mod
- internal/pluginhost/client.go
- internal/plugins/watch_sync_config.go
- docs/superpowers/specs/2026-07-25-plugin-watch-sync-provider-design.md
- internal/watchsync/plugin_provider.go
- internal/watchsync/service.go
|
Addressed the current review feedback through
Validation completed:
One intentional merge gate remains open: merge SDK PR #13, publish the approved SDK release, and replace the pseudo-version in |
|
The remaining SDK publication gate is closed in Fresh isolated dev-builder validation also completed against this head:
The sandbox and Floppy companion remain running for reviewer verification. |
Quick104
left a comment
Quick104
left a comment
There was a problem hiding this comment.
Verified at head faca64a: all review threads are resolved, the released SDK v0.13.0 dependency is pinned, focused/race/migration validation passed, and the exact head was deployed and exercised with the Floppy plugin on both isolated and shared development environments.
2 participants
Adds a complete host adapter for plugins implementing watch_sync_provider.v1, using Silo's existing watch-sync connection, import, export, list, scrobble, retry, and reconciliation machinery.
Problem
Silo's watch providers are compiled into the server. The released v0.12 plugin capability proved the RPC boundary, but PR 475 originally exposed only API-key authentication, watched export, and completed playback. That was not enough to extract Trakt, Simkl, MDBList, or to implement a Floppy provider without provider-specific host code.
Approach
The public /api/v1 surface remains additive. Existing built-in Trakt, Simkl, and MDBList providers remain registered while equivalent plugins and migration tooling are developed.
SDK dependency
Depends on silo-plugin-sdk PR #13, which expands the additive v0.13 contract with:
This PR is temporarily pinned to SDK commit 3b705d7e882f through its immutable Go pseudo-version. Replace it with v0.13.0 after the SDK PR merges and the release is tagged. Existing v0.12 plugins remain wire-compatible and expose only capabilities the older descriptor can execute.
Reference provider
A Floppy reference implementation has been built in Silo-Server/silo-plugin-watchprovider-floppy. It validates profile API tokens, imports watched history and durable progress, exports completed watches, and forwards live start/pause/stop events. It deliberately does not advertise favorites, watchlists, or unwatch because Floppy does not currently expose safe reconciliation semantics for those operations.
Reliability and security
Validation
Passed on the exact published head bcb31bc:
The repository-wide make test-go run reached the changed packages and migrations successfully, then failed two existing internal/jellycompat process-lock tests. Both tests fail identically on untouched main, confirming a baseline/environment failure rather than a PR regression.
The Floppy reference separately passes go test -race ./..., builds successfully, and renders a validated manifest through ./plugin manifest. The SDK branch passes go test ./....
Deferred work
AI disclosure
Summary by CodeRabbit
New Features
Bug Fixes
Documentation