Multi-instance Sonarr/Radarr integration mapped to path pairs#426
Merged
Conversation
Closes #425. The *arr integration now stores a list of named Sonarr/Radarr instances in a new integrations.json. Each path pair declares which instances to notify via arr_target_ids, so completion scans are routed to the *arrs that actually own the content. Files outside any path pair are skipped. The legacy [Integrations] section in settings.cfg is migrated on first boot: each configured *arr becomes a named instance attached to every existing path pair, preserving today's behavior. Config.from_dict drops the section so the next persist rewrites settings.cfg without it. Frontend: a new Integrations card lists configurable instances with add/edit/delete/test actions. Each path pair card grows a chip row ("Notify: [Sonarr — TV ✕] [+ Add]") for attaching/detaching configured instances; deletes detach automatically. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
📝 WalkthroughWalkthroughThis PR implements support for multiple Sonarr/Radarr instances with dedicated management. It introduces instance models, refactors the integrations service into a CRUD layer with per-instance endpoints, adds instance attachment to path pairs, and migrates legacy flat configs to instance-based lists. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~75 minutes Possibly related PRs
Suggested labels
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/angular/src/app/pages/settings/integrations.component.scss`:
- Around line 86-88: The Radarr badge (.kind-radarr) uses background-color
`#e6a532` with white text which fails WCAG contrast; update the .kind-radarr rule
to either use a darker background (e.g., a deeper amber/brown) or switch the
text color to a dark color to reach at least 4.5:1 contrast for small
text—modify the .kind-radarr selector in integrations.component.scss accordingly
and verify the new color combination meets WCAG AA contrast ratios.
In `@src/angular/src/app/pages/settings/path-pairs.component.ts`:
- Around line 187-203: Attach/detach currently calls PathPairsService.update()
and immediately closes the picker or silently ignores failures; change
onAttachInstance and onDetachInstance to handle the update Observable explicitly
by providing next and error handlers: perform the arrPickerPairId = null (close
picker) only in the next handler after a successful non-null response, and in
the error handler handle 409/conflict and other failures (e.g., show a
user-facing error toast, restore any optimistic changes or keep the picker open)
instead of silently subscribing; use the existing
takeUntilDestroyed(this.destroyRef) and reference the methods onAttachInstance,
onDetachInstance, PathPairsService.update and the arrPickerPairId field when
implementing these handlers.
In `@src/python/common/path_pairs_config.py`:
- Around line 30-39: The PathPair constructor currently assigns duplicates to
self.arr_target_ids; change the assignment in the initializer (the PathPair
__init__ that sets self.arr_target_ids) to deduplicate the incoming
arr_target_ids while preserving order (e.g., by converting to an ordered-unique
sequence) so duplicates are never stored and duplicate scan commands can't be
emitted.
In `@src/python/seedsync.py`:
- Around line 449-462: The migration updates PathPairsConfig in-memory (via
path_pairs_config.update_pair(pair)) but doesn't persist it, so if the process
dies after writing integrations (ic.to_file) the path_pairs.json changes are
lost; after the loop that sets pair.arr_target_ids and calls update_pair on each
pair, call the PathPairsConfig save API (e.g., path_pairs_config.persist() or
path_pairs_config.to_file(...)—use whatever save method exists on
PathPairsConfig) to write the updated pairs to disk before proceeding to
ic.to_file(file_path) and returning ic.
- Around line 125-131: The integrations migration is happening after defaults
may be written, which can delete the legacy [Integrations] section; move the
legacy integrations read/migration earlier so it runs before any config
rewrite/backfill. Specifically, call _load_integrations_config using
Seedsync.__FILE_INTEGRATIONS and integrations_path before invoking
_backfill_config_defaults (or any code path that calls Config.to_file), ensuring
the legacy section is preserved and migrated prior to any Config.to_file write.
🪄 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: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: a1abfe84-0e53-4554-a1a4-06b75df7aaf8
📒 Files selected for processing (33)
src/angular/src/app/models/arr-instance.tssrc/angular/src/app/models/config.tssrc/angular/src/app/models/path-pair.tssrc/angular/src/app/pages/settings/integrations.component.htmlsrc/angular/src/app/pages/settings/integrations.component.scsssrc/angular/src/app/pages/settings/integrations.component.tssrc/angular/src/app/pages/settings/options-list.tssrc/angular/src/app/pages/settings/path-pairs.component.htmlsrc/angular/src/app/pages/settings/path-pairs.component.scsssrc/angular/src/app/pages/settings/path-pairs.component.spec.tssrc/angular/src/app/pages/settings/path-pairs.component.tssrc/angular/src/app/pages/settings/settings-page.component.htmlsrc/angular/src/app/pages/settings/settings-page.component.spec.tssrc/angular/src/app/pages/settings/settings-page.component.tssrc/angular/src/app/services/files/view-file.service.spec.tssrc/angular/src/app/services/settings/config.service.spec.tssrc/angular/src/app/services/settings/integrations.service.spec.tssrc/angular/src/app/services/settings/integrations.service.tssrc/python/common/__init__.pysrc/python/common/config.pysrc/python/common/context.pysrc/python/common/integrations_config.pysrc/python/common/path_pairs_config.pysrc/python/controller/arr_notifier.pysrc/python/seedsync.pysrc/python/tests/integration/test_web/test_handler/test_integrations.pysrc/python/tests/integration/test_web/test_handler/test_path_pairs.pysrc/python/tests/unittests/test_common/test_config.pysrc/python/tests/unittests/test_common/test_integrations_config.pysrc/python/tests/unittests/test_controller/test_arr_notifier.pysrc/python/web/handler/integrations.pysrc/python/web/handler/path_pairs.pysrc/python/web/web_app_builder.py
💤 Files with no reviewable changes (4)
- src/angular/src/app/services/settings/config.service.spec.ts
- src/angular/src/app/pages/settings/options-list.ts
- src/angular/src/app/models/config.ts
- src/python/tests/unittests/test_common/test_config.py
- Reformat web_app_builder.py per ruff. - Drop the helper PathPairDict alias and accept dict[str, Any] in PathPair.from_dict; use cast(list[Any]) to satisfy pyright on element narrowing. - Inline the legacy-Integrations parser into __migrate_legacy_integrations so kwargs flow through with their declared types instead of being unpacked from a loose dict. - Apply the same Any+cast pattern in the path_pairs handler validator and annotate its return tuple as list[str]. - Make the integrations card use h3.card-header so the "Integrations" heading still satisfies the settings-page E2E expectation, and scope the path-pairs page object selectors to .path-pairs so Playwright's strict mode no longer collides with the new sibling card. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
- WCAG: switch the .kind-radarr badge to dark text on amber (#1a1a1a / #e6a532)
for ~7.3:1 contrast — white-on-amber was ~2.6:1 and failed AA.
- Surface failures from path-pair attach/detach instead of swallowing them.
Picker close now happens only on a non-null next, and error/null both set
errorMessage so the user gets feedback. Conflicts on detach are surfaced too.
- Dedupe arr_target_ids in PathPair.__init__ via dict.fromkeys, so a malformed
payload (or future regression) can't make ArrNotifier emit duplicate scans.
- Defer the legacy-Integrations rewrite until after migration:
- _backfill_config_defaults is now applied in-memory only; the to_file write
moves to AFTER _load_integrations_config, so the [Integrations] section in
settings.cfg survives long enough to be migrated.
- _load_integrations_config now persists path_pairs.json BEFORE
integrations.json. A crash between the two writes is recoverable: the
next boot has no integrations.json, re-runs the migration, and overwrites
the attached IDs cleanly. The reverse order would leave instances with no
pairs referencing them.
- Tests cover the dedup invariant on PathPair and the migration ordering
(path_pairs.json written first, no migration when integrations.json exists,
no migration when settings.cfg has no [Integrations] section).
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
… gaps Code fixes from PR #426 review: - Corrupt integrations.json returns empty config instead of re-migrating (C2) - IntegrationsService.refresh() preserves existing list on HTTP error (C3) - onToggleEnabled() shows error and refreshes on failure (C4) - ValueError catch in integrations handler maps to proper status codes (I3) - arr_target_ids validated against IntegrationsConfig on create/update (I4) - Migration logs warning when configparser.Error blocks legacy read (I5) - migrate_from_legacy uses add_instance() instead of direct append (I6) - _send_post log includes target URL for multi-instance disambiguation (M1) Test additions: - _backfill_config_defaults: 2 tests (T1) - _load_path_pairs_config migration: 4 tests (T2) - PathPairsConfig round-trip and malformed input: 2 tests (T4) - Radarr handler parity (missing_api_key, bad_scheme, leak): 3 tests (T6) - PathPairsComponent error paths (null create/update): 2 tests (T7) - _send_post scheme guard: 1 test (T8) - Unknown arr_target_ids validation: 2 tests (I4) - Refresh error preserves list: 1 test updated (C3) Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/angular/src/app/pages/settings/integrations.component.html`:
- Around line 39-40: The Sonarr/Radarr buttons currently lack explicit type
attributes which can cause unintended form submission; update both <button>
elements that call onStartAdd('sonarr') and onStartAdd('radarr') to include
type="button" so they behave as non-submit buttons (i.e., change the two buttons
that invoke the onStartAdd method to explicitly set type="button").
- Around line 53-54: The Save and Cancel buttons (the elements wired to
onSaveAdd() and onCancelAdd()) lack explicit types and may trigger form
submission if placed inside a form; update the two button elements that call
onSaveAdd and onCancelAdd to include type="button" so they act as non-submitting
action buttons and avoid unintended form submits.
🪄 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: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 50e19963-4190-418c-9627-27fba5c1b24c
📒 Files selected for processing (22)
src/angular/src/app/pages/settings/integrations.component.htmlsrc/angular/src/app/pages/settings/integrations.component.scsssrc/angular/src/app/pages/settings/integrations.component.tssrc/angular/src/app/pages/settings/path-pairs.component.spec.tssrc/angular/src/app/pages/settings/path-pairs.component.tssrc/angular/src/app/services/settings/integrations.service.spec.tssrc/angular/src/app/services/settings/integrations.service.tssrc/e2e-playwright/tests/pages/path-pairs.page.tssrc/e2e-playwright/tests/path-pairs.spec.tssrc/python/common/integrations_config.pysrc/python/common/path_pairs_config.pysrc/python/controller/arr_notifier.pysrc/python/seedsync.pysrc/python/tests/integration/test_web/test_handler/test_integrations.pysrc/python/tests/integration/test_web/test_handler/test_path_pairs.pysrc/python/tests/integration/test_web/test_web_app.pysrc/python/tests/unittests/test_common/test_path_pairs_config.pysrc/python/tests/unittests/test_controller/test_arr_notifier.pysrc/python/tests/unittests/test_seedsync.pysrc/python/web/handler/integrations.pysrc/python/web/handler/path_pairs.pysrc/python/web/web_app_builder.py
Comment on lines
+39
to
+40
| <button class="btn btn-sm btn-add" (click)="onStartAdd('sonarr')">+ Sonarr</button> | ||
| <button class="btn btn-sm btn-add" (click)="onStartAdd('radarr')">+ Radarr</button> |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Add type="button" to prevent unintended form submission.
The "Add Sonarr/Radarr" buttons lack explicit type attributes. While not inside a <form>, adding type="button" is defensive and aligns with HTMLHint's recommendation.
🔧 Proposed fix
- <button class="btn btn-sm btn-add" (click)="onStartAdd('sonarr')">+ Sonarr</button>
- <button class="btn btn-sm btn-add" (click)="onStartAdd('radarr')">+ Radarr</button>
+ <button type="button" class="btn btn-sm btn-add" (click)="onStartAdd('sonarr')">+ Sonarr</button>
+ <button type="button" class="btn btn-sm btn-add" (click)="onStartAdd('radarr')">+ Radarr</button>📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| <button class="btn btn-sm btn-add" (click)="onStartAdd('sonarr')">+ Sonarr</button> | |
| <button class="btn btn-sm btn-add" (click)="onStartAdd('radarr')">+ Radarr</button> | |
| <button type="button" class="btn btn-sm btn-add" (click)="onStartAdd('sonarr')">+ Sonarr</button> | |
| <button type="button" class="btn btn-sm btn-add" (click)="onStartAdd('radarr')">+ Radarr</button> |
🧰 Tools
🪛 HTMLHint (1.9.2)
[warning] 39-39: The type attribute must be present on elements.
(button-type-require)
[warning] 40-40: The type attribute must be present on
elements.(button-type-require)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/angular/src/app/pages/settings/integrations.component.html` around lines
39 - 40, The Sonarr/Radarr buttons currently lack explicit type attributes which
can cause unintended form submission; update both <button> elements that call
onStartAdd('sonarr') and onStartAdd('radarr') to include type="button" so they
behave as non-submit buttons (i.e., change the two buttons that invoke the
onStartAdd method to explicitly set type="button").
Comment on lines
+53
to
+54
| <button class="btn btn-sm btn-save" (click)="onSaveAdd()">Save</button> | ||
| <button class="btn btn-sm btn-cancel" (click)="onCancelAdd()">Cancel</button> |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Add type="button" to form action buttons.
These buttons should have explicit type="button" to prevent default form submission behavior if the template is ever wrapped in a form element.
🔧 Proposed fix
- <button class="btn btn-sm btn-save" (click)="onSaveAdd()">Save</button>
- <button class="btn btn-sm btn-cancel" (click)="onCancelAdd()">Cancel</button>
+ <button type="button" class="btn btn-sm btn-save" (click)="onSaveAdd()">Save</button>
+ <button type="button" class="btn btn-sm btn-cancel" (click)="onCancelAdd()">Cancel</button>📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| <button class="btn btn-sm btn-save" (click)="onSaveAdd()">Save</button> | |
| <button class="btn btn-sm btn-cancel" (click)="onCancelAdd()">Cancel</button> | |
| <button type="button" class="btn btn-sm btn-save" (click)="onSaveAdd()">Save</button> | |
| <button type="button" class="btn btn-sm btn-cancel" (click)="onCancelAdd()">Cancel</button> |
🧰 Tools
🪛 HTMLHint (1.9.2)
[warning] 53-53: The type attribute must be present on elements.
(button-type-require)
[warning] 54-54: The type attribute must be present on
elements.(button-type-require)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/angular/src/app/pages/settings/integrations.component.html` around lines
53 - 54, The Save and Cancel buttons (the elements wired to onSaveAdd() and
onCancelAdd()) lack explicit types and may trigger form submission if placed
inside a form; update the two button elements that call onSaveAdd and
onCancelAdd to include type="button" so they act as non-submitting action
buttons and avoid unintended form submits.
Prevents unintended form submission if the template structure changes to include a <form> wrapper. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
This was referenced Apr 30, 2026
1 task
nitrobass24
added a commit
that referenced
this pull request
May 20, 2026
In IntegrationsHandler.__handle_delete the two on-disk writes ran in the wrong order: integrations.json first, then path_pairs.json. A crash between writes left integrations.json without the instance but path_pairs.json still referencing it — a dangling arr_target_id that the cross-validation in #426 rejects on next load. Swap the order so path_pairs persists first. The worst-case crash outcome is now an orphaned (no-longer-referenced) instance in integrations.json, which is harmless — it survives load and gets cleaned up the next time the user retries the delete. Adds two integration tests: one spying both to_file calls to assert the call order, one patching the second write to raise OSError and verifying the persisted path_pairs.json has the detach applied. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
integrations.json.arr_target_idsand a chip-row UI for attaching/detaching configured instances.pair.arr_target_idsinstead of broadcasting to a global pair of services.[Integrations]section insettings.cfgis migrated on startup and dropped on the next persist; behavior is preserved by attaching the migrated instances to every existing path pair.Closes #425.
Scope
Backend
common/integrations_config.py(new):ArrInstance+IntegrationsConfigPersist with CRUD + legacy migration.common/path_pairs_config.py: PathPair gainsarr_target_ids; newdetach_arr_target(id)helper.common/config.py: drop the flatIntegrationsinner class; remove its sensitive-property entries.controller/arr_notifier.py: per-pair routing, no orphan-file fallback, kind dispatch (DownloadedEpisodesScan/DownloadedMoviesScan).web/handler/integrations.py: REST CRUD + per-instance test endpoint, redactsapi_keyand respects the redacted sentinel on PUT.web/handler/path_pairs.py: validates and persistsarr_target_idson create/update.seedsync.py: loadsintegrations.json; on first boot, reads the old[Integrations]section directly fromsettings.cfg, builds instances, attaches them to every existing pair, and writes JSON.Frontend
app-integrationsstandalone component (list + form + per-instance Test Connection).Notify:chip row with picker for available instances.IntegrationsServicerewritten as full CRUD withinstances$BehaviorSubject; static Sonarr/RadarrOPTIONS_CONTEXT_*removed.Migration behavior
sonarr_url/radarr_urlset: one named instance per service is created and attached to every existing path pair, so today's notifications keep firing.integrations.jsonstarts empty; nothing fires until the user adds an instance and attaches it.pair_id(orphans, outside any pair): skipped — there's no way to know which *arr owns them in a multi-instance world.Test plan
cd src/python && PYTHONPATH=. python3 -m pytest tests/unittests/test_common tests/unittests/test_controller/test_arr_notifier.py tests/integration/test_web/test_handler/test_integrations.py tests/integration/test_web/test_handler/test_path_pairs.py— 166 passed.cd src/angular && npx ng lint— clean.cd src/angular && npx ng test— 315 passed.cd src/angular && npx ng build— succeeds.[Integrations]section and a path pair: confirmintegrations.jsonis created with the migrated instance attached, settings.cfg is rewritten without the section on next save, and the Settings page renders the new Integrations card.Activity → Queueshows the scan. Repeat for Radarr.🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Bug Fixes