Repository navigation
fix(web): keep an autoscan source's stored config on unrelated edits - #2084
Conversation
Saving the source edit dialog always rebuilt source_config from the form, which fills in declared defaults, drops keys the descriptor no longer declares and folds legacy keys. When that differed from the stored map, the server took it as a config change and reset the source's poll marker, so a label or interval edit restarted the source from now. Send the stored config back unless the config form or the webhook provider was changed, as the list's enable switch already does.
A row holding the superseded nested-path keys is migrated forward by its first save from the editor. Keep doing that when only another field was edited, instead of leaving the legacy keys until the config is changed.
Silo Kody — review completeReview finished. Check the inline comments for findings and verify each suggestion against the code and tests. Reviewing changes in Silo
Review settingsReview OptionsThe following review options are enabled or disabled:
|
Quick104
added
priority: P2
Cursor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configuration
📒 Files selected for processing (2)
Included review availability: This review used your included allowance. Your plan provides up to 4 included reviews per hour; 0 remain after this review. 📝 WalkthroughWalkthroughThe source edit flow now preserves stored ChangesAutoscan source configuration
Priority: ⬇️ Low Estimated code review effort: 2 (Simple) | ~10 minutes Change: Bug fix Suggested reviewers: Merge Risk: ⚪ Minimal · up to Editing unrelated fields on an autoscan source now keeps its stored configuration, and legacy CephFS keys are still migrated on first save. No merge-blocking issues were found. Pre-merge checks |
 |
| Check name | Status | Explanation |
|---|---|---|
| Docstring Coverage |  |
Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 3 functions across 2 files. |
| Linked Issues check | |
Check skipped because no linked issues were found for this pull request. |
| Out of Scope Changes check | |
Check skipped because no linked issues were found for this pull request. |
| Title check | |
The title clearly and concisely describes the main change: preserving an autoscan source's stored configuration when unrelated fields are edited. |
| Description check | |
The description is directly related to the changeset. It explains the problem, implementation, migration behavior, tests, validation, and risks. |
✨ Finishing Touches
🧪 Generate unit tests (beta)
- Create a new PR
- Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
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 help to get the list of available commands.
Review summaryReviewed head: Fit: 8/10 · Readiness: 8/10 · conditional Fit. This is a bounded fix in the web client for a real correctness problem: a label or interval edit made a poll source lose its place. It saves the way the list's enable switch already does. It isn't 9 or higher because the root cause is in Readiness. I found no defect in the code, and the checks pass. It isn't 9 or higher because no save has been made against a live poll source, and #1232 C1 hasn't been run. Findings
What I checked
Validated featuresNo passed case on the v1.0 board is reached. The Autoscan web admin case, #1232 C1, hasn't been run yet. The PR body's Other checks
Reviewed with Claude Code (claude-opus-5-5). |
2 participants
Problem
Related issue: #1230
Validation tasks: touches #1232 C1 (no result yet)
Saving an autoscan source in the edit dialog can restart its polling from now, even when only the label, interval or path mappings changed. The dialog always rebuilds
source_configfrom the form before sending it. That fills in declared defaults, drops keys the descriptor no longer declares, and folds the legacy CephFS path keys. If the rebuilt map differs from the stored one,UpdateSourcetreats it as a config change and clears the poll marker (#1935), so whatever the source's server changed since the last poll is never reported. This change sends the stored config back unchanged unless the config form or the webhook provider was edited.Quick found this in the #1935 review, and it was left for after the editor rewrite in #1929. Sonarr and Radarr sources have no config form and aren't affected. The trigger is a plugin with one, such as CephFS: a source whose stored config lacks a field with a default (for example after Exclusions was cleared, which repeats on every save).
Approach
editedSourceBodysendssource_configexactly as stored unless the draft'sconfigDirtyflag is set (the schema form sets it on any change) or the webhook provider differs from the stored one. The provider has its own control that deliberately doesn't setconfigDirty, so it's compared directly.sourceUpdateBody, which the list's enable switch already uses and which sends the stored config unchanged.Validation
sourceFormtests: a label and interval edit sends the stored config even when the rebuilt one has an extra default, a provider change sends the rebuilt config, and a CephFS row with legacy keys is still migrated on a label edit. The first fails on ca186fe: it sends{ lookback: '24h', window: '7d' }instead of the stored{ lookback: '24h' }. The existing test that builds the PUT from an edited config now marks the config as edited, which is what it covers.pnpm exec vitest run src/pages/admin/autoscan/: 16 files, 179 tests pass.tsc -p tsconfig.app.json --noEmit, ESLint and Prettier on the changed files: clean.Evidencecheck waits on the evidence link below.Benchmarks
Not applicable. This changes which config the dialog sends, not how much work a save does.
Evidence
Evidence: https://evidence.siloserver.org/r/silo-server/pr-2084/
The dialog looks the same before and after. What changes is the request it sends for a save that didn't touch the config, shown as before and after PUT bodies from the test, and whether the source keeps its poll position afterwards. I don't have a CephFS source to capture from.
Risks
None identified. A save that edits the config behaves as before; one that doesn't sends what is already stored.
Checklist
AI Disclosure
AI-assisted with Claude Opus. I directed the task and designed the work.