Skip to content

Commit 1b3400f

Browse files
authored
fix: avoid pr hash collisions (#308)
1 parent 04a4372 commit 1b3400f

File tree

3 files changed

+37
-49
lines changed

3 files changed

+37
-49
lines changed

.github/workflows/stats.yml

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2,8 +2,6 @@ name: pkg.pr.new stats
22

33
on:
44
push:
5-
branches:
6-
- main
75

86
jobs:
97
stats:

packages/backend/server/routes/publish.post.ts

Lines changed: 19 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -254,46 +254,25 @@ export default eventHandler(async (event) => {
254254
},
255255
);
256256
} else {
257-
try {
258-
await installation.request(
259-
"POST /repos/{owner}/{repo}/issues/{issue_number}/comments",
260-
{
261-
owner: workflowData.owner,
262-
repo: workflowData.repo,
263-
issue_number: Number(workflowData.ref),
264-
body: generatePullRequestPublishMessage(
265-
origin,
266-
templatesHtmlMap,
267-
packagesWithoutPrefix,
268-
workflowData,
269-
compact,
270-
onlyTemplates,
271-
checkRunUrl,
272-
packageManager,
273-
comment === "update" ? "ref" : "sha",
274-
),
275-
},
276-
);
277-
} catch (error) {
278-
throw createError({
279-
statusCode: 500,
280-
message: `Failed to create pull request comment. Details:
281-
Error: ${error instanceof Error ? error.message : String(error)}
282-
Context:
283-
- Owner: ${workflowData.owner}
284-
- Repo: ${workflowData.repo}
285-
- Issue Number: ${Number(workflowData.ref)}
286-
- Origin: ${origin}
287-
- Templates: ${JSON.stringify(templatesHtmlMap)}
288-
- Packages: ${packagesWithoutPrefix.join(', ')}
289-
- Workflow Data: ${JSON.stringify(workflowData)}
290-
- Compact: ${compact}
291-
- Only Templates: ${onlyTemplates}
292-
- Check Run URL: ${checkRunUrl}
293-
- Package Manager: ${packageManager}
294-
- Comment Type: ${comment === "update" ? "ref" : "sha"}`,
295-
});
296-
}
257+
await installation.request(
258+
"POST /repos/{owner}/{repo}/issues/{issue_number}/comments",
259+
{
260+
owner: workflowData.owner,
261+
repo: workflowData.repo,
262+
issue_number: Number(workflowData.ref),
263+
body: generatePullRequestPublishMessage(
264+
origin,
265+
templatesHtmlMap,
266+
packagesWithoutPrefix,
267+
workflowData,
268+
compact,
269+
onlyTemplates,
270+
checkRunUrl,
271+
packageManager,
272+
comment === "update" ? "ref" : "sha",
273+
),
274+
},
275+
);
297276
}
298277
}
299278
}

packages/backend/server/routes/webhook.post.ts

Lines changed: 18 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,6 @@ export default eventHandler(async (event) => {
1616
const { test } = useRuntimeConfig(event);
1717
const workflowsBucket = useWorkflowsBucket(event);
1818
const pullRequestNumbersBucket = usePullRequestNumbersBucket(event);
19-
const cursorBucket = useCursorsBucket(event);
2019

2120
const workflowHandler: HandlerFunction<"workflow_run", unknown> = async ({
2221
payload,
@@ -42,9 +41,20 @@ export default eventHandler(async (event) => {
4241
full_name: payload.workflow_run.head_repository.full_name,
4342
ref: payload.workflow_run.head_branch,
4443
};
45-
const prDataHash = hash(prData);
46-
const isPullRequest = await pullRequestNumbersBucket.hasItem(prDataHash);
47-
const prNumber = await pullRequestNumbersBucket.getItem(prDataHash);
44+
// new: using the new key to avoid collision
45+
const prKey = `${prData.full_name}:${prData.ref}`;
46+
const isNewPullRequest = await pullRequestNumbersBucket.hasItem(prKey);
47+
48+
// old: the old of hashing the prData started to hit collision, so we need to use the new one (e.g. https://github.com/element-plus/element-plus/actions/runs/12351113750/job/34465376908)
49+
const oldPrDataHash = hash(prData);
50+
const isOldPullRequest = await pullRequestNumbersBucket.hasItem(
51+
oldPrDataHash,
52+
);
53+
54+
const isPullRequest = isNewPullRequest || isOldPullRequest;
55+
const prNumber = await pullRequestNumbersBucket.getItem(
56+
isNewPullRequest ? prKey : oldPrDataHash,
57+
);
4858

4959
const data: WorkflowData = {
5060
owner,
@@ -64,15 +74,16 @@ export default eventHandler(async (event) => {
6474
const pullRequestHandler: HandlerFunction<"pull_request", unknown> = async ({
6575
payload,
6676
}) => {
67-
const [owner, repo] = payload.repository.full_name.split("/");
6877
// TODO: functions that generate these kinda keys
6978
const key: PullRequestData = {
7079
full_name: payload.pull_request.head.repo?.full_name!,
7180
ref: payload.pull_request.head.ref,
7281
};
73-
const prDataHash = hash(key);
7482
if (prMarkEvents.includes(payload.action)) {
75-
await pullRequestNumbersBucket.setItem(prDataHash, payload.number);
83+
await pullRequestNumbersBucket.setItem(
84+
`${key.full_name}:${key.ref}`,
85+
payload.number,
86+
);
7687
}
7788
};
7889

0 commit comments

Comments
 (0)