From 03d3f2f91e9ea044503228f75adf718961fdb1ba Mon Sep 17 00:00:00 2001
From: Maurice <mauriceboe@icloud.com>
Date: Sat, 19 Sep 2026 21:01:45 +0200
Subject: [PATCH] fix(docsync): refuse an orphaned binding over MCP as the REST
 route does

A binding whose credential owner left the trip is orphaned: the
scheduler, the webhook and the REST "Sync now" route all refuse it,
because a run would use the credential of somebody who is no longer on
the trip. The MCP sync_trip_documents tool only checked the admin
switch, so any member with MCP could still run such a binding on the
ex-member's account.

The tool now skips an orphaned binding before anything is un-shelved,
reports it with the orphaned code, and answers with an error when no
binding of the trip could run at all.
---
 server/src/nest/doc-sync/doc-sync.mcp.ts      | 12 +++++++++
 .../unit/nest/doc-sync/doc-sync.mcp.test.ts   | 27 +++++++++++++++++--
 2 files changed, 37 insertions(+), 2 deletions(-)

diff --git a/server/src/nest/doc-sync/doc-sync.mcp.ts b/server/src/nest/doc-sync/doc-sync.mcp.ts
index 2e0596475..97ea486ca 100644
--- a/server/src/nest/doc-sync/doc-sync.mcp.ts
+++ b/server/src/nest/doc-sync/doc-sync.mcp.ts
@@ -98,6 +98,13 @@ export class DocSyncMcp {
     }
     const results = [];
     for (const link of links) {
+      // An orphaned binding stays stopped, as it does on the REST route and for
+      // the scheduler: its credential belongs to somebody who has left the trip,
+      // and a run would use it anyway.
+      if (link.last_sync_state === 'orphaned') {
+        results.push({ linkId: link.id, provider: link.provider_id, state: 'orphaned', errorCode: 'orphaned' });
+        continue;
+      }
       // Refused as the REST route refuses it, before the shelved rows below are
       // touched: a binding an admin switched off stays exactly as it was, so it
       // resumes where it stopped once the provider is back on.
@@ -117,6 +124,11 @@ export class DocSyncMcp {
         `${PROVIDER_DISABLED}: an administrator has switched off the document store this trip is bound to, so nothing was synced. The binding and its documents are kept, and syncing resumes once the store is switched back on.`,
       );
     }
+    if (results.every((r) => r.errorCode === PROVIDER_DISABLED || r.errorCode === 'orphaned')) {
+      return errorResult(
+        "orphaned: the person whose account this trip's document store was connected with has left the trip, so nothing was synced. The trip owner has to connect the store again in the trip's file manager.",
+      );
+    }
     return ok({ runs: results });
   }
 }
diff --git a/server/tests/unit/nest/doc-sync/doc-sync.mcp.test.ts b/server/tests/unit/nest/doc-sync/doc-sync.mcp.test.ts
index 1add73306..5b700fc54 100644
--- a/server/tests/unit/nest/doc-sync/doc-sync.mcp.test.ts
+++ b/server/tests/unit/nest/doc-sync/doc-sync.mcp.test.ts
@@ -27,8 +27,8 @@ import type { McpContext, McpTextResult, ToolOptions } from '../../../../src/nes
 
 const ctx = { userId: 7, scopes: null, isStaticToken: false } as McpContext;
 
-const link = (id: number, providerId = 'paperless'): LinkRow =>
-  ({ id, provider_id: providerId, trip_id: 3 } as LinkRow);
+const link = (id: number, providerId = 'paperless', lastSyncState: string | null = null): LinkRow =>
+  ({ id, provider_id: providerId, trip_id: 3, last_sync_state: lastSyncState } as LinkRow);
 
 const RUN = { state: 'ok', pulled: 2, pushed: 1, conflicts: 0, missing: 0 };
 
@@ -256,6 +256,29 @@ describe('sync_trip_documents', () => {
     expect(sync.syncLink).toHaveBeenCalledTimes(1);
   });
 
+  it('refuses an orphaned binding as the REST route does, without touching it', async () => {
+    const { mcp, sync } = makeMcp({ links: [link(1, 'paperless', 'orphaned')] });
+    const text = refusal(await mcp.syncNow({ tripId: 3 }, ctx));
+    expect(text.startsWith('orphaned:')).toBe(true);
+    expect(text).toContain('connect the store again');
+    // The credential belongs to somebody who left the trip: no run, and the
+    // shelved rows stay shelved.
+    expect(sync.retryShelvedItems).not.toHaveBeenCalled();
+    expect(sync.syncLink).not.toHaveBeenCalled();
+  });
+
+  it('runs the bindings that still have an owner and reports the orphaned one by its code', async () => {
+    const links = [link(1, 'paperless', 'orphaned'), link(2, 'nextcloud')];
+    const { mcp, sync } = makeMcp({ links });
+    const body = payload(await mcp.syncNow({ tripId: 3 }, ctx));
+    expect(body.runs).toEqual([
+      { linkId: 1, provider: 'paperless', state: 'orphaned', errorCode: 'orphaned' },
+      { linkId: 2, provider: 'nextcloud', ...RUN },
+    ]);
+    expect(sync.retryShelvedItems.mock.calls).toEqual([[2]]);
+    expect(sync.syncLink).toHaveBeenCalledTimes(1);
+  });
+
   it('asks for the cheap incremental run unless the caller says otherwise', async () => {
     const { mcp, sync } = makeMcp({ links: [link(1)] });
     await mcp.syncNow({ tripId: 3 }, ctx);