fix: keep the last sync's result when an upload sync finds nothing to do #34

Android starts an upload-only sync right after a manual sync, and 30 s
after any app write. When nothing waited on the device it ended at
once, but it still stored its status, so the settings showed a plain
"synced" and lost the conflicts and item errors of the run just
before it.

Such a run did nothing and now leaves the stored status alone.

src/device-sync/__tests__/engine/contacts-flow.test.ts

@@ -2,7 +2,7 @@
 // server: first sync, incremental /changes, uploads, echoes, selection
 // changes and groups (docs/device-sync.md, "A sync run").
 
-import { describe, expect, it } from 'vitest';
+import { describe, expect, it, vi } from 'vitest';
 import { Data, MimeType } from '../../android-columns';
 import { runDeviceSync } from '../../engine/run';
 import {
@@ -295,6 +295,23 @@ describe('device sync engine: contacts', () => {
     expect(h.server.requests.length).toBe(requests);
   });
 
+  it("keeps the last run's status (its conflicts and item errors) when an upload-only sync finds nothing to do", async () => {
+    const h = createHarness();
+    addServerCards(h, ['Ada']);
+    await h.run();
+    const recordStatus = vi.mocked(h.deps.recordStatus);
+    const recorded = recordStatus.mock.calls.length;
+
+    // The upload sync Android starts right after a manual one: nothing waits on the device.
+    expect((await h.run(CONTACTS_AUTHORITY, { upload: true })).outcome).toBe('ok');
+
+    expect(recordStatus.mock.calls.length).toBe(recorded);
+    // One that has something to upload is a run like any other.
+    renameDeviceContact(h, h.contactNamed('Ada')!.id, 'Ada King');
+    await h.run(CONTACTS_AUTHORITY, { upload: true });
+    expect(recordStatus.mock.calls.length).toBe(recorded + 1);
+  });
+
   it('reports disabled, unsupported and rows of another owner without touching anything', async () => {
     const off = createHarness();
     off.deps.isSyncEnabled = async () => false;

src/device-sync/engine/run.ts

@@ -157,15 +157,19 @@ export async function runDeviceSync(payload: RunPayload, deps: EngineDeps): Prom
   const key = lockKey(payload.registryId, payload.authority);
   const release = await acquireLock(key, Math.max(0, payload.deadline - deps.now() - tuning.deadlineMarginMs));
   let result: RunReport;
+  const run = { idle: false };
   if (!release) {
     result = report.build('cancelled', deps.now(), { message: 'Another sync of this account is still running', moreRecordsToGet: true });
   } else {
     try {
-      result = await runLocked(payload, deps, tuning, report, key);
+      result = await runLocked(payload, deps, tuning, report, key, run);
     } finally {
       release();
     }
   }
+  // An upload sync with nothing to upload (Android starts one right after a manual sync) did nothing:
+  // the last run's status, with its conflicts and item errors, stays what the settings show.
+  if (run.idle) return result;
   try {
     await deps.recordStatus(payload.registryId, payload.authority, statusOf(result, report.itemErrorCount));
   } catch (error) {
@@ -174,7 +178,14 @@ export async function runDeviceSync(payload: RunPayload, deps: EngineDeps): Prom
   return result;
 }
 
-async function runLocked(payload: RunPayload, deps: EngineDeps, tuning: Tuning, report: ReportBuilder, key: string): Promise<RunReport> {
+async function runLocked(
+  payload: RunPayload,
+  deps: EngineDeps,
+  tuning: Tuning,
+  report: ReportBuilder,
+  key: string,
+  run: { idle: boolean },
+): Promise<RunReport> {
   const finish = (outcome: RunReport['outcome'], extra: Parameters<ReportBuilder['build']>[2] = {}) =>
     report.build(outcome, deps.now(), extra);
   const extras = payload.extras ?? {};
@@ -189,7 +200,10 @@ async function runLocked(payload: RunPayload, deps: EngineDeps, tuning: Tuning,
       return finish('internal', { message: 'The rows on this device were written for another account' });
     }
     // Providers schedule an upload sync for every account 30 s after any app write.
-    if (extras.upload && !(await hasLocalWork(reader, payload.authority, stored))) return finish('ok');
+    if (extras.upload && !(await hasLocalWork(reader, payload.authority, stored))) {
+      run.idle = true;
+      return finish('ok');
+    }
 
     const connection = await deps.jmap(payload.registryId);
     if (stored.owner && stored.owner.origin !== connection.origin) {