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
90 changes: 81 additions & 9 deletions src/db/repo-identity-rename.ts
Original file line number Diff line number Diff line change
Expand Up @@ -356,15 +356,87 @@ export async function renameRepositoryIdentity(env: Env, oldFullName: string, ne
// carry no repo at all) and there is no index of any kind on this table. Plain rename.
await db.update(signalSnapshots).set({ repoFullName: newFullName }).where(eq(signalSnapshots.repoFullName, oldFullName));

// Deliberately OUT OF SCOPE: the request-scoped AI/LLM result caches (ai_review_cache, ai_slop_cache,
// linked_issue_satisfaction_cache, grounding_file_content_cache) and the RAG chunk/embedding cache
// (repo_chunks, a raw-SQL-only REES table). Every one of these is a rebuildable CACHE, not identity
// data: a miss after a rename just re-runs one LLM call or one re-index pass at the new name -- graceful,
// self-healing, and cheap, unlike an orphaned PR/issue/audit row a contributor or maintainer would
// otherwise need a full GitHub API backfill to recover. repo_chunks in particular stores its cache key as
// a LOWERCASED, TRUNCATED-TO-64-CHARS hash of `${ownerOnly}:${bareRepoName}` (packages layer), so a safe
// in-place string rename isn't even mechanically available without real risk of silently corrupting a
// truncated id -- letting it expire and re-index is both simpler and safer than attempting one.
// REES/parity tables below (review_audit, contributor_gate_history, submitter_stats) are raw-SQL-only --
// deliberately NOT added to the Drizzle schema (see each table's own migration header) -- so these three
// blocks use env.DB.prepare() directly instead of the query builder, matching how every other writer of
// these tables (parity-wire.ts, outcomes-wire.ts, contributor-calibration.ts, submitter-reputation.ts)
// already accesses them.

// reviewAudit: PK `id` alone (migrations/0049_review_audit_parity.sql), no separate unique index. `project`
// IS the repo full name (verified live at both writers: parity-wire.ts's recordNativeGateDecision and
// outcomes-wire.ts's appendReviewAudit); `target_id` is `${project}#${pullNumber}`, and `id` embeds
// `target_id` (hence project) as a substring in every writer's own id-construction scheme. Same
// PK-collision-only fold shape as pullRequestReviews/collisionEdges above -- no business-key unique index
// exists to fold on instead.
const oldReviewAuditIds = (
await env.DB.prepare("SELECT id FROM review_audit WHERE project = ?").bind(oldFullName).all<{ id: string }>()
).results.map((row) => row.id);
const renamedReviewAuditIds = oldReviewAuditIds.map((id) => id.split(oldFullName).join(newFullName));
if (renamedReviewAuditIds.length > 0) {
const placeholders = renamedReviewAuditIds.map(() => "?").join(",");
await env.DB.prepare(`DELETE FROM review_audit WHERE id IN (${placeholders})`)
.bind(...renamedReviewAuditIds)
.run();
}
await env.DB.prepare("UPDATE review_audit SET id = replace(id, ?, ?), project = ?, target_id = replace(target_id, ?, ?) WHERE project = ?")
.bind(oldFullName, newFullName, newFullName, oldFullName, newFullName, oldFullName)
.run();

// contributorGateHistory: same shape as reviewAudit -- PK `id` alone (migrations/0126_contributor_gate_
// history.sql), `project` is the repo full name (verified live at its sole writer, contributor-
// calibration.ts's recordContributorGateDecision), `target_id` and `id` both embed it the same way.
const oldContributorGateHistoryIds = (
await env.DB.prepare("SELECT id FROM contributor_gate_history WHERE project = ?").bind(oldFullName).all<{ id: string }>()
).results.map((row) => row.id);
const renamedContributorGateHistoryIds = oldContributorGateHistoryIds.map((id) => id.split(oldFullName).join(newFullName));
if (renamedContributorGateHistoryIds.length > 0) {
const placeholders = renamedContributorGateHistoryIds.map(() => "?").join(",");
await env.DB.prepare(`DELETE FROM contributor_gate_history WHERE id IN (${placeholders})`)
.bind(...renamedContributorGateHistoryIds)
.run();
}
await env.DB.prepare("UPDATE contributor_gate_history SET id = replace(id, ?, ?), project = ?, target_id = replace(target_id, ?, ?) WHERE project = ?")
.bind(oldFullName, newFullName, newFullName, oldFullName, newFullName, oldFullName)
.run();

// submitterStats: no `id` column at all -- PRIMARY KEY (project, submitter) directly (migrations/0046_
// submitter_stats.sql). `project` is the repo full name (verified live at its sole writer, submitter-
// reputation.ts's recordSubmissionOutcome). Fold on the OTHER half of the composite key, `submitter`,
// same single-column inArray shape used for repoSyncSegments/repoLabels above.
const collidingSubmitters = (
await env.DB.prepare("SELECT submitter FROM submitter_stats WHERE project = ?").bind(oldFullName).all<{ submitter: string }>()
).results.map((row) => row.submitter);
if (collidingSubmitters.length > 0) {
const placeholders = collidingSubmitters.map(() => "?").join(",");
await env.DB.prepare(`DELETE FROM submitter_stats WHERE project = ? AND submitter IN (${placeholders})`)
.bind(newFullName, ...collidingSubmitters)
.run();
}
await env.DB.prepare("UPDATE submitter_stats SET project = ? WHERE project = ?").bind(newFullName, oldFullName).run();

// Deliberately OUT OF SCOPE:
// - The request-scoped AI/LLM result caches (ai_review_cache, ai_slop_cache,
// linked_issue_satisfaction_cache, grounding_file_content_cache). Every one of these is a rebuildable
// CACHE, not identity data: a miss after a rename just re-runs one LLM call at the new name --
// graceful, self-healing, and cheap, unlike an orphaned PR/issue/audit row a contributor or maintainer
// would otherwise need a full GitHub API backfill to recover.
// - review_targets (migrations/0050_review_targets.sql, raw-SQL-only): has NO live writer anywhere in
// this codebase -- src/review/public-stats.ts's own comment confirms "the legacy review_targets
// ledger, which the convergence cutover orphaned (nothing writes it anymore)". Its data is a one-time
// historical bulk copy from reviewbot's original schema, and the table carries TWO different
// repo-identity-shaped columns (`project`, an agent/install-level slug that must NEVER be renamed --
// it is not a per-repo value at all; and `repo`, which current read-side code validates as full
// owner/repo format) whose actual on-disk semantics for that historical data can't be independently
// verified from this codebase. Mutating identity columns on an orphaned table we can't fully verify
// carries real risk of silently corrupting data no writer would ever repair.
// - repo_chunks (migrations/0051_repo_chunks.sql, raw-SQL-only): a rebuildable RAG chunk/embedding
// cache, not identity data -- a miss after a rename just re-indexes at the new name (self-healing,
// same reasoning as the AI/LLM caches above). Its `project`/`repo` columns ALSO don't hold a single
// owner/repo string the way every table above does -- `project` is the bare OWNER only and `repo` is
// the bare REPO NAME only (src/queue/processors.ts's splitRepoForRag / src/review/rag-index.ts's
// splitRepo each independently strip the other half at the write path), and its `id` is a
// LOWERCASED, TRUNCATED-TO-64-CHARS hash of `${project}:${repo}` -- a safe in-place string rename
// isn't mechanically available without risking a silently-corrupted truncated id.

// 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.
Expand Down
121 changes: 121 additions & 0 deletions test/unit/repo-identity-rename.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -791,6 +791,127 @@ describe("renameRepositoryIdentity", () => {
});
});

// review_audit, contributor_gate_history, and submitter_stats are raw-SQL-only REES/parity tables (never
// added to the Drizzle schema -- see each migration's own header comment), so these seed and verify via
// env.DB.prepare() directly, matching how every real writer (parity-wire.ts, outcomes-wire.ts,
// contributor-calibration.ts, submitter-reputation.ts) already accesses them -- there's no clean
// repositories.ts convenience function to call instead, and their real writers are gated behind
// self-host/parity-audit feature flags unrelated to what this migration itself needs to verify.
describe("review_audit", () => {
it("renames project, target_id, and id for a gate-decision row", async () => {
const env = createTestEnv();
const targetId = `${OLD}#21`;
await env.DB.prepare(
"INSERT INTO review_audit (id, project, target_id, event_type, decision, source, head_sha, summary, created_at) VALUES (?, ?, ?, 'gate_decision', 'merge', 'loopover-native', 'sha1', 'clean', CURRENT_TIMESTAMP)",
)
.bind(`gate:loopover-native:${targetId}@sha1`, OLD, targetId)
.run();
await renameRepositoryIdentity(env, OLD, NEW);
const oldRow = await env.DB.prepare("select count(*) as n from review_audit where project = ?").bind(OLD).first<{ n: number }>();
expect(oldRow?.n).toBe(0);
const renamed = await env.DB.prepare("select id, target_id as targetId from review_audit where project = ?").bind(NEW).first<{ id: string; targetId: string }>();
expect(renamed).toEqual({ id: `gate:loopover-native:${NEW}#21@sha1`, targetId: `${NEW}#21` });
});

it("REGRESSION (#repo-rename-migration): folds away a stray new-name row whose id already equals what the old row's id would become", async () => {
const env = createTestEnv();
const oldId = `gate:loopover-native:${OLD}#21@sha1`; // distinct from strayId at insert time (satisfies the PK)...
const strayId = `gate:loopover-native:${NEW}#21@sha1`; // ...but is EXACTLY what oldId becomes after the rename's replace()
await env.DB.prepare(
"INSERT INTO review_audit (id, project, target_id, event_type, decision, source, head_sha, summary, created_at) VALUES (?, ?, ?, 'gate_decision', 'merge', 'loopover-native', 'sha1', 'original', CURRENT_TIMESTAMP)",
)
.bind(oldId, OLD, `${OLD}#21`)
.run();
await env.DB.prepare(
"INSERT INTO review_audit (id, project, target_id, event_type, decision, source, head_sha, summary, created_at) VALUES (?, ?, ?, 'gate_decision', 'close', 'loopover-native', 'sha1', 'stray', CURRENT_TIMESTAMP)",
)
.bind(strayId, NEW, `${NEW}#21`)
.run();
await renameRepositoryIdentity(env, OLD, NEW);
const rows = await env.DB.prepare("select summary from review_audit where id = ?").bind(strayId).all<{ summary: string }>();
expect(rows.results).toHaveLength(1);
expect(rows.results[0]?.summary).toBe("original");
});
});

describe("contributor_gate_history", () => {
it("renames project, target_id, and id for a per-contributor gate-decision row", async () => {
const env = createTestEnv();
const targetId = `${OLD}#9`;
await env.DB.prepare(
"INSERT INTO contributor_gate_history (id, login, source, project, target_id, decision, head_sha, created_at) VALUES (?, 'alice', 'loopover-native', ?, ?, 'merge', 'sha1', CURRENT_TIMESTAMP)",
)
.bind(`contrib:alice:loopover-native:${targetId}@sha1`, OLD, targetId)
.run();
await renameRepositoryIdentity(env, OLD, NEW);
const oldRow = await env.DB.prepare("select count(*) as n from contributor_gate_history where project = ?").bind(OLD).first<{ n: number }>();
expect(oldRow?.n).toBe(0);
const renamed = await env.DB.prepare("select id, target_id as targetId from contributor_gate_history where project = ?").bind(NEW).first<{ id: string; targetId: string }>();
expect(renamed).toEqual({ id: `contrib:alice:loopover-native:${NEW}#9@sha1`, targetId: `${NEW}#9` });
});

it("REGRESSION (#repo-rename-migration): folds away a stray new-name row whose id already equals what the old row's id would become", async () => {
const env = createTestEnv();
const oldId = `contrib:alice:loopover-native:${OLD}#9@sha1`; // distinct from strayId at insert time (satisfies the PK)...
const strayId = `contrib:alice:loopover-native:${NEW}#9@sha1`; // ...but is EXACTLY what oldId becomes after the rename's replace()
await env.DB.prepare(
"INSERT INTO contributor_gate_history (id, login, source, project, target_id, decision, head_sha, created_at) VALUES (?, 'alice', 'loopover-native', ?, ?, 'merge', 'sha1', CURRENT_TIMESTAMP)",
)
.bind(oldId, OLD, `${OLD}#9`)
.run();
await env.DB.prepare(
"INSERT INTO contributor_gate_history (id, login, source, project, target_id, decision, head_sha, created_at) VALUES (?, 'alice', 'loopover-native', ?, ?, 'hold', 'sha1', CURRENT_TIMESTAMP)",
)
.bind(strayId, NEW, `${NEW}#9`)
.run();
await renameRepositoryIdentity(env, OLD, NEW);
const rows = await env.DB.prepare("select decision from contributor_gate_history where id = ?").bind(strayId).all<{ decision: string }>();
expect(rows.results).toHaveLength(1);
expect(rows.results[0]?.decision).toBe("merge");
});
});

describe("submitter_stats", () => {
it("renames project for a submitter's outcome-count row", async () => {
const env = createTestEnv();
await env.DB.prepare("INSERT INTO submitter_stats (project, submitter, submissions, merged, closed, manual, last_seen) VALUES (?, 'bob', 5, 3, 1, 0, CURRENT_TIMESTAMP)")
.bind(OLD)
.run();
await renameRepositoryIdentity(env, OLD, NEW);
const oldRow = await env.DB.prepare("select count(*) as n from submitter_stats where project = ?").bind(OLD).first<{ n: number }>();
expect(oldRow?.n).toBe(0);
const renamed = await env.DB.prepare("select submissions, merged from submitter_stats where project = ? and submitter = 'bob'").bind(NEW).first<{ submissions: number; merged: number }>();
expect(renamed).toEqual({ submissions: 5, merged: 3 });
});

it("REGRESSION (#repo-rename-migration): folds away a stray new-name row for the same submitter, keeping the pre-existing counts", async () => {
const env = createTestEnv();
await env.DB.prepare("INSERT INTO submitter_stats (project, submitter, submissions, merged, closed, manual, last_seen) VALUES (?, 'bob', 10, 8, 1, 0, CURRENT_TIMESTAMP)")
.bind(OLD)
.run();
await env.DB.prepare("INSERT INTO submitter_stats (project, submitter, submissions, merged, closed, manual, last_seen) VALUES (?, 'bob', 1, 0, 1, 0, CURRENT_TIMESTAMP)")
.bind(NEW)
.run();
await renameRepositoryIdentity(env, OLD, NEW);
const rows = await env.DB.prepare("select submissions from submitter_stats where project = ? and submitter = 'bob'").bind(NEW).all<{ submissions: number }>();
expect(rows.results).toHaveLength(1);
expect(rows.results[0]?.submissions).toBe(10);
});

it("does not disturb a different submitter's row under the new name", async () => {
const env = createTestEnv();
await env.DB.prepare("INSERT INTO submitter_stats (project, submitter, submissions, merged, closed, manual, last_seen) VALUES (?, 'bob', 5, 3, 1, 0, CURRENT_TIMESTAMP)")
.bind(OLD)
.run();
await env.DB.prepare("INSERT INTO submitter_stats (project, submitter, submissions, merged, closed, manual, last_seen) VALUES (?, 'carol', 2, 1, 0, 0, CURRENT_TIMESTAMP)")
.bind(NEW)
.run();
await renameRepositoryIdentity(env, OLD, NEW);
const rows = await env.DB.prepare("select submitter from submitter_stats where project = ? order by submitter").bind(NEW).all<{ submitter: string }>();
expect(rows.results.map((r) => r.submitter)).toEqual(["bob", "carol"]);
});
});

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();
Expand Down