fix(orgs): bulk import only clears source pins it set in the same run (#408)

Follow-up to #407.

The bulk import cleared an organization's pin whenever another connected source listed the same organization name. That also undid pins a user had set on purpose through the card picker or the add dialog: pin acme to GitHub because the GitLab group of the same name is unrelated, and the next Import widened it back to every source.

Now the import only clears pins it created itself in the same run. Organizations it inserts (or re-pins on recovery) are remembered by name for the rest of the run; when a later source lists one of those names, the pin is cleared, since the organization spans several sources, the same case migration 0020 leaves unpinned. Pins that existed before the run are never touched. The bookkeeping is applied after each source's transaction commits, so a source that fails part way leaves it unchanged.

- `selectSameRunPinsToClear` in `src/lib/sources.ts` holds the rule, with unit tests.
- `src/pages/api/sync/index.ts` tracks the run's pins and applies the rule; the existing-org lookup goes back to name to status.
- Changelog line under the per-organization source selection entry.

Tests: 878 pass. Build passes.

CHANGELOG.md

@@ -12,6 +12,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
   - Organizations remember which source they import and mirror from; migration 0020 backfills the pin from each organization's repositories when they agree on one
   - Source picker in the add-organization dialog and on organization cards when more than one source is connected
   - `POST /api/sync/organization` persists `sourceId`, and `PATCH /api/organizations/:id` sets or clears it
+  - The bulk import only clears a pin it created itself in the same run when a second source lists the same organization; pins set through the picker or the add dialog are kept
 - Multi-source support: connect multiple source services per user (#375 follow-up)
   - Sources list on the Configuration page with add, edit and remove per source
   - Per-repository source attribution, with credentials resolved from each repository's own source

src/lib/sources.test.ts

@@ -4,6 +4,7 @@ import {
   deriveSourceName,
   findSourceForOrganization,
   findSourceForRepository,
+  selectSameRunPinsToClear,
   primarySource,
   resolveGitHubApiBaseUrl,
   selectRepositoriesToRelink,
@@ -121,6 +122,32 @@ describe("findSourceForOrganization", () => {
   });
 });
 
+describe("selectSameRunPinsToClear", () => {
+  const discovered = [{ normalizedName: "acme" }, { normalizedName: "tools" }];
+
+  test("clears a pin an earlier source in the same run created", () => {
+    const pinnedThisRun = new Map([["acme", "src-1"]]);
+    expect(selectSameRunPinsToClear(discovered, pinnedThisRun, "src-2")).toEqual(["acme"]);
+  });
+
+  test("leaves organizations the run has not pinned alone", () => {
+    // "tools" was pinned before the run (or never), so it is not in the map.
+    const pinnedThisRun = new Map([["other", "src-1"]]);
+    expect(selectSameRunPinsToClear(discovered, pinnedThisRun, "src-2")).toEqual([]);
+  });
+
+  test("keeps a pin the same source created", () => {
+    const pinnedThisRun = new Map([["acme", "src-2"]]);
+    expect(selectSameRunPinsToClear(discovered, pinnedThisRun, "src-2")).toEqual([]);
+  });
+
+  test("names each organization once", () => {
+    const pinnedThisRun = new Map([["acme", "src-1"]]);
+    const twice = [...discovered, { normalizedName: "acme" }];
+    expect(selectSameRunPinsToClear(twice, pinnedThisRun, "src-2")).toEqual(["acme"]);
+  });
+});
+
 describe("selectRepositoriesToRelink", () => {
   const connected = new Set(["src-1"]);
   const base = {

src/lib/sources.ts

@@ -144,6 +144,29 @@ export type DanglingRepositoryRow = {
   updatedAt: Date | string | null;
 };
 
+/**
+ * Normalized names among `discovered` whose pin the current bulk import run
+ * created itself (`pinnedThisRun`, name -> source id) from a different source
+ * than `sourceId`. Such an organization spans several sources, the ambiguous
+ * case migration 0020 leaves unpinned, so the caller clears the pin and drops
+ * the name from the map. Pins that existed before the run (set through the
+ * picker or the add dialog) are a stored choice and never appear in the map.
+ */
+export function selectSameRunPinsToClear(
+  discovered: ReadonlyArray<{ normalizedName: string }>,
+  pinnedThisRun: ReadonlyMap<string, string>,
+  sourceId: string
+): string[] {
+  const names: string[] = [];
+  for (const org of discovered) {
+    const pinnedBy = pinnedThisRun.get(org.normalizedName);
+    if (pinnedBy !== undefined && pinnedBy !== sourceId && !names.includes(org.normalizedName)) {
+      names.push(org.normalizedName);
+    }
+  }
+  return names;
+}
+
 // Repositories whose sourceId no longer resolves (or was never set) but whose
 // provider and host match the given source: exactly the rows a deleted and
 // re-added source must take back. Without the re-link, the next import sees

src/pages/api/sync/index.ts

@@ -40,7 +40,7 @@ export const POST: APIRoute = async ({ request, locals }) => {
     // is the primary source). A source without a token still lists public
     // repositories through its provider. Configs written outside the sources
     // API get their first source row seeded here.
-    const { listSources, ensureSourcesFromConfig } = await import("@/lib/sources");
+    const { listSources, ensureSourcesFromConfig, selectSameRunPinsToClear } = await import("@/lib/sources");
     const { createSourceProviderFromSource } = await import("@/lib/source-providers");
     await ensureSourcesFromConfig(userId);
     const sources = (await listSources(userId)).filter((source) => source.enabled);
@@ -71,6 +71,12 @@ export const POST: APIRoute = async ({ request, locals }) => {
     const failedOrgNames: string[] = [];
     const failedSourceNames: string[] = [];
 
+    // Organization pins this run set itself (inserted, or re-pinned on
+    // recovery), by normalized name. A later source listing the same name
+    // clears only these; pins that existed before the run are a stored
+    // choice and stay as they are. See selectSameRunPinsToClear.
+    const pinnedThisRun = new Map<string, string>();
+
     for (const source of sources) {
       try {
         const sourceProvider = createSourceProviderFromSource(source, { userId });
@@ -135,6 +141,11 @@ export const POST: APIRoute = async ({ request, locals }) => {
         let insertedOrgs: typeof newOrgs = [];
         let insertedFailedOrgs: typeof failedOrgRecords = [];
         let recoveredOrgCount = 0;
+        // Names whose pin this source set or cleared; applied to pinnedThisRun
+        // only after the transaction commits, so a rolled-back source leaves
+        // the run's pin bookkeeping untouched.
+        let pinnedOrgNames: string[] = [];
+        let clearedOrgNames: string[] = [];
 
         // Transaction to insert only new items
         await db.transaction(async (tx) => {
@@ -147,11 +158,7 @@ export const POST: APIRoute = async ({ request, locals }) => {
               .from(repositories)
               .where(eq(repositories.userId, userId)),
             tx
-              .select({
-                normalizedName: organizations.normalizedName,
-                status: organizations.status,
-                sourceId: organizations.sourceId,
-              })
+              .select({ normalizedName: organizations.normalizedName, status: organizations.status })
               .from(organizations)
               .where(eq(organizations.userId, userId)),
           ]);
@@ -160,9 +167,7 @@ export const POST: APIRoute = async ({ request, locals }) => {
           const existingRepoKeys = new Set(
             existingRepos.map((r) => `${r.sourceId ?? ""}|${r.normalizedFullName}`)
           );
-          const existingOrgMap = new Map(
-            existingOrgs.map((o) => [o.normalizedName, { status: o.status, sourceId: o.sourceId }])
-          );
+          const existingOrgMap = new Map(existingOrgs.map((o) => [o.normalizedName, o.status]));
 
           insertedRepos = newRepos.filter(
             (r) =>
@@ -175,7 +180,7 @@ export const POST: APIRoute = async ({ request, locals }) => {
           // follow the recovering source, or the org mirror keeps scoping
           // to the source the failure was recorded under.
           const recoveredOrgs = newOrgs.filter(
-            (o) => existingOrgMap.get(o.normalizedName)?.status === "failed"
+            (o) => existingOrgMap.get(o.normalizedName) === "failed"
           );
           for (const org of recoveredOrgs) {
             await tx
@@ -198,29 +203,20 @@ export const POST: APIRoute = async ({ request, locals }) => {
           }
           recoveredOrgCount = recoveredOrgs.length;
 
-          // An org that a second source also lists must not stay pinned to
-          // the first one: the mirror and the card counts would drop the
-          // other source's repositories. Clearing the pin matches migration
-          // 0020, which leaves orgs spanning multiple sources unpinned.
-          // Failed orgs are excluded — their pin follows the recovering
-          // source in the update above.
-          const crossSourceOrgs = newOrgs.filter((o) => {
-            const existing = existingOrgMap.get(o.normalizedName);
-            return (
-              existing !== undefined &&
-              existing.status !== "failed" &&
-              existing.sourceId != null &&
-              existing.sourceId !== source.id
-            );
-          });
-          for (const org of crossSourceOrgs) {
+          // An organization that an earlier source in this run pinned and
+          // that this source lists as well spans several sources, so its pin
+          // is cleared, the same rule migration 0020 applies. Only pins this
+          // run created are touched: an org the user pinned on purpose keeps
+          // its pin even when another source lists the same name.
+          clearedOrgNames = selectSameRunPinsToClear(newOrgs, pinnedThisRun, source.id);
+          for (const normalizedName of clearedOrgNames) {
             await tx
               .update(organizations)
               .set({ sourceId: null, updatedAt: new Date() })
               .where(
                 and(
                   eq(organizations.userId, userId),
-                  eq(organizations.normalizedName, org.normalizedName),
+                  eq(organizations.normalizedName, normalizedName),
                 )
               );
           }
@@ -228,7 +224,7 @@ export const POST: APIRoute = async ({ request, locals }) => {
           // Insert or update failed orgs (only update orgs already in "failed" state — don't overwrite good state)
           insertedFailedOrgs = failedOrgRecords.filter((o) => !existingOrgMap.has(o.normalizedName));
           const stillFailedOrgs = failedOrgRecords.filter(
-            (o) => existingOrgMap.get(o.normalizedName)?.status === "failed"
+            (o) => existingOrgMap.get(o.normalizedName) === "failed"
           );
           for (const org of stillFailedOrgs) {
             await tx
@@ -270,8 +266,16 @@ export const POST: APIRoute = async ({ request, locals }) => {
               await tx.insert(organizations).values(batch);
             }
           }
+          pinnedOrgNames = [...recoveredOrgs, ...insertedOrgs].map((org) => org.normalizedName);
         });
 
+        for (const normalizedName of pinnedOrgNames) {
+          pinnedThisRun.set(normalizedName, source.id);
+        }
+        for (const normalizedName of clearedOrgNames) {
+          pinnedThisRun.delete(normalizedName);
+        }
+
         // Create mirror jobs only for newly inserted items
         const mirrorJobPromises = [
           ...insertedRepos.map((repo) =>