Skip to content

Commit 93f06ad

Browse files
committed
fix: skip stale link updates and page through comments
1 parent 44c209a commit 93f06ad

4 files changed

Lines changed: 124 additions & 18 deletions

File tree

โ€ŽAGENTS.mdโ€Ž

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -69,7 +69,7 @@ GitHub webhook
6969
```
7070

7171
`INTERNAL_SECRET` ๊ณผ `WORKER_URL` ์ด ๋‘˜ ๋‹ค ์žˆ์–ด์•ผ ์ผœ์ง„๋‹ค. ํ•˜๋‚˜๋ผ๋„ ์—†์œผ๋ฉด ๊ฐ™์€ invocation ์—์„œ ์ฐจ๋ก€๋กœ ์‹คํ–‰๋œ๋‹ค.
72-
`tests/subrequest-budget.test.js` ๊ฐ€ fetch ํšŸ์ˆ˜(`tagPatterns` ํŒŒ์ผ 15 ๊ฐœ์— 36, `postLearningStatus` ํŒŒ์ผ 5 ๊ฐœ์— 31)๋ฅผ ํšŒ๊ท€ ํ…Œ์ŠคํŠธ๋กœ ๊ณ ์ •ํ•œ๋‹ค.
72+
`tests/subrequest-budget.test.js` ๊ฐ€ fetch ํšŸ์ˆ˜(`tagPatterns` ํŒŒ์ผ 15 ๊ฐœ์— 37, `postLearningStatus` ํŒŒ์ผ 5 ๊ฐœ์— 31)๋ฅผ ํšŒ๊ท€ ํ…Œ์ŠคํŠธ๋กœ ๊ณ ์ •ํ•œ๋‹ค.
7373

7474
## ๋ฐฐํฌ์™€ ์‹œํฌ๋ฆฟ
7575

โ€Žhandlers/tag-patterns.jsโ€Ž

Lines changed: 66 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -124,6 +124,7 @@ export async function tagPatterns(
124124
repoOwner,
125125
repoName,
126126
prNumber,
127+
headSha,
127128
checkRun.html_url,
128129
appToken
129130
);
@@ -280,47 +281,98 @@ async function downloadFileEntries(solutionFiles) {
280281
/**
281282
* issue comment ๋ชฉ๋ก์„ ํ•œ ๋ฒˆ ์ฝ์–ด ๋‘ ๊ฐ€์ง€๋ฅผ ์ฒ˜๋ฆฌํ•œ๋‹ค.
282283
* - ๋งํฌ ๋Œ“๊ธ€: ๋ด‡์ด ๋‹จ ๊ฒƒ์ด ์žˆ์œผ๋ฉด ์ตœ์‹  check run ๋งํฌ๋กœ ๊ณ ์น˜๊ณ , ์—†์œผ๋ฉด ์ƒˆ๋กœ ๋‹จ๋‹ค.
284+
* ๋ถ„์„ ์ค‘์— ์ƒˆ ์ปค๋ฐ‹์ด ํ‘ธ์‹œ๋๋‹ค๋ฉด ๋” ๋Šฆ๊ฒŒ ๋๋‚œ ์ด์ „ ๋ถ„์„์ด ์ตœ์‹  ๋งํฌ๋ฅผ ๋ฎ์ง€ ์•Š๋„๋ก ๊ฑด๋„ˆ๋›ด๋‹ค.
283285
* - ๋ ˆ๊ฑฐ์‹œ ๋‹จ๋… ๋ณต์žก๋„ issue comment: ์žˆ์œผ๋ฉด ์‚ญ์ œํ•œ๋‹ค (๋งˆ์ด๊ทธ๋ ˆ์ด์…˜).
284286
* ์‹คํŒจํ•ด๋„ check run ์€ ์ด๋ฏธ ๊ฒŒ์‹œ๋์œผ๋ฏ€๋กœ ๋กœ๊ทธ๋งŒ ๋‚จ๊ธด๋‹ค.
285287
*/
286288
async function syncIssueComments(
287289
repoOwner,
288290
repoName,
289291
prNumber,
292+
headSha,
290293
checkRunUrl,
291294
appToken
292295
) {
293-
const listResponse = await fetch(
294-
`https://api.github.com/repos/${repoOwner}/${repoName}/issues/${prNumber}/comments?per_page=100`,
295-
{ headers: getGitHubHeaders(appToken) }
296+
const comments = await listIssueComments(
297+
repoOwner,
298+
repoName,
299+
prNumber,
300+
appToken
296301
);
302+
if (comments === null) return;
297303

298-
if (!listResponse.ok) {
299-
console.error(
300-
`[tagPatterns] Failed to list issue comments: ${listResponse.status}`
301-
);
302-
return;
303-
}
304-
305-
const comments = await listResponse.json();
306304
const findBotComment = (marker) =>
307305
comments.find((c) => c.user?.type === "Bot" && c.body?.includes(marker));
308306

309-
await upsertLinkComment(
307+
const latestHeadSha = await getHeadSha(
310308
repoOwner,
311309
repoName,
312310
prNumber,
313-
findBotComment(LINK_COMMENT_MARKER),
314-
checkRunUrl,
315311
appToken
316312
);
313+
if (latestHeadSha === headSha) {
314+
await upsertLinkComment(
315+
repoOwner,
316+
repoName,
317+
prNumber,
318+
findBotComment(LINK_COMMENT_MARKER),
319+
checkRunUrl,
320+
appToken
321+
);
322+
} else {
323+
console.log(
324+
`[tagPatterns] Skipping link comment on PR #${prNumber}: head moved from ${headSha} to ${latestHeadSha}`
325+
);
326+
}
317327

318328
const legacy = findBotComment(LEGACY_COMPLEXITY_MARKER);
319329
if (legacy) {
320330
await deleteIssueComment(repoOwner, repoName, legacy.id, appToken);
321331
}
322332
}
323333

334+
/**
335+
* PR ์˜ issue comment ๋ฅผ ๋ชจ๋“  ํŽ˜์ด์ง€์— ๊ฑธ์ณ ๋ชจ์€๋‹ค. ์‹คํŒจํ•˜๋ฉด null.
336+
*/
337+
async function listIssueComments(repoOwner, repoName, prNumber, appToken) {
338+
const comments = [];
339+
for (let page = 1; ; page++) {
340+
const response = await fetch(
341+
`https://api.github.com/repos/${repoOwner}/${repoName}/issues/${prNumber}/comments?per_page=100&page=${page}`,
342+
{ headers: getGitHubHeaders(appToken) }
343+
);
344+
345+
if (!response.ok) {
346+
console.error(
347+
`[tagPatterns] Failed to list issue comments: ${response.status}`
348+
);
349+
return null;
350+
}
351+
352+
const batch = await response.json();
353+
comments.push(...batch);
354+
if (batch.length < 100) return comments;
355+
}
356+
}
357+
358+
/**
359+
* PR ์˜ ํ˜„์žฌ head ์ปค๋ฐ‹ SHA. ์‹คํŒจํ•˜๋ฉด null ์ด๋ผ ๋งํฌ ๋Œ“๊ธ€ ๊ฐฑ์‹ ์„ ๊ฑด๋„ˆ๋›ด๋‹ค.
360+
*/
361+
async function getHeadSha(repoOwner, repoName, prNumber, appToken) {
362+
const response = await fetch(
363+
`https://api.github.com/repos/${repoOwner}/${repoName}/pulls/${prNumber}`,
364+
{ headers: getGitHubHeaders(appToken) }
365+
);
366+
367+
if (!response.ok) {
368+
console.error(`[tagPatterns] Failed to get PR #${prNumber}: ${response.status}`);
369+
return null;
370+
}
371+
372+
const pr = await response.json();
373+
return pr.head?.sha ?? null;
374+
}
375+
324376
async function upsertLinkComment(
325377
repoOwner,
326378
repoName,

โ€Žtests/subrequest-budget.test.jsโ€Ž

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -44,7 +44,7 @@ describe("subrequest ์˜ˆ์‚ฐ โ€” ํ•ธ๋“ค๋Ÿฌ๋ณ„ invocation", () => {
4444
vi.clearAllMocks();
4545
});
4646

47-
it("tagPatterns ๋Š” ๋ณ€๊ฒฝ ํŒŒ์ผ 15๊ฐœ์—์„œ 50ํšŒ ์ดํ•˜ subrequest๋ฅผ ํ˜ธ์ถœํ•œ๋‹ค (์˜ˆ์ƒ 36: files 1 + raw 15 + ํŒจํ„ด OpenAI 15 + ๋ณต์žก๋„ OpenAI 1 + check run 1 + issue ์ฝ”๋ฉ˜ํŠธ ๋ชฉ๋ก 1 + ๋งํฌ ๋Œ“๊ธ€ POST 1 + ๋ ˆ๊ฑฐ์‹œ DELETE 1)", async () => {
47+
it("tagPatterns ๋Š” ๋ณ€๊ฒฝ ํŒŒ์ผ 15๊ฐœ์—์„œ 50ํšŒ ์ดํ•˜ subrequest๋ฅผ ํ˜ธ์ถœํ•œ๋‹ค (์˜ˆ์ƒ 37: files 1 + raw 15 + ํŒจํ„ด OpenAI 15 + ๋ณต์žก๋„ OpenAI 1 + check run 1 + issue ์ฝ”๋ฉ˜ํŠธ ๋ชฉ๋ก 1 + PR head ์กฐํšŒ 1 + ๋งํฌ ๋Œ“๊ธ€ POST 1 + ๋ ˆ๊ฑฐ์‹œ DELETE 1)", async () => {
4848
const solutionFiles = makeSolutionFiles(15);
4949

5050
globalThis.fetch = vi.fn().mockImplementation((url, opts) => {
@@ -112,6 +112,10 @@ describe("subrequest ์˜ˆ์‚ฐ โ€” ํ•ธ๋“ค๋Ÿฌ๋ณ„ invocation", () => {
112112
return okJson({ id: 500 });
113113
}
114114

115+
if (urlStr.endsWith(`/pulls/${PR_NUMBER}`) && method === "GET") {
116+
return okJson({ head: { sha: HEAD_SHA } });
117+
}
118+
115119
// ๋ ˆ๊ฑฐ์‹œ ๋‹จ๋… ๋ณต์žก๋„ issue comment ๋งˆ์ด๊ทธ๋ ˆ์ด์…˜
116120
if (urlStr.includes(`/issues/${PR_NUMBER}/comments`) && method === "GET") {
117121
return okJson([
@@ -143,7 +147,7 @@ describe("subrequest ์˜ˆ์‚ฐ โ€” ํ•ธ๋“ค๋Ÿฌ๋ณ„ invocation", () => {
143147
const fetchCount = globalThis.fetch.mock.calls.length;
144148

145149
expect(result.analyzed).toBe(15);
146-
expect(fetchCount).toBe(36);
150+
expect(fetchCount).toBe(37);
147151
expect(fetchCount).toBeLessThanOrEqual(50);
148152
});
149153

โ€Žtests/tag-patterns.test.jsโ€Ž

Lines changed: 51 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -87,6 +87,7 @@ function makeFetchMock({
8787
complexityFails = false,
8888
checkRunFails = false,
8989
existingIssueComments = [],
90+
latestHeadSha = HEAD_SHA,
9091
} = {}) {
9192
const requests = [];
9293
const fetchMock = vi.fn().mockImplementation((url, opts) => {
@@ -139,8 +140,13 @@ function makeFetchMock({
139140
return okJson({ id: 777, html_url: CHECK_RUN_URL });
140141
}
141142

143+
if (urlStr.endsWith(`/pulls/${PR_NUMBER}`) && method === "GET") {
144+
return okJson({ head: { sha: latestHeadSha } });
145+
}
146+
142147
if (urlStr.includes(`/issues/${PR_NUMBER}/comments`) && method === "GET") {
143-
return okJson(existingIssueComments);
148+
const page = Number(new URL(urlStr).searchParams.get("page"));
149+
return okJson(existingIssueComments.slice((page - 1) * 100, page * 100));
144150
}
145151

146152
if (urlStr.includes(`/issues/${PR_NUMBER}/comments`) && method === "POST") {
@@ -428,6 +434,50 @@ describe("tagPatterns โ€” ๋Œ€ํ™” ํƒญ ๋งํฌ ๋Œ“๊ธ€", () => {
428434
expect(commentWrites[0].body.body).toContain(CHECK_RUN_URL);
429435
});
430436

437+
it("๋งํฌ ๋Œ“๊ธ€์ด ๋‘ ๋ฒˆ์งธ ํŽ˜์ด์ง€์— ์žˆ์–ด๋„ ์ฐพ์•„์„œ ๊ณ ์นœ๋‹ค", async () => {
438+
const requests = setupFetch({
439+
solutionFiles: [makeSolutionFile("two-sum")],
440+
existingIssueComments: [
441+
...Array.from({ length: 100 }, (_, i) => ({
442+
id: i + 1,
443+
user: { type: "User" },
444+
body: "๋ฆฌ๋ทฐ ๊ฐ์‚ฌํ•ฉ๋‹ˆ๋‹ค",
445+
})),
446+
{
447+
id: 321,
448+
user: { type: "Bot" },
449+
body: `${LINK_COMMENT_MARKER}\n์ด์ „ ๋งํฌ`,
450+
},
451+
],
452+
});
453+
454+
await runTagPatterns();
455+
456+
const commentWrites = requests.filter((r) => r.url.includes("/issues/"));
457+
expect(commentWrites).toHaveLength(1);
458+
expect(commentWrites[0].method).toBe("PATCH");
459+
expect(commentWrites[0].url).toContain("/issues/comments/321");
460+
});
461+
462+
it("๋ถ„์„ ์ค‘์— ์ƒˆ ์ปค๋ฐ‹์ด ํ‘ธ์‹œ๋์œผ๋ฉด ๋งํฌ ๋Œ“๊ธ€์„ ๊ฑด๋“œ๋ฆฌ์ง€ ์•Š๋Š”๋‹ค", async () => {
463+
const requests = setupFetch({
464+
solutionFiles: [makeSolutionFile("two-sum")],
465+
latestHeadSha: "newer-sha",
466+
existingIssueComments: [
467+
{
468+
id: 321,
469+
user: { type: "Bot" },
470+
body: `${LINK_COMMENT_MARKER}\n์ตœ์‹  ๋งํฌ`,
471+
},
472+
],
473+
});
474+
475+
await runTagPatterns();
476+
477+
expect(findCheckRun(requests).head_sha).toBe(HEAD_SHA);
478+
expect(requests.filter((r) => r.url.includes("/issues/"))).toHaveLength(0);
479+
});
480+
431481
it("์‚ฌ๋žŒ์ด ๋งˆ์ปค๋ฅผ ํฌํ•จํ•ด ๋‹จ ๋Œ“๊ธ€์€ ๋งํฌ ๋Œ“๊ธ€๋กœ ๋ณด์ง€ ์•Š๋Š”๋‹ค", async () => {
432482
const requests = setupFetch({
433483
solutionFiles: [makeSolutionFile("two-sum")],

0 commit comments

Comments
ย (0)