From abfe153cdc0e3a050d574973d9d6a0c96db9d9e1 Mon Sep 17 00:00:00 2001 From: Thomas Durieux <5577568+tdurieux@users.noreply.github.com> Date: Mon, 2 Nov 2026 22:10:28 +0000 Subject: [PATCH] fix: preserve saved snapshot during source replacement --- src/server/routes/repository-private.ts | 21 ++++++++----- test/large-repository-tree.test.js | 41 +++++++++++++++++++++++++ 2 files changed, 54 insertions(+), 8 deletions(-) diff --git a/src/server/routes/repository-private.ts b/src/server/routes/repository-private.ts index 0c3c9bd..ffe64f9 100644 --- a/src/server/routes/repository-private.ts +++ b/src/server/routes/repository-private.ts @@ -529,11 +529,14 @@ router.post( const sourceChanged = reconnecting || hasRepositorySourceChanged(repo.model, repoUpdate); const previousAccessRevision = repo.model.githubAccess?.revision; - updateRepoModel(repo.model, repoUpdate); - const reactivating = shouldReactivateInactiveRepository(repo.model); + // Cleanup must use the persisted snapshot. Keep proposed settings separate + // until removal has finished under the original source and access fences. + const proposed = new AnonymizedRepositoryModel(repo.model.toObject()); + updateRepoModel(proposed, repoUpdate); + const reactivating = shouldReactivateInactiveRepository(proposed); if (reactivating) { - repo.model.anonymizeDate = new Date(); + proposed.anonymizeDate = new Date(); } if (sourceChanged) { @@ -570,15 +573,17 @@ router.post( const commit = await repository.getCommitInfo(repoUpdate.source.commit, { accessToken: sourceAccess.token, }); - if (reconnecting) repo.model.source.commitDate = commit.commit.committer?.date + if (reconnecting) proposed.source.commitDate = commit.commit.committer?.date ? new Date(commit.commit.committer.date) : undefined; - repo.model.githubAccess = { ...sourceAccess.binding, revision: randomUUID() }; - repo.model.source.repositoryId = repository.model.id; - repo.model.source.repositoryName = + proposed.githubAccess = { ...sourceAccess.binding, revision: randomUUID() }; + proposed.source.repositoryId = repository.model.id; + proposed.source.repositoryName = repository.fullName || repoUpdate.fullName; - repo.model.anonymizeDate = new Date(); + proposed.anonymizeDate = new Date(); await repo.remove({ accessRevision: previousAccessRevision }); } + repo.model.set({ options: proposed.options, source: proposed.source, + githubAccess: proposed.githubAccess, anonymizeDate: proposed.anonymizeDate }); const removeRepoFromConference = async (conferenceID: string) => { const conf = await ConferenceModel.findOne({ diff --git a/test/large-repository-tree.test.js b/test/large-repository-tree.test.js index c23722a..481e818 100644 --- a/test/large-repository-tree.test.js +++ b/test/large-repository-tree.test.js @@ -293,6 +293,47 @@ const uri = process.env.PERFORMANCE_MONGO_URI; expect(await File.countDocuments({ repoId: row.repoId })).to.equal(0); }); + for (const reconnecting of [false, true]) it(`saves a ${reconnecting ? "reconnected repository" : "changed source"} through real removal`, async () => { + const row = await repository({ status: "ready", treeGeneration: "old", pathIndexKey: "old-key", + githubAccess: { kind: "oauth", revision: "old-access" }, anonymizeDate: new Date(1), + options: { terms: ["before"], expirationMode: "never" } }), repo = new Repository(row); + await rows(2); await Path.collection.insertOne({ repoId: row.repoId, key: "old-key", anonymousPath: "old.txt" }); + const utils = require("../src/server/routes/route-utils"), app = require("../src/core/github-app"); + const source = require("../src/core/source/GitHubRepository"), queue = require("../src/queue"); + const router = require("../src/server/routes/repository-private").default; + const handler = router.stack.find(layer => layer.route?.path === "/:repoId/" && layer.route.methods.post).route.stack[0].handle; + const restores = [], jobs = []; let response, cleanupSnapshot, removals = 0; + const stub = (object, key, value) => { const original = object[key]; restores.push(() => { object[key] = original; }); object[key] = value; }; + stub(utils, "getRepo", async () => repo); + stub(utils, "getUser", async () => ({ id: repo.owner.id, model: { id: repo.owner.id } })); + stub(utils, "handleError", error => { throw error; }); + stub(app, "selectRepositoryAccess", async () => ({ token: "fixture", binding: { kind: "oauth" } })); + stub(source, "getRepositoryFromGitHub", async () => ({ id: "gh_123", model: { id: "new-source" }, fullName: "fixture/repo", + branches: async () => [{ name: "replacement" }], getCommitInfo: async () => ({ commit: { committer: { date: "2026-11-01" } } }) })); + stub(queue, "downloadQueue", { add: async (...args) => { jobs.push(args); } }); + const remove = repo.remove; + stub(repo, "remove", async function (...args) { + removals++; + cleanupSnapshot = { commit: this.model.source.commit, accessRevision: this.model.githubAccess.revision, + anonymizeDate: this.model.anonymizeDate }; + return remove.apply(this, args); + }); + try { + await handler({ body: { repoId: row.repoId, fullName: "fixture/repo", terms: ["after"], + source: { branch: "replacement", commit: "b".repeat(40) }, options: { expirationMode: "never" }, + ...(reconnecting ? { reconnectRepositoryId: "gh_123" } : {}) } }, { json: value => { response = value; } }); + } finally { while (restores.length) restores.pop()(); } + expect(removals).to.equal(1); expect(response).to.deep.equal({ status: "preparing" }); + expect(cleanupSnapshot).to.deep.equal({ commit: "a".repeat(40), accessRevision: "old-access", anonymizeDate: new Date(1) }); + const saved = await Repo.findById(row._id); + expect(saved.status).to.equal("preparing"); expect(saved.source.commit).to.equal("b".repeat(40)); + expect(saved.source.branch).to.equal("replacement"); expect(saved.source.repositoryId).to.equal("new-source"); + expect(saved.githubAccess.revision).not.to.equal("old-access"); expect(saved.anonymizeDate.getTime()).to.be.greaterThan(1); + expect(saved.options.terms).to.deep.equal(["after"]); + expect(await File.countDocuments({ repoId: row.repoId })).to.equal(0); expect(await Path.countDocuments({ repoId: row.repoId })).to.equal(0); + expect(jobs).to.have.length(1); expect(jobs[0][1]).to.deep.equal({ repoId: row.repoId }); + }); + it("retains retirement markers when foreground cleanup reaches its batch budget", async () => { const row = await repository({ treeGeneration: "active", retiredTreeGenerations: ["old"] }), repo = new Repository(row); await rows(501);