Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 9 additions & 5 deletions src/scoring/pending-pr-scenarios.ts
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ export async function loadContributorRepoOpenPrSignalRecords(
pullRequests: PullRequestRecord[],
): Promise<{ pullRequestReviews: PullRequestReviewRecord[]; pullRequestChecks: CheckSummaryRecord[] }> {
const open = pullRequests.filter(
(pr) => pr.repoFullName === repoFullName && pr.state === "open" && sameLogin(pr.authorLogin, login),
(pr) => sameRepoFullName(pr.repoFullName, repoFullName) && pr.state === "open" && sameLogin(pr.authorLogin, login),
);
const signals = await loadContributorRepoOpenPrSignals(env, repoFullName, open);
return {
Expand All @@ -62,14 +62,14 @@ export async function loadContributorRepoOpenPrSignals(
repoFullName: string,
pullRequests: PullRequestRecord[],
): Promise<ContributorRepoOpenPrSignals> {
const open = pullRequests.filter((pr) => pr.repoFullName === repoFullName && pr.state === "open");
const open = pullRequests.filter((pr) => sameRepoFullName(pr.repoFullName, repoFullName) && pr.state === "open");
const reviewsByPullNumber = new Map<number, PullRequestReviewRecord[]>();
const checksByPullNumber = new Map<number, CheckSummaryRecord[]>();
await Promise.all(
open.map(async (pr) => {
const [reviews, checks] = await Promise.all([
listPullRequestReviews(env, repoFullName, pr.number),
listCheckSummaries(env, repoFullName, pr.number),
listPullRequestReviews(env, pr.repoFullName, pr.number),
listCheckSummaries(env, pr.repoFullName, pr.number),
]);
reviewsByPullNumber.set(pr.number, reviews);
checksByPullNumber.set(pr.number, checks);
Expand Down Expand Up @@ -113,7 +113,7 @@ export function detectPendingPrScenario(args: {
const excluded = new Set(args.excludePullNumbers ?? []);
const contributorOpen = args.pullRequests.filter(
(pr) =>
pr.repoFullName === args.repoFullName &&
sameRepoFullName(pr.repoFullName, args.repoFullName) &&
pr.state === "open" &&
sameLogin(pr.authorLogin, args.login) &&
!excluded.has(pr.number),
Expand Down Expand Up @@ -229,6 +229,10 @@ function isDraftPullRequest(pr: PullRequestRecord): boolean {
return pr.labels.some((label) => label.toLowerCase() === "draft" || label.toLowerCase() === "wip");
}

function sameRepoFullName(left: string, right: string): boolean {
return left.toLowerCase() === right.toLowerCase();
}

function sameLogin(value: string | null | undefined, login: string): boolean {
return Boolean(value && value.toLowerCase() === login.toLowerCase());
}
Expand Down
10 changes: 5 additions & 5 deletions src/signals/contributor-open-pr-monitor.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { listContributorPullRequests, listPullRequestFiles, listPullRequests, listRepositories } from "../db/repositories";
import { listContributorPullRequests, listPullRequestFiles, listRepositories } from "../db/repositories";
import { sanitizePublicComment } from "../github/commands";
import {
classifyOpenPullRequest,
Expand Down Expand Up @@ -60,8 +60,8 @@ export async function buildContributorOpenPrMonitor(env: Env, login: string): Pr
const packets: ContributorOpenPrNextStepPacket[] = [];

for (const repoOpen of byRepo.values()) {
// The bucket is keyed case-insensitively (see groupByRepo); use a PR's original repoFullName casing for
// the case-sensitive DB lookups below so the per-repo open-PR set stays whole and queries still resolve.
// The bucket is keyed case-insensitively (see groupByRepo); keep one representative casing for
// repo-level context, but use each PR's stored casing for exact DB lookups below.
const repoFullName = repoOpen[0]!.repoFullName;
const repo = repositories.find((entry) => entry.fullName.toLowerCase() === repoFullName.toLowerCase()) ?? null;
const roleContext = buildRoleContext({
Expand All @@ -73,13 +73,13 @@ export async function buildContributorOpenPrMonitor(env: Env, login: string): Pr
profile: null,
});
const signals = await loadContributorRepoOpenPrSignals(env, repoFullName, repoOpen);
const repoPullRequests = await listPullRequests(env, repoFullName);
const repoPullRequests = pullRequests.filter((pr) => pr.repoFullName.toLowerCase() === repoFullName.toLowerCase());
const duplicateNumbers = duplicatePronePullNumbers(repoOpen);

for (const pr of repoOpen) {
const reviews = signals.reviewsByPullNumber.get(pr.number) ?? [];
const checks = signals.checksByPullNumber.get(pr.number) ?? [];
const files = await listPullRequestFiles(env, repoFullName, pr.number);
const files = await listPullRequestFiles(env, pr.repoFullName, pr.number);
const classified = classifyOpenPullRequest({
pr,
roleContext,
Expand Down
33 changes: 22 additions & 11 deletions test/unit/contributor-open-pr-monitor.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -184,27 +184,38 @@ describe("contributor open PR monitor", () => {
expect(monitor.guidance.length).toBeGreaterThan(0);
});

it("groups case-variant repoFullName for one repo into a single open-PR set", async () => {
it("loads signals and files with each PR casing in a case-variant repo group", async () => {
const env = createTestEnv();
vi.spyOn(repositories, "listRepositories").mockResolvedValue([
{ fullName: "entrius/allways-ui", owner: "entrius", name: "allways-ui", isInstalled: true, isRegistered: true, isPrivate: false },
] as Awaited<ReturnType<typeof repositories.listRepositories>>);
// The same repo arrives under two casings — these must be one group, not two.
vi.spyOn(repositories, "listContributorPullRequests").mockResolvedValue([
const pullRequests = [
pr({ number: 30, repoFullName: "entrius/allways-ui" }),
pr({ number: 31, repoFullName: "Entrius/Allways-UI" }),
]);
const listPrSpy = vi.spyOn(repositories, "listPullRequests").mockResolvedValue([pr({ number: 30 }), pr({ number: 31 })]);
vi.spyOn(repositories, "listPullRequestReviews").mockResolvedValue([]);
];
vi.spyOn(repositories, "listContributorPullRequests").mockResolvedValue(pullRequests);
vi.spyOn(repositories, "listPullRequestReviews").mockImplementation(async (_env, repo, pullNumber) =>
repo === pullRequests.find((entry) => entry.number === pullNumber)?.repoFullName ? [{ ...approvedReview(pullNumber), repoFullName: repo }] : [],
);
vi.spyOn(repositories, "listCheckSummaries").mockResolvedValue([]);
vi.spyOn(repositories, "listPullRequestFiles").mockResolvedValue([]);
const fileSpy = vi.spyOn(repositories, "listPullRequestFiles").mockImplementation(async (_env, repo, pullNumber) =>
repo === pullRequests.find((entry) => entry.number === pullNumber)?.repoFullName
? [
{ repoFullName: repo, pullNumber, path: `src/${pullNumber}.ts`, additions: 1, deletions: 0, changes: 1, status: "modified", payload: {} },
{ repoFullName: repo, pullNumber, path: `src/${pullNumber}.test.ts`, additions: 1, deletions: 0, changes: 1, status: "added", payload: {} },
]
: [],
);

const monitor = await buildContributorOpenPrMonitor(env, "miner-a");
expect(monitor.openPrCount).toBe(2);
// One merged group → the case-sensitive per-repo query runs only against a real repo casing, never the
// case-variant. Before the fix the two casings split into two groups and queried both.
expect(listPrSpy).toHaveBeenCalledWith(env, "entrius/allways-ui");
expect(listPrSpy).not.toHaveBeenCalledWith(env, "Entrius/Allways-UI");
expect(monitor.pullRequests.map((entry) => [entry.number, entry.classification])).toEqual([
[30, "approved"],
[31, "approved"],
]);
expect(monitor.pendingScenarios[0]?.detection.pendingMergedPrCount).toBe(2);
expect(fileSpy).toHaveBeenCalledWith(env, "entrius/allways-ui", 30);
expect(fileSpy).toHaveBeenCalledWith(env, "Entrius/Allways-UI", 31);
});

it("keeps public monitor output free of forbidden private language", async () => {
Expand Down
Loading