Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
101 changes: 101 additions & 0 deletions src/db/repo-identity-rename.ts
Original file line number Diff line number Diff line change
@@ -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<void> {
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}%`}`);
}
70 changes: 69 additions & 1 deletion src/queue/processors.ts
Original file line number Diff line number Diff line change
Expand Up @@ -87,6 +87,7 @@ import {
upsertPullRequestFromGitHub,
upsertRepositoryFromGitHub,
} from "../db/repositories";
import { renameRepositoryIdentity } from "../db/repo-identity-rename";
import {
effectiveIssueCapForAccountAge,
isBelowAccountAgeThreshold,
Expand Down Expand Up @@ -490,6 +491,7 @@ import {
} from "../signals/focus-manifest";
import { decideReviewEligibility } from "../review/review-eligibility";
import {
hasLocalManifest,
loadPublicRepoFocusManifest,
loadRepoFocusManifest,
loadRepoFocusManifests,
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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<void> {
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<void> {
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
Expand Down Expand Up @@ -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) {
Expand Down
17 changes: 17 additions & 0 deletions src/signals/focus-manifest-loader.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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<boolean> {
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.
Expand Down
10 changes: 10 additions & 0 deletions src/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 = {
Expand Down
1 change: 1 addition & 0 deletions test/unit/focus-manifest-loader.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import { createTestEnv } from "../helpers/d1";
import type { JsonValue } from "../../src/types";
import {
fetchRepoFocusManifestFile,
hasLocalManifest,
loadPublicRepoFocusManifest,
loadRepoFocusManifest,
loadRepoFocusManifests,
Expand Down
Loading