fix(github): extend repo-rename identity migration to review/audit tables - #5950
Merged
Conversation
…bles Follow-up to the repo-rename webhook handler (#5918): renameRepositoryIdentity only covered the anchor tables (repositories, repository_settings, pull_requests, issues, audit_events). Extends it to every review/audit table keyed on repo_full_name so a GitHub repository rename migrates their history too, instead of orphaning it under the old name: - gate_outcomes, active_review_tracking, pull_request_detail_sync_state, recent_merged_pull_requests: same fold-then-rename shape as pull_requests/issues -- each has a unique (repo_full_name, pull_number| number) index and an id that embeds the repo name. - pull_request_files: unique (repo_full_name, pull_number, path), a 3-column key -- folds per (pull_number, path) pair rather than a single-column IN, since row counts here are small and this avoids a raw composite-tuple SQL construct. - check_summaries: unique (repo_full_name, head_sha, name) -- same per-pair fold, but head_sha is nullable so the collision lookup branches on isNull vs eq per row (SQL NULL never equals NULL via `=`). - pull_request_reviews: no separate unique index, just the PK id (which embeds the repo name) -- folds on the id the rename would produce rather than a business-key tuple. - advisories: id is a random UUID (never repo-derived) with no unique constraint on repo columns, so this is a plain rename of repo_full_name plus the repo-embedded target_key, same LIKE+replace shape as audit_events.target_key. 30 new tests covering the rename, the collision-fold, an unrelated-row-untouched case, and (for check_summaries) the NULL head_sha fold specifically.
Contributor
|
Superagent didn't find any vulnerabilities or security issues in this PR. |
This was referenced Jul 15, 2026
JSONbored
added a commit
that referenced
this pull request
Jul 15, 2026
#5953) Follow-up to #5918/#5950: extends renameRepositoryIdentity to 14 more repo_full_name-keyed tables: - burden_forecasts, repo_queue_trend_snapshots, repo_sync_state: repo_full_name itself IS the primary key (single row per repo) -- same fold-then-rename shape as the anchor tables. - repo_sync_segments (unique on segment), contributor_repo_stats (unique on login), repo_labels (unique on name): each has an id that embeds the repo name and a unique index pairing repo_full_name with one other column -- folds on that column. - collision_edges: id embeds the repo name but is built in packages/loopover-engine and passed through verbatim, with no unique index -- folds on the id collision the rename would produce, same shape as pull_request_reviews. - notification_deliveries, github_agent_command_answers, repo_snapshots, repo_github_totals_snapshots, github_rate_limit_observations, product_usage_events, signal_snapshots: id is always a random UUID with no unique constraint tied to repo_full_name (several nullable) -- plain renames. notification_deliveries.deeplink and github_agent_command_answers.response_url are also rewritten, same as the anchor tables' own html_url treatment (their own canonical GitHub link, not incidental content). Deliberately excludes 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 cache (repo_chunks) -- every one 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, which is graceful and self-healing, unlike an orphaned PR/issue/audit row a maintainer would otherwise need a GitHub API backfill to recover. Documented inline so the omission reads as deliberate, not forgotten. 24 new tests: a rename test for all 14 tables, plus a collision-fold regression test for the 7 that have a real fold path (three PK-is-repo_full_name tables, three single-column-unique-index tables, and the id-collision fold for collision_edges).
JSONbored
added a commit
that referenced
this pull request
Jul 15, 2026
Follow-up to #5918/#5950/#5953: extends renameRepositoryIdentity to review_audit, contributor_gate_history, and submitter_stats -- raw-SQL- only REES/parity tables, never added to the Drizzle schema (see each migration's own header comment), so these three blocks use env.DB.prepare() directly instead of the query builder, matching how every real writer (parity-wire.ts, outcomes-wire.ts, contributor- calibration.ts, submitter-reputation.ts) already accesses them. - review_audit, contributor_gate_history: PK `id` alone, no separate unique index. `project` IS the repo full name at every live writer; `target_id` (`${project}#${pullNumber}`) and `id` both embed it. Fold on the id collision the rename would produce -- same PK-collision-only shape as pull_request_reviews/collision_edges. - submitter_stats: no id column -- PRIMARY KEY (project, submitter) directly. Fold on the other half of the composite key, `submitter`. Deliberately excludes two other raw-SQL REES tables: - review_targets: has NO live writer anywhere in this codebase (explicitly confirmed orphaned by src/review/public-stats.ts's own comment). Its `project` column is an agent/install-level slug that must NEVER be renamed (not a per-repo value), and its `repo` column's real on-disk semantics for that historical, one-time-bulk-copied data can't be independently verified from current code alone -- mutating identity columns on data no writer would ever repair carries real corruption risk for uncertain benefit. - repo_chunks: a rebuildable RAG chunk/embedding cache (same reasoning as the AI/LLM caches #5953 excluded), whose project/repo columns hold the bare owner and bare repo name SEPARATELY (not a single owner/repo string) and whose id is a lowercased, truncated-to-64-chars hash of the two -- a safe in-place string rename isn't mechanically available without risking a silently-corrupted truncated id. 7 new tests: a rename test for all 3 tables, a collision-fold regression test for each, plus an unrelated-row-untouched check for submitter_stats.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Follow-up to #5918 (the repo-rename webhook handler).
renameRepositoryIdentityonly covered the anchor tables (repositories,repository_settings,pull_requests,issues,audit_events). This extends it to every review/audit table keyed onrepo_full_name, so a GitHub repository rename migrates their history too instead of silently orphaning it under the old name.What changed
gate_outcomes,active_review_tracking,pull_request_detail_sync_state,recent_merged_pull_requests: same fold-then-rename shape already established forpull_requests/issues— each has a unique(repo_full_name, pull_number|number)index and anidthat embeds the repo name.pull_request_files: unique(repo_full_name, pull_number, path), a 3-column key — folds per(pull_number, path)pair rather than a single-columnIN, since row counts here are small (a rename is rare; a PR's file list is bounded) and this avoids a raw composite-tuple SQL construct.check_summaries: unique(repo_full_name, head_sha, name)— same per-pair fold, buthead_shais nullable, so the collision lookup branches onisNullvseqper row (SQLNULLnever equalsNULLvia=).pull_request_reviews: no separate unique index, just the PKid(which embeds the repo name in every real call site) — folds on theidthe rename would produce rather than a business-key tuple.advisories:idis a random UUID (never repo-derived) with no unique constraint on repo columns, so this is a plain rename ofrepo_full_nameplus the repo-embeddedtarget_key, sameLIKE+replaceshape asaudit_events.target_key.Scope
Still deliberately narrow to structural repo-identity columns (see the module's own header comment). Remaining follow-ups, tracked separately:
ai_review_cache,signal_snapshots,product_usage_events, etc.)review_targets,contributor_gate_history,review_audit,repo_chunks)Test plan
npm run typecheck— cleannpx vitest run test/unit/repo-identity-rename.test.ts— 30/30 passing (13 new tests: rename + collision-fold + unrelated-row-untouched per table, plus a dedicated NULLhead_shafold regression forcheck_summaries)npm run test:ci— full local gate, greennpm audit --audit-level=moderate— 0 vulnerabilities