From d159f4bc7d11ffd4bbe62ca331a1a805f800d9ad Mon Sep 17 00:00:00 2001 From: CaptainTimon Date: Fri, 8 May 2026 23:40:31 +0200 Subject: [PATCH] fix: store base content for deleted PR files --- .../webhook/github-fetcher.service.spec.ts | 107 ++++++++++++++++++ .../das/src/webhook/github-fetcher.service.ts | 19 ++-- 2 files changed, 119 insertions(+), 7 deletions(-) create mode 100644 packages/das/src/webhook/github-fetcher.service.spec.ts diff --git a/packages/das/src/webhook/github-fetcher.service.spec.ts b/packages/das/src/webhook/github-fetcher.service.spec.ts new file mode 100644 index 0000000..b2e9552 --- /dev/null +++ b/packages/das/src/webhook/github-fetcher.service.spec.ts @@ -0,0 +1,107 @@ +import assert from "node:assert/strict"; +import test from "node:test"; +import type { ConfigService } from "@nestjs/config"; +import type { Repository } from "typeorm"; +import { + Issue, + LabelEvent, + PrFile, + PrFileContent, + PullRequest, + Repo, + Review, +} from "../entities"; +import { GitHubFetcherService } from "./github-fetcher.service"; + +type PullFile = { + filename: string; + previous_filename?: string; + status: string; +}; + +type FetcherInternals = { + fetchContentBatch( + repoFullName: string, + prNumber: number, + batch: PullFile[], + owner: string, + repo: string, + token: string, + headSha: string, + baseSha: string | null, + ): Promise; + githubFetch(url: string, init: RequestInit): Promise; +}; + +const emptyRepo = (): Repository => ({}) as Repository; + +void test("fetchContentBatch stores only base content for removed files", async () => { + let storedContent: Partial | null = null; + const contentRepo = { + upsert: (content: Partial): Promise => { + storedContent = content; + return Promise.resolve(); + }, + } as unknown as Repository; + + const service = new GitHubFetcherService( + { getOrThrow: (): string => "123" } as unknown as ConfigService, + emptyRepo(), + contentRepo, + emptyRepo(), + emptyRepo(), + emptyRepo(), + emptyRepo(), + emptyRepo(), + ) as unknown as FetcherInternals; + + service.githubFetch = ( + _url: string, + init: RequestInit, + ): Promise => { + const body = init.body; + assert.equal(typeof body, "string"); + if (typeof body !== "string") { + throw new Error("Expected GraphQL request body to be a string"); + } + assert.ok(body.includes('base0: object(expression: \\"BASE:src/old.ts\\"')); + assert.doesNotMatch(body, /head0:/); + + return Promise.resolve({ + ok: true, + json: () => + Promise.resolve({ + data: { + repository: { + base0: { + text: "export const removed = true;\n", + byteSize: 29, + isBinary: false, + }, + }, + }, + }), + } as Response); + }; + + await service.fetchContentBatch( + "owner/repo", + 7, + [{ filename: "src/old.ts", status: "removed" }], + "owner", + "repo", + "token", + "HEAD", + "BASE", + ); + + assert.deepEqual(storedContent, { + repoFullName: "owner/repo", + prNumber: 7, + filename: "src/old.ts", + baseContent: "export const removed = true;\n", + headContent: null, + isBinary: false, + byteSize: 29, + }); +}); diff --git a/packages/das/src/webhook/github-fetcher.service.ts b/packages/das/src/webhook/github-fetcher.service.ts index 98c8de4..cfec157 100644 --- a/packages/das/src/webhook/github-fetcher.service.ts +++ b/packages/das/src/webhook/github-fetcher.service.ts @@ -477,8 +477,11 @@ export class GitHubFetcherService implements OnModuleInit { headSha: string, baseSha: string | null, ): Promise { - // Only fetch contents for files that have a meaningful version to fetch - const scored = files.filter((f) => f.status !== "removed"); + // Only fetch contents for files that have a meaningful version to fetch. + // Removed files still need their base-side blob for scoring. + const scored = files.filter( + (f) => f.status !== "removed" || baseSha !== null, + ); if (scored.length === 0) return; let batchSize = GRAPHQL_FILES_BATCH_SIZE; @@ -537,11 +540,13 @@ export class GitHubFetcherService implements OnModuleInit { `base${i}: object(expression: "${baseExpr}") { ... on Blob { text byteSize isBinary } }`, ); } - // Head version (already filtered out removed files at caller) - const headExpr = this.escapeGraphql(`${headSha}:${file.filename}`); - fields.push( - `head${i}: object(expression: "${headExpr}") { ... on Blob { text byteSize isBinary } }`, - ); + // Head version: skip for removed files because the path no longer exists. + if (file.status !== "removed") { + const headExpr = this.escapeGraphql(`${headSha}:${file.filename}`); + fields.push( + `head${i}: object(expression: "${headExpr}") { ... on Blob { text byteSize isBinary } }`, + ); + } } const query = `