From f6d62489822d09a87325658da1cf9ef15cf54bb2 Mon Sep 17 00:00:00 2001 From: galuis116 Date: Thu, 11 Jun 2026 04:23:50 -0700 Subject: [PATCH 1/2] fix(registry): resolve repo casing case-insensitively on sync persistRegistrySnapshot keyed the upsert conflict target and the de-registration on the case-sensitive fullName primary key, while repo names arrive from multiple sources (registry vs GitHub-canonical) and the rest of the system resolves repos case-insensitively. A casing mismatch inserted a duplicate row and de-registered the wrong one (same class as #223/#225). Resolve each snapshot repo to an existing row by lowercased name before upsert, and de-register by lowercased comparison. --- src/registry/sync.ts | 23 +++++++++++++++------ test/unit/registry.test.ts | 41 +++++++++++++++++++++++++++++++++++++- 2 files changed, 57 insertions(+), 7 deletions(-) diff --git a/src/registry/sync.ts b/src/registry/sync.ts index cd645fe2f2..07ea1da2aa 100644 --- a/src/registry/sync.ts +++ b/src/registry/sync.ts @@ -1,4 +1,4 @@ -import { and, desc, eq, notInArray } from "drizzle-orm"; +import { and, desc, eq, inArray } from "drizzle-orm"; import { getDb } from "../db/client"; import { registrySnapshots, repositories, syncRuns } from "../db/schema"; import type { RegistrySnapshot } from "../types"; @@ -91,12 +91,20 @@ export async function persistRegistrySnapshot(env: Env, snapshot: RegistrySnapsh payloadJson: jsonString(snapshot as unknown as Record), }); + // repositories.fullName is a case-sensitive primary key, but repo names arrive from multiple sources + // (the upstream registry vs GitHub-canonical webhook/API casing) and the rest of the system resolves + // repos case-insensitively (getRepository). Resolve each snapshot repo to an existing row by lowercased + // name so a casing variant updates that row instead of inserting a duplicate primary key. + const existingFullNames = (await db.select({ fullName: repositories.fullName }).from(repositories)).map((row) => row.fullName); + const canonicalByLower = new Map(existingFullNames.map((name) => [name.toLowerCase(), name])); + for (const repo of snapshot.repositories) { - const parts = repoParts(repo.repo); + const fullName = canonicalByLower.get(repo.repo.toLowerCase()) ?? repo.repo; + const parts = repoParts(fullName); await db .insert(repositories) .values({ - fullName: repo.repo, + fullName, owner: parts.owner, name: parts.name, isRegistered: true, @@ -123,8 +131,11 @@ export async function persistRegistrySnapshot(env: Env, snapshot: RegistrySnapsh }); } - const registeredFullNames = snapshot.repositories.map((repo) => repo.repo); - if (registeredFullNames.length > 0) { + // De-register case-insensitively: only existing rows whose lowercased name is absent from the snapshot, + // so a casing variant of a still-registered repo is never wrongly de-registered. + const registeredLower = new Set(snapshot.repositories.map((repo) => repo.repo.toLowerCase())); + const staleFullNames = existingFullNames.filter((name) => !registeredLower.has(name.toLowerCase())); + if (staleFullNames.length > 0) { await db .update(repositories) .set({ @@ -136,7 +147,7 @@ export async function persistRegistrySnapshot(env: Env, snapshot: RegistrySnapsh labelMultipliersJson: "{}", updatedAt: nowIso(), }) - .where(and(eq(repositories.isRegistered, true), notInArray(repositories.fullName, registeredFullNames))); + .where(and(eq(repositories.isRegistered, true), inArray(repositories.fullName, staleFullNames))); } } diff --git a/test/unit/registry.test.ts b/test/unit/registry.test.ts index 69afa3676d..b72154212d 100644 --- a/test/unit/registry.test.ts +++ b/test/unit/registry.test.ts @@ -1,5 +1,5 @@ import { afterEach, describe, expect, it, vi } from "vitest"; -import { getRepository } from "../../src/db/repositories"; +import { getRepository, upsertRepositoryFromGitHub } from "../../src/db/repositories"; import { normalizeRegistryPayload } from "../../src/registry/normalize"; import { getLatestRegistrySnapshot, persistRegistrySnapshot, refreshRegistry } from "../../src/registry/sync"; import { createTestEnv } from "../helpers/d1"; @@ -147,6 +147,45 @@ describe("registry normalization", () => { }); }); + it("updates an existing case-variant repo row instead of inserting a duplicate", async () => { + const env = createTestEnv(); + // A GitHub-sourced row already exists under canonical casing. + await upsertRepositoryFromGitHub(env, { name: "gittensory", full_name: "JSONbored/gittensory", private: false, owner: { login: "JSONbored" } }); + + // The registry supplies the same repo with different casing. + await persistRegistrySnapshot( + env, + normalizeRegistryPayload( + { "jsonbored/gittensory": { emission_share: 0.02, issue_discovery_share: 0 } }, + { kind: "raw-github", url: "fixture://registry" }, + "2026-05-22T00:00:00.000Z", + ), + ); + + // The existing canonical row is updated to registered -- no duplicate primary-key row. + await expect(getRepository(env, "JSONbored/gittensory")).resolves.toMatchObject({ isRegistered: true }); + const rows = await env.DB.prepare("SELECT full_name FROM repositories WHERE lower(full_name) = ?").bind("jsonbored/gittensory").all(); + expect(rows.results).toHaveLength(1); + expect((rows.results[0] as { full_name: string }).full_name).toBe("JSONbored/gittensory"); + }); + + it("does not de-register a repo whose snapshot casing differs from the stored row", async () => { + const env = createTestEnv(); + await persistRegistrySnapshot( + env, + normalizeRegistryPayload({ "JSONbored/gittensory": { emission_share: 0.02, issue_discovery_share: 0 } }, { kind: "raw-github", url: "fixture://old" }, "2026-05-22T00:00:00.000Z"), + ); + // The next snapshot uses different casing for the same repo. + await persistRegistrySnapshot( + env, + normalizeRegistryPayload({ "jsonbored/gittensory": { emission_share: 0.02, issue_discovery_share: 0 } }, { kind: "raw-github", url: "fixture://new" }, "2026-05-23T00:00:00.000Z"), + ); + + await expect(getRepository(env, "JSONbored/gittensory")).resolves.toMatchObject({ isRegistered: true }); + const rows = await env.DB.prepare("SELECT full_name FROM repositories WHERE lower(full_name) = ?").bind("jsonbored/gittensory").all(); + expect(rows.results).toHaveLength(1); + }); + it("falls back to raw GitHub when registry API probes fail", async () => { vi.stubGlobal("fetch", async (input: RequestInfo | URL) => { const url = input.toString(); From 7a8d11024d95938337d891d695f2873cc6bae328 Mon Sep 17 00:00:00 2001 From: galuis116 Date: Fri, 12 Jun 2026 10:19:36 -0700 Subject: [PATCH 2/2] fix(registry): guard empty snapshots and intra-snapshot case dups Address review: (1) restore the empty-snapshot guard so a failed/empty registry fetch never de-registers every repo; (2) record each resolved canonical name within the sync loop so a case-variant duplicate in the same snapshot upserts the existing row instead of inserting a second case-only-different primary key. Add tests for both. --- src/registry/sync.ts | 7 ++++++- test/unit/registry.test.ts | 29 +++++++++++++++++++++++++++++ 2 files changed, 35 insertions(+), 1 deletion(-) diff --git a/src/registry/sync.ts b/src/registry/sync.ts index 07ea1da2aa..0a0a419c06 100644 --- a/src/registry/sync.ts +++ b/src/registry/sync.ts @@ -100,6 +100,9 @@ export async function persistRegistrySnapshot(env: Env, snapshot: RegistrySnapsh for (const repo of snapshot.repositories) { const fullName = canonicalByLower.get(repo.repo.toLowerCase()) ?? repo.repo; + // Record the resolved name so a later case-variant of the same repo within this snapshot maps to + // the same row (upsert) instead of inserting a second case-only-different primary key. + canonicalByLower.set(repo.repo.toLowerCase(), fullName); const parts = repoParts(fullName); await db .insert(repositories) @@ -133,9 +136,11 @@ export async function persistRegistrySnapshot(env: Env, snapshot: RegistrySnapsh // De-register case-insensitively: only existing rows whose lowercased name is absent from the snapshot, // so a casing variant of a still-registered repo is never wrongly de-registered. + // Never de-register on an empty snapshot (e.g. a failed/empty registry fetch) -- that would wipe every + // registration. Only de-register when the snapshot actually lists repos and some stored row is absent. const registeredLower = new Set(snapshot.repositories.map((repo) => repo.repo.toLowerCase())); const staleFullNames = existingFullNames.filter((name) => !registeredLower.has(name.toLowerCase())); - if (staleFullNames.length > 0) { + if (snapshot.repositories.length > 0 && staleFullNames.length > 0) { await db .update(repositories) .set({ diff --git a/test/unit/registry.test.ts b/test/unit/registry.test.ts index b72154212d..0bbf0edcf9 100644 --- a/test/unit/registry.test.ts +++ b/test/unit/registry.test.ts @@ -186,6 +186,35 @@ describe("registry normalization", () => { expect(rows.results).toHaveLength(1); }); + it("does not de-register existing repos when the snapshot is empty", async () => { + const env = createTestEnv(); + await persistRegistrySnapshot( + env, + normalizeRegistryPayload({ "JSONbored/gittensory": { emission_share: 0.02, issue_discovery_share: 0 } }, { kind: "raw-github", url: "fixture://seed" }, "2026-05-22T00:00:00.000Z"), + ); + // An empty snapshot (e.g. a failed/empty registry fetch) must preserve registrations, not wipe them. + await persistRegistrySnapshot(env, normalizeRegistryPayload({}, { kind: "raw-github", url: "fixture://empty" }, "2026-05-23T00:00:00.000Z")); + await expect(getRepository(env, "JSONbored/gittensory")).resolves.toMatchObject({ isRegistered: true }); + }); + + it("collapses case-variant duplicates within a single snapshot to one row", async () => { + const env = createTestEnv(); + await persistRegistrySnapshot( + env, + normalizeRegistryPayload( + { + "JSONbored/gittensory": { emission_share: 0.02, issue_discovery_share: 0 }, + "jsonbored/gittensory": { emission_share: 0.03, issue_discovery_share: 0 }, + }, + { kind: "raw-github", url: "fixture://dup-casing" }, + "2026-05-22T00:00:00.000Z", + ), + ); + const rows = await env.DB.prepare("SELECT full_name FROM repositories WHERE lower(full_name) = ?").bind("jsonbored/gittensory").all(); + expect(rows.results).toHaveLength(1); + await expect(getRepository(env, "JSONbored/gittensory")).resolves.toMatchObject({ isRegistered: true }); + }); + it("falls back to raw GitHub when registry API probes fail", async () => { vi.stubGlobal("fetch", async (input: RequestInfo | URL) => { const url = input.toString();