From 852b3c144243d8ea7302e764f7b633ea8be9989f Mon Sep 17 00:00:00 2001 From: JSONbored <49853598+JSONbored@users.noreply.github.com> Date: Tue, 14 Jul 2026 15:43:33 -0700 Subject: [PATCH] fix(github): migrate repo identity on a GitHub repository rename webhook Nothing in the codebase handled the GitHub App's `repository` webhook with `action: "renamed"` -- a repo rename (e.g. gittensory -> loopover) left every repo-identity-keyed row pointing at the old full_name until the next unrelated write happened to touch it, and any row keyed only by the old name (repositories, repository_settings) never updated at all without a webhook-driven path. Adds maybeHandleRepositoryRenamedWebhookEvent, wired into processGitHubWebhook right before the existing upsertRepositoryFromGitHub(payload.repository) call so that upsert UPDATEs the now-renamed anchor row instead of inserting a fresh, disconnected duplicate. renameRepositoryIdentity (src/db/repo-identity-rename.ts) migrates the structural identity columns for repositories, repository_settings, pull_requests, issues, and audit_events' target_key -- explicit per-table code (matching this codebase's house convention in repositories.ts) rather than a generic Drizzle helper, with a fold-on-collision pattern for the unique (repo_full_name, number) constraints. Also detects the one thing a rename can't migrate on its own: a self-host operator's container-private per-repo config folder is derived from the CURRENT repo name and is read-only from the app's perspective, so a folder that existed under the old name but not the new one means the operator's gate/autonomy/review policy silently reverted to defaults. maybeWarnOnMissingLocalConfigAfterRename surfaces that as an error-level audit event + log line instead of a routine info line, using the new hasLocalManifest() (focus-manifest-loader.ts). Scope is deliberately narrow to structural repo-identity columns -- review/audit tables (gate_outcomes, advisories, pull_request_reviews, etc.), caches/analytics tables, and REES/enrichment tables are tracked as separate follow-up PRs so each stays independently reviewable. --- src/db/repo-identity-rename.ts | 101 +++++++++++++++ src/queue/processors.ts | 70 ++++++++++- src/signals/focus-manifest-loader.ts | 17 +++ src/types.ts | 10 ++ test/unit/focus-manifest-loader.test.ts | 1 + test/unit/repo-identity-rename.test.ts | 156 ++++++++++++++++++++++++ test/unit/repo-rename-webhook.test.ts | 114 +++++++++++++++++ 7 files changed, 468 insertions(+), 1 deletion(-) create mode 100644 src/db/repo-identity-rename.ts create mode 100644 test/unit/repo-identity-rename.test.ts create mode 100644 test/unit/repo-rename-webhook.test.ts diff --git a/src/db/repo-identity-rename.ts b/src/db/repo-identity-rename.ts new file mode 100644 index 0000000000..9814311585 --- /dev/null +++ b/src/db/repo-identity-rename.ts @@ -0,0 +1,101 @@ +// #repo-rename-migration: GitHub identifies a repository by a stable numeric id, but this schema keys +// almost everything off the full_name STRING (repositories.full_name is itself the primary key, and +// most other tables carry a plain repo_full_name column with no foreign-key cascade). A GitHub repo +// rename webhook carries the SAME installation and the new current full_name, but nothing here +// recognizes it as the same repo -- upsertRepositoryFromGitHub's onConflictDoUpdate keys on full_name, +// so the very next webhook after a rename creates a second, disconnected row instead of updating the +// existing one, silently orphaning every PR/issue/audit-trail row already recorded under the old name. +// +// This module is the fix: renameRepositoryIdentity walks every repo-identity-bearing table and moves +// the old name's rows forward to the new name, so a rename preserves history instead of forking it. +// Idempotent (safe to re-run for a redelivered webhook -- every step only touches rows still under +// oldFullName) and collision-safe (where a unique constraint exists, a row that already exists under +// newFullName -- e.g. from a webhook that slipped in under the new name before this ran -- is folded +// away in favor of the pre-existing oldFullName row, never the reverse, so history is never dropped). +// +// Deliberately narrow in scope: only structural identity columns (the ones that determine which repo a +// row belongs to, or serve as part of a primary/unique key) are touched. Free-text content (titles, +// summaries, audit detail), *_json snapshots, and URL columns are left as an accurate historical record +// of what was true when they were captured -- GitHub's own redirect keeps old html_url values working, +// and rewriting historical text/audit content is not what this fix is for. +// +// One explicit block per table, deliberately not a generic cross-table helper: Drizzle's table/column +// types don't generalize cleanly across tables with different secondary keys, and this codebase's own +// convention (repositories.ts) is explicit per-table queries throughout, not a shared query abstraction. +// New tables extend this function directly, following the same shape. +import { and, eq, inArray, sql } from "drizzle-orm"; +import { getDb } from "./client"; +import { auditEvents, issues, pullRequests, repositories, repositorySettings } from "./schema"; + +function repoParts(fullName: string): { owner: string; name: string } { + const slash = fullName.indexOf("/"); + return slash === -1 ? { owner: fullName, name: fullName } : { owner: fullName.slice(0, slash), name: fullName.slice(slash + 1) }; +} + +/** + * Renames a repository's identity across every structural repo-identity column this module covers so + * far. Call this BEFORE the normal upsertRepositoryFromGitHub(env, payload.repository, ...) call that + * every webhook triggers -- once the anchor `repositories` row is renamed, that upsert correctly UPDATEs + * it instead of inserting a fresh duplicate. A no-op when oldFullName === newFullName. + */ +export async function renameRepositoryIdentity(env: Env, oldFullName: string, newFullName: string): Promise { + if (oldFullName === newFullName) return; + const db = getDb(env.DB); + const { owner, name } = repoParts(newFullName); + + // repositories (PK: full_name alone) -- fold a stray new-name row first, then rename the anchor row. + await db.delete(repositories).where(eq(repositories.fullName, newFullName)); + await db + .update(repositories) + .set({ + fullName: newFullName, + owner, + name, + htmlUrl: sql`replace(${repositories.htmlUrl}, ${oldFullName}, ${newFullName})`, + }) + .where(eq(repositories.fullName, oldFullName)); + + // repositorySettings (PK: repo_full_name alone) -- same fold-then-rename shape. + await db.delete(repositorySettings).where(eq(repositorySettings.repoFullName, newFullName)); + await db.update(repositorySettings).set({ repoFullName: newFullName }).where(eq(repositorySettings.repoFullName, oldFullName)); + + // pullRequests: unique (repo_full_name, number) -- fold any new-name row whose number already exists + // under the old name, favoring the pre-existing (oldFullName) row's history. + const collidingPullNumbers = ( + await db.select({ number: pullRequests.number }).from(pullRequests).where(eq(pullRequests.repoFullName, oldFullName)) + ).map((row) => row.number); + if (collidingPullNumbers.length > 0) { + await db.delete(pullRequests).where(and(eq(pullRequests.repoFullName, newFullName), inArray(pullRequests.number, collidingPullNumbers))); + } + await db + .update(pullRequests) + .set({ + repoFullName: newFullName, + id: sql`replace(${pullRequests.id}, ${oldFullName}, ${newFullName})`, + htmlUrl: sql`replace(${pullRequests.htmlUrl}, ${oldFullName}, ${newFullName})`, + }) + .where(eq(pullRequests.repoFullName, oldFullName)); + + // issues: same shape as pullRequests -- unique (repo_full_name, number). + const collidingIssueNumbers = ( + await db.select({ number: issues.number }).from(issues).where(eq(issues.repoFullName, oldFullName)) + ).map((row) => row.number); + if (collidingIssueNumbers.length > 0) { + await db.delete(issues).where(and(eq(issues.repoFullName, newFullName), inArray(issues.number, collidingIssueNumbers))); + } + await db + .update(issues) + .set({ + repoFullName: newFullName, + id: sql`replace(${issues.id}, ${oldFullName}, ${newFullName})`, + htmlUrl: sql`replace(${issues.htmlUrl}, ${oldFullName}, ${newFullName})`, + }) + .where(eq(issues.repoFullName, oldFullName)); + + // auditEvents.target_key: an append-only log with no uniqueness on target_key (many rows legitimately + // share one), so a plain substring rename with no dedupe step is correct and sufficient. + await db + .update(auditEvents) + .set({ targetKey: sql`replace(${auditEvents.targetKey}, ${oldFullName}, ${newFullName})` }) + .where(sql`${auditEvents.targetKey} like ${`%${oldFullName}%`}`); +} diff --git a/src/queue/processors.ts b/src/queue/processors.ts index d744ac4b96..abe39e6a33 100644 --- a/src/queue/processors.ts +++ b/src/queue/processors.ts @@ -87,6 +87,7 @@ import { upsertPullRequestFromGitHub, upsertRepositoryFromGitHub, } from "../db/repositories"; +import { renameRepositoryIdentity } from "../db/repo-identity-rename"; import { effectiveIssueCapForAccountAge, isBelowAccountAgeThreshold, @@ -490,6 +491,7 @@ import { } from "../signals/focus-manifest"; import { decideReviewEligibility } from "../review/review-eligibility"; import { + hasLocalManifest, loadPublicRepoFocusManifest, loadRepoFocusManifest, loadRepoFocusManifests, @@ -626,7 +628,7 @@ import type { RepositorySettings, } from "../types"; import { sha256Hex } from "../utils/crypto"; -import { errorMessage, nowIso } from "../utils/json"; +import { errorMessage, nowIso, repoParts } from "../utils/json"; import { maybeSuggestMilestoneMatchForPr } from "../integrations/project-tracker-adapter"; const OFFICIAL_MINER_DETECTION_TTL_MS = 5 * 60 * 1000; @@ -5184,6 +5186,69 @@ async function maybeHandleForeignAppInstallationWebhookEvent( return false; } +/** + * Handles a `repository` webhook with `action: "renamed"`: migrates the repo's identity forward across + * every structural repo-identity column (see repo-identity-rename.ts) BEFORE the caller's normal + * upsertRepositoryFromGitHub(payload.repository) runs, so that upsert correctly UPDATEs the now-renamed + * anchor row instead of inserting a fresh, disconnected duplicate. A no-op (and safely so) for any other + * event/action, for a payload missing the old-name field, or when the old and new names are identical + * (e.g. a case-only GitHub-side normalization with nothing to migrate). + */ +async function maybeHandleRepositoryRenamedWebhookEvent( + env: Env, + eventName: string, + payload: GitHubWebhookPayload, +): Promise { + if (eventName !== "repository" || payload.action !== "renamed") return; + const oldName = payload.changes?.repository?.name?.from; + const newFullName = payload.repository?.full_name; + if (!oldName || !newFullName) return; + const owner = payload.repository?.owner?.login ?? repoParts(newFullName).owner; + const oldFullName = `${owner}/${oldName}`; + if (oldFullName === newFullName) return; + await renameRepositoryIdentity(env, oldFullName, newFullName); + await recordAuditEvent(env, { + eventType: "github_app.repository_renamed", + actor: "loopover", + targetKey: newFullName, + outcome: "completed", + detail: `repository identity migrated from ${oldFullName} to ${newFullName}`, + metadata: { oldFullName, newFullName }, + }); + await maybeWarnOnMissingLocalConfigAfterRename(env, oldFullName, newFullName); +} + +/** + * #repo-rename-migration (self-host follow-up): the ONE piece of a rename that renameRepositoryIdentity + * cannot fix on its own -- a self-host operator's container-private per-repo config folder + * (LOOPOVER_REPO_CONFIG_DIR/{owner}__{repo}/...), which the app can only READ (the mount is read-only) + * and which private-config.ts derives from the repo's CURRENT name. If one existed under the old name but + * not the new one, the operator's gate/autonomy/review policy for this repo just silently reverted to + * global defaults -- loud enough to reach Sentry (level:"error"), not a routine info line, because the + * failure mode is exactly "reviews quietly stop matching what the operator configured," not a crash. + * A cloud deployment (no local reader registered) always resolves both sides false, so this never fires there. + */ +async function maybeWarnOnMissingLocalConfigAfterRename(env: Env, oldFullName: string, newFullName: string): Promise { + const [hadOldLocalConfig, hasNewLocalConfig] = await Promise.all([hasLocalManifest(oldFullName), hasLocalManifest(newFullName)]); + if (!hadOldLocalConfig || hasNewLocalConfig) return; + await recordAuditEvent(env, { + eventType: "selfhost.repo_rename_local_config_missing", + actor: "loopover", + targetKey: newFullName, + outcome: "denied", + detail: `${oldFullName} had a container-private per-repo config folder, but ${newFullName} does not -- the operator's gate/autonomy/review policy for this repo has silently reverted to defaults. Rename or copy the config folder on the host to match the new repo name.`, + metadata: { oldFullName, newFullName }, + }); + console.error( + JSON.stringify({ + level: "error", + event: "selfhost_repo_rename_local_config_missing", + oldFullName, + newFullName, + }), + ); +} + /** * Handles the `installation_repositories` webhook event: upserts added repos, marks removed repos, and * records product-usage telemetry for both. Extracted from processGitHubWebhook (#4607) — pure code @@ -6142,6 +6207,9 @@ export async function processGitHubWebhook( : undefined); await handleInstallationRepositoriesWebhookEvent(env, eventName, payload, installationActor); await handleInstallationCreatedWebhookEvent(env, eventName, payload, installationActor); + // Must run BEFORE the upsertRepositoryFromGitHub(payload.repository) call just below: that upsert keys + // on full_name, so renaming the anchor row first is what makes it an UPDATE instead of a fresh INSERT. + await maybeHandleRepositoryRenamedWebhookEvent(env, eventName, payload); const installationId = getInstallationId(payload); if (payload.repositories) { diff --git a/src/signals/focus-manifest-loader.ts b/src/signals/focus-manifest-loader.ts index 1a5ae53ef3..8b66a7d2e5 100644 --- a/src/signals/focus-manifest-loader.ts +++ b/src/signals/focus-manifest-loader.ts @@ -71,6 +71,23 @@ export async function loadRepoReviewContext( } } +/** + * True iff a container-private local manifest is registered (self-host only -- always false in cloud, + * where no reader is ever set) AND resolves non-null for `repoFullName`. A read error degrades to false, + * matching every other local-reader consumer's fail-safe-empty behavior. Used by the repo-rename webhook + * handler (#repo-rename-migration) to detect the ONE thing a rename can't migrate on its own: the + * operator's own container-private per-repo config folder, which this module derives from the CURRENT + * repo name and can only read, never write (the mount is read-only from the app's own perspective). + */ +export async function hasLocalManifest(repoFullName: string): Promise { + if (!localManifestReader) return false; + try { + return (await localManifestReader(repoFullName)) !== null; + } catch { + return false; + } +} + /** * Fetch a maintainer-owned manifest file from the public GitHub raw endpoint. Network or HTTP * failures resolve to null so the loader falls back to deterministic signals. diff --git a/src/types.ts b/src/types.ts index 9939ea6430..950912108a 100644 --- a/src/types.ts +++ b/src/types.ts @@ -311,6 +311,16 @@ export type GitHubWebhookPayload = { label?: { name?: string; }; + /** Present on a `repository` webhook with `action: "renamed"` -- `changes.repository.name.from` is the + * OLD bare repo name (not full_name); `repository.full_name` on this same payload is already the NEW + * current identity. See maybeHandleRepositoryRenamedWebhookEvent in queue/processors.ts. */ + changes?: { + repository?: { + name?: { + from?: string; + }; + }; + }; }; export type GitHubWebhookUserPayload = { diff --git a/test/unit/focus-manifest-loader.test.ts b/test/unit/focus-manifest-loader.test.ts index 94b0f7c9d7..c2fe1f1d87 100644 --- a/test/unit/focus-manifest-loader.test.ts +++ b/test/unit/focus-manifest-loader.test.ts @@ -3,6 +3,7 @@ import { createTestEnv } from "../helpers/d1"; import type { JsonValue } from "../../src/types"; import { fetchRepoFocusManifestFile, + hasLocalManifest, loadPublicRepoFocusManifest, loadRepoFocusManifest, loadRepoFocusManifests, diff --git a/test/unit/repo-identity-rename.test.ts b/test/unit/repo-identity-rename.test.ts new file mode 100644 index 0000000000..c15551103a --- /dev/null +++ b/test/unit/repo-identity-rename.test.ts @@ -0,0 +1,156 @@ +import { describe, expect, it } from "vitest"; +import { renameRepositoryIdentity } from "../../src/db/repo-identity-rename"; +import { + getIssue, + getPullRequest, + getRepository, + getRepositorySettings, + listPullRequests, + recordAuditEvent, + upsertIssueFromGitHub, + upsertPullRequestFromGitHub, + upsertRepositoryFromGitHub, + upsertRepositorySettings, +} from "../../src/db/repositories"; +import { createTestEnv } from "../helpers/d1"; + +const OLD = "owner/gittensory"; +const NEW = "owner/loopover"; + +describe("renameRepositoryIdentity", () => { + it("is a no-op when oldFullName and newFullName are identical", async () => { + const env = createTestEnv(); + await upsertRepositoryFromGitHub(env, { name: "gittensory", full_name: OLD, private: false, owner: { login: "owner" } }, 1); + await renameRepositoryIdentity(env, OLD, OLD); + const repo = await getRepository(env, OLD); + expect(repo?.fullName).toBe(OLD); + }); + + it("is a safe no-op when nothing exists yet under the old name", async () => { + const env = createTestEnv(); + await expect(renameRepositoryIdentity(env, OLD, NEW)).resolves.toBeUndefined(); + expect(await getRepository(env, NEW)).toBeNull(); + }); + + describe("repositories", () => { + it("renames the anchor row's full_name, owner, name, and html_url", async () => { + const env = createTestEnv(); + await upsertRepositoryFromGitHub(env, { name: "gittensory", full_name: OLD, private: false, html_url: `https://github.com/${OLD}`, owner: { login: "owner" } }, 42); + await renameRepositoryIdentity(env, OLD, NEW); + expect(await getRepository(env, OLD)).toBeNull(); + const renamed = await getRepository(env, NEW); + expect(renamed).toMatchObject({ fullName: NEW, owner: "owner", name: "loopover", installationId: 42, htmlUrl: `https://github.com/${NEW}` }); + }); + + it("REGRESSION (#repo-rename-migration): folds away a stray new-name row (already created by a webhook that slipped in under the new name) rather than colliding, keeping the old row's richer state", async () => { + const env = createTestEnv(); + await upsertRepositoryFromGitHub(env, { name: "gittensory", full_name: OLD, private: false, owner: { login: "owner" } }, 42); + // Simulate the exact drift this module exists to fix: a webhook already created a fresh row under the + // new name (installationId set, but none of the old row's accumulated state). + await upsertRepositoryFromGitHub(env, { name: "loopover", full_name: NEW, private: false, owner: { login: "owner" } }, 42); + await renameRepositoryIdentity(env, OLD, NEW); + const renamed = await getRepository(env, NEW); + expect(renamed?.installationId).toBe(42); + // Exactly one row survives -- the fold, not a second insert. + expect(await getRepository(env, OLD)).toBeNull(); + }); + }); + + describe("repository_settings", () => { + // getRepositorySettings always returns a (possibly all-default) RepositorySettings, never null, so + // these assert on the raw row directly to distinguish "no row" / "renamed row" / "folded row". + it("renames the settings row's repo_full_name", async () => { + const env = createTestEnv(); + await upsertRepositorySettings(env, { repoFullName: OLD, commentMode: "off" }); + await renameRepositoryIdentity(env, OLD, NEW); + const oldRow = await env.DB.prepare("select count(*) as n from repository_settings where repo_full_name = ?").bind(OLD).first<{ n: number }>(); + expect(oldRow?.n).toBe(0); + const settings = await getRepositorySettings(env, NEW); + expect(settings.commentMode).toBe("off"); + }); + + it("REGRESSION (#repo-rename-migration): folds away a stray new-name settings row, keeping the pre-existing configured settings", async () => { + const env = createTestEnv(); + await upsertRepositorySettings(env, { repoFullName: OLD, commentMode: "detected_contributors_only" }); + await upsertRepositorySettings(env, { repoFullName: NEW, commentMode: "off" }); // stray, should be discarded + await renameRepositoryIdentity(env, OLD, NEW); + const settings = await getRepositorySettings(env, NEW); + expect(settings.commentMode).toBe("detected_contributors_only"); + const newRowCount = await env.DB.prepare("select count(*) as n from repository_settings where repo_full_name = ?").bind(NEW).first<{ n: number }>(); + expect(newRowCount?.n).toBe(1); // exactly one surviving row, not two + }); + }); + + describe("pull_requests", () => { + it("renames repo_full_name, id, and html_url for every PR under the old name", async () => { + const env = createTestEnv(); + await upsertPullRequestFromGitHub(env, OLD, { number: 1, title: "PR one", state: "open", html_url: `https://github.com/${OLD}/pull/1`, labels: [] }); + await upsertPullRequestFromGitHub(env, OLD, { number: 2, title: "PR two", state: "closed", labels: [] }); + await renameRepositoryIdentity(env, OLD, NEW); + expect(await getPullRequest(env, OLD, 1)).toBeNull(); + const pr1 = await getPullRequest(env, NEW, 1); + expect(pr1).toMatchObject({ repoFullName: NEW, title: "PR one", htmlUrl: `https://github.com/${NEW}/pull/1` }); + const pr2 = await getPullRequest(env, NEW, 2); + expect(pr2?.title).toBe("PR two"); + }); + + it("REGRESSION (#repo-rename-migration): a colliding PR number under the new name is folded away, preserving the pre-existing PR's history instead of the sparse post-rename duplicate", async () => { + const env = createTestEnv(); + await upsertPullRequestFromGitHub(env, OLD, { number: 5, title: "Original, full history", state: "open", labels: [], body: "the real one" }); + // The sparse duplicate a webhook could have created under the new name before this migration ran. + await upsertPullRequestFromGitHub(env, NEW, { number: 5, title: "Fragment", state: "open", labels: [] }); + await renameRepositoryIdentity(env, OLD, NEW); + const rows = await listPullRequests(env, NEW); + expect(rows.filter((pr) => pr.number === 5)).toHaveLength(1); + expect(rows.find((pr) => pr.number === 5)?.title).toBe("Original, full history"); + }); + + it("does not disturb a PR that only ever existed under the new name (no matching number under the old name)", async () => { + const env = createTestEnv(); + await upsertPullRequestFromGitHub(env, OLD, { number: 1, title: "old-name PR", state: "open", labels: [] }); + await upsertPullRequestFromGitHub(env, NEW, { number: 99, title: "genuinely new PR", state: "open", labels: [] }); + await renameRepositoryIdentity(env, OLD, NEW); + expect(await getPullRequest(env, NEW, 99)).toMatchObject({ title: "genuinely new PR" }); + expect(await getPullRequest(env, NEW, 1)).toMatchObject({ title: "old-name PR" }); + }); + }); + + describe("issues", () => { + it("renames repo_full_name, id, and html_url for every issue under the old name", async () => { + const env = createTestEnv(); + await upsertIssueFromGitHub(env, OLD, { number: 7, title: "Issue seven", state: "open", html_url: `https://github.com/${OLD}/issues/7`, labels: [] }); + await renameRepositoryIdentity(env, OLD, NEW); + expect(await getIssue(env, OLD, 7)).toBeNull(); + expect(await getIssue(env, NEW, 7)).toMatchObject({ repoFullName: NEW, title: "Issue seven", htmlUrl: `https://github.com/${NEW}/issues/7` }); + }); + + it("REGRESSION (#repo-rename-migration): a colliding issue number under the new name is folded away, keeping the pre-existing issue", async () => { + const env = createTestEnv(); + await upsertIssueFromGitHub(env, OLD, { number: 3, title: "Original issue", state: "open", labels: [] }); + await upsertIssueFromGitHub(env, NEW, { number: 3, title: "Fragment issue", state: "open", labels: [] }); + await renameRepositoryIdentity(env, OLD, NEW); + expect(await getIssue(env, NEW, 3)).toMatchObject({ title: "Original issue" }); + }); + }); + + describe("audit_events", () => { + it("renames every target_key containing the old full name, including composite repo#number keys, leaving unrelated keys untouched", async () => { + const env = createTestEnv(); + await recordAuditEvent(env, { eventType: "test.event", actor: "loopover", targetKey: OLD, outcome: "completed", detail: "repo-level" }); + await recordAuditEvent(env, { eventType: "test.event", actor: "loopover", targetKey: `${OLD}#42`, outcome: "completed", detail: "pr-level" }); + await recordAuditEvent(env, { eventType: "test.event", actor: "loopover", targetKey: `${OLD}#42`, outcome: "completed", detail: "pr-level, second event, same target_key" }); + await recordAuditEvent(env, { eventType: "test.event", actor: "loopover", targetKey: "some/other-repo#1", outcome: "completed", detail: "unrelated" }); + + await renameRepositoryIdentity(env, OLD, NEW); + + const oldRepoLevel = await env.DB.prepare("select count(*) as n from audit_events where target_key = ?").bind(OLD).first<{ n: number }>(); + expect(oldRepoLevel?.n).toBe(0); + const newRepoLevel = await env.DB.prepare("select count(*) as n from audit_events where target_key = ?").bind(NEW).first<{ n: number }>(); + expect(newRepoLevel?.n).toBe(1); + const newPrLevel = await env.DB.prepare("select count(*) as n from audit_events where target_key = ?").bind(`${NEW}#42`).first<{ n: number }>(); + expect(newPrLevel?.n).toBe(2); // both rows sharing the same target_key survive -- no uniqueness on this column + const unrelated = await env.DB.prepare("select count(*) as n from audit_events where target_key = ?").bind("some/other-repo#1").first<{ n: number }>(); + expect(unrelated?.n).toBe(1); + }); + }); +}); diff --git a/test/unit/repo-rename-webhook.test.ts b/test/unit/repo-rename-webhook.test.ts new file mode 100644 index 0000000000..b6c7b9b960 --- /dev/null +++ b/test/unit/repo-rename-webhook.test.ts @@ -0,0 +1,114 @@ +import { describe, expect, it, vi } from "vitest"; +import { processJob } from "../../src/queue/job-dispatch"; +import { getPullRequest, getRepository, upsertInstallation, upsertPullRequestFromGitHub, upsertRepositoryFromGitHub } from "../../src/db/repositories"; +import { createTestEnv } from "../helpers/d1"; + +async function seedInstalledRepo(env: Env, fullName: string, installationId: number): Promise { + await upsertInstallation(env, { + action: "created", + installation: { id: installationId, account: { login: "owner", id: 1, type: "Organization" }, repository_selection: "selected", permissions: {}, events: [] }, + }); + await upsertRepositoryFromGitHub(env, { name: fullName.split("/")[1]!, full_name: fullName, private: false, owner: { login: "owner" } }, installationId); +} + +function renamedWebhookPayload(fromName: string, toFullName: string, installationId: number) { + return { + action: "renamed", + changes: { repository: { name: { from: fromName } } }, + repository: { name: toFullName.split("/")[1]!, full_name: toFullName, private: false, owner: { login: "owner" } }, + installation: { id: installationId, account: { login: "owner", id: 1, type: "Organization" }, repository_selection: "selected", permissions: {}, events: [] }, + sender: { login: "owner", type: "User" }, + }; +} + +describe("repository renamed webhook", () => { + it("REGRESSION (#repo-rename-migration): a repository/renamed webhook migrates PR history forward instead of creating a disconnected duplicate repo", async () => { + const env = createTestEnv(); + await seedInstalledRepo(env, "owner/gittensory", 9700); + await upsertPullRequestFromGitHub(env, "owner/gittensory", { number: 1, title: "Pre-rename PR", state: "open", labels: [] }); + vi.stubGlobal("fetch", async () => new Response("{}", { status: 200 })); + + await processJob(env, { + type: "github-webhook", + deliveryId: "rename-1", + eventName: "repository", + payload: renamedWebhookPayload("gittensory", "owner/loopover", 9700) as never, + }); + + expect(await getRepository(env, "owner/gittensory")).toBeNull(); + const renamed = await getRepository(env, "owner/loopover"); + expect(renamed?.installationId).toBe(9700); + const migratedPr = await getPullRequest(env, "owner/loopover", 1); + expect(migratedPr?.title).toBe("Pre-rename PR"); + }, 30_000); + + it("records a github_app.repository_renamed audit event with the old and new names", async () => { + const env = createTestEnv(); + await seedInstalledRepo(env, "owner/gittensory", 9701); + vi.stubGlobal("fetch", async () => new Response("{}", { status: 200 })); + + await processJob(env, { + type: "github-webhook", + deliveryId: "rename-2", + eventName: "repository", + payload: renamedWebhookPayload("gittensory", "owner/loopover", 9701) as never, + }); + + const row = await env.DB.prepare("select target_key, detail from audit_events where event_type = 'github_app.repository_renamed'").first<{ + target_key: string; + detail: string; + }>(); + expect(row?.target_key).toBe("owner/loopover"); + expect(row?.detail).toContain("owner/gittensory"); + expect(row?.detail).toContain("owner/loopover"); + }, 30_000); + + it("does not migrate anything for a repository webhook with a different action (e.g. created)", async () => { + const env = createTestEnv(); + await seedInstalledRepo(env, "owner/gittensory", 9702); + vi.stubGlobal("fetch", async () => new Response("{}", { status: 200 })); + + await processJob(env, { + type: "github-webhook", + deliveryId: "not-a-rename", + eventName: "repository", + payload: { action: "created", repository: { name: "gittensory", full_name: "owner/gittensory", private: false, owner: { login: "owner" } }, installation: { id: 9702, account: { login: "owner", id: 1, type: "Organization" }, repository_selection: "selected", permissions: {}, events: [] } } as never, + }); + + expect(await getRepository(env, "owner/gittensory")).not.toBeNull(); + }, 30_000); + + it("does not crash and does not migrate when the payload is missing the old-name field (a sparse/unexpected renamed payload)", async () => { + const env = createTestEnv(); + await seedInstalledRepo(env, "owner/gittensory", 9703); + vi.stubGlobal("fetch", async () => new Response("{}", { status: 200 })); + + await expect( + processJob(env, { + type: "github-webhook", + deliveryId: "rename-missing-from", + eventName: "repository", + payload: { action: "renamed", repository: { name: "loopover", full_name: "owner/loopover", private: false, owner: { login: "owner" } }, installation: { id: 9703, account: { login: "owner", id: 1, type: "Organization" }, repository_selection: "selected", permissions: {}, events: [] } } as never, + }), + ).resolves.toBeUndefined(); + + // No migration happened (nothing to migrate from), but the normal upsert still records the current repo state. + expect(await getRepository(env, "owner/gittensory")).not.toBeNull(); + }, 30_000); + + it("is a safe no-op when the computed old and new full names are identical (e.g. a case-only GitHub-side rename with nothing to migrate)", async () => { + const env = createTestEnv(); + await seedInstalledRepo(env, "owner/loopover", 9704); + vi.stubGlobal("fetch", async () => new Response("{}", { status: 200 })); + + await processJob(env, { + type: "github-webhook", + deliveryId: "rename-same-name", + eventName: "repository", + payload: renamedWebhookPayload("loopover", "owner/loopover", 9704) as never, + }); + + const renamed = await getRepository(env, "owner/loopover"); + expect(renamed?.installationId).toBe(9704); + }, 30_000); +});