fix(docsync): an orphaned binding stays stopped on every path that starts a run

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

@@ -411,6 +411,14 @@ export class DocSyncConfigService {
   updateLink(id: number, patch: Partial<DocsyncLinkInput>): LinkRow | undefined {
     const sets: string[] = [];
     const values: unknown[] = [];
+    // Sync automatically is the way back for an orphaned binding, but only once
+    // the person whose credential it runs under is on the trip again. Until
+    // then the switch stays off rather than handing that credential back.
+    const current = patch.syncEnabled === true ? this.getLink(id) : undefined;
+    if (current?.last_sync_state === 'orphaned') {
+      if (this.ownerLeft(current.connection_id)) patch = { ...patch, syncEnabled: undefined };
+      else sets.push("last_sync_state = 'never'");
+    }
     if (patch.direction !== undefined) { sets.push('direction = ?'); values.push(patch.direction); }
     if (patch.deletePolicy !== undefined) { sets.push('delete_policy = ?'); values.push(patch.deletePolicy); }
     if (patch.conflictPolicy !== undefined) { sets.push('conflict_policy = ?'); values.push(patch.conflictPolicy); }
@@ -467,6 +475,29 @@ export class DocSyncConfigService {
     };
   }
 
+  /**
+   * Whether a binding must not run. The sweep below marks it, but only while
+   * Sync automatically is on, so a paused binding whose owner left is not
+   * marked; every path that starts a run asks here rather than reading the
+   * mark alone.
+   */
+  isOrphaned(link: LinkRow): boolean {
+    return link.last_sync_state === 'orphaned' || this.ownerLeft(link.connection_id);
+  }
+
+  private ownerLeft(connectionId: number): boolean {
+    return !!this.db.connection
+      .prepare(
+        `SELECT 1 FROM document_connections c
+          WHERE c.id = ?
+            AND c.owner_user_id NOT IN (
+              SELECT tm.user_id FROM trip_members tm WHERE tm.trip_id = c.trip_id
+              UNION SELECT t.user_id FROM trips t WHERE t.id = c.trip_id
+            )`,
+      )
+      .get(connectionId);
+  }
+
   /**
    * Bindings whose credential owner is no longer on the trip.
    *

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

@@ -354,10 +354,9 @@ export class DocSyncController {
     const link = this.config.getLink(Number(linkId));
     if (!link || link.trip_id !== Number(tripId)) throw new HttpException('Link not found', 404);
     // An orphaned binding stays orphaned: its credential belongs to somebody who
-    // is no longer on this trip, and a manual run would use it anyway. The
-    // scheduler already refuses it (sync_enabled is 0), so without this the
-    // button was a way around that.
-    if (link.last_sync_state === 'orphaned') {
+    // is no longer on this trip. The run itself would stand down as well; this
+    // says why, instead of answering with an empty run.
+    if (this.config.isOrphaned(link)) {
       throw new HttpException({ error: 'This binding lost its owner and has to be reconnected' }, 409);
     }
     // Refused before the shelved rows are touched, so a binding an admin

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

@@ -101,7 +101,7 @@ export class DocSyncMcp {
       // 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') {
+      if (this.config.isOrphaned(link)) {
         results.push({ linkId: link.id, provider: link.provider_id, state: 'orphaned', errorCode: 'orphaned' });
         continue;
       }

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

@@ -112,6 +112,7 @@ export class DocSyncService {
       .prepare(
         `SELECT * FROM trip_document_links
           WHERE sync_enabled = 1
+            AND last_sync_state != 'orphaned'
             AND failure_count < ?
             AND (next_attempt_at IS NULL OR next_attempt_at <= CURRENT_TIMESTAMP)
             AND provider_id IN (SELECT id FROM document_providers WHERE enabled = 1)
@@ -176,6 +177,12 @@ export class DocSyncService {
     if (this.isSwitchedOff(link)) {
       return { state: 'disabled', pulled: 0, pushed: 0, conflicts: 0, missing: 0 };
     }
+    // An orphaned binding runs for nobody. Its credential belongs to somebody
+    // who has left the trip, and a webhook or a resolved conflict reach this
+    // without asking beforehand.
+    if (this.config.isOrphaned(link)) {
+      return { state: 'orphaned', pulled: 0, pushed: 0, conflicts: 0, missing: 0 };
+    }
     const conn = this.config.getConnection(link.connection_id);
     if (!conn) {
       this.recordLinkFailure(link, 'not_found');

server/tests/unit/nest/doc-sync/doc-sync-config.service.test.ts

@@ -645,3 +645,58 @@ describe('markOrphanedLinks', () => {
     expect(svc.getLink(paused.id)?.last_sync_state).toBe('never');
   });
 });
+
+describe('isOrphaned', () => {
+  async function bind(ownerUserId: number): Promise<LinkRow> {
+    return link((await connect({}, ownerUserId)).id);
+  }
+
+  it('answers yes for a paused binding whose owner left, which the sweep never marks', async () => {
+    const paused = await bind(MEMBER);
+    svc.updateLink(paused.id, { syncEnabled: false });
+    testDb.prepare('DELETE FROM trip_members WHERE trip_id = ? AND user_id = ?').run(TRIP, MEMBER);
+
+    expect(svc.markOrphanedLinks()).toBe(0);
+    expect(svc.isOrphaned(svc.getLink(paused.id)!)).toBe(true);
+  });
+
+  it('answers yes for a marked binding, and no while the owner is on the trip', async () => {
+    const kept = await bind(OWNER);
+    expect(svc.isOrphaned(kept)).toBe(false);
+
+    testDb.prepare("UPDATE trip_document_links SET last_sync_state = 'orphaned' WHERE id = ?").run(kept.id);
+    expect(svc.isOrphaned(svc.getLink(kept.id)!)).toBe(true);
+  });
+
+  it('leaves a connection that is gone to the run, which reports it as not found', async () => {
+    const bound = await bind(OWNER);
+    expect(svc.isOrphaned({ ...bound, connection_id: 999999 })).toBe(false);
+  });
+});
+
+describe('switching an orphaned binding back on', () => {
+  async function orphan(): Promise<LinkRow> {
+    const bound = link((await connect({}, MEMBER)).id);
+    testDb.prepare('DELETE FROM trip_members WHERE trip_id = ? AND user_id = ?').run(TRIP, MEMBER);
+    svc.markOrphanedLinks();
+    return bound;
+  }
+
+  it('keeps it off while its owner is still gone, and changes the rest of the patch', async () => {
+    const bound = await orphan();
+
+    const res = svc.updateLink(bound.id, { syncEnabled: true, direction: 'pull' });
+
+    expect(res).toMatchObject({ sync_enabled: 0, last_sync_state: 'orphaned', direction: 'pull' });
+  });
+
+  it('lets it run again once its owner is back on the trip', async () => {
+    const bound = await orphan();
+    testDb.prepare('INSERT INTO trip_members (trip_id, user_id) VALUES (?, ?)').run(TRIP, MEMBER);
+
+    const res = svc.updateLink(bound.id, { syncEnabled: true });
+
+    expect(res).toMatchObject({ sync_enabled: 1, last_sync_state: 'never' });
+    expect(svc.isOrphaned(res!)).toBe(false);
+  });
+});

server/tests/unit/nest/doc-sync/doc-sync.controller.test.ts

@@ -668,6 +668,23 @@ describe('a manual run', () => {
     expect(sync.retryShelvedItems).not.toHaveBeenCalled();
     expect(sync.syncLink).not.toHaveBeenCalled();
   });
+
+  it('refuses a paused binding whose owner left, although the sweep never marked it', async () => {
+    const res = await config.upsertConnection(tripId, Number(member.id), connBody({ credentials: { api_token: 'member-token' } }));
+    if ('error' in res) throw new Error('fixture failed');
+    const created = (await controller.createLink(String(tripId), owner, linkBody(res.data.id, { syncEnabled: false }), makeReq())) as { id: number };
+    sync.syncLink.mockClear();
+    testDb.prepare('DELETE FROM trip_members WHERE trip_id = ? AND user_id = ?').run(tripId, member.id);
+    try {
+      const err = await thrown(() => controller.syncNow(String(tripId), String(created.id), { full: false }));
+
+      expect(err.getStatus()).toBe(409);
+      expect(sync.retryShelvedItems).not.toHaveBeenCalled();
+      expect(sync.syncLink).not.toHaveBeenCalled();
+    } finally {
+      testDb.prepare('INSERT INTO trip_members (trip_id, user_id) VALUES (?, ?)').run(tripId, member.id);
+    }
+  });
 });
 
 describe('the document list', () => {

server/tests/unit/nest/doc-sync/doc-sync.mcp.test.ts

@@ -46,7 +46,12 @@ interface Setup {
 function makeMcp(over: Partial<Setup> = {}) {
   const setup: Setup = { access: true, links: [], status: {}, issues: [], addonOn: true, off: [], ...over };
   const files = { verifyTripAccess: vi.fn(() => (setup.access ? { id: 3, user_id: 7 } : undefined)) };
-  const config = { listLinks: vi.fn(() => setup.links) };
+  const config = {
+    listLinks: vi.fn(() => setup.links),
+    // The real helper also asks whether the owner is still on the trip; here
+    // the mark stands in for both.
+    isOrphaned: vi.fn((l: LinkRow) => l.last_sync_state === 'orphaned'),
+  };
   const sync = {
     status: vi.fn(() => setup.status),
     issues: vi.fn(() => setup.issues),

server/tests/unit/nest/doc-sync/doc-sync.service.test.ts

@@ -394,6 +394,16 @@ describe('DocSyncService', () => {
       const nearly = makeLink({ failureCount: LINK_CIRCUIT_OPEN_AFTER - 1 });
       expect(service.dueLinks().map((l) => l.id)).toEqual([nearly.id]);
     });
+
+    it('leaves out an orphaned binding even when Sync automatically was switched back on', () => {
+      // It would never run, so it would never get a last_sync_at and would take
+      // one of the slots on every tick.
+      const orphaned = makeLink({ lastSyncAt: null });
+      testDb.prepare("UPDATE trip_document_links SET last_sync_state = 'orphaned' WHERE id = ?").run(orphaned.id);
+      const live = makeLink({ lastSyncAt: sqlTime('-1 minute') });
+
+      expect(service.dueLinks().map((l) => l.id)).toEqual([live.id]);
+    });
   });
 
   describe('syncLink', () => {
@@ -1054,6 +1064,21 @@ describe('DocSyncService', () => {
       expect(fileRows()).toHaveLength(1);
     });
 
+    it('never runs an orphaned binding, whoever asks', async () => {
+      // Sync automatically switched back on puts sync_enabled to 1 again, and a
+      // webhook or a resolved conflict calls syncLink without asking first.
+      const link = makeLink();
+      testDb.prepare("UPDATE trip_document_links SET last_sync_state = 'orphaned' WHERE id = ?").run(link.id);
+      const before = linkRow(link.id);
+
+      const res = await service.syncLink(config.getLink(link.id));
+
+      expect(res.state).toBe('orphaned');
+      expect(provider.resolveScope).not.toHaveBeenCalled();
+      expect(provider.list).not.toHaveBeenCalled();
+      expect(linkRow(link.id)).toEqual(before);
+    });
+
     it('answers the same question for any caller that has to refuse up front', () => {
       const link = makeLink();
       expect(service.isSwitchedOff(link)).toBe(false);