Skip to content

Commit 0810fca

Browse files
gregbergeclaude
andauthored
fix(backend): scope automatic reviews per project and team (#2349)
The "reapply previous approvals" flow (reviews marked `automatic`) resolved candidate previous builds by bucket name + branch + mode only. Bucket names ("default", …) are unique within a project, so a build could inherit approvals from a same-named build in another project — and therefore another team. Scope `getPreviousBuildsQuery` to `build.projectId`, which isolates the whole automatic-review flow per project and, since a project belongs to one account, per team. Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 parent cd28590 commit 0810fca

2 files changed

Lines changed: 35 additions & 0 deletions

File tree

apps/backend/src/build/approval.e2e.test.ts

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -395,6 +395,35 @@ describe("getPreviousApproverUserIds", () => {
395395
const userIds = await getPreviousApproverUserIds({ build, compareBucket });
396396
expect(userIds.sort()).toEqual([userA.id, userB.id].sort());
397397
});
398+
399+
test("ignores approvals from a same-named build in another project", async ({
400+
build,
401+
compareBucket,
402+
}) => {
403+
// A build in a different project (and therefore a different team) that
404+
// shares the same bucket name and branch must never contribute approvers.
405+
const otherProject = await factory.Project.create();
406+
const otherBucket = await factory.ScreenshotBucket.create({
407+
projectId: otherProject.id,
408+
branch: compareBucket.branch,
409+
name: compareBucket.name,
410+
});
411+
const otherBuild = await factory.Build.create({
412+
compareScreenshotBucketId: otherBucket.id,
413+
conclusion: "changes-detected",
414+
projectId: otherProject.id,
415+
createdAt: new Date(Date.now() - 1000).toISOString(),
416+
});
417+
const otherUser = await factory.User.create();
418+
await factory.BuildReview.create({
419+
buildId: otherBuild.id,
420+
state: "approved",
421+
userId: otherUser.id,
422+
});
423+
424+
const userIds = await getPreviousApproverUserIds({ build, compareBucket });
425+
expect(userIds).toEqual([]);
426+
});
398427
});
399428

400429
describe("getBuildReviewableDiffIds", () => {

apps/backend/src/build/approval.ts

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -92,6 +92,12 @@ function getPreviousBuildsQuery(args: {
9292
const buildQuery = Build.query()
9393
.select("builds.id")
9494
.joinRelated("compareScreenshotBucket")
95+
// Scope to the current build's project. Bucket names ("default", …) are
96+
// only unique within a project, so without this a build could inherit
97+
// approvals from a same-named build in another project — and therefore
98+
// another team. Scoping by project keeps automatic reviews isolated per
99+
// project and per team.
100+
.where("builds.projectId", build.projectId)
95101
.where("builds.createdAt", "<", build.createdAt)
96102
.where("builds.mode", build.mode)
97103
.where("builds.conclusion", "changes-detected")

0 commit comments

Comments
 (0)