diff --git a/server/src/nest/doc-sync/doc-sync.helpers.ts b/server/src/nest/doc-sync/doc-sync.helpers.ts index c59997ce8..03ddcfcf0 100644 --- a/server/src/nest/doc-sync/doc-sync.helpers.ts +++ b/server/src/nest/doc-sync/doc-sync.helpers.ts @@ -9,8 +9,8 @@ import type { RemoteDocument } from './document-provider'; * * `planReconcile` takes a snapshot of both sides and returns the actions to * take. It touches no database, no HTTP and no container, which is what lets - * the hard cases — conflict, echo, mass deletion, rename, a document that - * exists on neither side any more — be tested exhaustively as plain data. The + * the hard cases (conflict, echo, mass deletion, rename, a document that + * exists on neither side any more) be tested exhaustively as plain data. The * service does nothing but execute the plan. */ @@ -26,7 +26,7 @@ export function newTrekDocUid(): string { * separators, control characters, leading dots and NTFS-illegal characters, and * they end up both in a DB column and in a Content-Disposition header. * - * The extension is preserved deliberately — it is what TREK's download route + * The extension is preserved deliberately: it is what TREK's download route * derives the Content-Type from, and what the blocklist check reads. */ export function sanitizeIncomingName(raw: string): string { @@ -84,11 +84,17 @@ export interface SyncItemState { trekDocUid: string; remoteId: string | null; remoteVersion: string | null; - /** The name the provider had at the last agreed state — the rename arbiter. */ + /** The name the provider had at the last agreed state: the rename arbiter. */ remoteName: string | null; + /** + * Size and modification time of the provider copy as last seen. Only read + * where ids are paths, to recognise the same copy after a rename or a move. + */ + remoteSize: number | null; + remoteModifiedAt: string | null; /** The bytes both sides last agreed on. */ contentSha256: string | null; - /** What TREK itself last uploaded — the echo guard. */ + /** What TREK itself last uploaded: the echo guard. */ pushedSha256: string | null; state: string; attempts: number; @@ -123,6 +129,11 @@ export type PlanAction = | { kind: 'pull_update'; remote: RemoteDocument; itemId: number } /** TREK moved on while upstream did not: push a new revision. */ | { kind: 'push_update'; local: LocalDocument; itemId: number; remoteId: string } + /** + * Where the id is the path: the paired copy is listed under a new one, + * because it was renamed or moved. The pairing follows; nothing else changes. + */ + | { kind: 'relocate'; itemId: number; remote: RemoteDocument } /** A name changed on one side only. */ | { kind: 'rename_remote'; itemId: number; remoteId: string; name: string } | { kind: 'rename_local'; itemId: number; fileId: number; name: string } @@ -132,8 +143,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 while the provider copy stayed: the pairing counts + * again. With `missing`, the copy is gone and TREK did not bin it, so the row + * comes back flagged the way a vanished copy is. + */ + | { kind: 'local_restored'; itemId: number; missing?: true } /** 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. */ @@ -145,7 +160,7 @@ export interface ReconcilePlan { actions: PlanAction[]; /** True when the listing looked like a mass deletion and was not trusted. */ massDeleteGuardTripped: boolean; - /** Vanished upstream but below the guard threshold. */ + /** Newly gone upstream: vanished since the last run, or found gone on a restore in TREK. */ missingCount: number; } @@ -159,8 +174,8 @@ export interface ReconcileInput { /** * The provider says its side has not moved since the stored cursor. * - * Every adapter derives that cursor from the scope's contents — a digest over - * the documents for Paperless, Papra and Synology, the root ETag for WebDAV — + * Every adapter derives that cursor from the scope's contents (a digest over + * the documents for Paperless, Papra and Synology, the root ETag for WebDAV), * so the flag means "nothing changed upstream" for all of them. The WebDAV * adapter acts on it by returning an EMPTY list instead of walking the folder, * which is only safe if this is read: otherwise a quiet binding looks like a @@ -173,7 +188,7 @@ export interface ReconcileInput { * * `manual` parks the row for a person. The other two are the reason this * field exists: the binding was asked at setup, the answer was stored, and - * nothing ever read it — so every conflict was manual whatever the binding + * nothing ever read it, so every conflict was manual whatever the binding * said, and a person who had chosen a side still had to pick it again. */ conflictPolicy: 'manual' | 'trek_wins' | 'provider_wins'; @@ -202,7 +217,7 @@ export function planReconcile(input: ReconcileInput): ReconcilePlan { /** * Rows that must not be tried again on this run. * - * A provider that refuses a document refuses it every time — a file it reads + * A provider that refuses a document refuses it every time: a file it reads * as corrupt, a type it does not accept, a quota that is full. Without this * the row is re-planned on every pass, so a single bad document turns into an * upload attempt every cron tick forever; rows were found at 46 attempts @@ -233,7 +248,8 @@ export function planReconcile(input: ReconcileInput): ReconcilePlan { }; /** - * Both sides moved. The binding's answer decides, and `manual` parks it. + * Both sides changed the bytes. The binding's answer decides, and `manual` + * parks it. * * A side that wins takes the whole document, name and bytes together, because * the two halves of one document coming from different sides is not a state @@ -242,8 +258,8 @@ export function planReconcile(input: ReconcileInput): ReconcilePlan { * the opposite of what was chosen. */ const settle = (it: SyncItemState, r: RemoteDocument, l: LocalDocument | null): void => { - // Only the two answers that name a winner act; anything else — `manual`, a - // value from a future version, a column somebody edited by hand — parks the + // Only the two answers that name a winner act; anything else (`manual`, a + // value from a future version, a column somebody edited by hand) parks the // row. Overwriting one side is the destructive move, so it needs a yes. const park = (): void => { add({ kind: 'conflict', itemId: it.id, remote: r, local: l }); }; if (conflictPolicy === 'provider_wins' && direction !== 'push') { @@ -255,43 +271,141 @@ export function planReconcile(input: ReconcileInput): ReconcilePlan { return park(); }; + /** + * Both sides renamed a document whose bytes still agree. Only the name is in + * dispute, so the winning side's name is carried over and nothing is sent. + * + * This went through `settle` once, and `trek_wins` re-uploaded identical + * bytes. Where the provider replaces in place (WebDAV, Synology) the upload + * also kept the provider's name, so the next run read that as a rename made + * upstream and gave TREK's file the name TREK was supposed to win with. + */ + const settleName = (it: SyncItemState, r: RemoteDocument, l: LocalDocument, incoming: string): void => { + if (conflictPolicy === 'provider_wins' && direction !== 'push') { + return add({ kind: 'rename_local', itemId: it.id, fileId: l.fileId, name: incoming }); + } + if (conflictPolicy === 'trek_wins' && direction !== 'pull') { + return add({ kind: 'rename_remote', itemId: it.id, remoteId: it.remoteId, name: l.name }); + } + return add({ kind: 'conflict', itemId: it.id, remote: r, local: l }); + }; + const remoteById = new Map(); for (const r of remote) if (!r.isDeleted) remoteById.set(r.remoteId, r); const localById = new Map(); for (const l of local) localById.set(l.fileId, l); + /** + * Where the id is the path, a rename or a move upstream reads as one copy + * gone and another one new. + * + * This used to be patched up in the upstream-only loop with a `touch`, which + * went wrong twice. The pairing had already been counted as vanished by then, + * so the same run flagged it missing (and counted it towards the mass-delete + * guard); and `touch` wrote the provider's new name into the arbiter, so the + * next run read the name TREK still had as a rename made in TREK and renamed + * the provider's copy back. + * + * So the pairing is moved first, before anything else looks at it, `reopen` + * included: onto a new entry that is recognisably the same copy (see + * `sameCopy`). Only the id and the version move. The agreed name stays, so + * the pair loop reads the new name the way it reads any rename, and the rest + * of the plan sees a document that is listed. A rename then ends exactly as + * it does where ids are stable. A rename TREK sent upstream is found again + * the same way: the adapter cannot say what the new path is, so the stored id + * is stale until here. + * + * Only a one-to-one match moves anything. Two copies of the same bytes + * renamed at once cannot be told apart, and pairing them crosswise puts each + * name on the other file; unless one of them kept its name (a move), neither + * is moved, and both are handled like any other gap and new document. + * + * A copy TREK binned itself only gets an entry nobody else fits. Its gap is + * TREK's own doing, and letting it make a tie meant that deleting one of two + * identical files in TREK stopped a rename of the other from being followed, + * which came back as a missing flag plus a second download. It is still + * recognised on its own, as a copy restored at the provider somewhere else. + */ + const followMoves = (list: readonly SyncItemState[]): SyncItemState[] => { + const paired = new Set(list.map((it) => it.remoteId)); + const fresh = remote.filter((r) => !r.isDeleted && !paired.has(r.remoteId)); + const gone = list.filter((it) => it.remoteId !== null && !remoteById.has(it.remoteId)); + + const owner = (r: RemoteDocument): SyncItemState | null => { + const fits = gone.filter((o) => sameCopy(o, r)); + return soleMatch(fits, (o) => keptName(o, r)) + ?? soleMatch(fits.filter((o) => o.remoteTrashedAt === null), (o) => keptName(o, r)); + }; + + const moved = new Map(); + for (const it of gone) { + const r = soleMatch(fresh.filter((d) => sameCopy(it, d)), (d) => keptName(it, d)); + if (!r || owner(r) !== it) continue; + // Counted above so a shelved row still makes a tie a tie, but not moved: + // `add` would drop the action while the plan went on with the new id. + if (blocked.has(it.id)) continue; + moved.set(it.id, r); + } + + return list.map((it) => { + const r = moved.get(it.id); + if (!r) return it; + add({ kind: 'relocate', itemId: it.id, remote: r }); + // Same bytes, so the new version marker is not a change: on a path-keyed + // provider it differs only because the path is part of it. + return { ...it, remoteId: r.remoteId, remoteVersion: r.remoteVersion }; + }); + }; + + // Restored rows whose copy turned out to be gone: missing, but not vanished. + let foundGone = 0; + /** * 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 + * depends on the provider copy. If it is listed, 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. + * in the meantime comes down. 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. + * + * A copy gone by somebody else's hand is flagged as missing rather than + * uploaded over their deletion, and flagged right here rather than counted + * as vanished. Nothing is recorded about a copy while its row sits in + * `local_deleted`, so it may have been gone for weeks, and counting it + * towards the mass-delete guard read a handful of such restores as a listing + * that shrank: the guard tripped, the run was abandoned before the restores + * were written, and the next run found the same plan, for good. Only a + * listing that could have shown the copy may say it is gone, so otherwise + * the row waits; `needsFullListing` keeps that to a single run. * * 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) { + if (!isBackFromTrash(it, localById)) return it; + if (!input.remoteUnchanged && remoteById.has(it.remoteId)) { add({ kind: 'local_restored', itemId: it.id }); return { ...it, state: 'synced', remoteMissingAt: null, remoteTrashedAt: null }; } + if (it.remoteTrashedAt === null) { + if (input.remoteUnchanged || remoteTruncated) return it; + foundGone += 1; + add({ kind: 'local_restored', itemId: it.id, missing: true }); + return { ...it, state: 'remote_missing', remoteMissingAt: now, 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)); + + // Upstream unchanged means there is no listing to follow anything into. + const followed = stableRemoteIds || input.remoteUnchanged ? input.items : followMoves(input.items); + const items = followed.map(reopen); const itemsByRemote = new Map(); const itemsByFile = new Map(); @@ -326,6 +440,8 @@ export function planReconcile(input: ReconcileInput): ReconcilePlan { // A listing that lost most of what it had last time is far more likely to be // a broken mount, a revoked token or a moved folder than a real mass delete. + // A row `reopen` resumed is only `synced` here when its copy is listed, so a + // restore never reads as part of a listing that shrank. const known = items.filter((i) => i.remoteId && i.state === 'synced'); const vanished = known.filter((i) => !remoteById.has(i.remoteId as string)); const guardTripped = @@ -343,7 +459,7 @@ export function planReconcile(input: ReconcileInput): ReconcilePlan { const remoteChanged = it.remoteVersion !== null && r.remoteVersion !== it.remoteVersion; // The echo guard: bytes TREK itself pushed come back as a change on the // provider side. They are only TREK's own write if the hash still matches - // what was pushed, so this compares content, never timestamps — a time + // what was pushed, so this compares content, never timestamps: a time // window would misfire on every clock skew. const isEcho = remoteChanged && @@ -377,14 +493,14 @@ export function planReconcile(input: ReconcileInput): ReconcilePlan { * * A failed pull stores the remote's version alongside the error, so on the * next run `remoteChanged` is false, there is no local file for the other - * branches to compare against, and the row fell through to `touch` — which + * branches to compare against, and the row fell through to `touch`, which * refreshes it and changes nothing. Nothing anywhere planned a second * attempt: the document never arrived, no retry ever happened, and pressing * "Sync now" did not help either, because the plan was the same. * * A rejected type and an oversized file are left out on purpose. Those are * not transport failures; they are answers, and they stay true until the - * document upstream changes — which the `remoteChanged` branch below picks + * document upstream changes, which the `remoteChanged` branch below picks * up on its own. */ if (!l && it.fileId === null && direction !== 'push' @@ -409,21 +525,27 @@ export function planReconcile(input: ReconcileInput): ReconcilePlan { } // Same bytes, different name. Which side renamed is decided by comparing - // each against the name they last agreed on — not by direction. Reading + // each against the name they last agreed on, not by direction. Reading // `both` as "TREK always wins" silently renamed the provider's copy back // every time somebody tidied up a folder, which is the opposite of a // two-way sync. - if (l && sanitizeIncomingName(r.name) !== l.name) { - // Compared through the same sanitiser the local name went through, or a - // provider name TREK had to clean up (a slash, a control character, a - // leading dot) reads as "TREK renamed this" on the very next run — and - // the provider's copy gets renamed to the cleaned version, unasked. + // + // Compared through the same sanitiser the local name went through, or a + // provider name TREK had to clean up (a slash, a control character, a + // leading dot) reads as "TREK renamed this" on the very next run, and the + // provider's copy gets renamed to the cleaned version, unasked. For the + // same reason TREK takes a new name in its cleaned form, the one a download + // would have given it: a Paperless title with a slash in it used to land + // in the file manager as it was, and the next run read that as a rename + // made in TREK and sent it upstream again. + const incoming = sanitizeIncomingName(r.name); + if (l && incoming !== l.name) { const agreed = it.remoteName === null ? null : sanitizeIncomingName(it.remoteName); - const providerRenamed = agreed !== null && sanitizeIncomingName(r.name) !== agreed; + const providerRenamed = agreed !== null && incoming !== agreed; const localRenamed = agreed !== null && l.name !== agreed; if (providerRenamed && !localRenamed && direction !== 'push') { - add({ kind: 'rename_local', itemId: it.id, fileId: l.fileId, name: r.name }); + add({ kind: 'rename_local', itemId: it.id, fileId: l.fileId, name: incoming }); continue; } if (localRenamed && !providerRenamed && direction !== 'pull') { @@ -432,14 +554,14 @@ export function planReconcile(input: ReconcileInput): ReconcilePlan { } if (providerRenamed && localRenamed) { // Both sides renamed. Nothing here can pick the right one on its own, - // and picking wrong loses a name somebody chose — so it follows the + // and picking wrong loses a name somebody chose, so it follows the // binding's answer, and goes to a person when there is none. - settle(it, r, l); + settleName(it, r, l, incoming); continue; } // No stored name to arbitrate with (a row from before this column, or a // freshly paired document): follow the binding's direction. - if (direction === 'pull') add({ kind: 'rename_local', itemId: it.id, fileId: l.fileId, name: r.name }); + if (direction === 'pull') add({ kind: 'rename_local', itemId: it.id, fileId: l.fileId, name: incoming }); else if (direction === 'push') add({ kind: 'rename_remote', itemId: it.id, remoteId: it.remoteId, name: l.name }); else add({ kind: 'touch', itemId: it.id, remote: r }); continue; @@ -452,21 +574,8 @@ export function planReconcile(input: ReconcileInput): ReconcilePlan { if (direction !== 'push') { for (const r of remote) { if (r.isDeleted) continue; - const existing = itemsByRemote.get(r.remoteId); - if (existing) continue; - - // Without stable ids a rename reads as "old one gone, new one appeared". - // Re-pair on content instead of downloading the same bytes again and - // leaving a phantom behind. - if (!stableRemoteIds && r.contentHash) { - const reuse = items.find( - (i) => i.contentSha256 === r.contentHash && i.remoteId !== null && !remoteById.has(i.remoteId), - ); - if (reuse) { - add({ kind: 'touch', itemId: reuse.id, remote: r }); - continue; - } - } + // A copy that was only renamed or moved is paired already (followMoves). + if (itemsByRemote.has(r.remoteId)) continue; add({ kind: 'pull', remote: r, itemId: null }); } } @@ -478,7 +587,7 @@ export function planReconcile(input: ReconcileInput): ReconcilePlan { const it = itemsByFile.get(l.fileId); if (it?.remoteId && remoteById.has(it.remoteId)) continue; // A row that has a remoteId which is no longer listed is handled below as - // "missing", not re-uploaded — otherwise a broken listing duplicates the + // "missing", not re-uploaded. Otherwise a broken listing duplicates the // entire trip upstream. if (it?.remoteId) continue; add({ kind: 'push', local: l, itemId: it?.id ?? null, remoteId: null }); @@ -493,7 +602,54 @@ export function planReconcile(input: ReconcileInput): ReconcilePlan { } } - return { actions, massDeleteGuardTripped: guardTripped, missingCount: vanished.length }; + return { actions, massDeleteGuardTripped: guardTripped, missingCount: vanished.length + foundGone }; +} + +/** + * Whether this run has to list the provider in full, whatever its cursor says. + * + * A file back out of TREK's trash whose copy TREK did not bin can only be + * settled by a listing that shows whether that copy is still there (see + * `reopen`). An unchanged cursor gives the planner nothing to read, and on a + * quiet binding the cursor stays unchanged until somebody touches the folder, + * so the restore would wait that long. + */ +export function needsFullListing(items: readonly SyncItemState[], local: readonly LocalDocument[]): boolean { + const localById = new Map(local.map((l) => [l.fileId, l])); + return items.some((it) => it.remoteTrashedAt === null && isBackFromTrash(it, localById)); +} + +/** A deletion TREK acted on whose file has since come back out of TREK's trash. */ +function isBackFromTrash(it: SyncItemState, localById: ReadonlyMap): boolean { + if (it.state !== 'local_deleted' || !it.remoteId || it.fileId === null) return false; + const l = localById.get(it.fileId); + return !!l && !l.deletedAt; +} + +/** + * Whether a listing entry is recognisably the copy a pairing had, judged by + * content rather than by id. + * + * A hash on both sides settles it either way. Without one (Synology offers + * none), size and modification time together: a rename or a move leaves both + * alone, and an edit does not. + */ +function sameCopy(it: SyncItemState, r: RemoteDocument): boolean { + if (r.contentHash !== null && it.contentSha256 !== null) return r.contentHash === it.contentSha256; + return r.size !== null && r.remoteModifiedAt !== null + && r.size === it.remoteSize && r.remoteModifiedAt === it.remoteModifiedAt; +} + +/** The copy is listed under the name both sides last agreed on: moved, not renamed. */ +function keptName(it: SyncItemState, r: RemoteDocument): boolean { + return it.remoteName !== null && sanitizeIncomingName(it.remoteName) === sanitizeIncomingName(r.name); +} + +/** The only candidate, or else the only one that kept its name; otherwise none. */ +function soleMatch(candidates: T[], keptItsName: (candidate: T) => boolean): T | null { + if (candidates.length === 1) return candidates[0]; + const named = candidates.filter(keptItsName); + return named.length === 1 ? named[0] : null; } /** @@ -502,11 +658,16 @@ export function planReconcile(input: ReconcileInput): ReconcilePlan { * `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. + * policy never ran. A first download that failed has no file either, and is + * retried, so `error` and `pending` count only with an agreed hash: that is + * written by a pull or a push that landed and by nothing else, and a failed + * update keeps the file it had. A row shelved in `error` that the person then + * deleted and purged is a deletion, not a document to fetch again. */ function lostItsFile(it: SyncItemState): boolean { - return it.fileId === null && (it.state === 'synced' || it.state === 'conflict' || it.state === 'remote_missing'); + if (it.fileId !== null) return false; + if (it.state === 'synced' || it.state === 'conflict' || it.state === 'remote_missing') return true; + return (it.state === 'error' || it.state === 'pending') && it.contentSha256 !== null; } /** Backoff lookup that saturates instead of running off the end of the curve. */ diff --git a/server/src/nest/doc-sync/doc-sync.service.ts b/server/src/nest/doc-sync/doc-sync.service.ts index 020866874..5b6b5c735 100644 --- a/server/src/nest/doc-sync/doc-sync.service.ts +++ b/server/src/nest/doc-sync/doc-sync.service.ts @@ -29,6 +29,7 @@ import { backoffSeconds, isAllowedByOperator, isBlockedName, + needsFullListing, newTrekDocUid, planReconcile, sanitizeIncomingName, @@ -37,6 +38,20 @@ import { type SyncItemState, } from './doc-sync.helpers'; +/** + * The SET clause for a pairing whose copy is listed again: `touch`, `relocate` + * and both renames. + * + * Only `touch` lifted the missing flag, so a copy found again under a new name + * (a rename, or a move where ids are paths) stayed on the issues list for one + * more run. `error` is left alone on purpose: that row either never got its + * bytes or holds older ones than the provider, and calling it synced would hide + * it for good. So is a row without a file, which is a deletion the planner + * turns into `local_deleted`. + */ +const FOUND_AGAIN = `remote_missing_at = NULL, + state = CASE WHEN state = 'remote_missing' AND file_id IS NOT NULL THEN 'synced' ELSE state END`; + /** * The reconciler: it executes what `planReconcile` decided, and does nothing * else of consequence. @@ -46,7 +61,7 @@ import { * deletion at all; Papra does not bump `updatedAt` when a tag changes (measured, * not assumed); Paperless's bulk edit changes documents without touching * `modified`; and Synology FileStation has no change feed whatsoever. A webhook - * is therefore only ever "look now", never a source of truth — the same + * is therefore only ever "look now", never a source of truth: the same * conclusion AirTrail and Dawarich reached for their own providers, written * down in dawarich-sync.service.ts. * @@ -82,7 +97,7 @@ export class DocSyncService { * * The second sort key is what makes the limit fair. A successful run clears * `next_attempt_at`, so on a healthy instance every binding sorts equal on the - * first key and SQLite falls back to insertion order — which means the twenty + * first key and SQLite falls back to insertion order, which means the twenty * oldest bindings were picked on every tick and everything created after them * was never synced automatically at all. Found on a dev database with 170 * bindings, where a freshly created one was still untouched minutes later @@ -156,7 +171,6 @@ export class DocSyncService { const ref = this.config.toRef(conn); const scope = this.config.toScopeRef(link); - if (opts.full) scope.cursor = null; // A binding whose folder or tag is gone needs a human, not a retry: the // alternative is re-creating someone's deleted folder and filling it again. @@ -167,6 +181,12 @@ export class DocSyncService { return { state: code === 'scope_missing' ? 'scope_lost' : 'failed', pulled: 0, pushed: 0, conflicts: 0, missing: 0, errorCode: code }; } + // Read before the listing because they decide how much of it is needed: a + // file restored in TREK waits for a listing that shows its copy. + const items = this.loadItems(link.id); + const local = this.loadLocalDocuments(link); + if (opts.full || needsFullListing(items, local)) scope.cursor = null; + const listing = await provider.list(ref, scope); if (docFailed(listing)) { const state = listing.error.code === 'unauthorized' ? 'needs_reauth' : 'failed'; @@ -174,9 +194,6 @@ export class DocSyncService { return { state, pulled: 0, pushed: 0, conflicts: 0, missing: 0, errorCode: listing.error.code }; } - const items = this.loadItems(link.id); - const local = this.loadLocalDocuments(link); - const plan = planReconcile({ items, remote: listing.data.documents, @@ -241,11 +258,31 @@ export class DocSyncService { case 'push': case 'push_update': return this.push(action.local, 'itemId' in action ? action.itemId : null, 'remoteId' in action ? action.remoteId : null, ctx); + case 'relocate': { + // The id and the version only. `remote_name` is the rename arbiter and + // keeps the agreed name; the rename, if there was one, is the planner's + // next action for this row. That may be a rename rather than a `touch`, + // so a copy on record as missing is lifted here already. + this.db.connection + .prepare( + `UPDATE document_sync_items + SET remote_id = ?, remote_version = ?, + remote_size = COALESCE(?, remote_size), remote_modified_at = COALESCE(?, remote_modified_at), + ${FOUND_AGAIN}, last_seen_at = CURRENT_TIMESTAMP + WHERE id = ?`, + ) + .run(action.remote.remoteId, action.remote.remoteVersion, action.remote.size, action.remote.remoteModifiedAt, action.itemId); + return 'ok'; + } case 'rename_remote': { const res = await ctx.provider.rename(ctx.ref, ctx.scope, action.remoteId, action.name); if (docFailed(res)) return res.error.code; this.db.connection - .prepare('UPDATE document_sync_items SET remote_name = ?, remote_version = ?, last_seen_at = CURRENT_TIMESTAMP WHERE id = ?') + .prepare( + `UPDATE document_sync_items + SET remote_name = ?, remote_version = ?, ${FOUND_AGAIN}, last_seen_at = CURRENT_TIMESTAMP + WHERE id = ?`, + ) .run(action.name, res.data.remoteVersion, action.itemId); return 'ok'; } @@ -255,7 +292,7 @@ export class DocSyncService { file: this.files.getFileById(action.fileId, ctx.link.trip_id), }); this.db.connection - .prepare('UPDATE document_sync_items SET remote_name = ?, last_seen_at = CURRENT_TIMESTAMP WHERE id = ?') + .prepare(`UPDATE document_sync_items SET remote_name = ?, ${FOUND_AGAIN}, last_seen_at = CURRENT_TIMESTAMP WHERE id = ?`) .run(action.name, action.itemId); return 'ok'; } @@ -298,13 +335,17 @@ export class DocSyncService { return 'ok'; } case 'local_restored': { + // A copy found gone is flagged the way `mark_remote_missing` flags one, + // and recovers the same way: a later `touch` lifts it once it is back. + const missing = action.missing === true; this.db.connection .prepare( `UPDATE document_sync_items - SET state = 'synced', remote_missing_at = NULL, remote_trashed_at = NULL, last_seen_at = CURRENT_TIMESTAMP + SET state = ?, remote_missing_at = CASE WHEN ? = 1 THEN CURRENT_TIMESTAMP ELSE NULL END, + remote_trashed_at = NULL, last_seen_at = CURRENT_TIMESTAMP WHERE id = ?`, ) - .run(action.itemId); + .run(missing ? 'remote_missing' : 'synced', missing ? 1 : 0, action.itemId); return 'ok'; } case 'detach': { @@ -329,10 +370,13 @@ export class DocSyncService { return 'ok'; } case 'touch': { - // `error` is NOT cleared here. A row in that state has no bytes in TREK - // — the download failed — and marking it synced would hide a document - // that nobody will ever fetch again. Only a `remote_missing` row, whose - // file is present and whose provider copy came back, recovers this way. + // `error` is NOT cleared here (see FOUND_AGAIN). Only a `remote_missing` + // row, whose file is present and whose provider copy came back, + // recovers this way. + // + // Size and modification time are kept current as well: where the id is + // the path, they are how the planner recognises this copy once it has + // been renamed or moved. this.db.connection .prepare( `UPDATE document_sync_items @@ -340,11 +384,15 @@ export class DocSyncService { remote_version = COALESCE(?, remote_version), remote_name = COALESCE(?, remote_name), remote_id = COALESCE(?, remote_id), - remote_missing_at = NULL, - state = CASE WHEN state = 'remote_missing' AND file_id IS NOT NULL THEN 'synced' ELSE state END + remote_size = COALESCE(?, remote_size), + remote_modified_at = COALESCE(?, remote_modified_at), + ${FOUND_AGAIN} WHERE id = ?`, ) - .run(action.remote?.remoteVersion ?? null, action.remote?.name ?? null, action.remote?.remoteId ?? null, action.itemId); + .run( + action.remote?.remoteVersion ?? null, action.remote?.name ?? null, action.remote?.remoteId ?? null, + action.remote?.size ?? null, action.remote?.remoteModifiedAt ?? null, action.itemId, + ); return 'ok'; } default: @@ -362,7 +410,7 @@ export class DocSyncService { // The same defences an upload goes through. A provider folder routinely // holds .svg and .html, and TREK serves downloads inline with a - // Content-Type derived from the extension — letting those through would be + // Content-Type derived from the extension: letting those through would be // stored XSS. Rejected documents become a visible row, never a silent skip. if (isBlockedName(name) || !isAllowedByOperator(name, this.allowedTypes.get())) { this.upsertItem(ctx.link, { itemId, remote, state: 'rejected_type', errorCode: 'unsupported_type' }); @@ -427,7 +475,7 @@ export class DocSyncService { /** * The file row, the retirement of the copy it replaces and the pairing are * one fact, so they are one transaction. Written separately, a crash - * between them leaves a trip file with no sync item — which the next run + * between them leaves a trip file with no sync item, which the next run * reads as a document TREK gained and pushes straight back up, turning one * upstream edit into two documents on both sides. * @@ -457,7 +505,7 @@ export class DocSyncService { return { created: file, retired: gone ? (supersededId as number) : null }; }); - // Announced only once it is committed — a rolled-back transaction that had + // Announced only once it is committed: a rolled-back transaction that had // already told every client the file exists cannot be taken back. if (retired !== null) this.realtime.broadcast(ctx.link.trip_id, 'file:deleted', { fileId: retired }); this.realtime.broadcast(ctx.link.trip_id, 'file:created', { file: created }); @@ -489,7 +537,7 @@ export class DocSyncService { if (!file) return 'not_found'; // Re-read rather than trust the plan: a run walks a whole folder, and a // document somebody deleted in the meantime would otherwise still be - // uploaded — a delete that looks ignored, and a document that reappears + // uploaded: a delete that looks ignored, and a document that reappears // upstream after the person watched it go. if ((file as { deleted_at?: string | null }).deleted_at) return 'not_found'; @@ -528,8 +576,8 @@ export class DocSyncService { // A provider may answer a push with a document it already held rather than // a new one: Papra deduplicates identical bytes, and `deduplicated` says so. // Two trip files with the same content therefore push to ONE document, and - // the unique index on (link_id, remote_id) would abort this run — and every - // run after it — with a constraint error. The upload happened, so nothing is + // the unique index on (link_id, remote_id) would abort this run (and every + // run after it) with a constraint error. The upload happened, so nothing is // lost; what must not happen is the pairing moving off the file that owns it. const heldBy = this.pairingOwner(ctx.link.id, res.data.remoteId, itemId); if (heldBy !== null) { @@ -553,10 +601,15 @@ export class DocSyncService { pushedSha256: sha256, // The name both sides now agree on. Leaving it null meant the first // `touch` filled the arbiter with whatever the provider had made of the - // name — Paperless stores a title and keeps its own filename — and the + // name (Paperless stores a title and keeps its own filename), and the // run after that read the difference as TREK having renamed the document // and renamed the provider's copy to match, unasked. remoteNameOverride: local.name, + // What the copy looks like at the provider, for the same reason as the + // name: on a provider whose id is the path, a rename before the next run + // could otherwise not be told from a deletion. + remoteSize: local.size, + remoteModifiedAt: res.data.remoteModifiedAt, trekDocUid: uid, errorCode: null, }); @@ -570,7 +623,7 @@ export class DocSyncService { * and pressing "Sync now" is that person saying otherwise: they have taken the * bad document out of the folder, cleared the quota, or fixed the permission, * and the only way to find out is to try again. The scheduler never calls - * this — automatic retries are exactly what the limit exists to stop. + * this: automatic retries are exactly what the limit exists to stop. */ retryShelvedItems(linkId: number): void { this.db.connection @@ -587,8 +640,8 @@ export class DocSyncService { private loadItems(linkId: number): SyncItemState[] { 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, remote_trashed_at + `SELECT id, file_id, trek_doc_uid, remote_id, remote_version, remote_name, remote_size, remote_modified_at, + content_sha256, pushed_sha256, state, attempts, next_attempt_at, remote_missing_at, remote_trashed_at FROM document_sync_items WHERE link_id = ?`, ) .all(linkId) as Array>; @@ -599,6 +652,8 @@ export class DocSyncService { remoteId: r.remote_id === null ? null : String(r.remote_id), remoteVersion: r.remote_version === null ? null : String(r.remote_version), remoteName: r.remote_name === null || r.remote_name === undefined ? null : String(r.remote_name), + remoteSize: r.remote_size === null ? null : Number(r.remote_size), + remoteModifiedAt: r.remote_modified_at === null ? null : String(r.remote_modified_at), contentSha256: r.content_sha256 === null ? null : String(r.content_sha256), pushedSha256: r.pushed_sha256 === null ? null : String(r.pushed_sha256), state: String(r.state), @@ -613,14 +668,14 @@ export class DocSyncService { * The trip's own documents, minus the ones that are not really documents. * * Chat attachments (`message_id`) and note attachments (`note_id`) live in - * the same table but belong to a conversation, not to the trip's paperwork — + * the same table but belong to a conversation, not to the trip's paperwork: * syncing them would push someone's chat screenshot into a shared Paperless. * * The set is per TRIP, not per binding, and that is a property rather than an * oversight: a binding mirrors the trip's documents, so two bindings on one * trip each hold a full copy. That is the right answer for "the same papers, * in both my Nextcloud and my Paperless", and the wrong one for "receipts to - * Paperless, everything else to Nextcloud" — splitting a trip across bindings + * Paperless, everything else to Nextcloud": splitting a trip across bindings * would need a per-document assignment that nothing in the UI offers yet. */ private loadLocalDocuments(link: LinkRow): LocalDocument[] { @@ -639,7 +694,7 @@ export class DocSyncService { // Deliberately null, and not the agreed hash out of document_sync_items: // reading that column here would compare it against itself, and // `localChanged` in the planner could then never be true. TREK has no way - // to replace a document's bytes — an upload creates a new row — so there + // to replace a document's bytes (an upload creates a new row), so there // is no local change to detect, and claiming otherwise would be worse // than admitting it. The day the file manager grows a replace, this needs // a real hash column on trip_files, filled at upload time. @@ -681,6 +736,9 @@ export class DocSyncService { /** The agreed name, where the caller knows it better than the listing does. */ remoteNameOverride?: string; remoteVersion?: string; + /** Size and modification time at the provider, where there is no listing entry to take them from. */ + remoteSize?: number; + remoteModifiedAt?: string | null; fileId?: number; state: string; errorCode?: DocsyncErrorCode | null; @@ -692,6 +750,8 @@ export class DocSyncService { const remoteId = patch.remoteIdOverride ?? patch.remote?.remoteId ?? null; const remoteVersion = patch.remoteVersion ?? patch.remote?.remoteVersion ?? null; const remoteName = patch.remoteNameOverride ?? patch.remote?.name ?? null; + const remoteSize = patch.remoteSize ?? patch.remote?.size ?? null; + const remoteModifiedAt = patch.remoteModifiedAt ?? patch.remote?.remoteModifiedAt ?? null; const failed = patch.state === 'error'; if (patch.itemId !== null) { @@ -699,7 +759,7 @@ export class DocSyncService { * The attempt counter, read before it is written. * * Two things needed it. The backoff was computed as - * `backoffSeconds(curve, 1)` — always the first step, so three of the + * `backoffSeconds(curve, 1)`, always the first step, so three of the * four steps in the curve were unreachable and a provider that was down * got asked again at the same short interval. And the counter only ever * grew: a row that failed once a month reached the limit after six months @@ -737,7 +797,7 @@ export class DocSyncService { .run( patch.state, patch.errorCode ?? null, patch.fileId ?? null, remoteId, remoteVersion, - remoteName, patch.remote?.size ?? null, patch.remote?.remoteModifiedAt ?? null, + remoteName, remoteSize, remoteModifiedAt, patch.contentSha256 ?? null, patch.pushedSha256 ?? null, attempts, failed ? 1 : 0, attempts, ITEM_MAX_ATTEMPTS, @@ -770,8 +830,8 @@ export class DocSyncService { ) .run( link.id, link.trip_id, patch.fileId ?? null, patch.trekDocUid ?? newTrekDocUid(), - remoteId, remoteName, remoteVersion, patch.remote?.size ?? null, - patch.remote?.remoteModifiedAt ?? null, patch.contentSha256 ?? null, patch.pushedSha256 ?? null, + remoteId, remoteName, remoteVersion, remoteSize, + remoteModifiedAt, patch.contentSha256 ?? null, patch.pushedSha256 ?? null, patch.state, patch.errorCode ?? null, failed ? 1 : 0, failed ? 1 : 0, backoffSeconds(ITEM_BACKOFF_SECONDS, 1), // a new row is always at its first failure patch.state, @@ -817,7 +877,7 @@ export class DocSyncService { * * `tripId` is not decoration: the route authorises the caller against the trip * in its URL, and without checking the row against the same trip an owner of - * any trip could resolve a conflict in somebody else's — the id is a plain + * any trip could resolve a conflict in somebody else's: the id is a plain * integer and nothing else tied the two together. Same rule as everywhere in * this codebase: every referenced id must exist AND belong to the same trip. */ @@ -826,7 +886,7 @@ export class DocSyncService { * Rename the provider's copy, outside a run. * * Only `resolveConflict` needs this: every other rename is planned and - * executed by a run. A failure is not fatal here — the row is left for the + * executed by a run. A failure is not fatal here: the row is left for the * next run to sort out rather than the owner's choice being refused. */ private async renameRemoteTo(link: LinkRow, remoteId: string, name: string, itemId: number): Promise { @@ -856,7 +916,7 @@ export class DocSyncService { * The name has to be decided too, not only the bytes. * * The only conflict the planner can actually raise today is a double - * rename, and that is arbitrated entirely by `remote_name` — the name both + * rename, and that is arbitrated entirely by `remote_name`, the name both * sides last agreed on. Clearing a version marker leaves all three names * exactly as they were, so the next run compared them again, found both * sides changed again, and wrote the conflict straight back. Whatever the @@ -884,7 +944,7 @@ export class DocSyncService { // Carry the name over as well, here rather than by moving the arbiter: // TREK winning means the provider's copy takes TREK's name, and the only // way to say that through `remote_name` would be to write the provider's - // CURRENT name into it — which this method does not know without asking. + // CURRENT name into it, which this method does not know without asking. if (localName && item.remote_id && localName !== item.remote_name) { await this.renameRemoteTo(link, String(item.remote_id), localName, itemId); } @@ -894,11 +954,14 @@ export class DocSyncService { .run(itemId); // Take the local rename back to the agreed name. Only the provider's // rename is then left standing, and the next run follows it the ordinary - // way — through the planner, with no special case in it. - if (item.file_id !== null && item.remote_name && localName !== item.remote_name) { + // way, through the planner, with no special case in it. Cleaned the way + // a download cleans it: the planner compares cleaned names, so a raw one + // with a slash in it would read as TREK renaming the file all over again. + const agreedName = item.remote_name ? sanitizeIncomingName(String(item.remote_name)) : null; + if (item.file_id !== null && agreedName && localName !== agreedName) { this.db.connection .prepare('UPDATE trip_files SET original_name = ? WHERE id = ?') - .run(item.remote_name, item.file_id); + .run(agreedName, item.file_id); } } else { // Detach the pairing and let the next run pull the provider copy as a new @@ -913,7 +976,7 @@ export class DocSyncService { /** * The documents a person has to decide about: conflicts, refusals and things - * that vanished upstream. Deliberately not "everything not synced" — a row + * that vanished upstream. Deliberately not "everything not synced": a row * waiting for its turn is not a problem anyone should be shown. */ issues(tripId: number): Array> { @@ -941,7 +1004,7 @@ export class DocSyncService { * What each side is actually holding, per binding. * * The UI showed the last run's transfer counts, which are zero on a binding - * that is already in step — so the interesting screen said nothing. These + * that is already in step, so the interesting screen said nothing. These * are the standing numbers instead: how many documents this trip has, how * many of them the store has, and how many are only on one side. */ diff --git a/server/tests/unit/nest/doc-sync/doc-sync.helpers.test.ts b/server/tests/unit/nest/doc-sync/doc-sync.helpers.test.ts index 4bf2e28ab..0019eaa23 100644 --- a/server/tests/unit/nest/doc-sync/doc-sync.helpers.test.ts +++ b/server/tests/unit/nest/doc-sync/doc-sync.helpers.test.ts @@ -3,6 +3,7 @@ import { backoffSeconds, isAllowedByOperator, isBlockedName, + needsFullListing, planReconcile, sanitizeIncomingName, type LocalDocument, @@ -13,8 +14,8 @@ import type { RemoteDocument } from '../../../../src/nest/doc-sync/document-prov /** * The planner decides everything that matters about two-way sync, and it does * it without a database, a container or a network. That is the point of having - * it: the cases that are painful to reproduce against a live provider — an - * unmounted share, a rename with no stable id, TREK's own write echoing back — + * it: the cases that are painful to reproduce against a live provider (an + * unmounted share, a rename with no stable id, TREK's own write echoing back) * are cheap to state here as data. */ @@ -47,6 +48,8 @@ const item = (over: Partial = {}): SyncItemState => ({ remoteId: 'r1', remoteVersion: 'v1', remoteName: 'boarding.pdf', + remoteSize: null, + remoteModifiedAt: null, contentSha256: 'aaa', pushedSha256: null, state: 'synced', @@ -156,22 +159,6 @@ describe('planReconcile', () => { expect(p.actions.filter((a) => a.kind === 'mark_remote_missing')).toHaveLength(0); }); - /** - * Synology FileStation has no stable file id, so a rename upstream looks like - * a deletion plus a new file. Re-pairing on content avoids downloading the - * same bytes again and leaving a phantom behind. - */ - it('re-pairs a renamed document when the provider has no stable ids', () => { - const p = plan({ - items: [item({ remoteId: '/trek/old.pdf', contentSha256: 'hash-x' })], - remote: [remote({ remoteId: '/trek/new.pdf', contentHash: 'hash-x' })], - local: [local()], - stableRemoteIds: false, - }); - expect(p.actions.some((a) => a.kind === 'pull')).toBe(false); - expect(p.actions.some((a) => a.kind === 'touch')).toBe(true); - }); - /** * Which side renamed is decided against the name both sides last agreed on. * Reading a `both` binding as "TREK always wins" renamed the provider's copy @@ -346,8 +333,8 @@ describe('backoffSeconds', () => { * * Found in a live database at 46 attempts against a limit of 6: the counter and * the backoff were written on every failure and never consulted when planning - * the next run, so a document a provider refuses — a file it reads as corrupt, - * a type it will not take, a full quota — was re-uploaded on every cron tick + * the next run, so a document a provider refuses (a file it reads as corrupt, + * a type it will not take, a full quota) was re-uploaded on every cron tick * for as long as it existed. */ describe('planReconcile > shelving a row that keeps failing', () => { @@ -416,7 +403,7 @@ describe('planReconcile > shelving a row that keeps failing', () => { it('leaves a shelved row alone even when its provider copy is gone too', () => { // "Vanished" is only said about a row that was synced last run, and a row is // only shelved while it is in error, so these two never describe the same - // row — nothing is planned for it either way. + // row. Nothing is planned for it either way. const p = plan({ items: [item({ id: 30, attempts: 99, state: 'error', remoteId: 'r-gone' })], remote: [], @@ -434,7 +421,7 @@ describe('planReconcile > shelving a row that keeps failing', () => { * they then return an empty document list. The core ignored the flag and read * that emptiness as a folder somebody had emptied: from the third run of every * idle Nextcloud or OpenCloud binding onward, every run ended in the mass-delete - * guard — `partial`, `missing: 6`, forever, until the failure counter opened the + * guard: `partial`, `missing: 6`, forever, until the failure counter opened the * circuit and the scheduler dropped the binding entirely. Reproduced against a * live Nextcloud before this was written. */ @@ -600,8 +587,8 @@ describe('planReconcile > a pairing whose download never landed', () => { /** * Names are compared through the same sanitiser the local one went through. * - * A provider name TREK had to clean up — a slash, a control character, a - * leading dot — otherwise reads as "TREK renamed this document" on the very + * A provider name TREK had to clean up (a slash, a control character, a + * leading dot) otherwise reads as "TREK renamed this document" on the very * next run, and the provider's copy is renamed to the cleaned version without * anybody asking for it. */ @@ -623,6 +610,325 @@ describe('planReconcile > a provider name TREK had to clean up', () => { }); expect(p.actions.map(a => a.kind)).toEqual(['rename_local']); }); + + it('takes a new name into TREK the way a download would have written it, so it stays', () => { + // Taken over raw, the next run compared the cleaned provider name with the + // raw one in TREK, read the difference as a rename made in TREK, and sent + // the name back upstream: a Paperless title with a colon, renamed to itself. + const renamed = remote({ remoteId: 'r1', name: 'Hotel: Kyoto.pdf', remoteVersion: 'v1' }); + const first = plan({ + items: [item({ remoteName: 'b.pdf' })], + remote: [renamed], + local: [local({ fileId: 1, name: 'b.pdf' })], + }); + expect(first.actions).toEqual([{ kind: 'rename_local', itemId: 10, fileId: 1, name: 'Hotel_ Kyoto.pdf' }]); + + const second = plan({ + items: [item({ remoteName: 'Hotel_ Kyoto.pdf' })], + remote: [renamed], + local: [local({ fileId: 1, name: 'Hotel_ Kyoto.pdf' })], + }); + expect(second.actions).toEqual([{ kind: 'touch', itemId: 10, remote: renamed }]); + }); +}); + +/** + * A rename upstream where the id is the path, which is Synology FileStation. + * + * It bounced. The upstream-only loop re-paired the renamed copy with a `touch`, + * and `touch` wrote the provider's new name into the rename arbiter while the + * trip file kept the old one: the next run read that as a rename made in TREK + * and renamed the provider's copy back. The same run had flagged the pairing + * missing too, because the vanished list was drawn up before the re-pairing. + * Observed: run 1 [touch b.pdf, mark_remote_missing], run 2 [rename_remote a.pdf]. + */ +describe('planReconcile > a rename upstream where the id is the path', () => { + const at = '2026-09-18T10:00:00Z'; + const paired = (over: Partial = {}) => item({ + remoteId: '/trek/a.pdf', remoteVersion: 'va', remoteName: 'a.pdf', contentSha256: 'hash-a', + remoteSize: 1024, remoteModifiedAt: at, ...over, + }); + const listed = (over: Partial = {}) => remote({ + remoteId: '/trek/b.pdf', name: 'b.pdf', remoteVersion: 'vb', contentHash: 'hash-a', + size: 1024, remoteModifiedAt: at, ...over, + }); + // No local hash, as in production: TREK has no way to change a file's bytes, + // so the service never reports one (see loadLocalDocuments). + const file = (over: Partial = {}) => local({ name: 'a.pdf', sha256: null, ...over }); + const pathIds = (over: Partial[0]> = {}) => plan({ stableRemoteIds: false, ...over }); + const kinds = (p: ReturnType) => p.actions.map(a => a.kind); + + it('takes the new name into TREK and leaves it there on the next run', () => { + const first = pathIds({ items: [paired()], remote: [listed()], local: [file()] }); + expect(first.actions).toEqual([ + { kind: 'relocate', itemId: 10, remote: listed() }, + { kind: 'rename_local', itemId: 10, fileId: 1, name: 'b.pdf' }, + ]); + + // What the executor leaves behind: the pairing on the new path, the agreed + // name moved on by rename_local, the trip file renamed. + const second = pathIds({ + items: [paired({ remoteId: '/trek/b.pdf', remoteVersion: 'vb', remoteName: 'b.pdf' })], + remote: [listed()], + local: [file({ name: 'b.pdf' })], + }); + expect(second.actions).toEqual([{ kind: 'touch', itemId: 10, remote: listed() }]); + }); + + it('ends the way it ends where ids are stable, apart from moving the pairing', () => { + const pathKeyed = pathIds({ items: [paired()], remote: [listed()], local: [file()] }); + const stable = plan({ + items: [paired({ remoteId: '/trek/b.pdf', remoteVersion: 'vb' })], + remote: [listed()], + local: [file()], + }); + expect(pathKeyed.actions.filter(a => a.kind !== 'relocate')).toEqual(stable.actions); + }); + + it('does not flag the renamed copy as missing', () => { + const p = pathIds({ items: [paired()], remote: [listed()], local: [file()] }); + expect(p.missingCount).toBe(0); + expect(kinds(p)).not.toContain('mark_remote_missing'); + }); + + it('does not read a whole folder moved in one go as a mass deletion', () => { + const rows = Array.from({ length: 10 }, (_, i) => paired({ + id: 100 + i, fileId: 100 + i, remoteId: `/trek/${i}.pdf`, remoteName: `${i}.pdf`, contentSha256: `hash-${i}`, + })); + const p = pathIds({ + items: rows, + remote: rows.map((_, i) => listed({ remoteId: `/trek/2026/${i}.pdf`, name: `${i}.pdf`, contentHash: `hash-${i}` })), + local: rows.map((r, i) => file({ fileId: r.fileId as number, name: `${i}.pdf` })), + }); + expect(p.massDeleteGuardTripped).toBe(false); + expect(p.missingCount).toBe(0); + expect(kinds(p).filter(k => k === 'relocate')).toHaveLength(10); + expect(kinds(p)).not.toContain('pull'); + }); + + it('follows a copy moved to another folder and leaves its name alone', () => { + const moved = listed({ remoteId: '/trek/receipts/a.pdf', name: 'a.pdf' }); + const p = pathIds({ items: [paired()], remote: [moved], local: [file()] }); + expect(p.actions).toEqual([ + { kind: 'relocate', itemId: 10, remote: moved }, + { kind: 'touch', itemId: 10, remote: moved }, + ]); + }); + + it('recognises the copy by size and time where the listing carries no hash', () => { + // Synology offers no sha256 at all, so this is the case that actually + // happens there; the hash-only version of this never fired. + const p = pathIds({ items: [paired()], remote: [listed({ contentHash: null })], local: [file()] }); + expect(kinds(p)).toEqual(['relocate', 'rename_local']); + }); + + it('does not follow into a copy with other bytes, whatever its size and time', () => { + const p = pathIds({ items: [paired()], remote: [listed({ contentHash: 'hash-other' })], local: [file()] }); + expect(kinds(p)).toEqual(['pull', 'mark_remote_missing']); + }); + + it('does not follow into a copy that was edited as well', () => { + const edited = listed({ contentHash: null, remoteModifiedAt: '2026-09-19T08:00:00Z' }); + const p = pathIds({ items: [paired()], remote: [edited], local: [file()] }); + expect(kinds(p)).toEqual(['pull', 'mark_remote_missing']); + }); + + it('does not guess from size and time it never recorded', () => { + const p = pathIds({ + items: [paired({ remoteSize: null, remoteModifiedAt: null })], + remote: [listed({ contentHash: null })], + local: [file()], + }); + expect(kinds(p)).toEqual(['pull', 'mark_remote_missing']); + }); + + it('pairs nothing when two copies of the same bytes were renamed at once', () => { + // Either way round fits the content, and the crosswise one puts each name + // on the other trip file. A missing flag and a second download can be seen + // and undone; a swapped pairing cannot be seen at all. + const p = pathIds({ + items: [ + paired({ id: 1, fileId: 1, remoteId: '/trek/a1.pdf', remoteName: 'a1.pdf' }), + paired({ id: 2, fileId: 2, remoteId: '/trek/a2.pdf', remoteName: 'a2.pdf' }), + ], + remote: [listed({ remoteId: '/trek/b1.pdf', name: 'b1.pdf' }), listed({ remoteId: '/trek/b2.pdf', name: 'b2.pdf' })], + local: [file({ fileId: 1, name: 'a1.pdf' }), file({ fileId: 2, name: 'a2.pdf' })], + }); + expect(kinds(p).sort()).toEqual(['mark_remote_missing', 'mark_remote_missing', 'pull', 'pull']); + }); + + it('tells two copies of the same bytes apart when they were moved and kept their names', () => { + const one = listed({ remoteId: '/trek/2026/a1.pdf', name: 'a1.pdf' }); + const two = listed({ remoteId: '/trek/2026/a2.pdf', name: 'a2.pdf' }); + const p = pathIds({ + items: [ + paired({ id: 1, fileId: 1, remoteId: '/trek/a1.pdf', remoteName: 'a1.pdf' }), + paired({ id: 2, fileId: 2, remoteId: '/trek/a2.pdf', remoteName: 'a2.pdf' }), + ], + remote: [two, one], + local: [file({ fileId: 1, name: 'a1.pdf' }), file({ fileId: 2, name: 'a2.pdf' })], + }); + expect(p.actions.filter(a => a.kind === 'relocate')).toEqual([ + { kind: 'relocate', itemId: 1, remote: one }, + { kind: 'relocate', itemId: 2, remote: two }, + ]); + expect(kinds(p).filter(k => k !== 'relocate')).toEqual(['touch', 'touch']); + }); + + it('follows the one of two identical copies that was renamed and leaves the other', () => { + const kept = listed({ remoteId: '/trek/a2.pdf', name: 'a2.pdf', remoteVersion: 'va' }); + const renamed = listed({ remoteId: '/trek/b1.pdf', name: 'b1.pdf' }); + const p = pathIds({ + items: [ + paired({ id: 1, fileId: 1, remoteId: '/trek/a1.pdf', remoteName: 'a1.pdf' }), + paired({ id: 2, fileId: 2, remoteId: '/trek/a2.pdf', remoteName: 'a2.pdf' }), + ], + remote: [kept, renamed], + local: [file({ fileId: 1, name: 'a1.pdf' }), file({ fileId: 2, name: 'a2.pdf' })], + }); + expect(p.actions).toEqual([ + { kind: 'relocate', itemId: 1, remote: renamed }, + { kind: 'rename_local', itemId: 1, fileId: 1, name: 'b1.pdf' }, + { kind: 'touch', itemId: 2, remote: kept }, + ]); + }); + + it('finds a copy again after TREK renamed it upstream', () => { + // The adapter cannot say what path a rename produced, so the row keeps the + // old one (and the name rename_remote agreed) until the next listing. + const p = pathIds({ + items: [paired({ remoteName: 'b.pdf', remoteVersion: 'vb' })], + remote: [listed({ contentHash: null })], + local: [file({ name: 'b.pdf' })], + }); + expect(kinds(p)).toEqual(['relocate', 'touch']); + }); + + it('keeps a document deleted in TREK deleted when its copy is renamed upstream', () => { + // Where ids are stable a pairing already acted on stays put. Here the + // renamed copy would otherwise come back as a new document. + const p = pathIds({ + items: [paired({ state: 'local_deleted' })], + remote: [listed()], + local: [file({ deletedAt: '2026-09-18 08:00:00' })], + }); + expect(p.actions).toEqual([{ kind: 'relocate', itemId: 10, remote: listed() }]); + }); + + describe('next to a twin TREK binned', () => { + // Deleting one of two identical files in TREK is the usual way two rows + // come to share content. The binned one used to count as a candidate, so a + // rename of the live one was a tie and came back as missing plus new. + const binned = (over: Partial = {}) => paired({ + id: 1, fileId: 1, remoteId: '/trek/a1.pdf', remoteName: 'a1.pdf', + state: 'local_deleted', remoteTrashedAt: '2026-09-18 08:05:00', ...over, + }); + const live = paired({ id: 2, fileId: 2, remoteId: '/trek/a2.pdf', remoteName: 'a2.pdf' }); + const renamed = listed({ remoteId: '/trek/b.pdf', name: 'b.pdf', contentHash: null }); + + it('follows the rename of the live one', () => { + const p = pathIds({ + items: [binned(), live], + remote: [renamed], + local: [file({ fileId: 1, name: 'a1.pdf', deletedAt: '2026-09-18 08:00:00' }), file({ fileId: 2, name: 'a2.pdf' })], + }); + expect(p.actions).toEqual([ + { kind: 'relocate', itemId: 2, remote: renamed }, + { kind: 'rename_local', itemId: 2, fileId: 2, name: 'b.pdf' }, + ]); + expect(p.missingCount).toBe(0); + }); + + it('does so once the binned one is purged as well', () => { + const p = pathIds({ items: [binned({ fileId: null }), live], remote: [renamed], local: [file({ fileId: 2, name: 'a2.pdf' })] }); + expect(kinds(p)).toEqual(['relocate', 'rename_local']); + expect(p.actions[0]).toMatchObject({ itemId: 2 }); + }); + + it('still recognises the binned copy when it comes back elsewhere on its own', () => { + const back = listed({ remoteId: '/trek/restored/a1.pdf', name: 'a1.pdf', contentHash: null }); + const p = pathIds({ + items: [binned()], + remote: [back], + local: [file({ fileId: 1, name: 'a1.pdf', deletedAt: '2026-09-18 08:00:00' })], + }); + expect(p.actions).toEqual([ + { kind: 'relocate', itemId: 1, remote: back }, + { kind: 'remote_restored', itemId: 1 }, + ]); + }); + }); + + it('leaves a shelved row where it is: that waits for a person', () => { + const p = pathIds({ + items: [paired({ state: 'error', attempts: 6, fileId: null, contentSha256: null })], + remote: [listed({ contentHash: null })], + local: [], + }); + expect(kinds(p)).not.toContain('relocate'); + }); + + it('follows nothing where ids are stable: a new id there is a new document', () => { + const p = plan({ items: [paired()], remote: [listed()], local: [file()] }); + expect(kinds(p)).toEqual(['pull', 'mark_remote_missing']); + }); + + it('follows nothing under an unchanged upstream, which lists nothing to follow into', () => { + const p = pathIds({ items: [paired()], remote: [listed()], local: [file()], remoteUnchanged: true }); + expect(p.actions).toEqual([]); + }); +}); + +/** + * Both sides renamed a document whose bytes agree: only the name is in dispute. + * + * `trek_wins` used to settle it like changed bytes, with a push_update. That + * re-sent identical bytes, and where the provider replaces in place (WebDAV, + * and Synology once renames there were followed) the upload kept the + * provider's name: the next run read it as a rename made upstream and renamed + * TREK's file to it. Observed: run 1 [relocate, push_update], run 2 [rename_local]. + */ +describe('planReconcile > both sides renamed', () => { + const at = '2026-09-18T10:00:00Z'; + const paired = item({ remoteId: '/trek/a.pdf', remoteName: 'a.pdf', remoteSize: 1024, remoteModifiedAt: at }); + const theirs = remote({ remoteId: '/trek/c.pdf', name: 'c.pdf', remoteModifiedAt: at }); + const mine = local({ name: 'b.pdf', sha256: null }); + const pathIds = (over: Partial[0]> = {}) => + plan({ stableRemoteIds: false, items: [paired], remote: [theirs], local: [mine], ...over }); + + it('carries TREK\'s name upstream when TREK wins, and sends no bytes', () => { + const p = pathIds({ conflictPolicy: 'trek_wins' }); + expect(p.actions).toEqual([ + { kind: 'relocate', itemId: 10, remote: theirs }, + { kind: 'rename_remote', itemId: 10, remoteId: '/trek/c.pdf', name: 'b.pdf' }, + ]); + }); + + it('takes the provider\'s name into TREK when the provider wins, and fetches nothing', () => { + const p = pathIds({ conflictPolicy: 'provider_wins' }); + expect(p.actions).toEqual([ + { kind: 'relocate', itemId: 10, remote: theirs }, + { kind: 'rename_local', itemId: 10, fileId: 1, name: 'c.pdf' }, + ]); + }); + + it('does the same where ids are stable', () => { + const stable = (conflictPolicy: 'trek_wins' | 'provider_wins') => plan({ + conflictPolicy, + items: [item({ remoteName: 'old.pdf' })], + remote: [remote({ name: 'theirs.pdf' })], + local: [local({ name: 'mine.pdf' })], + }).actions; + expect(stable('trek_wins')).toEqual([{ kind: 'rename_remote', itemId: 10, remoteId: 'r1', name: 'mine.pdf' }]); + expect(stable('provider_wins')).toEqual([{ kind: 'rename_local', itemId: 10, fileId: 1, name: 'theirs.pdf' }]); + }); + + it('parks it when the direction forbids what the winner would need', () => { + expect(pathIds({ conflictPolicy: 'trek_wins', direction: 'pull' }).actions.map(a => a.kind)) + .toEqual(['relocate', 'conflict']); + expect(pathIds({ conflictPolicy: 'provider_wins', direction: 'push' }).actions.map(a => a.kind)) + .toEqual(['relocate', 'conflict']); + }); }); /** @@ -685,6 +991,47 @@ describe('planReconcile > a deletion TREK already acted on', () => { }); expect(p.actions).toEqual([{ kind: 'remote_restored', itemId: 50 }]); }); + + /** + * A failed update keeps the file it had and sets `error`. Deleted and purged + * while the row was held back, it read as a first download that never + * landed, and the next attempt ("Sync now" clears the counter) brought the + * deleted document straight back. + */ + describe('on a row that failed an update first', () => { + const failedUpdate = (over: Partial = {}) => + item({ fileId: null, state: 'error', attempts: 2, contentSha256: 'aaa', ...over }); + + it('reads the purge as a deletion', () => { + const p = plan({ items: [failedUpdate()], remote: [remote()], local: [] }); + expect(p.actions).toEqual([{ kind: 'local_deleted', itemId: 10, remoteId: 'r1' }]); + }); + + it('does the same under an unchanged upstream', () => { + const p = plan({ items: [failedUpdate()], remote: [], local: [], remoteUnchanged: true }); + expect(p.actions).toEqual([{ kind: 'local_deleted', itemId: 10, remoteId: 'r1' }]); + }); + + it('waits out the backoff like any other action for the row', () => { + const p = plan({ + items: [failedUpdate({ nextAttemptAt: '2030-01-01 00:00:00' })], + remote: [remote()], + local: [], + now: '2026-09-19 08:00:00', + }); + expect(p.actions).toEqual([]); + }); + + it('treats a row waiting on a conflict answer the same way', () => { + const p = plan({ items: [failedUpdate({ state: 'pending', attempts: 0 })], remote: [remote()], local: [] }); + expect(p.actions).toEqual([{ kind: 'local_deleted', itemId: 10, remoteId: 'r1' }]); + }); + + it('still retries a first download, which never agreed on a hash', () => { + const p = plan({ items: [failedUpdate({ contentSha256: null })], remote: [remote()], local: [] }); + expect(p.actions.map(a => a.kind)).toEqual(['pull_update']); + }); + }); }); /** @@ -713,15 +1060,25 @@ describe('planReconcile > a file taken back out of TREK trash', () => { it('flags a copy somebody else deleted meanwhile instead of uploading it', () => { const p = plan({ items: [settled()], remote: [], local: [back] }); - expect(p.actions).toEqual([ - { kind: 'local_restored', itemId: 50 }, - { kind: 'mark_remote_missing', itemId: 50 }, - ]); + expect(p.actions).toEqual([{ kind: 'local_restored', itemId: 50, missing: true }]); + expect(p.missingCount).toBe(1); }); - it('resumes under an unchanged upstream and leaves the listing to a later run', () => { + it('waits under an unchanged upstream, which shows nothing of the copy', () => { + // Resumed as synced here, a copy that was long gone counted as one that + // vanished on the next real listing, and could trip the guard with it. const p = plan({ items: [settled()], remote: [], local: [back], remoteUnchanged: true }); - expect(p.actions).toEqual([{ kind: 'local_restored', itemId: 50 }]); + expect(p.actions).toEqual([]); + }); + + it('waits on a truncated listing that does not show the copy', () => { + const p = plan({ items: [settled()], remote: [], local: [back], remoteTruncated: true }); + expect(p.actions).toEqual([]); + }); + + it('resumes from a truncated listing that does show the copy', () => { + const p = plan({ items: [settled()], remote: [remote({ remoteId: 'r5' })], local: [back], remoteTruncated: true }); + expect(p.actions.map(a => a.kind)).toEqual(['local_restored', 'touch']); }); it('uploads it again when TREK was the one that binned the copy', () => { @@ -766,10 +1123,94 @@ describe('planReconcile > a file taken back out of TREK trash', () => { expect(p.actions.filter(a => a.kind === 'push')).toHaveLength(10); }); - it('counts many restored rows whose copies vanished like any synced rows, guard included', () => { + it('does not read many restores whose copies are gone as a mass deletion', () => { + // Nothing was recorded about those copies while TREK's files sat in the + // bin, so they may have been gone for weeks: the listing did not shrink. + // Counted as vanished, they tripped the guard, the run was dropped before + // the restores were written, and every run after it planned the same. const rows = Array.from({ length: 10 }, (_, i) => settled({ id: 100 + i, fileId: 100 + i, remoteId: `r${100 + i}` })); const p = plan({ items: rows, remote: [], local: rows.map((r) => local({ fileId: r.fileId as number })) }); - expect(p.massDeleteGuardTripped).toBe(true); + expect(p.massDeleteGuardTripped).toBe(false); + expect(p.actions).toEqual(rows.map((r) => ({ kind: 'local_restored', itemId: r.id, missing: true }))); + expect(p.missingCount).toBe(10); + }); + + describe('beside a binding that is otherwise in step', () => { + const synced = Array.from({ length: 6 }, (_, i) => item({ id: 200 + i, fileId: 200 + i, remoteId: `s${i}` })); + const restored = Array.from({ length: 5 }, (_, i) => settled({ id: 300 + i, fileId: 300 + i, remoteId: `g${i}` })); + const files = [...synced, ...restored].map((r) => local({ fileId: r.fileId as number })); + const newUpstream = remote({ remoteId: 'fresh', name: 'visa.pdf' }); + + it('leaves the guard alone and syncs the rest as usual', () => { + const p = plan({ + items: [...synced, ...restored], + remote: [...synced.map((s) => remote({ remoteId: s.remoteId as string })), newUpstream], + local: files, + }); + expect(p.massDeleteGuardTripped).toBe(false); + expect(p.actions.filter(a => a.kind === 'local_restored')) + .toEqual(restored.map((r) => ({ kind: 'local_restored', itemId: r.id, missing: true }))); + expect(p.actions).toContainEqual({ kind: 'pull', remote: newUpstream, itemId: null }); + expect(p.actions.map(a => a.kind)).not.toContain('mark_remote_missing'); + }); + + it('still trips the guard when the synced rows themselves vanish', () => { + const p = plan({ items: synced, remote: [remote({ remoteId: 's0' })], local: files }); + expect(p.massDeleteGuardTripped).toBe(true); + }); + }); + + it('resumes when the binned copy was restored at the provider under a new path', () => { + // Detached first, the row lost its id before the move could be followed: + // the restored copy came down as a new document and TREK's file went up as + // another, two of each. + const p = plan({ + stableRemoteIds: false, + items: [binnedByTrek({ remoteId: '/trek/a.pdf', remoteName: 'a.pdf', remoteSize: 1024, remoteModifiedAt: '2026-09-18T10:00:00Z' })], + remote: [remote({ remoteId: '/trek/a restored.pdf', name: 'a restored.pdf' })], + local: [local({ fileId: 5, name: 'a.pdf', sha256: null })], + }); + expect(p.actions.map(a => a.kind)).toEqual(['relocate', 'local_restored', 'rename_local']); + }); + + it('follows a copy moved while the file was in the bin, when TREK did not bin it', () => { + const moved = remote({ remoteId: '/trek/2026/a.pdf', name: 'a.pdf' }); + const p = plan({ + stableRemoteIds: false, + items: [settled({ remoteId: '/trek/a.pdf', remoteName: 'a.pdf', remoteSize: 1024, remoteModifiedAt: '2026-09-18T10:00:00Z' })], + remote: [moved], + local: [local({ fileId: 5, name: 'a.pdf', sha256: null })], + }); + expect(p.actions).toEqual([ + { kind: 'relocate', itemId: 50, remote: moved }, + { kind: 'local_restored', itemId: 50 }, + { kind: 'touch', itemId: 50, remote: moved }, + ]); + }); +}); + +describe('needsFullListing', () => { + const settled = (over: Partial = {}) => + item({ id: 50, fileId: 5, remoteId: 'r5', state: 'local_deleted', ...over }); + + it('asks for one once a file whose copy TREK left alone is back', () => { + expect(needsFullListing([settled()], [local({ fileId: 5 })])).toBe(true); + }); + + it('does not while the file is still in the trash', () => { + expect(needsFullListing([settled()], [local({ fileId: 5, deletedAt: '2026-09-18 08:00:00' })])).toBe(false); + }); + + it('does not for a copy TREK binned: that gap is known, and the file goes up again', () => { + expect(needsFullListing([settled({ remoteTrashedAt: '2026-09-18 08:05:00' })], [local({ fileId: 5 })])).toBe(false); + }); + + it('does not once the file is purged', () => { + expect(needsFullListing([settled({ fileId: null })], [])).toBe(false); + }); + + it('does not for pairings that are simply in step', () => { + expect(needsFullListing([item()], [local()])).toBe(false); }); }); diff --git a/server/tests/unit/nest/doc-sync/doc-sync.service.test.ts b/server/tests/unit/nest/doc-sync/doc-sync.service.test.ts index 2d608a128..ef5248383 100644 --- a/server/tests/unit/nest/doc-sync/doc-sync.service.test.ts +++ b/server/tests/unit/nest/doc-sync/doc-sync.service.test.ts @@ -8,7 +8,7 @@ * read back with plain SQL rather than through the service that wrote it. * * The provider is a fake because a unit test must not open a socket. Storage - * and the files domain are fakes because the bytes are not what is under test — + * and the files domain are fakes because the bytes are not what is under test, * but the fake files service writes real `trip_files` rows, so "nothing was * downloaded" is asserted against the table the file manager reads, not against * a spy alone. @@ -129,7 +129,7 @@ const provider = { listScopes: vi.fn(async (): Promise> => ok([SCOPE])), createScope: vi.fn(async (): Promise> => ok(SCOPE)), resolveScope: vi.fn(async (): Promise> => ok(SCOPE)), - list: vi.fn(async (): Promise> => ok(listing())), + list: vi.fn(async (_conn: DocumentConnectionRef, _scope: DocumentScopeRef): Promise> => ok(listing())), fetch: vi.fn(async (): Promise> => ok({ body: Readable.from([FILE_BYTES]), size: FILE_BYTES.length, mimeType: 'application/pdf', remoteVersion: 'v9' })), push: vi.fn(async ( @@ -188,6 +188,7 @@ let scopeSeq = 0; function makeLink(over: { direction?: string; deletePolicy?: string; + conflictPolicy?: string; syncEnabled?: number; failureCount?: number; nextAttemptAt?: string | null; @@ -199,11 +200,11 @@ function makeLink(over: { `INSERT INTO trip_document_links (trip_id, connection_id, provider_id, remote_scope_key, remote_root_id, remote_root_path, remote_label, direction, delete_policy, conflict_policy, sync_enabled, failure_count, next_attempt_at, created_by) - VALUES (?, ?, 'paperless', ?, '1', '/TREK/japan', 'Japan 2026', ?, ?, 'manual', ?, ?, ?, ?)`, + VALUES (?, ?, 'paperless', ?, '1', '/TREK/japan', 'Japan 2026', ?, ?, ?, ?, ?, ?, ?)`, ) .run( tripId, connectionId, `tag:${scopeSeq}`, - over.direction ?? 'both', over.deletePolicy ?? 'unlink', + over.direction ?? 'both', over.deletePolicy ?? 'unlink', over.conflictPolicy ?? 'manual', over.syncEnabled ?? 1, over.failureCount ?? 0, over.nextAttemptAt ?? null, ownerId, ); if (over.lastSyncAt !== undefined) { @@ -222,19 +223,24 @@ function seedItem(link: LinkRow, over: { state?: string; contentSha256?: string; pushedSha256?: string; + remoteSize?: number; + remoteModifiedAt?: string; + remoteMissingAt?: string; } = {}): number { uidSeq += 1; const info = testDb .prepare( `INSERT INTO document_sync_items (link_id, trip_id, file_id, trek_doc_uid, remote_id, remote_name, remote_version, - content_sha256, pushed_sha256, state) - VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?)`, + remote_size, remote_modified_at, content_sha256, pushed_sha256, state, remote_missing_at) + VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?)`, ) .run( link.id, link.trip_id, over.fileId ?? null, `uid-${uidSeq}`, over.remoteId ?? null, over.remoteName ?? null, over.remoteVersion ?? null, + over.remoteSize ?? null, over.remoteModifiedAt ?? null, over.contentSha256 ?? null, over.pushedSha256 ?? null, over.state ?? 'synced', + over.remoteMissingAt ?? null, ); return Number(info.lastInsertRowid); } @@ -549,7 +555,7 @@ describe('DocSyncService', () => { expect(afterPush.content_sha256).toBe(FILE_SHA); expect(afterPush.remote_id).toBe('r-pushed'); - // Same bytes back, under a new version marker — which is what a provider + // Same bytes back, under a new version marker, which is what a provider // reports for TREK's own write. provider.list.mockResolvedValueOnce(ok(listing([ remoteDoc({ remoteId: 'r-pushed', name: 'boarding.pdf', remoteVersion: 'v2', contentHash: FILE_SHA }), @@ -567,7 +573,7 @@ describe('DocSyncService', () => { it('refuses to move a pairing onto a document another file already owns', async () => { // Papra answers a push of bytes it already holds with the document that // has them. Two trip files with the same content therefore push to ONE - // document — and the unique index on (link_id, remote_id) turned that + // document, and the unique index on (link_id, remote_id) turned that // into a constraint error that aborted this run and every run after it. const link = makeLink(); makeFile({ name: 'boarding.pdf' }); @@ -576,8 +582,8 @@ describe('DocSyncService', () => { const run = await service.syncLink(link); - // Counted as a conflict, which is what it is — a document two files want - // — and reported the same way as any other conflict rather than as a + // Counted as a conflict, which is what it is (a document two files + // want), and reported the same way as any other conflict rather than as a // failed run. expect(run.pushed).toBe(1); expect(run.conflicts).toBe(1); @@ -639,7 +645,7 @@ describe('DocSyncService', () => { /** * A chat screenshot and a note attachment live in `trip_files` like every * other document, but they belong to a conversation rather than to the - * trip's paperwork — pushing them would put someone's chat into a shared + * trip's paperwork: pushing them would put someone's chat into a shared * Paperless. */ it('leaves chat and note attachments out of the upload entirely', async () => { @@ -749,6 +755,64 @@ describe('DocSyncService', () => { expect(service.issues(tripId).map((r) => r.id)).toContain(itemId); }); + it('settles a double rename TREK wins with a rename, not a second upload', async () => { + const link = makeLink({ conflictPolicy: 'trek_wins' }); + const fileId = makeFile({ name: 'mine.pdf' }); + const itemId = seedItem(link, { + remoteId: 'r1', remoteName: 'old.pdf', remoteVersion: 'v1', fileId, state: 'synced', + }); + provider.list.mockResolvedValueOnce(ok(listing([remoteDoc({ remoteId: 'r1', name: 'theirs.pdf' })]))); + + const res = await service.syncLink(link); + + expect(res.conflicts).toBe(0); + expect(provider.push).not.toHaveBeenCalled(); + expect(provider.rename).toHaveBeenCalledWith(expect.anything(), expect.anything(), 'r1', 'mine.pdf'); + expect(itemRow(itemId)).toMatchObject({ state: 'synced', remote_name: 'mine.pdf' }); + + provider.list.mockResolvedValueOnce(ok(listing([remoteDoc({ remoteId: 'r1', name: 'mine.pdf', remoteVersion: 'v2' })]))); + await service.syncLink(config.getLink(link.id)); + + expect(provider.rename).toHaveBeenCalledTimes(1); + expect(fileRows().map((f) => f.original_name)).toEqual(['mine.pdf']); + }); + + /** + * Only `touch` used to lift the missing flag, and a copy that came back + * under another name gets a rename instead, so it stayed on the issues list + * for another run. + */ + it('takes a copy on record as missing off the list when it is back under a new name', async () => { + const link = makeLink(); + const fileId = makeFile({ name: 'old.pdf' }); + const itemId = seedItem(link, { + remoteId: 'r1', remoteName: 'old.pdf', remoteVersion: 'v1', fileId, state: 'remote_missing', + remoteMissingAt: '2026-09-18 09:00:00', + }); + provider.list.mockResolvedValueOnce(ok(listing([remoteDoc({ remoteId: 'r1', name: 'new-upstream.pdf' })]))); + + await service.syncLink(link); + + expect(fileRows()[0].original_name).toBe('new-upstream.pdf'); + expect(itemRow(itemId)).toMatchObject({ state: 'synced', remote_missing_at: null, remote_name: 'new-upstream.pdf' }); + expect(service.issues(tripId)).toEqual([]); + }); + + it('does the same when the name TREK gave it meanwhile is carried upstream', async () => { + const link = makeLink(); + const fileId = makeFile({ name: 'new.pdf' }); + const itemId = seedItem(link, { + remoteId: 'r1', remoteName: 'old.pdf', remoteVersion: 'v1', fileId, state: 'remote_missing', + remoteMissingAt: '2026-09-18 09:00:00', + }); + provider.list.mockResolvedValueOnce(ok(listing([remoteDoc({ remoteId: 'r1', name: 'old.pdf' })]))); + + await service.syncLink(link); + + expect(provider.rename).toHaveBeenCalledWith(expect.anything(), expect.anything(), 'r1', 'new.pdf'); + expect(itemRow(itemId)).toMatchObject({ state: 'synced', remote_missing_at: null, remote_name: 'new.pdf' }); + }); + /** * The size a listing claims is not a promise. The transfer is cut off at * the cap while it is still being written, so an oversized body can never @@ -792,7 +856,7 @@ describe('DocSyncService', () => { /** * Pins what happens today, and it is wrong. The failed pull already wrote * the provider's version marker into the pairing, so the next run sees no - * change and emits `touch` — and `touch` promotes `error` to `synced`. The + * change and emits `touch`, and `touch` promotes `error` to `synced`. The * document is then booked as synced with no `file_id`, out of the issues * list and never retried. Reported rather than fixed here; this is the test * that turns red once it is. @@ -958,7 +1022,7 @@ describe('DocSyncService', () => { * pin what the code does TODAY, and it is the wrong way round: 'trek' * clears `content_sha256`, which is the only record that TREK's copy ever * moved, so the run pulls the provider's version instead of sending TREK's. - * Reported rather than fixed here — these are the tests that turn red when + * Reported rather than fixed here; these are the tests that turn red when * the two branches are put back. */ it('does not pull the provider copy when the conflict was resolved in TREK favour', async () => { @@ -994,8 +1058,8 @@ describe('DocSyncService', () => { /** * The double rename is the only conflict a person can actually produce, and * it is arbitrated by `remote_name` alone. Clearing a version marker left - * all three names untouched, so the next run raised the same conflict again - * — whatever the owner chose, the issue came back within minutes, forever. + * all three names untouched, so the next run raised the same conflict again: + * whatever the owner chose, the issue came back within minutes, forever. */ describe('a double rename', () => { function seedRenameConflict(link: LinkRow) { @@ -1039,6 +1103,27 @@ describe('DocSyncService', () => { expect(file.original_name).toBe('Rechnung.pdf'); }); + it('takes the provider name back the way a download would have written it', async () => { + // Written raw, the colon made the next run read TREK's name as a second + // rename of its own, and the conflict the owner had just settled came + // straight back. + const link = makeLink(); + const fileId = makeFile({ name: 'Invoice.pdf' }); + const itemId = seedItem(link, { + remoteId: 'r1', remoteName: 'Hotel: Kyoto.pdf', remoteVersion: 'v1', fileId, state: 'conflict', contentSha256: 'agreed-hash', + }); + provider.list.mockResolvedValue(ok(listing([ + remoteDoc({ remoteId: 'r1', name: 'Hotel: Kyoto 2026.pdf', remoteVersion: 'v1' }), + ]))); + + await service.resolveConflict(itemId, 'provider'); + + expect(itemRow(itemId).state).not.toBe('conflict'); + expect(provider.rename).not.toHaveBeenCalled(); + const file = testDb.prepare('SELECT original_name FROM trip_files WHERE id = ?').get(fileId) as { original_name: string }; + expect(file.original_name).toBe('Hotel_ Kyoto 2026.pdf'); + }); + it('does not leave the row in conflict either way', async () => { for (const keep of ['trek', 'provider'] as const) { const link = makeLink(); @@ -1090,7 +1175,7 @@ describe('DocSyncService', () => { * An upstream edit replaces TREK's copy instead of joining it. * * `pull_update` routes into the same `pull()` as a first download, and - * `FilesService.createFile` only inserts — so the edit arrived as a SECOND + * `FilesService.createFile` only inserts, so the edit arrived as a SECOND * document while the first stayed in the file manager holding the old bytes. * Worse, the pairing moves to the new row, which leaves the old one with no * sync item: the next run reads it as a document TREK gained and pushes the @@ -1317,6 +1402,15 @@ describe('DocSyncService', () => { const restore = (fileId: number) => testDb.prepare('UPDATE trip_files SET deleted_at = NULL WHERE id = ?').run(fileId); + /** + * A provider answering the way the WebDAV adapters do: asked with the + * cursor it handed out, "nothing changed" and no documents at all; asked + * without one, the whole folder. + */ + const quietUnlessAsked = (documents: RemoteDocument[]) => + async (_conn: DocumentConnectionRef, scope: DocumentScopeRef) => + ok(scope.cursor === null ? listing(documents) : listing([], { cursorUnchanged: true })); + /** A synced pairing whose file was deleted in TREK and whose deletion one run has seen. */ async function deletedAndSeen(link: LinkRow): Promise<{ itemId: number; fileId: number }> { const fileId = makeFile({ name: 'boarding.pdf', deletedAt: '2026-09-18 08:00:00' }); @@ -1447,8 +1541,8 @@ describe('DocSyncService', () => { it('does not upload a copy TREK binned that was restored at the provider meanwhile', async () => { // Seen while TREK's file was still in the trash, so the note that TREK - // binned it is dropped. Without that, a later restore under an unchanged - // upstream (no listing to look at) would upload a second copy. + // binned it is dropped. Without that, a later restore on a quiet binding + // would upload a second copy. const link = makeLink({ deletePolicy: 'trash' }); const { itemId, fileId } = await deletedAndSeen(link); @@ -1458,7 +1552,7 @@ describe('DocSyncService', () => { expect(provider.fetch).not.toHaveBeenCalled(); restore(fileId); - provider.list.mockResolvedValueOnce(ok(listing([], { cursorUnchanged: true }))); + provider.list.mockImplementationOnce(quietUnlessAsked([remoteDoc({ remoteId: 'r1' })])); await service.syncLink(config.getLink(link.id)); expect(provider.push).not.toHaveBeenCalled(); @@ -1466,6 +1560,60 @@ describe('DocSyncService', () => { expect(itemRow(itemId).remote_id).toBe('r1'); }); + it('asks a quiet binding for the whole listing and flags a copy that is gone', async () => { + // An unchanged cursor shows nothing of the copy, and on a quiet binding + // it stays unchanged until somebody touches the folder. Resumed blind, + // the row was booked as synced and counted as vanished later on. + const link = makeLink({ deletePolicy: 'unlink' }); + const { itemId, fileId } = await deletedAndSeen(link); + const visa = remoteDoc({ remoteId: 'r-visa', name: 'visa.pdf' }); + + restore(fileId); + provider.list.mockImplementation(quietUnlessAsked([visa])); + const run = await service.syncLink(config.getLink(link.id)); + + expect(provider.list.mock.calls[1][1].cursor).toBeNull(); + expect(run).toMatchObject({ state: 'ok', pulled: 1, pushed: 0, missing: 1 }); + expect(itemRow(itemId)).toMatchObject({ state: 'remote_missing', remote_trashed_at: null }); + expect(itemRow(itemId).remote_missing_at).not.toBeNull(); + + // Once the restore is settled the binding goes back to its cursor. + await service.syncLink(config.getLink(link.id)); + expect(provider.list.mock.calls[2][1].cursor).toBe('cursor-1'); + }); + + it('keeps the binding running when several restored files lost their copies meanwhile', async () => { + // Counted as vanished, five of them next to six in step tripped the + // mass-delete guard. The run was dropped before the restores were + // written, the next run planned the same, and the binding synced nothing + // at all, new documents included, until its circuit opened. + const link = makeLink({ deletePolicy: 'unlink' }); + const inStep = Array.from({ length: 6 }, (_, i) => { + const fileId = makeFile({ name: `doc-${i}.pdf`, storageKey: `key-doc-${i}` }); + seedItem(link, { remoteId: `s${i}`, remoteName: `doc-${i}.pdf`, remoteVersion: 'v1', fileId, contentSha256: FILE_SHA }); + return remoteDoc({ remoteId: `s${i}`, name: `doc-${i}.pdf` }); + }); + const restored = Array.from({ length: 5 }, (_, i) => seedItem(link, { + remoteId: `g${i}`, remoteName: `gone-${i}.pdf`, remoteVersion: 'v1', state: 'local_deleted', contentSha256: FILE_SHA, + fileId: makeFile({ name: `gone-${i}.pdf`, storageKey: `key-gone-${i}` }), + })); + makeFile({ name: 'new-here.pdf', storageKey: 'key-new-here' }); + const newThere = remoteDoc({ remoteId: 'r-new', name: 'new-there.pdf' }); + + provider.list.mockResolvedValueOnce(ok(listing([...inStep, newThere]))); + const first = await service.syncLink(link); + provider.list.mockResolvedValueOnce(ok(listing([ + ...inStep, newThere, remoteDoc({ remoteId: 'r-pushed', name: 'new-here.pdf' }), + ]))); + const second = await service.syncLink(config.getLink(link.id)); + + expect(first).toMatchObject({ state: 'ok', pulled: 1, pushed: 1, missing: 5 }); + expect(first.errorCode).toBeUndefined(); + expect(second).toMatchObject({ state: 'ok', pulled: 0, pushed: 0, missing: 0 }); + expect(linkRow(link.id)).toMatchObject({ failure_count: 0, last_sync_state: 'ok' }); + for (const id of restored) expect(itemRow(id).state).toBe('remote_missing'); + }); + it('flags the copy as missing when it vanished upstream by somebody else', async () => { const link = makeLink({ deletePolicy: 'unlink' }); const { itemId, fileId } = await deletedAndSeen(link); @@ -1520,6 +1668,174 @@ describe('DocSyncService', () => { expect(fileRows()).toHaveLength(0); expect(itemRow(itemId).state).toBe('local_deleted'); }); + + it('does not bring back a document purged while its failed update was held back', async () => { + // A failed update keeps the file and leaves the row in `error`, where the + // backoff drops the deletion. Purged in that window, the row read as a + // first download that never landed, and "Sync now" fetched it again. + const link = makeLink({ deletePolicy: 'trash' }); + const fileId = makeFile({ name: 'boarding.pdf' }); + const itemId = seedItem(link, { + remoteId: 'r1', remoteName: 'boarding.pdf', remoteVersion: 'v1', fileId, state: 'synced', contentSha256: FILE_SHA, + }); + provider.list.mockResolvedValue(ok(listing([remoteDoc({ remoteId: 'r1', remoteVersion: 'v2' })]))); + provider.fetch.mockResolvedValueOnce(fail('timeout')); + await service.syncLink(link); + expect(itemRow(itemId)).toMatchObject({ state: 'error', file_id: fileId }); + + purge(fileId); + service.retryShelvedItems(link.id); + await service.syncLink(config.getLink(link.id)); + + expect(provider.fetch).toHaveBeenCalledTimes(1); + expect(fileRows()).toHaveLength(0); + expect(provider.trash).toHaveBeenCalledTimes(1); + expect(itemRow(itemId).state).toBe('local_deleted'); + }); + }); + + /** + * A rename where the id is the path, walked through real runs. + * + * On Synology a rename upstream lists the same copy under a new id. The + * pairing used to be re-attached with a `touch` that wrote the new name into + * the arbiter while the trip file kept the old one, and was flagged missing + * in the same run; the run after that renamed the provider's copy back. + */ + describe('a rename where the id is the path', () => { + const AT = '2026-09-18T10:00:00Z'; + + beforeEach(() => { + capabilities = { ...BASE_CAPABILITIES, stableId: false, contentHashInListing: false }; + }); + + it('takes a rename made at the provider into TREK and does not rename it back', async () => { + const link = makeLink(); + const fileId = makeFile({ name: 'a.pdf' }); + const itemId = seedItem(link, { + remoteId: '/trek/a.pdf', remoteName: 'a.pdf', remoteVersion: 'va', fileId, state: 'synced', + contentSha256: FILE_SHA, remoteSize: 1024, remoteModifiedAt: AT, + }); + provider.list.mockResolvedValue(ok(listing([ + remoteDoc({ remoteId: '/trek/b.pdf', name: 'b.pdf', remoteVersion: 'vb', contentHash: FILE_SHA }), + ]))); + + const first = await service.syncLink(link); + + expect(first).toMatchObject({ state: 'ok', missing: 0, pulled: 0 }); + expect(fileRows().map((f) => [f.id, f.original_name, f.deleted_at])).toEqual([[fileId, 'b.pdf', null]]); + expect(itemRow(itemId)).toMatchObject({ + remote_id: '/trek/b.pdf', remote_name: 'b.pdf', remote_version: 'vb', state: 'synced', remote_missing_at: null, + }); + + const second = await service.syncLink(config.getLink(link.id)); + + expect(second).toMatchObject({ state: 'ok', missing: 0 }); + expect(provider.rename).not.toHaveBeenCalled(); + expect(provider.fetch).not.toHaveBeenCalled(); + expect(provider.push).not.toHaveBeenCalled(); + expect(fileRows().map((f) => f.original_name)).toEqual(['b.pdf']); + expect(itemRows()).toHaveLength(1); + }); + + it('takes a copy on record as missing off the list in the run that finds it renamed', async () => { + const link = makeLink(); + const fileId = makeFile({ name: 'a.pdf' }); + const itemId = seedItem(link, { + remoteId: '/trek/a.pdf', remoteName: 'a.pdf', remoteVersion: 'va', fileId, state: 'remote_missing', + contentSha256: FILE_SHA, remoteSize: 1024, remoteModifiedAt: AT, remoteMissingAt: '2026-09-18 09:00:00', + }); + provider.list.mockResolvedValue(ok(listing([ + remoteDoc({ remoteId: '/trek/b.pdf', name: 'b.pdf', remoteVersion: 'vb' }), + ]))); + + await service.syncLink(link); + + expect(itemRow(itemId)).toMatchObject({ remote_id: '/trek/b.pdf', state: 'synced', remote_missing_at: null }); + expect(fileRows().map((f) => f.original_name)).toEqual(['b.pdf']); + expect(service.issues(tripId)).toEqual([]); + + await service.syncLink(config.getLink(link.id)); + + expect(itemRow(itemId).state).toBe('synced'); + expect(provider.rename).not.toHaveBeenCalled(); + }); + + it('recognises the copy by size and time where the listing has no hash', async () => { + // What Synology actually sends. The pull records size and time, which is + // all there is to go on once the path has changed. + const link = makeLink(); + provider.list.mockResolvedValueOnce(ok(listing([ + remoteDoc({ remoteId: '/trek/a.pdf', name: 'a.pdf', remoteVersion: 'va' }), + ]))); + await service.syncLink(link); + expect(provider.fetch).toHaveBeenCalledTimes(1); + const fileId = Number(fileRows()[0].id); + + provider.list.mockResolvedValue(ok(listing([ + remoteDoc({ remoteId: '/trek/2026/b.pdf', name: 'b.pdf', remoteVersion: 'vb' }), + ]))); + const renamed = await service.syncLink(config.getLink(link.id)); + await service.syncLink(config.getLink(link.id)); + + expect(renamed.missing).toBe(0); + expect(provider.fetch).toHaveBeenCalledTimes(1); + expect(provider.rename).not.toHaveBeenCalled(); + expect(fileRows().map((f) => [f.id, f.original_name, f.deleted_at])).toEqual([[fileId, 'b.pdf', null]]); + expect(itemRows().map((r) => [r.remote_id, r.remote_name, r.state])).toEqual([['/trek/2026/b.pdf', 'b.pdf', 'synced']]); + }); + + it('recognises a copy it uploaded itself when it is renamed before the next run', async () => { + const link = makeLink(); + makeFile({ name: 'boarding.pdf' }); + provider.list.mockResolvedValueOnce(ok(listing([]))); + provider.push.mockResolvedValueOnce(ok({ + remoteId: '/trek/boarding.pdf', remoteVersion: 'v1', remoteModifiedAt: AT, deduplicated: false, + })); + await service.syncLink(link); + + provider.list.mockResolvedValue(ok(listing([ + remoteDoc({ remoteId: '/trek/flight.pdf', name: 'flight.pdf', remoteVersion: 'v2', size: FILE_BYTES.length }), + ]))); + await service.syncLink(config.getLink(link.id)); + await service.syncLink(config.getLink(link.id)); + + expect(provider.push).toHaveBeenCalledTimes(1); + expect(provider.fetch).not.toHaveBeenCalled(); + expect(provider.rename).not.toHaveBeenCalled(); + expect(fileRows().map((f) => f.original_name)).toEqual(['flight.pdf']); + expect(itemRows().map((r) => [r.remote_id, r.state])).toEqual([['/trek/flight.pdf', 'synced']]); + }); + + it('finds the copy again after sending a rename made in TREK upstream', async () => { + // The adapter cannot report the path its rename produced, so the row + // keeps the old one until the next listing shows where the copy went. + const link = makeLink(); + const fileId = makeFile({ name: 'b.pdf' }); + const itemId = seedItem(link, { + remoteId: '/trek/a.pdf', remoteName: 'a.pdf', remoteVersion: 'va', fileId, state: 'synced', + contentSha256: FILE_SHA, remoteSize: 1024, remoteModifiedAt: AT, + }); + provider.list.mockResolvedValueOnce(ok(listing([ + remoteDoc({ remoteId: '/trek/a.pdf', name: 'a.pdf', remoteVersion: 'va' }), + ]))); + provider.rename.mockResolvedValueOnce(ok({ remoteVersion: 'vb' })); + await service.syncLink(link); + expect(provider.rename).toHaveBeenCalledWith(expect.anything(), expect.anything(), '/trek/a.pdf', 'b.pdf'); + + provider.list.mockResolvedValue(ok(listing([ + remoteDoc({ remoteId: '/trek/b.pdf', name: 'b.pdf', remoteVersion: 'vb' }), + ]))); + const next = await service.syncLink(config.getLink(link.id)); + await service.syncLink(config.getLink(link.id)); + + expect(next.missing).toBe(0); + expect(provider.rename).toHaveBeenCalledTimes(1); + expect(provider.fetch).not.toHaveBeenCalled(); + expect(provider.push).not.toHaveBeenCalled(); + expect(fileRows()).toHaveLength(1); + expect(itemRow(itemId)).toMatchObject({ remote_id: '/trek/b.pdf', remote_name: 'b.pdf', state: 'synced' }); + }); }); describe('resolveConflict across trips', () => {