Harden external object sync edge cases

server/src/services/external-objects.ts

@@ -515,58 +515,64 @@ export function externalObjectService(
   }
 
   async function syncComment(commentId: string, dbOrTx: any = db) {
-    if (!(await isEnabled())) return;
-    const comment = await dbOrTx
-      .select({
-        id: issueComments.id,
-        companyId: issueComments.companyId,
-        issueId: issueComments.issueId,
-        body: issueComments.body,
-      })
-      .from(issueComments)
-      .where(eq(issueComments.id, commentId))
-      .then((rows: Array<{ id: string; companyId: string; issueId: string; body: string }>) => rows[0] ?? null);
-    if (!comment) throw notFound("Issue comment not found");
-    await replaceSourceMentions({
-      companyId: comment.companyId,
-      sourceIssueId: comment.issueId,
-      sourceKind: "comment",
-      sourceRecordId: comment.id,
-      documentKey: null,
-      propertyKey: null,
-      text: comment.body,
-    }, dbOrTx);
+    const runSync = async (tx: any) => {
+      if (!(await isEnabled())) return;
+      const comment = await tx
+        .select({
+          id: issueComments.id,
+          companyId: issueComments.companyId,
+          issueId: issueComments.issueId,
+          body: issueComments.body,
+        })
+        .from(issueComments)
+        .where(eq(issueComments.id, commentId))
+        .then((rows: Array<{ id: string; companyId: string; issueId: string; body: string }>) => rows[0] ?? null);
+      if (!comment) throw notFound("Issue comment not found");
+      await replaceSourceMentions({
+        companyId: comment.companyId,
+        sourceIssueId: comment.issueId,
+        sourceKind: "comment",
+        sourceRecordId: comment.id,
+        documentKey: null,
+        propertyKey: null,
+        text: comment.body,
+      }, tx);
+    };
+    return dbOrTx === db ? db.transaction(runSync) : runSync(dbOrTx);
   }
 
   async function syncDocument(documentId: string, dbOrTx: any = db) {
-    if (!(await isEnabled())) return;
-    const document = await dbOrTx
-      .select({
-        documentId: documents.id,
-        companyId: documents.companyId,
-        issueId: issueDocuments.issueId,
-        key: issueDocuments.key,
-        body: documents.latestBody,
-      })
-      .from(issueDocuments)
-      .innerJoin(documents, eq(issueDocuments.documentId, documents.id))
-      .where(eq(documents.id, documentId))
-      .then((rows: Array<{ documentId: string; companyId: string; issueId: string; key: string; body: string }>) => rows[0] ?? null);
-    if (!document) {
-      await dbOrTx
-        .delete(externalObjectMentions)
-        .where(and(eq(externalObjectMentions.sourceKind, "document"), eq(externalObjectMentions.sourceRecordId, documentId)));
-      return;
-    }
-    await replaceSourceMentions({
-      companyId: document.companyId,
-      sourceIssueId: document.issueId,
-      sourceKind: "document",
-      sourceRecordId: document.documentId,
-      documentKey: document.key,
-      propertyKey: null,
-      text: document.body,
-    }, dbOrTx);
+    const runSync = async (tx: any) => {
+      if (!(await isEnabled())) return;
+      const document = await tx
+        .select({
+          documentId: documents.id,
+          companyId: documents.companyId,
+          issueId: issueDocuments.issueId,
+          key: issueDocuments.key,
+          body: documents.latestBody,
+        })
+        .from(issueDocuments)
+        .innerJoin(documents, eq(issueDocuments.documentId, documents.id))
+        .where(eq(documents.id, documentId))
+        .then((rows: Array<{ documentId: string; companyId: string; issueId: string; key: string; body: string }>) => rows[0] ?? null);
+      if (!document) {
+        await tx
+          .delete(externalObjectMentions)
+          .where(and(eq(externalObjectMentions.sourceKind, "document"), eq(externalObjectMentions.sourceRecordId, documentId)));
+        return;
+      }
+      await replaceSourceMentions({
+        companyId: document.companyId,
+        sourceIssueId: document.issueId,
+        sourceKind: "document",
+        sourceRecordId: document.documentId,
+        documentKey: document.key,
+        propertyKey: null,
+        text: document.body,
+      }, tx);
+    };
+    return dbOrTx === db ? db.transaction(runSync) : runSync(dbOrTx);
   }
 
   async function safeSync(label: string, fn: () => Promise<void>) {

ui/src/components/IssuesList.tsx

@@ -1006,6 +1006,9 @@ export function IssuesList({
     externalObjectSummaryByIssueId,
     externalObjectSummariesReady: externalObjectSummariesReady && !externalObjectSummariesLoading,
   }), [externalObjectSummariesLoading, externalObjectSummariesReady, externalObjectSummaryByIssueId, issueFilterWorkspaceContext]);
+  const externalObjectFilterLoading = hasExternalObjectStatusFilters
+    && externalObjectSummariesLoading
+    && !externalObjectSummariesReady;
 
   const filtered = useMemo(() => {
     const filteredByControls = applyIssueFilters(
@@ -1597,7 +1600,7 @@ export function IssuesList({
         </div>
       </div>
 
-      {isLoading && <PageSkeleton variant="issues-list" />}
+      {(isLoading || externalObjectFilterLoading) && <PageSkeleton variant="issues-list" />}
       {error && <p className="text-sm text-destructive">{error.message}</p>}
       {!searchWithinLoadedIssues && normalizedIssueSearch.length > 0 && searchedIssues.length === ISSUE_SEARCH_RESULT_LIMIT && (
         <p className="text-xs text-muted-foreground">
@@ -1609,7 +1612,7 @@ export function IssuesList({
           Some board columns are showing up to {ISSUE_BOARD_COLUMN_RESULT_LIMIT} tasks. Refine filters or search to reveal the rest.
         </p>
       )}
-      {!isLoading && filtered.length === 0 && viewState.viewMode === "list" && (
+      {!isLoading && !externalObjectFilterLoading && filtered.length === 0 && viewState.viewMode === "list" && (
         <EmptyState
           icon={CircleDot}
           message="No tasks match the current filters or search."

ui/src/hooks/useIssueExternalObjects.test.ts

@@ -1,48 +0,0 @@
-import { afterEach, describe, expect, it, vi } from "vitest";
-import type { ExternalObjectSummary } from "@paperclipai/shared";
-import { externalObjectsApi } from "../api/externalObjects";
-import {
-  EXTERNAL_OBJECT_SUMMARY_BATCH_SIZE,
-  fetchIssueExternalObjectSummariesInBatches,
-} from "./useIssueExternalObjects";
-
-vi.mock("../api/externalObjects", () => ({
-  externalObjectsApi: {
-    getIssueSummaries: vi.fn(),
-  },
-}));
-
-const emptySummary: ExternalObjectSummary = {
-  total: 0,
-  byStatusCategory: {},
-  byLiveness: {},
-  highestSeverity: "neutral",
-  objects: [],
-};
-
-describe("fetchIssueExternalObjectSummariesInBatches", () => {
-  afterEach(() => {
-    vi.mocked(externalObjectsApi.getIssueSummaries).mockReset();
-  });
-
-  it("chunks bulk summary requests below the server validation cap and merges results", async () => {
-    const issueIds = Array.from(
-      { length: EXTERNAL_OBJECT_SUMMARY_BATCH_SIZE * 2 + 3 },
-      (_entry, index) => `issue-${index}`,
-    );
-    vi.mocked(externalObjectsApi.getIssueSummaries).mockImplementation(async (_companyId, ids) => ({
-      summaries: Object.fromEntries(ids.map((id) => [id, { ...emptySummary, total: id.endsWith("-0") ? 1 : 0 }])),
-    }));
-
-    const result = await fetchIssueExternalObjectSummariesInBatches("company-1", issueIds);
-
-    expect(externalObjectsApi.getIssueSummaries).toHaveBeenCalledTimes(3);
-    expect(vi.mocked(externalObjectsApi.getIssueSummaries).mock.calls.map((call) => call[1].length)).toEqual([
-      EXTERNAL_OBJECT_SUMMARY_BATCH_SIZE,
-      EXTERNAL_OBJECT_SUMMARY_BATCH_SIZE,
-      3,
-    ]);
-    expect(Object.keys(result.summaries)).toHaveLength(issueIds.length);
-    expect(result.summaries["issue-0"]?.total).toBe(1);
-  });
-});

ui/src/lib/external-objects.test.ts

@@ -1,4 +1,10 @@
-import { describe, expect, it } from "vitest";
+import { afterEach, describe, expect, it, vi } from "vitest";
+import type { ExternalObjectSummary } from "@paperclipai/shared";
+import { externalObjectsApi } from "../api/externalObjects";
+import {
+  EXTERNAL_OBJECT_SUMMARY_BATCH_SIZE,
+  fetchIssueExternalObjectSummariesInBatches,
+} from "../hooks/useIssueExternalObjects";
 import {
   dominantExternalObjectTone,
   externalObjectCategoryLabel,
@@ -14,6 +20,20 @@ import {
 } from "./external-objects";
 import { normalizeExternalObjectHref } from "./external-object-href";
 
+vi.mock("../api/externalObjects", () => ({
+  externalObjectsApi: {
+    getIssueSummaries: vi.fn(),
+  },
+}));
+
+const emptySummary: ExternalObjectSummary = {
+  total: 0,
+  byStatusCategory: {},
+  byLiveness: {},
+  highestSeverity: "neutral",
+  objects: [],
+};
+
 describe("normalizeExternalObjectHref", () => {
   it("lowercases the host (case preserving the path) and drops query/fragment", () => {
     expect(
@@ -35,6 +55,33 @@ describe("normalizeExternalObjectHref", () => {
   });
 });
 
+describe("fetchIssueExternalObjectSummariesInBatches", () => {
+  afterEach(() => {
+    vi.mocked(externalObjectsApi.getIssueSummaries).mockReset();
+  });
+
+  it("chunks bulk summary requests below the server validation cap and merges results", async () => {
+    const issueIds = Array.from(
+      { length: EXTERNAL_OBJECT_SUMMARY_BATCH_SIZE * 2 + 3 },
+      (_entry, index) => `issue-${index}`,
+    );
+    vi.mocked(externalObjectsApi.getIssueSummaries).mockImplementation(async (_companyId, ids) => ({
+      summaries: Object.fromEntries(ids.map((id) => [id, { ...emptySummary, total: id.endsWith("-0") ? 1 : 0 }])),
+    }));
+
+    const result = await fetchIssueExternalObjectSummariesInBatches("company-1", issueIds);
+
+    expect(externalObjectsApi.getIssueSummaries).toHaveBeenCalledTimes(3);
+    expect(vi.mocked(externalObjectsApi.getIssueSummaries).mock.calls.map((call) => call[1].length)).toEqual([
+      EXTERNAL_OBJECT_SUMMARY_BATCH_SIZE,
+      EXTERNAL_OBJECT_SUMMARY_BATCH_SIZE,
+      3,
+    ]);
+    expect(Object.keys(result.summaries)).toHaveLength(issueIds.length);
+    expect(result.summaries["issue-0"]?.total).toBe(1);
+  });
+});
+
 describe("external-objects helpers", () => {
   it("labels categories with copy from the UX spec", () => {
     expect(externalObjectCategoryLabel("auth_required")).toBe("Authorization required");