From 1e08f3e18381ca431fbed325223dcc5bb5430e36 Mon Sep 17 00:00:00 2001 From: volcano303 <75143900+volcano303@users.noreply.github.com> Date: Wed, 13 May 2026 15:59:50 +0200 Subject: [PATCH] fix(webhook): reconcile repository renames by GitHub id --- packages/das/src/entities/Repo.entity.ts | 3 + .../webhook/handlers/installation.handler.ts | 46 ++---- .../das/src/webhook/repo-identity.service.ts | 155 ++++++++++++++++++ packages/das/src/webhook/webhook.module.ts | 2 + packages/das/src/webhook/webhook.service.ts | 10 +- packages/db/01_repos.sql | 7 + 6 files changed, 182 insertions(+), 41 deletions(-) create mode 100644 packages/das/src/webhook/repo-identity.service.ts diff --git a/packages/das/src/entities/Repo.entity.ts b/packages/das/src/entities/Repo.entity.ts index 92ba728..bcbaca1 100644 --- a/packages/das/src/entities/Repo.entity.ts +++ b/packages/das/src/entities/Repo.entity.ts @@ -5,6 +5,9 @@ export class Repo { @PrimaryColumn({ name: "repo_full_name" }) repoFullName: string; + @Column({ name: "github_repo_id", type: "bigint", nullable: true }) + githubRepoId: string | null; + @Column({ name: "installation_id", type: "bigint", nullable: true }) installationId: string | null; diff --git a/packages/das/src/webhook/handlers/installation.handler.ts b/packages/das/src/webhook/handlers/installation.handler.ts index 2638652..ac10d38 100644 --- a/packages/das/src/webhook/handlers/installation.handler.ts +++ b/packages/das/src/webhook/handlers/installation.handler.ts @@ -1,17 +1,12 @@ /* eslint-disable @typescript-eslint/no-unsafe-member-access, @typescript-eslint/no-unsafe-assignment, @typescript-eslint/no-unsafe-argument, @typescript-eslint/no-unsafe-return, @typescript-eslint/no-unsafe-call, @typescript-eslint/no-explicit-any */ import { Injectable, Logger } from "@nestjs/common"; -import { InjectRepository } from "@nestjs/typeorm"; -import { Repository } from "typeorm"; -import { Repo } from "../../entities"; +import { RepoIdentityService } from "../repo-identity.service"; @Injectable() export class InstallationHandler { private readonly logger = new Logger(InstallationHandler.name); - constructor( - @InjectRepository(Repo) - private readonly repoRepo: Repository, - ) {} + constructor(private readonly repoIdentity: RepoIdentityService) {} async handle(event: string, payload: Record): Promise { const installationId = payload.installation?.id; @@ -22,12 +17,7 @@ export class InstallationHandler { this.logger.warn( `Installation ${installationId} deleted, clearing repos`, ); - await this.repoRepo - .createQueryBuilder() - .update() - .set({ installationId: null, registered: false }) - .where("installationId = :id", { id: String(installationId) }) - .execute(); + await this.repoIdentity.clearInstallation(String(installationId)); return; } @@ -38,33 +28,19 @@ export class InstallationHandler { payload.repositories ?? payload.repositories_added ?? []; for (const repo of repos) { - // Check existence first so we only set added_at on insert, not on every - // re-fire of installation.created / installation_repositories.added. - const existing = await this.repoRepo.findOneBy({ - repoFullName: repo.full_name, - }); - if (existing) { - await this.repoRepo.update(repo.full_name, { - installationId: String(installationId), - }); - } else { - await this.repoRepo.insert({ - repoFullName: repo.full_name, - installationId: String(installationId), - addedAt: new Date().toISOString(), - }); - } - this.logger.log(`Tracking repo: ${repo.full_name}`); + const repoFullName = await this.repoIdentity.upsertInstalled( + repo, + String(installationId), + ); + if (repoFullName) this.logger.log(`Tracking repo: ${repoFullName}`); } // installation_repositories.removed — soft clear, preserve historical data. const removed: any[] = payload.repositories_removed ?? []; for (const repo of removed) { - await this.repoRepo.update(repo.full_name, { - installationId: null, - registered: false, - }); - this.logger.log(`Stopped tracking repo: ${repo.full_name}`); + const repoFullName = await this.repoIdentity.markRemoved(repo); + if (repoFullName) + this.logger.log(`Stopped tracking repo: ${repoFullName}`); } } } diff --git a/packages/das/src/webhook/repo-identity.service.ts b/packages/das/src/webhook/repo-identity.service.ts new file mode 100644 index 0000000..c517cd5 --- /dev/null +++ b/packages/das/src/webhook/repo-identity.service.ts @@ -0,0 +1,155 @@ +import { Injectable, Logger } from "@nestjs/common"; +import { InjectRepository } from "@nestjs/typeorm"; +import { DataSource, Repository } from "typeorm"; +import { Repo } from "../entities"; + +interface GitHubRepositoryPayload { + id?: number | string | null; + full_name?: string | null; +} + +const REPO_SCOPED_TABLES = [ + "pull_requests", + "issues", + "reviews", + "comments", + "review_comments", + "label_events", + "pr_files", + "pr_file_contents", +]; + +@Injectable() +export class RepoIdentityService { + private readonly logger = new Logger(RepoIdentityService.name); + + constructor( + @InjectRepository(Repo) + private readonly repoRepo: Repository, + private readonly dataSource: DataSource, + ) {} + + async reconcile(repository: GitHubRepositoryPayload): Promise { + const repoFullName = repository.full_name ?? null; + if (!repoFullName) return null; + + const githubRepoId = this.repoId(repository); + const exact = await this.repoRepo.findOneBy({ repoFullName }); + if (exact) { + if (githubRepoId && exact.githubRepoId !== githubRepoId) { + await this.repoRepo.update(repoFullName, { githubRepoId }); + exact.githubRepoId = githubRepoId; + } + return exact; + } + + if (!githubRepoId) return null; + + const renamed = await this.repoRepo.findOneBy({ githubRepoId }); + if (!renamed) return null; + + await this.renameRepoFullName( + renamed.repoFullName, + repoFullName, + githubRepoId, + ); + this.logger.warn( + `Reconciled repository rename ${renamed.repoFullName} -> ${repoFullName}`, + ); + + renamed.repoFullName = repoFullName; + renamed.githubRepoId = githubRepoId; + return renamed; + } + + async upsertInstalled( + repository: GitHubRepositoryPayload, + installationId: string, + ): Promise { + const repoFullName = repository.full_name ?? null; + if (!repoFullName) return null; + + const githubRepoId = this.repoId(repository); + const existing = await this.reconcile(repository); + + if (existing) { + await this.repoRepo.update(existing.repoFullName, { + githubRepoId, + installationId, + }); + } else { + await this.repoRepo.insert({ + repoFullName, + githubRepoId, + installationId, + addedAt: new Date().toISOString(), + }); + } + + return repoFullName; + } + + async markRemoved( + repository: GitHubRepositoryPayload, + ): Promise { + const repoFullName = repository.full_name ?? null; + if (!repoFullName) return null; + + const existing = await this.reconcile(repository); + const resolvedFullName = existing?.repoFullName ?? repoFullName; + await this.repoRepo.update(resolvedFullName, { + installationId: null, + registered: false, + }); + + return resolvedFullName; + } + + async clearInstallation(installationId: string): Promise { + await this.repoRepo + .createQueryBuilder() + .update() + .set({ installationId: null, registered: false }) + .where("installationId = :id", { id: installationId }) + .execute(); + } + + private repoId(repository: GitHubRepositoryPayload): string | null { + return repository.id === undefined || repository.id === null + ? null + : String(repository.id); + } + + private async renameRepoFullName( + oldFullName: string, + newFullName: string, + githubRepoId: string, + ): Promise { + if (oldFullName === newFullName) return; + + await this.dataSource.transaction(async (manager) => { + const target = await manager.getRepository(Repo).findOneBy({ + repoFullName: newFullName, + }); + if (target) { + throw new Error( + `Cannot reconcile repository rename ${oldFullName} -> ${newFullName}: target repo row already exists`, + ); + } + + for (const table of REPO_SCOPED_TABLES) { + await manager.query( + `UPDATE ${table} SET repo_full_name = $1 WHERE repo_full_name = $2`, + [newFullName, oldFullName], + ); + } + + await manager.query( + `UPDATE repos + SET repo_full_name = $1, github_repo_id = $2 + WHERE repo_full_name = $3`, + [newFullName, githubRepoId, oldFullName], + ); + }); + } +} diff --git a/packages/das/src/webhook/webhook.module.ts b/packages/das/src/webhook/webhook.module.ts index a5e5144..ddbea08 100644 --- a/packages/das/src/webhook/webhook.module.ts +++ b/packages/das/src/webhook/webhook.module.ts @@ -14,6 +14,7 @@ import { FETCH_QUEUE } from "../queue/constants"; import { WebhookController } from "./webhook.controller"; import { WebhookService } from "./webhook.service"; import { WebhookPruneService } from "./webhook-prune.service"; +import { RepoIdentityService } from "./repo-identity.service"; import { PullRequestHandler } from "./handlers/pull-request.handler"; import { IssueHandler } from "./handlers/issue.handler"; import { ReviewHandler } from "./handlers/review.handler"; @@ -39,6 +40,7 @@ import { InstallationHandler } from "./handlers/installation.handler"; providers: [ WebhookService, WebhookPruneService, + RepoIdentityService, PullRequestHandler, IssueHandler, ReviewHandler, diff --git a/packages/das/src/webhook/webhook.service.ts b/packages/das/src/webhook/webhook.service.ts index ab4d707..4df3cee 100644 --- a/packages/das/src/webhook/webhook.service.ts +++ b/packages/das/src/webhook/webhook.service.ts @@ -1,8 +1,6 @@ /* eslint-disable @typescript-eslint/no-unsafe-member-access, @typescript-eslint/no-unsafe-assignment, @typescript-eslint/no-unsafe-argument, @typescript-eslint/no-unsafe-return, @typescript-eslint/no-unsafe-call, @typescript-eslint/no-explicit-any */ import { Injectable, Logger } from "@nestjs/common"; -import { InjectRepository } from "@nestjs/typeorm"; -import { DataSource, Repository } from "typeorm"; -import { Repo } from "../entities"; +import { DataSource } from "typeorm"; import { PullRequestHandler } from "./handlers/pull-request.handler"; import { IssueHandler } from "./handlers/issue.handler"; import { ReviewHandler } from "./handlers/review.handler"; @@ -10,15 +8,15 @@ import { CommentHandler } from "./handlers/comment.handler"; import { ReviewCommentHandler } from "./handlers/review-comment.handler"; import { LabelHandler } from "./handlers/label.handler"; import { InstallationHandler } from "./handlers/installation.handler"; +import { RepoIdentityService } from "./repo-identity.service"; @Injectable() export class WebhookService { private readonly logger = new Logger(WebhookService.name); constructor( - @InjectRepository(Repo) - private readonly repoRepo: Repository, private readonly dataSource: DataSource, + private readonly repoIdentity: RepoIdentityService, private readonly pullRequestHandler: PullRequestHandler, private readonly issueHandler: IssueHandler, private readonly reviewHandler: ReviewHandler, @@ -81,7 +79,7 @@ export class WebhookService { // All other events carry repo context and only persist data for registered repos. if (repoFullName) { - const repo = await this.repoRepo.findOneBy({ repoFullName }); + const repo = await this.repoIdentity.reconcile(payload.repository); if (!repo?.registered) { this.logger.log( `Skipping ${event}: repo ${repoFullName} not registered`, diff --git a/packages/db/01_repos.sql b/packages/db/01_repos.sql index db48434..b52f736 100644 --- a/packages/db/01_repos.sql +++ b/packages/db/01_repos.sql @@ -2,6 +2,7 @@ CREATE TABLE IF NOT EXISTS repos ( repo_full_name VARCHAR(255) PRIMARY KEY, + github_repo_id BIGINT, installation_id BIGINT, webhook_secret VARCHAR(255), added_at TIMESTAMPTZ NOT NULL DEFAULT NOW(), @@ -11,3 +12,9 @@ CREATE TABLE IF NOT EXISTS repos ( -- Manually flipped today; a future reconciler will sync from on-chain registration. registered BOOLEAN NOT NULL DEFAULT FALSE ); + +ALTER TABLE repos ADD COLUMN IF NOT EXISTS github_repo_id BIGINT; + +CREATE UNIQUE INDEX IF NOT EXISTS idx_repos_github_repo_id + ON repos(github_repo_id) + WHERE github_repo_id IS NOT NULL;