fix(docsync): act on a deletion once, and know what to do when it is undone

Both planner branches planned local_deleted for as long as a file sat in
TREK's trash. Under unlink nothing showed, until the binding was switched
to trash: the next run then binned every document ever deleted in TREK,
months back included. A binding on trash from the start asked the provider
to bin the same copy on every run. A local_deleted row is now left alone,
so the policy is applied once, under the policy in force at that moment.

That guard also meant a file taken back out of TREK's trash stayed on
local_deleted for good, which it already did before: no transfer, no
upload, no flag, and a second deletion of the same file would never have
propagated. The row now leaves that state by what became of the provider
copy:

- still there: the pairing counts again and the row is judged like any
  synced one, so an upstream edit made meanwhile comes down;
- binned by TREK: the pairing is dropped and the file goes up again as a
  new document through the ordinary push, keeping its trek_doc_uid;
- deleted by somebody else: flagged remote_missing, never uploaded over
  their deletion.

Telling the last two apart needs a record of what TREK did, so the new
remote_trashed_at column (migration 239) is written when TREK bins a copy
and cleared when the pairing is back, or when the binned copy is listed
again because somebody restored it at the provider.

A file purged from the trash came straight back as well: file_id is ON
DELETE SET NULL, and the row read as a download that never landed. A
tombstone now stays a tombstone, and a purge no run had seen yet is
treated as the deletion it is.

Also drops snapshotHash, which nothing read.

server/src/db/migrations.ts

@@ -5125,6 +5125,27 @@ function runMigrations(db: Database.Database): void {
         CREATE INDEX IF NOT EXISTS idx_document_sync_items_due ON document_sync_items(link_id, next_attempt_at);
       `);
     },
+
+    /*
+     * Document providers: when TREK itself put a provider copy in the bin.
+     *
+     * A file deleted in TREK under the `trash` policy takes its provider copy
+     * with it. Taken back out of TREK's trash, it has to go up again; a copy
+     * somebody else deleted in the meantime must not. Both leave the same gap
+     * in a listing, so the difference is written down when TREK acts rather
+     * than guessed at later.
+     *
+     * No backfill from the binding's current policy: that policy may not be
+     * the one the deletion ran under, and reading it back is the retroactive
+     * mistake this column exists to avoid. A row left NULL is treated like a
+     * copy somebody else deleted, which flags it instead of uploading it.
+     */
+    () => {
+      const hasColumn = db
+        .prepare("SELECT 1 FROM pragma_table_info('document_sync_items') WHERE name = 'remote_trashed_at'")
+        .get();
+      if (!hasColumn) db.exec('ALTER TABLE document_sync_items ADD COLUMN remote_trashed_at TEXT');
+    },
   ];
 
   if (currentVersion < migrations.length) {

server/src/nest/doc-sync/doc-sync.helpers.ts

@@ -14,20 +14,7 @@ import type { RemoteDocument } from './document-provider';
  * service does nothing but execute the plan.
  */
 
-// ── Identity and fingerprints ────────────────────────────────────────────────
-
-/**
- * A stable fingerprint over an explicit field order.
- *
- * The order is written out rather than derived from the object for the reason
- * dawarich.helpers.ts gives: a hash over `Object.keys` changes when someone
- * adds a field or a provider reorders its JSON, and a changed hash means
- * "changed upstream" to every later run. Explicit means a reviewer can see what
- * participates.
- */
-export function snapshotHash(parts: readonly (string | number | null | undefined)[]): string {
-  return crypto.createHash('sha256').update(parts.map((p) => String(p ?? '')).join('\u0000')).digest('hex');
-}
+// ── Identity and incoming names ──────────────────────────────────────────────
 
 /** Per-document anchor, written into provider metadata where there is room. */
 export function newTrekDocUid(): string {
@@ -108,6 +95,13 @@ export interface SyncItemState {
   /** When the backoff lets this row be tried again; null once it is shelved. */
   nextAttemptAt: string | null;
   remoteMissingAt: string | null;
+  /**
+   * When TREK itself put the provider copy in the recycle bin, under the
+   * `trash` policy. A copy binned by TREK and one deleted by somebody else
+   * leave the same gap in a listing; only this tells them apart once the file
+   * comes back out of TREK's trash.
+   */
+  remoteTrashedAt: string | null;
 }
 
 /** What TREK currently holds for this trip. */
@@ -138,6 +132,12 @@ export type PlanAction =
   | { kind: 'mark_remote_missing'; itemId: number }
   /** Deleted in TREK; what happens upstream is the link's delete policy. */
   | { kind: 'local_deleted'; itemId: number; remoteId: string }
+  /** Back out of TREK's trash while the provider copy stayed: the pairing counts again. */
+  | { kind: 'local_restored'; itemId: number }
+  /** Back out of TREK's trash after TREK binned the copy: unpaired, so it goes up as a new document. */
+  | { kind: 'detach'; itemId: number }
+  /** The copy TREK binned is listed again: somebody restored it at the provider. */
+  | { kind: 'remote_restored'; itemId: number }
   /** Nothing to do but the row should stop looking stale. */
   | { kind: 'touch'; itemId: number; remote: RemoteDocument | null };
 
@@ -195,7 +195,7 @@ export interface ReconcileInput {
  * or ping-pong forever.
  */
 export function planReconcile(input: ReconcileInput): ReconcilePlan {
-  const { items, remote, local, direction, remoteTruncated, stableRemoteIds, maxAttempts, conflictPolicy } = input;
+  const { remote, local, direction, remoteTruncated, stableRemoteIds, maxAttempts, conflictPolicy } = input;
   const now = input.now ?? new Date().toISOString().replace('T', ' ').slice(0, 19);
   const actions: PlanAction[] = [];
 
@@ -213,7 +213,7 @@ export function planReconcile(input: ReconcileInput): ReconcilePlan {
    * which is what "waits for a person" means.
    */
   const blocked = new Set<number>();
-  for (const it of items) {
+  for (const it of input.items) {
     if (it.state !== 'error') continue;
     if (it.attempts >= maxAttempts) { blocked.add(it.id); continue; }
     if (it.nextAttemptAt !== null && it.nextAttemptAt > now) blocked.add(it.id);
@@ -260,6 +260,39 @@ export function planReconcile(input: ReconcileInput): ReconcilePlan {
   const localById = new Map<number, LocalDocument>();
   for (const l of local) localById.set(l.fileId, l);
 
+  /**
+   * A file taken back out of TREK's trash.
+   *
+   * A deletion TREK has acted on is otherwise left alone for good (see the
+   * pair loop), so this is the one way out of `local_deleted`, and which way
+   * depends on the provider copy. If it stayed, the pairing counts again and
+   * the row is judged like any synced one from here on: an edit made upstream
+   * in the meantime comes down, and a copy somebody else deleted is flagged as
+   * missing instead of being uploaded over their deletion. If TREK binned it,
+   * TREK took it away and the person asked for it back, so the pairing is
+   * dropped and the file goes up as a new document through the ordinary push.
+   *
+   * The returned row is what the rest of the plan sees, so it has to match
+   * what the executor writes for the action added here.
+   */
+  const reopen = (it: SyncItemState): SyncItemState => {
+    if (it.state !== 'local_deleted' || !it.remoteId || it.fileId === null) return it;
+    const l = localById.get(it.fileId);
+    if (!l || l.deletedAt) return it;
+    const listed = !input.remoteUnchanged && remoteById.has(it.remoteId);
+    if (it.remoteTrashedAt === null || listed) {
+      add({ kind: 'local_restored', itemId: it.id });
+      return { ...it, state: 'synced', remoteMissingAt: null, remoteTrashedAt: null };
+    }
+    // A pull-only binding cannot upload, and a gap in a truncated listing says
+    // nothing about the copy. The row waits for a run that can decide.
+    if (direction === 'pull' || remoteTruncated) return it;
+    add({ kind: 'detach', itemId: it.id });
+    return { ...it, state: 'pending', remoteId: null, remoteVersion: null, remoteTrashedAt: null };
+  };
+  const items: SyncItemState[] = [];
+  for (const it of input.items) items.push(reopen(it));
+
   const itemsByRemote = new Map<string, SyncItemState>();
   const itemsByFile = new Map<number, SyncItemState>();
   for (const it of items) {
@@ -271,9 +304,14 @@ export function planReconcile(input: ReconcileInput): ReconcilePlan {
   // remote half of the plan would be reading a list the adapter did not fetch.
   if (input.remoteUnchanged) {
     for (const it of items) {
-      if (it.fileId === null || !it.remoteId) continue;
-      const l = localById.get(it.fileId);
-      if (l?.deletedAt) add({ kind: 'local_deleted', itemId: it.id, remoteId: it.remoteId });
+      // A deletion already acted on stays acted on, as in the pair loop. A copy
+      // on record as gone is absent from a full listing too, so a full run
+      // plans nothing for it until it comes back; binning it here would also
+      // write the gap down as TREK's doing, and a restore would then upload
+      // what somebody else deleted.
+      if (!it.remoteId || it.state === 'local_deleted' || it.state === 'remote_missing') continue;
+      const l = it.fileId !== null ? localById.get(it.fileId) : undefined;
+      if (l?.deletedAt || lostItsFile(it)) add({ kind: 'local_deleted', itemId: it.id, remoteId: it.remoteId });
     }
     if (direction !== 'pull') {
       for (const l of local) {
@@ -315,7 +353,21 @@ export function planReconcile(input: ReconcileInput): ReconcilePlan {
 
     const localChanged = !!l && l.sha256 !== null && it.contentSha256 !== null && l.sha256 !== it.contentSha256;
 
-    if (l?.deletedAt) {
+    /**
+     * A deletion is acted on once, under the policy in force at that moment.
+     *
+     * This used to be planned again on every run for as long as the file sat
+     * in TREK's trash. Under `unlink` nothing showed; switching the binding to
+     * `trash` later then binned every document ever deleted in TREK, months
+     * back included, and a binding on `trash` from the start asked the
+     * provider to bin the same copy on every run. A row still here is one
+     * `reopen` did not take back: its file is still in the trash or purged.
+     */
+    if (it.state === 'local_deleted') {
+      if (it.remoteTrashedAt !== null) add({ kind: 'remote_restored', itemId: it.id });
+      continue;
+    }
+    if (l?.deletedAt || lostItsFile(it)) {
       add({ kind: 'local_deleted', itemId: it.id, remoteId: it.remoteId });
       continue;
     }
@@ -444,6 +496,19 @@ export function planReconcile(input: ReconcileInput): ReconcilePlan {
   return { actions, massDeleteGuardTripped: guardTripped, missingCount: vanished.length };
 }
 
+/**
+ * A file purged from TREK's trash before any run saw it deleted.
+ *
+ * `file_id` is ON DELETE SET NULL, so the row outlives the file with nothing
+ * but a NULL to show for it. Read as a download that never landed, which is
+ * what happened until now, the document came straight back and the delete
+ * policy never ran. Only states a row holds together with a file count here: a
+ * first download that failed has no file either, and is retried.
+ */
+function lostItsFile(it: SyncItemState): boolean {
+  return it.fileId === null && (it.state === 'synced' || it.state === 'conflict' || it.state === 'remote_missing');
+}
+
 /** Backoff lookup that saturates instead of running off the end of the curve. */
 export function backoffSeconds(curve: readonly number[], failures: number): number {
   if (failures <= 0) return curve[0];

server/src/nest/doc-sync/doc-sync.service.ts

@@ -277,12 +277,54 @@ export class DocSyncService {
         return 'ok';
       }
       case 'local_deleted': {
-        if (ctx.link.delete_policy === 'trash') {
+        const binned = ctx.link.delete_policy === 'trash';
+        if (binned) {
           const res = await ctx.provider.trash(ctx.ref, ctx.scope, action.remoteId);
           if (docFailed(res)) return res.error.code;
         }
+        // The policy is applied here and never again: the planner leaves a
+        // `local_deleted` row alone, so a policy changed later cannot reach
+        // back. Whether TREK binned the copy is written down with it, because a
+        // restore in TREK later has to know whether the gap upstream is TREK's.
         this.db.connection
-          .prepare("UPDATE document_sync_items SET state = 'local_deleted', last_seen_at = CURRENT_TIMESTAMP WHERE id = ?")
+          .prepare(
+            `UPDATE document_sync_items
+                SET state = 'local_deleted', remote_missing_at = NULL,
+                    remote_trashed_at = CASE WHEN ? = 1 THEN CURRENT_TIMESTAMP ELSE NULL END,
+                    last_seen_at = CURRENT_TIMESTAMP
+              WHERE id = ?`,
+          )
+          .run(binned ? 1 : 0, action.itemId);
+        return 'ok';
+      }
+      case 'local_restored': {
+        this.db.connection
+          .prepare(
+            `UPDATE document_sync_items
+                SET state = 'synced', remote_missing_at = NULL, remote_trashed_at = NULL, last_seen_at = CURRENT_TIMESTAMP
+              WHERE id = ?`,
+          )
+          .run(action.itemId);
+        return 'ok';
+      }
+      case 'detach': {
+        // The trek_doc_uid stays: it is the same document to TREK, only the
+        // provider copy is new. The push the planner queued after this fills
+        // the pairing in again, and if that push fails the row is an ordinary
+        // unpaired file that the next run retries.
+        this.db.connection
+          .prepare(
+            `UPDATE document_sync_items
+                SET state = 'pending', remote_id = NULL, remote_version = NULL, remote_trashed_at = NULL,
+                    error_code = NULL, last_seen_at = CURRENT_TIMESTAMP
+              WHERE id = ?`,
+          )
+          .run(action.itemId);
+        return 'ok';
+      }
+      case 'remote_restored': {
+        this.db.connection
+          .prepare('UPDATE document_sync_items SET remote_trashed_at = NULL, last_seen_at = CURRENT_TIMESTAMP WHERE id = ?')
           .run(action.itemId);
         return 'ok';
       }
@@ -546,7 +588,7 @@ export class DocSyncService {
     const rows = this.db.connection
       .prepare(
         `SELECT id, file_id, trek_doc_uid, remote_id, remote_version, remote_name, content_sha256,
-                pushed_sha256, state, attempts, next_attempt_at, remote_missing_at
+                pushed_sha256, state, attempts, next_attempt_at, remote_missing_at, remote_trashed_at
            FROM document_sync_items WHERE link_id = ?`,
       )
       .all(linkId) as Array<Record<string, unknown>>;
@@ -563,6 +605,7 @@ export class DocSyncService {
       attempts: Number(r.attempts ?? 0),
       nextAttemptAt: r.next_attempt_at === null || r.next_attempt_at === undefined ? null : String(r.next_attempt_at),
       remoteMissingAt: r.remote_missing_at === null ? null : String(r.remote_missing_at),
+      remoteTrashedAt: r.remote_trashed_at === null ? null : String(r.remote_trashed_at),
     }));
   }
 
@@ -913,7 +956,8 @@ export class DocSyncService {
       .prepare(
         `SELECT link_id,
                 SUM(CASE WHEN file_id IS NOT NULL AND remote_id IS NOT NULL AND state = 'synced' THEN 1 ELSE 0 END) AS paired,
-                SUM(CASE WHEN remote_id IS NOT NULL AND state != 'remote_missing' THEN 1 ELSE 0 END) AS atProvider,
+                SUM(CASE WHEN remote_id IS NOT NULL AND state != 'remote_missing' AND remote_trashed_at IS NULL
+                         THEN 1 ELSE 0 END) AS atProvider,
                 SUM(CASE WHEN state = 'remote_missing' THEN 1 ELSE 0 END) AS missing
            FROM document_sync_items
           WHERE trip_id = ?