fix(sync): detect a missing sync engine and stop reporting false success (closes #209) - #211
Merged
Merged
Conversation
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.
Problem
nextsyncdelegates reconciliation to an external engine (nextcloudcmd/opencloudcmd). When that binary is not on$PATH:Failed("Synchronization failed - view the log"), andEngineRun::Direct(SyncOutcome::Success)) without ever resolving the engine - so the row reported "Synchronization completed" for days while nothing was actually synchronized.The concrete trigger: removing the official Nextcloud desktop package (e.g. to stop its D-Bus/Nautilus auto-start, #208) also removes
nextcloudcmd, and the app kept looking healthy.Fix
engine_threadnow builds the engine command before the remote ensurer and the ETag gate, mappingCommandError::MissingBinaryto the new distinct outcomeSyncOutcome::EngineMissing(other command errors keep the genericFailed).resolve_binarytreats an explicit executable override pointing at a path that no longer exists asMissingBinarytoo (a stale configured path surfaces the actionable state instead of an opaque spawn failure).EngineMissingtoAppState::Errorwith a clear message ("Synchronization blocked: the sync engine is not installed"), never counts it as a success, and keeps retrying on every trigger - installing the engine package recovers without an app restart.outcome_log_line) and raises a desktop notification (failure_notification), so "view the log" is meaningful.Tests (red-green)
missing_engine_blocks_the_etag_gate_success- unchanged ETag + missing binary must not reportSuccess(was:Successwithout spawning anything).missing_engine_reports_engine_missing_on_manual_run- manual run with a missing engine reportsEngineMissing(was:Failed).missing_binary_maps_to_failedupdated: now expectsEngineMissing(the old behavior this issue changes).engine_missing_sets_error_and_recovers_on_retry- scheduler state, message mentions the engine, and recovery on the next trigger.resolve_binary_rejects_a_missing_explicit_override, notifications coverage, outcome-log coverage.Gate: 712 passed + 1 ignored (known flaky), clippy
--all-targetsclean, fmt clean, i18n parity diff clean.Note
Local package built and running (0.2.22-1). With this merged, #208 becomes safe to execute.