Autosync: decouple pull cadence from push quiet window - #690
Merged
Conversation
fiskus
marked this pull request as ready for review
May 21, 2026 12:10
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
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
Shipping as v0.18.1 — disk JSON stays backward compatible, so this is a UX iteration on the autosync settings popup rather than a new feature.
AutosyncSettingsinto nestedpull: PullSettings { enabled, focused_secs, unfocused_secs, closed_secs }andpush: PushSettings { enabled, idle_timeout_secs }. The disk JSON shape stays flat (via#[serde(flatten)]) so existing 0.18.0 files load unchanged.push.idle_timeout_secs(default 30 s) instead ofcadence_for_mode(...). Pull cadence and push quiet window are now independent knobs.update_autosync_settingsbecomes a merge over disk state: it takes a singleAutosyncSettingsDataarg, validates zero, and preservesclosed_secsflowing through untouched.LEGACY_FILE_NAME, rename step,#[serde(alias = "enabled")],any_enabled()helper) — 0.18.0 already shipped the new file name and flat schema.Behaviour change worth flagging
For users running an unfocused window on 0.18.0, the quiet window shrinks from 120 s to a constant 30 s. Quiet on the focused window path stays at 30 s. The new
idle_timeout_secsfield defaults to 30 and is editable in the popup.Test plan
cargo test --workspace— 581 passed / 0 failed / 1 ignoredcargo clippy --workspace -- -D warnings— cleanrumdl check . --respect-gitignore— cleanidle_timeout_secs, missing-keys default to false, publish defers while inside the idle window, focused→pull_interval projection on divergent files, merge preservesclosed_secs, validation rejects zero in either numeric fieldquilt-sync, open Settings → Autosync, verify two inputs render with current values and Save persistsautosync_settings.jsonto givefocused_secsandunfocused_secsdifferent values, reopen Settings, verify Save unifies them (UI ties focused == unfocused) whileclosed_secssurvives untouchedGreptile Summary
This PR decouples the autosync push quiet window from the pull cadence by splitting
AutosyncSettingsinto nestedPullSettingsandPushSettingsstructs. The disk JSON shape remains flat via#[serde(flatten)], so 0.18.0 files load unchanged, andidle_timeout_secsdefaults to 30 s when missing. The Settings popup is simplified to two numeric inputs, the legacy migration code is removed, andupdate_autosync_settingsis refactored to merge over disk state so thatclosed_secsis preserved without appearing in the UI.push.idle_timeout_secs(constant 30 s default) instead ofcadence_for_mode(...), meaning unfocused-window users who were on 0.18.0 will see their push quiet window drop from 120 s to 30 s — this behavioral change is clearly documented in the PR description and changelog.update_autosync_settingscommand now holds theSharedAutosyncSettingswrite lock across the async disk save in order to keep the disk and in-memory state atomic; this is correct but may briefly block the tick loop if it fires concurrently with a settings save.Confidence Score: 4/5
Safe to merge — the refactor is well-tested, backward-compatible on disk, and the one concern (holding a write lock during file I/O) is unlikely to cause observable problems in practice given the small file size and infrequent save operations.
The settings restructure is clean and comprehensively tested (round-trip, 0.18.0 compat, missing-key defaults, merge-preserves-closed_secs, idle-window deferral). The only code-level concern is that update_autosync_settings now holds the SharedAutosyncSettings write lock across the async save() call, whereas previously the save ran before the lock was acquired. The tick loop reads settings under a read lock, so a concurrent tick could block briefly during a settings save. Given the file is tiny and user-triggered saves are infrequent, this is not expected to surface as a user-visible issue.
quilt-sync/src-tauri/src/commands.rs — the write lock scope around the async save is the only area that warrants a second look.
Important Files Changed
Sequence Diagram
sequenceDiagram participant UI as UI (settings.rs) participant FECmd as ui/src/commands.rs participant BECmd as commands.rs (backend) participant Disk as autosync_settings.json participant Watcher as Watcher / Tick UI->>FECmd: update_autosync_settings(AutosyncSettingsData) FECmd->>BECmd: tauri::invoke(update_autosync_settings) BECmd->>BECmd: validate_autosync_settings_data() BECmd->>BECmd: autosync_settings.write() [WRITE LOCK acquired] BECmd->>BECmd: merge_autosync_settings_data(current, incoming) BECmd->>Disk: merged.save() [async I/O while lock held] Disk-->>BECmd: Ok BECmd->>BECmd: "*current = merged" BECmd->>BECmd: [WRITE LOCK released] BECmd->>Watcher: clear_all_paused() [if off-on edge] BECmd-->>UI: Ok(()) Note over Watcher: On each tick Watcher->>BECmd: settings.read() — pull.enabled / push.enabled Watcher->>BECmd: settings.read() — push.idle_timeout_secs Watcher->>BECmd: "settings.read() — cadence_for_mode(&pull, mode)"Reviews (1): Last reviewed commit: "chore(autosync): retarget release as v0...." | Re-trigger Greptile