feat: sync ETag concurrency + fix chunk-transfer hang + sync overlay + auto-VACUUM

Sync optimistic concurrency (ETag/If-Match) - webdav GET captures the response ETag; PUT sends it back as If-Match so the server rejects (412) our write when another device changed the file between our pull and push. A 412 re-runs the whole pull→merge→push (bounded to 3) so the other device's changes are folded in instead of clobbered. Servers without ETags → empty etag → no If-Match → falls back to last-write-wins (no regression). onWebdavResult gained a 4th etag arg. Chunked webdav PUT (big vaults no longer black-screen on sync) - The whole encrypted snapshot base64'd into a single cmd://webdav/put URL blew past WebView2's cap → black screen once the vault grew (20MB of attachments). PUT bodies now stream through the file/chunk transport and commit via a new webdav/put-commit (reads the accumulated buffer). Chunk-transfer hang fix (root cause of the stuck "Preparing…" sync) - All chunked transfers (saveFile/writeFile/webdav PUT) share one reqId-keyed resolver. A resolved chunk's stale 30s timeout would later delete the CURRENT chunk's resolver and fire the wrong res(), leaving that chunk's await pending forever. Extracted a single _streamChunks() helper whose ack CLEARS the pending timeout, so resolvers stay strictly one-at-a-time. Also fixed _webdavCall referencing the Bridge-local cmd() from module scope (latent ReferenceError). Sync busy overlay - syncStatus() now drives the global busy overlay too, so a running sync blocks stray clicks (e.g. the auto-backup "Choose…" picker) and reads like the manual backup. The account-mismatch confirm hideBusy()s first so it's visible above the overlay. Auto-VACUUM (reclaim space after deleting large attachments) - SQLite never shrinks the file on DELETE, so deleting big attachments left vault.db bloated (35MB for 11 tiny entries). DB.CompactIfBloated VACUUMs when >20% of pages are free AND >~2MB is reclaimable — called on startup and after each attachment delete. A healthy small vault pays nothing. (Verified: 35MB → 695KB after the deletes.) Rebuild: BuildAssets + F9 (UMainForm + PM.Database + PM.Handler.Attachments). Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
2026-07-04 00:02:25 +01:00
parent b367d031b5
commit ac909f4f09
6 changed files with 195 additions and 57 deletions
@@ -499,8 +499,14 @@ device, never transmitted.
   restore attachments ; both sides have it = compare `updated_at`,
   PUT if remote newer.
 7. `loadEntries()` + `buildSyncSnapshot()` for the post-merge state.
-8. `webdav/put` push the merged snapshot.
-9. Toast `X added · Y updated · Z deleted`.
+8. `webdav/put` push the merged snapshot, with `If-Match: <etag>` where
+   the etag was captured from the step-2 pull (optimistic concurrency).
+   A **412** means another device changed the file between our pull and
+   push → re-run the whole pull→merge→push (`runSyncNow(_attempt+1)`,
+   bounded to 3) so their changes are folded in instead of clobbered.
+   Servers without ETag support return an empty etag → no If-Match sent
+   → falls back to last-write-wins (same as before).
+9. Toast `pulled X new · Y updated · Z deleted · pushed N entries`.

 Sensitive actions (export, change master pw, recovery code…) still
 require master pw via `askReauth` — sync never substitutes.
@@ -431,6 +431,11 @@ begin
    DB.Unlock;
  end;

+  // Attachments are the big rows; deleting one leaves the file bloated
+  // until VACUUM'd. Compact now (no-op unless the free ratio is high) so
+  // disk space is reclaimed immediately, not just on the next launch.
+  DB.CompactIfBloated;
+
  LogAudit(LUserId, 'delete_attachment', GetClientIP(ARequest));
  TJSONHelper.SendOK(AResponse, 'Deleted');
 end;
@@ -33,6 +33,11 @@ type
    destructor Destroy; override;
    procedure Lock;
    procedure Unlock;
+    // VACUUM the file when a large fraction of its pages are free (SQLite
+    // never shrinks on DELETE — deleting big attachments leaves the file
+    // bloated). No-op when the free ratio is low, so the common case pays
+    // nothing. Caller must NOT hold the lock or be in a transaction.
+    procedure CompactIfBloated;
    property Connection: TFDConnection read FConn;
    property DBPath: string read FDBPath;
  end;
@@ -61,6 +66,10 @@ begin
  CreateSchema;
  ApplyMigrations;
  CleanupExpired;
+  // Reclaim space left behind by previously-deleted large rows
+  // (attachments especially). Only actually rewrites the file when it's
+  // meaningfully bloated, so a healthy small vault opens instantly.
+  CompactIfBloated;
 end;

 destructor TPMDatabase.Destroy;
@@ -70,6 +79,36 @@ begin
  inherited;
 end;

+procedure TPMDatabase.CompactIfBloated;
+var
+  LQ: TFDQuery;
+  LFree, LTotal: Int64;
+begin
+  FLock.Enter;
+  try
+    LFree := 0; LTotal := 0;
+    LQ := TFDQuery.Create(nil);
+    try
+      LQ.Connection := FConn;
+      LQ.SQL.Text := 'PRAGMA freelist_count';
+      LQ.Open; if not LQ.IsEmpty then LFree := LQ.Fields[0].AsLargeInt;
+      LQ.Close;
+      LQ.SQL.Text := 'PRAGMA page_count';
+      LQ.Open; if not LQ.IsEmpty then LTotal := LQ.Fields[0].AsLargeInt;
+      LQ.Close;
+    finally
+      LQ.Free;
+    end;
+    // Rewrite only when >20% of the pages are free AND there's at least a
+    // few MB to reclaim (avoids churning a tiny vault). VACUUM can't run
+    // inside a transaction, so this must be called outside one.
+    if (LTotal > 0) and (LFree * 5 > LTotal) and (LFree > 512) then
+      FConn.ExecSQL('VACUUM');
+  finally
+    FLock.Leave;
+  end;
+end;
+
 procedure TPMDatabase.Lock;
 begin
  FLock.Enter;
@@ -1101,7 +1101,8 @@ begin
  // - PUT ok  → status=200/201/204, body=''
  // - test    → status=200..399 means reachable, body=''
  // Network errors → status=0, body=exception message.
-  else if (ACmd = 'webdav/get') or (ACmd = 'webdav/put') or (ACmd = 'webdav/test') then
+  else if (ACmd = 'webdav/get') or (ACmd = 'webdav/put') or (ACmd = 'webdav/test')
+       or (ACmd = 'webdav/put-commit') then
  begin
    var LMethod := ACmd;
    var LReqId  := GetParam('reqId');
@@ -1109,6 +1110,26 @@ begin
    var LUser   := GetParam('user');
    var LPwd    := GetParam('pwd');
    var LData   := GetParam('data');
+    // put-commit: the (large) body arrived in chunks via file/chunk,
+    // accumulated in FFileSaveChunks keyed by reqId. Pull it out and treat
+    // the rest as a normal PUT.
+    if ACmd = 'webdav/put-commit' then
+    begin
+      var LSB: TStringBuilder;
+      if FFileSaveChunks.TryGetValue(LReqId, LSB) then
+      begin
+        LData := LSB.ToString;
+        LSB.Free;
+        FFileSaveChunks.Remove(LReqId);
+      end
+      else
+        LData := '';
+      LMethod := 'webdav/put';
+    end;
+    // Optimistic concurrency: JS passes the ETag it saw at pull time; we
+    // send it as If-Match on the push so the server rejects (412) the
+    // write when another device changed the file in between.
+    var LIfMatch := GetParam('ifmatch');
    TThread.CreateAnonymousThread(
      procedure
      var
@@ -1120,10 +1141,12 @@ begin
        LBodyB64: string;
        LStatus: Integer;
        LErr: string;
+        LEtag: string;
      begin
        LStatus  := 0;
        LBodyB64 := '';
        LErr     := '';
+        LEtag    := '';
        try
          LHttp := System.Net.HttpClient.THTTPClient.Create;
          try
@@ -1141,6 +1164,7 @@ begin
              try
                LResp := LHttp.Get(LUrl, LBodyStream);
                LStatus := LResp.StatusCode;
+                LEtag := LResp.HeaderValue['ETag'];
                if (LStatus >= 200) and (LStatus < 300) and (LBodyStream.Size > 0) then
                begin
                  SetLength(LBytes, LBodyStream.Size);
@@ -1158,8 +1182,13 @@ begin
              LBytes := TNetEncoding.Base64.DecodeStringToBytes(LData);
              LReqStream := TBytesStream.Create(LBytes);
              try
-                LResp := LHttp.Put(LUrl, LReqStream);
+                if LIfMatch <> '' then
+                  LResp := LHttp.Put(LUrl, LReqStream, nil,
+                    [System.Net.URLClient.TNetHeader.Create('If-Match', LIfMatch)])
+                else
+                  LResp := LHttp.Put(LUrl, LReqStream);
                LStatus := LResp.StatusCode;
+                LEtag := LResp.HeaderValue['ETag'];
              finally
                LReqStream.Free;
              end;
@@ -1182,7 +1211,7 @@ begin
        TThread.Queue(nil,
          procedure
          var
-            LEscReq, LEscPayload: string;
+            LEscReq, LEscPayload, LEscEtag: string;
          begin
            LEscReq := StringReplace(LReqId, '"', '\"', [rfReplaceAll]);
            // GET success path → ship body base64. Otherwise the field
@@ -1196,12 +1225,16 @@ begin
            LEscPayload := StringReplace(LEscPayload, '"', '\"', [rfReplaceAll]);
            LEscPayload := StringReplace(LEscPayload, #13, '', [rfReplaceAll]);
            LEscPayload := StringReplace(LEscPayload, #10, '\n', [rfReplaceAll]);
+            LEscEtag := StringReplace(LEtag, '\', '\\', [rfReplaceAll]);
+            LEscEtag := StringReplace(LEscEtag, '"', '\"', [rfReplaceAll]);
+            LEscEtag := StringReplace(LEscEtag, #13, '', [rfReplaceAll]);
+            LEscEtag := StringReplace(LEscEtag, #10, '', [rfReplaceAll]);
            WebBrowser.ExecuteJavaScript(
              'if(window.Bridge&&Bridge.onWebdavResult)' +
              'Bridge.onWebdavResult("' + LEscReq + '",' +
-              IntToStr(LStatus) + ',"' + LEscPayload + '")');
-            LogLine(Format('%s %s → %d (%d bytes payload)',
-              [LMethod, LUrl, LStatus, Length(LEscPayload)]));
+              IntToStr(LStatus) + ',"' + LEscPayload + '","' + LEscEtag + '")');
+            LogLine(Format('%s %s → %d (%d bytes payload, etag=%s)',
+              [LMethod, LUrl, LStatus, Length(LEscPayload), LEtag]));
          end);
      end).Start;
  end
@@ -16,6 +16,36 @@ const API = (location.pathname.indexOf('/password-manager/') === 0)
 const prefResolvers = {};
 const fileSaveResolvers = {};
 const fileChunkResolvers = {};
+// Stream a base64 string to Delphi in URL-sized pieces via cmd://file/chunk,
+// each acked (onFileChunkAck) before the next is sent. Returns true when all
+// pieces are buffered server-side, false on a stalled chunk. The ack CLEARS
+// the pending timeout — without that, a resolved chunk's stale 30s timeout
+// would later delete the CURRENT chunk's resolver and hang the transfer
+// forever (only the last location.href navigation "wins" per event loop, so
+// resolvers must be strictly 1-at-a-time and their timers torn down).
+async function _streamChunks(reqId, b64, onProgress) {
+    const CHUNK = 1000000;
+    const total = Math.ceil(b64.length / CHUNK);
+    let done = 0;
+    for (let off = 0; off < b64.length; off += CHUNK) {
+        const piece = b64.slice(off, off + CHUNK);
+        const ok = await new Promise(res => {
+            const timer = setTimeout(() => {
+                if (fileChunkResolvers[reqId]) {
+                    delete fileChunkResolvers[reqId];
+                    res(false);
+                }
+            }, 30000);
+            fileChunkResolvers[reqId] = { resolve: res, timer };
+            window.location.href = 'cmd://file/chunk?reqId=' + encodeURIComponent(reqId) +
+                '&data=' + encodeURIComponent(piece);
+        });
+        if (!ok) return false;
+        done++;
+        if (typeof onProgress === 'function') onProgress(Math.round(done / total * 100));
+    }
+    return true;
+}
 let versionResolver = null;
 let launchModeResolver = null;
 const folderPickResolvers = {};
@@ -241,26 +271,8 @@ const Bridge = (() => {
                });
            }
            return (async () => {
-                const totalChunks = Math.ceil(b64.length / CHUNK);
-                let done = 0;
-                for (let off = 0; off < b64.length; off += CHUNK) {
-                    const piece = b64.slice(off, off + CHUNK);
-                    const ackOk = await new Promise(res => {
-                        fileChunkResolvers[reqId] = res;
-                        cmd('cmd://file/chunk?reqId=' + encodeURIComponent(reqId) +
-                            '&data=' + encodeURIComponent(piece));
-                        setTimeout(() => {
-                            if (fileChunkResolvers[reqId]) {
-                                delete fileChunkResolvers[reqId];
-                                res(false);
-                            }
-                        }, 30000);
-                    });
-                    if (!ackOk) return { ok: false, error: 'chunk transfer failed' };
-                    done++;
-                    if (typeof onProgress === 'function')
-                        onProgress(Math.round(done / totalChunks * 100));
-                }
+                const ok = await _streamChunks(reqId, b64, onProgress);
+                if (!ok) return { ok: false, error: 'chunk transfer failed' };
                return await new Promise(resolve => {
                    fileSaveResolvers[reqId] = resolve;
                    cmd('cmd://file/save-commit?reqId=' + encodeURIComponent(reqId) +
@@ -276,7 +288,11 @@ const Bridge = (() => {
        },
        onFileChunkAck(reqId) {
            const r = fileChunkResolvers[reqId];
-            if (r) { delete fileChunkResolvers[reqId]; r(true); }
+            if (r) {
+                delete fileChunkResolvers[reqId];
+                if (r.timer) clearTimeout(r.timer);
+                r.resolve(true);
+            }
        },
        onFileSaveResult(reqId, ok, path, err) {
            const r = fileSaveResolvers[reqId];
@@ -332,26 +348,8 @@ const Bridge = (() => {
                });
            }
            return (async () => {
-                const totalChunks = Math.ceil(b64.length / CHUNK);
-                let done = 0;
-                for (let off = 0; off < b64.length; off += CHUNK) {
-                    const piece = b64.slice(off, off + CHUNK);
-                    const ackOk = await new Promise(res => {
-                        fileChunkResolvers[reqId] = res;
-                        cmd('cmd://file/chunk?reqId=' + encodeURIComponent(reqId) +
-                            '&data=' + encodeURIComponent(piece));
-                        setTimeout(() => {
-                            if (fileChunkResolvers[reqId]) {
-                                delete fileChunkResolvers[reqId];
-                                res(false);
-                            }
-                        }, 30000);
-                    });
-                    if (!ackOk) return { ok: false, error: 'chunk transfer failed' };
-                    done++;
-                    if (typeof onProgress === 'function')
-                        onProgress(Math.round(done / totalChunks * 100));
-                }
+                const ok = await _streamChunks(reqId, b64, onProgress);
+                if (!ok) return { ok: false, error: 'chunk transfer failed' };
                return await new Promise(resolve => {
                    fileWriteResolvers[reqId] = resolve;
                    cmd('cmd://file/write-commit?reqId=' + encodeURIComponent(reqId) +
@@ -9889,15 +9887,45 @@ const SYNC_PREFS = {
 };

 let _webdavResolvers = {};
-Bridge.onWebdavResult = function(reqId, status, payload) {
+Bridge.onWebdavResult = function(reqId, status, payload, etag) {
    const r = _webdavResolvers[reqId];
    if (!r) return;
    delete _webdavResolvers[reqId];
-    r({ status: status | 0, payload: payload || '' });
+    r({ status: status | 0, payload: payload || '', etag: etag || '' });
 };
-function _webdavCall(method, url, user, pwd, dataB64) {
+// method: 'get'|'put'|'test'. opts.ifMatch → sent as If-Match on a put so
+// the server rejects (412) a write when the remote changed since our pull.
+function _webdavCall(method, url, user, pwd, dataB64, opts) {
+    opts = opts || {};
+    const reqId = 'dav_' + Date.now() + '_' + Math.random().toString(36).slice(2, 8);
+    const CHUNK = 1000000;
+    // Large PUT bodies (a big encrypted vault) can't ride in a single
+    // cmd:// URL — WebView2 caps it and the navigation blanks the page.
+    // Stream the base64 through the file/chunk transport, then commit via
+    // webdav/put-commit (which reads the accumulated buffer server-side).
+    if (method === 'put' && dataB64 && dataB64.length > CHUNK) {
+        return (async () => {
+            const ok = await _streamChunks(reqId, dataB64);
+            if (!ok) return { status: 0, payload: 'chunk transfer failed', etag: '' };
+            return await new Promise(resolve => {
+                _webdavResolvers[reqId] = resolve;
+                let q = 'cmd://webdav/put-commit'
+                      + '?reqId=' + encodeURIComponent(reqId)
+                      + '&url='   + encodeURIComponent(url)
+                      + '&user='  + encodeURIComponent(user || '')
+                      + '&pwd='   + encodeURIComponent(pwd  || '');
+                if (opts.ifMatch) q += '&ifmatch=' + encodeURIComponent(opts.ifMatch);
+                window.location.href = q;
+                setTimeout(() => {
+                    if (_webdavResolvers[reqId]) {
+                        delete _webdavResolvers[reqId];
+                        resolve({ status: 0, payload: 'timeout', etag: '' });
+                    }
+                }, 60000);
+            });
+        })();
+    }
    return new Promise(resolve => {
-        const reqId = 'dav_' + Date.now() + '_' + Math.random().toString(36).slice(2, 8);
        _webdavResolvers[reqId] = resolve;
        let q = 'cmd://webdav/' + method
              + '?reqId=' + encodeURIComponent(reqId)
@@ -9905,11 +9933,12 @@ function _webdavCall(method, url, user, pwd, dataB64) {
              + '&user='  + encodeURIComponent(user || '')
              + '&pwd='   + encodeURIComponent(pwd  || '');
        if (dataB64) q += '&data=' + encodeURIComponent(dataB64);
+        if (opts.ifMatch) q += '&ifmatch=' + encodeURIComponent(opts.ifMatch);
        window.location.href = q;
        setTimeout(() => {
            if (_webdavResolvers[reqId]) {
                delete _webdavResolvers[reqId];
-                resolve({ status: 0, payload: 'timeout' });
+                resolve({ status: 0, payload: 'timeout', etag: '' });
            }
        }, 60000);
    });
@@ -10245,6 +10274,10 @@ function syncStatus(text) {
        if (text) { el.textContent = text; el.style.display = ''; }
        else      { el.textContent = '';  el.style.display = 'none'; }
    }
+    // Also drive the global busy overlay so a running sync blocks stray
+    // clicks (e.g. the auto-backup "Choose…" picker) and reads the same
+    // as the manual backup. Empty text = clear.
+    if (text) showBusy(text); else hideBusy();
    // Lock the Sync/Test buttons while a run is in flight so the user
    // can't double-click a second concurrent sync.
    const active = !!text;
@@ -10254,14 +10287,15 @@ function syncStatus(text) {
    });
 }

-async function runSyncNow() {
+async function runSyncNow(_attempt) {
+    _attempt = _attempt || 0;
    const cfg = await loadSyncConfig();
    if (!cfg) return toast('Bridge not available', 'error');
    if (!cfg.url) return toast('Sync not configured', 'warning');
    if (!cfg.encPwd) return toast('Set the sync password first', 'warning');
    if (!state.cryptoKey) return toast('Vault is locked', 'warning');

-    toast('Syncing…');
+    if (_attempt === 0) toast('Syncing…');
    let merged = { added: 0, updated: 0, deleted: 0 };

    // Fail-fast connectivity: hit the remote FIRST so a dead server /
@@ -10285,6 +10319,10 @@ async function runSyncNow() {
        syncStatus('');
        return toast('Pull failed: HTTP ' + pullResp.status, 'error');
    }
+    // ETag of the version we just pulled — sent back as If-Match on the
+    // push so the server rejects our write if another device changed the
+    // file in between (optimistic concurrency, avoids lost updates).
+    const remoteEtag = pullResp.etag || '';

    // Decrypt + cross-account guard BEFORE touching local state, so a
    // wrong sync password or a foreign account aborts cleanly.
@@ -10300,6 +10338,9 @@ async function runSyncNow() {
        }
        if (remoteSnap && remoteSnap.username && state.username &&
            remoteSnap.username !== state.username) {
+            // Drop the busy overlay so this confirm (z-index below it)
+            // is visible; a later syncStatus() re-shows it if we continue.
+            hideBusy();
            const ok = await confirmDialog({
                title:   'Different account on remote',
                message: 'The remote snapshot belongs to <b>' +
@@ -10369,7 +10410,21 @@ async function runSyncNow() {
        const bodyBytes = new TextEncoder().encode(JSON.stringify(container, null, 2));
        syncStatus('Pushing…');
        const r = await _webdavCall('put', cfg.url, cfg.user, cfg.pwd,
-                                    bytesToBase64(bodyBytes));
+                                    bytesToBase64(bodyBytes),
+                                    { ifMatch: remoteEtag });
+        // 412 Precondition Failed = the remote changed since our pull
+        // (another device pushed). Re-run the whole pull→merge→push so we
+        // fold in their changes instead of clobbering them. Bounded to a
+        // few tries to avoid a livelock against a device syncing in a hot
+        // loop.
+        if (r.status === 412) {
+            syncStatus('');
+            if (_attempt < 3) {
+                toast('Remote changed — re-syncing…');
+                return await runSyncNow(_attempt + 1);
+            }
+            return toast('Sync gave up after repeated remote changes — try again', 'error');
+        }
        if (!(r.status >= 200 && r.status < 300)) {
            syncStatus('');
            return toast('Push failed: HTTP ' + r.status, 'error');