fix: handle settings saves before repository cleanup

src/core/Repository.ts

@@ -38,6 +38,7 @@ import AnonymizedRepositoryModel from "./model/anonymizedRepositories/anonymized
 import { createLogger, serializeError } from "./logger";
 
 const logger = createLogger("repository");
+const snapshotValue = <T>(value: T | undefined) => value === undefined ? { $exists: false } : value;
 /** Capture and retire the stored key while clearing it, including concurrent index publication. */
 async function updateRetiringPathIndex(
   filter: () => FilterQuery<IAnonymizedRepositoryDocument>,
@@ -1027,6 +1028,19 @@ export default class Repository {
     }
   }
 
+  /** Fields that identify the cached source and its current lifecycle owner. */
+  private fileSnapshotFilter() {
+    return {
+      treeGeneration: snapshotValue(this.model.treeGeneration),
+      status: snapshotValue(this.model.status), statusDate: snapshotValue(this.model.statusDate),
+      anonymizeDate: snapshotValue(this.model.anonymizeDate),
+      fileMetadataRevision: snapshotValue(this.model.fileMetadataRevision),
+      "source.type": snapshotValue(this.model.source.type), "source.repositoryName": snapshotValue(this.model.source.repositoryName),
+      "source.commit": snapshotValue(this.model.source.commit), "source.branch": snapshotValue(this.model.source.branch),
+      "githubAccess.revision": snapshotValue(this.model.githubAccess?.revision),
+    };
+  }
+
   /**
    * Remove the repository
    */
@@ -1036,8 +1050,9 @@ export default class Repository {
       this.assertNotArchived();
       const statusDate = new Date();
       const result = await AnonymizedRepositoryModel.updateOne({ _id: this.model._id,
-        status: this.model.status,
-        "githubAccess.revision": expected.accessRevision || { $exists: false },
+        ...this.fileSnapshotFilter(), ...this.refreshFilter(),
+        settingsSavedAt: snapshotValue(this.model.settingsSavedAt),
+        "githubAccess.revision": snapshotValue(expected.accessRevision),
       }, { $set: { status: RepositoryStatus.REMOVING, statusDate } });
       if (!result.matchedCount) throw new AnonymousError("connection_changed", { httpStatus: 409 });
       this.model.status = RepositoryStatus.REMOVING;
@@ -1058,19 +1073,13 @@ export default class Repository {
     const defer = isConnected && (status || this.status) === RepositoryStatus.PREPARING;
     const previousGeneration = this.model.treeGeneration;
     const generation = defer ? randomUUID() : previousGeneration;
-    const expected = <T>(value: T | undefined) => value === undefined ? { $exists: false } : value;
     const refreshToken = this.refreshToken;
-    const snapshot = {
-      status: expected(this.model.status), statusDate: expected(this.model.statusDate),
-      anonymizeDate: expected(this.model.anonymizeDate), settingsSavedAt: expected(this.model.settingsSavedAt),
-      fileMetadataRevision: expected(this.model.fileMetadataRevision),
-      "source.type": expected(this.model.source.type), "source.repositoryName": expected(this.model.source.repositoryName),
-      "source.commit": expected(this.model.source.commit), "source.branch": expected(this.model.source.branch),
-      "githubAccess.revision": expected(this.model.githubAccess?.revision),
-    };
+    // Options-only saves apply at read time and leave this source snapshot valid.
+    // They must not cancel a queued worker without scheduling replacement work.
+    const snapshot = this.fileSnapshotFilter();
     if (isConnected) {
       const result = await updateRetiringPathIndex(() => ({ _id: this.model._id,
-        treeGeneration: expected(previousGeneration), ...snapshot,
+        ...snapshot,
         ...(refreshToken ? { refreshToken, refreshUntil: { $gt: new Date() } } : {}),
       }), { $set: { fileMetadataRevision: revision, ...(defer ? { treeGeneration: generation } : {}) },
         ...(defer ? { $addToSet: { retiredTreeGenerations: previousGeneration || "",

test/large-repository-tree.test.js

@@ -227,7 +227,6 @@ const uri = process.env.PERFORMANCE_MONGO_URI;
     ["source branch", { "source.branch": "replacement" }],
     ["source type", { "source.type": "Zip" }],
     ["anonymization snapshot", { anonymizeDate: new Date(1) }],
-    ["settings revision", { settingsSavedAt: new Date(1) }],
     ["metadata revision", { fileMetadataRevision: "replacement" }],
     ["access revision", { "githubAccess.revision": "replacement" }],
   ];
@@ -334,6 +333,56 @@ const uri = process.env.PERFORMANCE_MONGO_URI;
     expect(jobs).to.have.length(1); expect(jobs[0][1]).to.deep.equal({ repoId: row.repoId });
   });
 
+  for (const [label, change] of [...changedSnapshots, ["settings revision", { settingsSavedAt: new Date(1) }]] ) {
+    it(`rejects removal before claiming the lifecycle after a changed ${label}`, async () => {
+      const row = await repository({ status: "ready", treeGeneration: "old", pathIndexKey: "old-key" }), stale = new Repository(row);
+      await rows(1); await Repo.updateOne({ _id: row._id }, { $set: change });
+      const before = await Repo.findById(row._id).lean();
+      stale.removeCache = () => { throw Error("stale removal touched storage"); };
+      const error = await stale.remove({}).then(() => null, error => error);
+      expect(error.message).to.equal("connection_changed");
+      expect(await Repo.findById(row._id).lean()).to.deep.equal(before);
+      expect(await File.countDocuments({ repoId: row.repoId })).to.equal(1);
+    });
+  }
+
+  it("finishes preparation after an accepted options-only save with the same source", async () => {
+    const row = await repository({ status: "preparing", treeGeneration: "old", anonymizeDate: new Date(1),
+      source: { type: "GitHubStream", repositoryName: "fixture/repo", branch: "main", commit: "a".repeat(40) },
+      options: { terms: ["before"], expirationMode: "never" } });
+    const worker = new Repository(row), editor = new Repository(await Repo.findById(row._id));
+    await rows(1);
+    Object.defineProperty(worker, "source", { get: () => ({ async *iterateFiles() {
+      yield { name: "replacement.txt", path: "", size: 2, sha: "b".repeat(40) };
+    } }) });
+    const utils = require("../src/server/routes/route-utils"), 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 savedSettingsAt;
+    const stub = (object, key, value) => { const original = object[key]; restores.push(() => { object[key] = original; }); object[key] = value; };
+    stub(utils, "getRepo", async () => editor);
+    stub(utils, "getUser", async () => ({ id: editor.owner.id, model: { id: editor.owner.id } }));
+    stub(utils, "handleError", error => { throw error; });
+    stub(queue, "downloadQueue", { add: async (...args) => { jobs.push(args); } });
+    stub(db, "connect", async () => {});
+    stub(db, "getRepository", async () => {
+      // The worker snapshot was loaded before this accepted owner edit.
+      await handler({ body: { repoId: row.repoId, fullName: "fixture/repo", terms: ["after"],
+        source: { branch: "main", commit: "a".repeat(40) }, options: { expirationMode: "never" } } }, { json() {} });
+      savedSettingsAt = (await Repo.findById(row._id)).settingsSavedAt;
+      return worker;
+    });
+    stub(github, "getToken", async () => "fixture"); stub(github, "getRedisGateResetAt", async () => 0);
+    try {
+      await require("../src/queue/processes/downloadRepository").default({ data: { repoId: row.repoId }, updateProgress: async () => {} });
+    } finally { while (restores.length) restores.pop()(); }
+    const saved = await Repo.findById(row._id);
+    expect(saved.status).to.equal("ready"); expect(saved.options.terms).to.deep.equal(["after"]);
+    expect(saved.settingsSavedAt).to.deep.equal(savedSettingsAt); expect(savedSettingsAt).to.be.instanceOf(Date);
+    expect(jobs).to.have.length(0);
+    expect((await File.find({ repoId: row.repoId })).map(file => file.name)).to.deep.equal(["replacement.txt"]);
+  });
+
   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);