Skip to content

Commit 5cc8d1f

Browse files
authored
fix(db): rename four more repo-identity tables and guard the list against drift (#9843)
`renameRepositoryIdentity` walks every repo-identity table to move a renamed repo's rows forward, but four tables carrying a repo-identity column were never walked: `linked_issue_claims`, `bounty_lifecycle_events`, `webhook_events`, and `score_previews`. A rename orphaned their rows under the old name. Add an explicit per-table block for each (matching the module's convention): `linked_issue_claims` (PK repo_full_name/pull_number/issue_number) folds a stray new-name row first, keeping the pre-existing old-name row's claimed_at, exactly as pullRequests/issues do; the other three have surrogate primary keys with no unique constraint on the repo column, so each is a plain UPDATE with no fold. Export `RENAME_OUT_OF_SCOPE_TABLES` (the eight deliberately-excluded tables the prose block already documents) and add a completeness drift guard: every schema.ts table with a repo_full_name/repository_full_name column must be renamed here or be exempt, no table may be both, and no exempt entry may be dead — the list had drifted three times before. Mirrors retention.test.ts's RETENTION_POLICY guard. Closes #9650
1 parent 60d900a commit 5cc8d1f

2 files changed

Lines changed: 154 additions & 1 deletion

File tree

src/db/repo-identity-rename.ts

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,7 @@ import {
4141
githubRateLimitObservations,
4242
issues,
4343
issueWatchSubscriptions,
44+
linkedIssueClaims,
4445
notificationDeliveries,
4546
productUsageEvents,
4647
pullRequestDetailSyncState,
@@ -606,4 +607,42 @@ export async function renameRepositoryIdentity(env: Env, oldFullName: string, ne
606607
.update(auditEvents)
607608
.set({ targetKey: sql`replace(${auditEvents.targetKey}, ${oldFullName}, ${newFullName})` })
608609
.where(sql`${auditEvents.targetKey} like ${`%${oldFullName}%`}`);
610+
611+
// linkedIssueClaims: PK (repo_full_name, pull_number, issue_number) -- a stray new-name row can collide, so
612+
// fold-then-rename exactly as pullRequests/issues do, keeping the pre-existing old-name row's claimed_at (#9650).
613+
const collidingClaimKeys = (
614+
await db.select({ pullNumber: linkedIssueClaims.pullNumber, issueNumber: linkedIssueClaims.issueNumber }).from(linkedIssueClaims).where(eq(linkedIssueClaims.repoFullName, oldFullName))
615+
).map((row) => `${row.pullNumber}:${row.issueNumber}`);
616+
if (collidingClaimKeys.length > 0) {
617+
await db
618+
.delete(linkedIssueClaims)
619+
.where(and(eq(linkedIssueClaims.repoFullName, newFullName), sql`(${linkedIssueClaims.pullNumber} || ':' || ${linkedIssueClaims.issueNumber}) in ${collidingClaimKeys}`));
620+
}
621+
await db.update(linkedIssueClaims).set({ repoFullName: newFullName }).where(eq(linkedIssueClaims.repoFullName, oldFullName));
622+
623+
// bountyLifecycleEvents: PK is surrogate `id`, no unique constraint on repo_full_name -- plain rename, same
624+
// as orb_webhook_events above (#9650).
625+
await env.DB.prepare("UPDATE bounty_lifecycle_events SET repo_full_name = ? WHERE repo_full_name = ?").bind(newFullName, oldFullName).run();
626+
627+
// webhookEvents: PK is delivery_id; repository_full_name is nullable with no unique constraint -- plain
628+
// rename (#9650).
629+
await env.DB.prepare("UPDATE webhook_events SET repository_full_name = ? WHERE repository_full_name = ?").bind(newFullName, oldFullName).run();
630+
631+
// scorePreviews: PK is surrogate `id`, no unique constraint on repo_full_name -- plain rename (#9650).
632+
await env.DB.prepare("UPDATE score_previews SET repo_full_name = ? WHERE repo_full_name = ?").bind(newFullName, oldFullName).run();
609633
}
634+
635+
/** Tables carrying a repo-identity column that renameRepositoryIdentity DELIBERATELY does not rename -- the
636+
* reasons are the prose block above. The completeness drift guard in repo-identity-rename.test.ts asserts
637+
* every schema.ts table with a repo_full_name/repository_full_name column is either renamed here or listed
638+
* here, so the list can never silently drift again (#9650). */
639+
export const RENAME_OUT_OF_SCOPE_TABLES: ReadonlySet<string> = new Set([
640+
"ai_review_cache",
641+
"ai_slop_cache",
642+
"linked_issue_satisfaction_cache",
643+
"grounding_file_content_cache",
644+
"impact_map_query_cache",
645+
"review_targets",
646+
"repo_chunks",
647+
"upstream_source_snapshots",
648+
]);

test/unit/repo-identity-rename.test.ts

Lines changed: 115 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
1+
import { readdirSync, readFileSync } from "node:fs";
12
import { describe, expect, it } from "vitest";
2-
import { renameRepositoryIdentity } from "../../src/db/repo-identity-rename";
3+
import { RENAME_OUT_OF_SCOPE_TABLES, renameRepositoryIdentity } from "../../src/db/repo-identity-rename";
34
import {
45
getAgentCommandAnswer,
56
getBurdenForecast,
@@ -1224,4 +1225,117 @@ describe("renameRepositoryIdentity", () => {
12241225
expect(unrelated?.n).toBe(1);
12251226
});
12261227
});
1228+
1229+
describe("linked_issue_claims (#9650)", () => {
1230+
it("folds a colliding new-name row, keeping the pre-existing old-name row's claimed_at", async () => {
1231+
const env = createTestEnv();
1232+
const ins = (repo: string, pn: number, iss: number, at: string) =>
1233+
env.DB.prepare("INSERT INTO linked_issue_claims (repo_full_name, pull_number, issue_number, claimed_at) VALUES (?,?,?,?)").bind(repo, pn, iss, at).run();
1234+
await ins(OLD, 1, 2, "2026-01-01T00:00:00.000Z"); // pre-existing old-name row
1235+
await ins(NEW, 1, 2, "2026-09-09T00:00:00.000Z"); // stray new-name row that would collide on the composite PK
1236+
await ins(OLD, 3, 4, "2026-02-02T00:00:00.000Z"); // non-colliding old-name row
1237+
1238+
await renameRepositoryIdentity(env, OLD, NEW);
1239+
1240+
const oldLeft = await env.DB.prepare("select count(*) as n from linked_issue_claims where repo_full_name = ?").bind(OLD).first<{ n: number }>();
1241+
expect(oldLeft?.n).toBe(0);
1242+
// The OLD row's claimed_at survives at (1,2) -- history is never dropped in favor of the new-name row.
1243+
const kept = await env.DB.prepare("select claimed_at from linked_issue_claims where repo_full_name = ? and pull_number = 1 and issue_number = 2").bind(NEW).first<{ claimed_at: string }>();
1244+
expect(kept?.claimed_at).toBe("2026-01-01T00:00:00.000Z");
1245+
});
1246+
});
1247+
1248+
for (const { table, col, label, insert } of [
1249+
{
1250+
table: "bounty_lifecycle_events",
1251+
col: "repo_full_name",
1252+
label: "bounty_lifecycle_events",
1253+
insert: (env: Env, repo: string) =>
1254+
env.DB.prepare("INSERT INTO bounty_lifecycle_events (id, bounty_id, repo_full_name, issue_number, status) VALUES (?,?,?,?,?)").bind("ble-1", "b1", repo, 7, "open").run(),
1255+
},
1256+
{
1257+
table: "webhook_events",
1258+
col: "repository_full_name",
1259+
label: "webhook_events",
1260+
insert: (env: Env, repo: string) =>
1261+
env.DB.prepare("INSERT INTO webhook_events (delivery_id, event_name, repository_full_name, payload_hash, status) VALUES (?,?,?,?,?)").bind("wh-1", "push", repo, "h1", "processed").run(),
1262+
},
1263+
{
1264+
table: "score_previews",
1265+
col: "repo_full_name",
1266+
label: "score_previews",
1267+
insert: (env: Env, repo: string) =>
1268+
env.DB.prepare("INSERT INTO score_previews (id, scoring_model_snapshot_id, repo_full_name, target_type, target_key) VALUES (?,?,?,?,?)").bind("sp-1", "sm1", repo, "pull_request", "k1").run(),
1269+
},
1270+
]) {
1271+
describe(`${label} (#9650)`, () => {
1272+
it("renames the repo-identity column, leaving zero rows under the old name", async () => {
1273+
const env = createTestEnv();
1274+
await insert(env, OLD);
1275+
await renameRepositoryIdentity(env, OLD, NEW);
1276+
const oldLeft = await env.DB.prepare(`select count(*) as n from ${table} where ${col} = ?`).bind(OLD).first<{ n: number }>();
1277+
expect(oldLeft?.n).toBe(0);
1278+
const newRows = await env.DB.prepare(`select count(*) as n from ${table} where ${col} = ?`).bind(NEW).first<{ n: number }>();
1279+
expect(newRows?.n).toBe(1);
1280+
});
1281+
});
1282+
}
1283+
1284+
describe("renameRepositoryIdentity completeness (drift guard, #9650)", () => {
1285+
// Every schema.ts table carrying a repo_full_name / repository_full_name column must be either renamed by
1286+
// this module or explicitly exempt — so the list (which has drifted before) can never silently regress.
1287+
// Mirrors retention.test.ts's RETENTION_POLICY completeness guard.
1288+
const REPO_IDENTITY_COLUMNS = new Set(["repo_full_name", "repository_full_name"]);
1289+
1290+
/** Map each schema.ts table that carries a repo-identity column to its Drizzle EXPORT variable name (the
1291+
* name the rename module references it by, e.g. `pull_requests` -> `pullRequests`). */
1292+
async function repoIdentityTables(): Promise<Array<{ sqlName: string; varName: string }>> {
1293+
const { getTableColumns, getTableName, isTable } = await import("drizzle-orm");
1294+
const schema = await import("../../src/db/schema");
1295+
const out: Array<{ sqlName: string; varName: string }> = [];
1296+
for (const [varName, value] of Object.entries(schema)) {
1297+
if (!isTable(value)) continue;
1298+
const columns = getTableColumns(value);
1299+
if (Object.values(columns).some((c) => REPO_IDENTITY_COLUMNS.has((c as { name: string }).name))) {
1300+
out.push({ sqlName: getTableName(value), varName });
1301+
}
1302+
}
1303+
return out;
1304+
}
1305+
1306+
it("every schema.ts table with a repo-identity column is renamed here or explicitly exempt", async () => {
1307+
const source = readFileSync("src/db/repo-identity-rename.ts", "utf8");
1308+
const unaccounted: string[] = [];
1309+
for (const { sqlName, varName } of await repoIdentityTables()) {
1310+
// Referenced either by its Drizzle variable (the fold-style blocks) or by its raw SQL name (the plain
1311+
// UPDATE blocks). Word-boundary on the var name so `pullRequests` doesn't also satisfy `pullRequestFiles`.
1312+
const referenced = new RegExp(`\\b${varName}\\b`).test(source) || source.includes(sqlName);
1313+
if (!referenced && !RENAME_OUT_OF_SCOPE_TABLES.has(sqlName)) unaccounted.push(sqlName);
1314+
}
1315+
expect(unaccounted).toEqual([]);
1316+
});
1317+
1318+
it("no table is both renamed here and listed as out-of-scope", async () => {
1319+
const source = readFileSync("src/db/repo-identity-rename.ts", "utf8");
1320+
const tablesByName = new Map((await repoIdentityTables()).map((t) => [t.sqlName, t.varName]));
1321+
const both = [...RENAME_OUT_OF_SCOPE_TABLES].filter((sqlName) => {
1322+
const varName = tablesByName.get(sqlName);
1323+
// "Renamed here" means an actual UPDATE, not merely a mention in the out-of-scope prose. The prose names
1324+
// the exempt tables in comments only; a real rename references the Drizzle var or `<sql_name> SET`.
1325+
return source.includes(`${sqlName} SET`) || (varName !== undefined && new RegExp(`\\.update\\(${varName}\\)`).test(source));
1326+
});
1327+
expect(both).toEqual([]);
1328+
});
1329+
1330+
it("no RENAME_OUT_OF_SCOPE_TABLES entry is dead (still exists in schema.ts or migrations/)", async () => {
1331+
const schemaSqlNames = new Set((await repoIdentityTables()).map((t) => t.sqlName));
1332+
const migrationsSql = readdirSync("migrations")
1333+
.filter((f) => f.endsWith(".sql"))
1334+
.map((f) => readFileSync(`migrations/${f}`, "utf8"))
1335+
.join("\n")
1336+
.toLowerCase();
1337+
const dead = [...RENAME_OUT_OF_SCOPE_TABLES].filter((table) => !schemaSqlNames.has(table) && !migrationsSql.includes(table.toLowerCase()));
1338+
expect(dead).toEqual([]);
1339+
});
1340+
});
12271341
});

0 commit comments

Comments
 (0)