Address review findings

cockatrice/src/interface/deck_sync/deck_sync_manager.cpp

@@ -858,6 +858,16 @@ void DeckSyncManager::queueConflict(const QString &relativePath,
                                .guid = serverGuid,
                                .hash = serverHash,
                                .lastModifiedSecs = serverLastModifiedSecs};
+    if (autoRun) {
+        // An auto run has no user in front of it: opening the modal conflict
+        // dialog from a timer or a file-watcher event would pop up mid-game.
+        // Leave the path untouched so the next manual sync classifies it again
+        // and asks — and skip the server fetch, which would only be discarded
+        // and repeated on every poll.
+        ++conflictCount;
+        ++skippedCount;
+        return;
+    }
     pendingConflicts.insert(relativePath, ctx);
     conflictPaths.insert(relativePath);
     applyDownload(relativePath, deckId, serverGuid, serverHash, serverLastModifiedSecs);
@@ -865,17 +875,10 @@ void DeckSyncManager::queueConflict(const QString &relativePath,
 
 void DeckSyncManager::handleConflictDownload(const QString &relativePath, const QString &serverDeckString)
 {
+    // Only a manual run fetches a conflicting server copy, so this point is
+    // never reached with autoRun set: queueConflict() records the conflict and
+    // leaves the path alone instead.
     const ConflictDownload ctx = pendingConflicts.take(relativePath);
-    if (autoRun) {
-        // An auto run has no user in front of it: opening the modal conflict
-        // dialog from a timer or a file-watcher event would pop up mid-game.
-        // Leave the path untouched so the next manual sync classifies it again
-        // and asks.
-        ++conflictCount;
-        ++skippedCount;
-        processNext();
-        return;
-    }
     if (sessionDefault == DeckSyncConflictKeepMine) {
         applyUpload(relativePath, ctx.serverPath, ctx.deckId, ctx.guid);
         processNext();
@@ -1349,7 +1352,8 @@ void DeckSyncManager::onDeleteFinished(const Response &response,
         }
     }
     // Both outcomes converge the record: pushed deletes and server-side
-    // deletes (already tombstoned, answering NameNotFound) are done.
+    // deletes (already tombstoned, answering NameNotFound) are done. Either
+    // way the server tree the client is showing is stale, so both count.
     if (response.response_code() == Response::RespOk || response.response_code() == Response::RespNameNotFound) {
         DeckSyncSidecar sidecar(sidecarPath);
         sidecar.load();
@@ -1418,10 +1422,9 @@ void DeckSyncManager::finishSync()
     if (!summary.note.isEmpty()) {
         message += QStringLiteral(" ") + summary.note;
     }
+    // The conflict count travels in the summary: a statusMessage here would be
+    // overwritten by the syncFinished() summary the tab renders next.
     emit statusMessage(message);
-    if (conflictCount > 0) {
-        emit statusMessage(tr("%1 conflicting decks skipped. Press Sync now to resolve them.").arg(conflictCount));
-    }
     emit syncFinished(summary, QString());
 }
 
@@ -1433,6 +1436,7 @@ DeckSyncManager::SyncSummary DeckSyncManager::currentSummary() const
         .removed = removedCount,
         .unchanged = unchangedCount,
         .skipped = skippedCount,
+        .conflicts = conflictCount,
         .note = syncNote,
     };
 }

cockatrice/src/interface/deck_sync/deck_sync_manager.h

@@ -70,6 +70,8 @@ public:
         int removed = 0; ///< Decks deleted on the server, or locally to mirror a server delete.
         int unchanged = 0;
         int skipped = 0;
+        /** @brief Both-sides-changed decks left untouched for a manual verdict. */
+        int conflicts = 0;
         QString note; ///< Non-error caveat about this run, empty when there is none.
     };
 

cockatrice/src/interface/widgets/tabs/tab_deck_storage.cpp

@@ -668,7 +668,9 @@ void TabDeckStorage::onSyncFinished(const SyncSummary &summary, const QString &e
         lastSyncSummary = summary;
     }
     renderSyncStatus();
-    if (error.isEmpty()) {
+    // An idle poll changes nothing: refreshing the tree there would send a
+    // fresh Command_DeckList every interval for every connected client.
+    if (error.isEmpty() && summary.uploaded + summary.downloaded + summary.removed > 0) {
         serverDirView->refreshTree();
     }
 }
@@ -689,6 +691,10 @@ void TabDeckStorage::renderSyncStatus()
                                .arg(lastSyncSummary.removed)
                                .arg(lastSyncSummary.unchanged)
                                .arg(lastSyncSummary.skipped);
+            if (lastSyncSummary.conflicts > 0) {
+                text += QStringLiteral(" ") + tr("%n conflicting deck(s) skipped. Press Sync now to resolve them.", "",
+                                                 lastSyncSummary.conflicts);
+            }
             if (!lastSyncSummary.note.isEmpty()) {
                 text += QStringLiteral("\n") + lastSyncSummary.note;
             }

servatrice/src/serversocketinterface.cpp

@@ -905,7 +905,7 @@ bool AbstractServerSocketInterface::syncCheckHelper(int userId, qint64 sinceSecs
     // their files linger as tombstones, so walking the live folder tree would
     // miss deletes pointing at nonexistent folders. The client only checks
     // changed_files_size(), so folder paths are not reconstructed here.
-    // last_modified is backfilled by the 37 -> 38 migration, so a plain range
+    // last_modified is backfilled by the 0040 -> 0041 migration, so a plain range
     // check is enough.
     QSqlQuery *query = sqlInterface->prepareQuery("select id, guid, hash, last_modified, deleted from "
                                                   "{prefix}_decklist_files where id_user = :id_user "