Skip to content

fix: paginate listComments to prevent duplicate bot comments #13

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
mafredri wants to merge 1 commit into
base: main
Choose a base branch
from
Open

fix: paginate listComments to prevent duplicate bot comments #13

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
38 changes: 29 additions & 9 deletions dist/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,25 +3,43 @@ var __getProtoOf = Object.getPrototypeOf;
var __defProp = Object.defineProperty;
var __getOwnPropNames = Object.getOwnPropertyNames;
var __hasOwnProp = Object.prototype.hasOwnProperty;
function __accessProp(key) {
return this[key];
}
var __toESMCache_node;
var __toESMCache_esm;
var __toESM = (mod, isNodeMode, target) => {
var canCache = mod != null && typeof mod === "object";
if (canCache) {
var cache = isNodeMode ? __toESMCache_node ??= new WeakMap : __toESMCache_esm ??= new WeakMap;
var cached = cache.get(mod);
if (cached)
return cached;
}
target = mod != null ? __create(__getProtoOf(mod)) : {};
const to = isNodeMode || !mod || !mod.__esModule ? __defProp(target, "default", { value: mod, enumerable: true }) : target;
for (let key of __getOwnPropNames(mod))
if (!__hasOwnProp.call(to, key))
__defProp(to, key, {
get: () => mod[key],
get: __accessProp.bind(mod, key),
enumerable: true
});
if (canCache)
cache.set(mod, to);
return to;
};
var __commonJS = (cb, mod) => () => (mod || cb((mod = { exports: {} }).exports, mod), mod.exports);
var __returnValue = (v) => v;
function __exportSetter(name, newValue) {
this[name] = __returnValue.bind(null, newValue);
}
var __export = (target, all) => {
for (var name in all)
__defProp(target, name, {
get: all[name],
enumerable: true,
configurable: true,
set: (newValue) => all[name] = () => newValue
set: __exportSetter.bind(all, name)
});
};

Expand Down Expand Up @@ -3447,7 +3465,7 @@ var require_constants2 = __commonJS((exports2, module2) => {
}
})();
var channel;
var structuredClone = globalThis.structuredClone ?? function structuredClone(value, options = undefined) {
var structuredClone = globalThis.structuredClone ?? function structuredClone2(value, options = undefined) {
if (arguments.length === 0) {
throw new TypeError("missing argument");
}
Expand Down Expand Up @@ -16372,7 +16390,7 @@ var require_undici = __commonJS((exports2, module2) => {
module2.exports.getGlobalDispatcher = getGlobalDispatcher;
if (util.nodeMajor > 16 || util.nodeMajor === 16 && util.nodeMinor >= 8) {
let fetchImpl = null;
module2.exports.fetch = async function fetch(resource) {
module2.exports.fetch = async function fetch2(resource) {
if (!fetchImpl) {
fetchImpl = require_fetch().fetch;
}
Expand Down Expand Up @@ -22708,11 +22726,11 @@ var require_github = __commonJS((exports2) => {
});

// src/index.ts
var core2 = __toESM(require_core());
var github = __toESM(require_github());
var core2 = __toESM(require_core(), 1);
var github = __toESM(require_github(), 1);

// src/action.ts
var core = __toESM(require_core());
var core = __toESM(require_core(), 1);

class CoderAgentChatAction {
coder;
Expand Down Expand Up @@ -22747,9 +22765,11 @@ class CoderAgentChatAction {
const { data: comments } = await this.octokit.rest.issues.listComments({
owner,
repo,
issue_number: issueNumber
issue_number: issueNumber,
per_page: 100,
direction: "desc"
});
const existingComment = comments.reverse().find((comment) => comment.body?.startsWith("Agent chat created:"));
const existingComment = comments.find((comment) => comment.body?.startsWith("Agent chat created:"));
if (existingComment) {
await this.octokit.rest.issues.updateComment({
owner,
Expand Down
50 changes: 46 additions & 4 deletions src/action.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -134,6 +134,13 @@ describe("CoderAgentChatAction", () => {

await action.commentOnIssue("chat-url", "owner", "repo", 123);

expect(octokit.rest.issues.listComments).toHaveBeenCalledWith({
owner: "owner",
repo: "repo",
issue_number: 123,
per_page: 100,
direction: "desc",
});
expect(octokit.rest.issues.createComment).toHaveBeenCalledWith({
owner: "owner",
repo: "repo",
Expand All @@ -142,11 +149,14 @@ describe("CoderAgentChatAction", () => {
});
});

test("updates existing Agent chat created comment", async () => {
test("updates most recent matching comment (desc order)", async () => {
// With direction: "desc", the API returns newest first.
// The first match is the most recent one.
octokit.rest.issues.listComments.mockResolvedValue({
data: [
{ id: 1, body: "Agent chat created: old-url" },
{ id: 2, body: "Other comment" },
{ id: 3, body: "Other comment" },
{ id: 2, body: "Agent chat created: newer-url" },
{ id: 1, body: "Agent chat created: oldest-url" },
],
} as ReturnType<typeof octokit.rest.issues.listComments>);
octokit.rest.issues.updateComment.mockResolvedValue(
Expand All @@ -165,11 +175,43 @@ describe("CoderAgentChatAction", () => {
expect(octokit.rest.issues.updateComment).toHaveBeenCalledWith({
owner: "owner",
repo: "repo",
comment_id: 1,
comment_id: 2,
body: "Agent chat created: new-url",
});
});

test("creates new comment when match is not in first page", async () => {
// Simulate 100 comments with no match. The real matching
// comment would be beyond this page, but creating a new
// one is acceptable.
const comments = Array.from({ length: 100 }, (_, i) => ({
id: 100 - i,
body: `Unrelated comment ${100 - i}`,
}));
octokit.rest.issues.listComments.mockResolvedValue({
data: comments,
} as ReturnType<typeof octokit.rest.issues.listComments>);
octokit.rest.issues.createComment.mockResolvedValue(
{} as ReturnType<typeof octokit.rest.issues.createComment>,
);

const inputs = createMockInputs();
const action = new CoderAgentChatAction(
coderClient,
octokit as unknown as Octokit,
inputs,
);

await action.commentOnIssue("chat-url", "owner", "repo", 123);

expect(octokit.rest.issues.createComment).toHaveBeenCalledWith({
owner: "owner",
repo: "repo",
issue_number: 123,
body: "Agent chat created: chat-url",
});
});

test("warns but doesn't fail on GitHub API error", async () => {
octokit.rest.issues.listComments.mockRejectedValue(
new Error("API Error"),
Expand Down
14 changes: 9 additions & 5 deletions src/action.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -57,17 +57,21 @@ export class CoderAgentChatAction {
const body = `Agent chat created: ${chatUrl}`;

try {
// Fetch the most recent comments first so we find the latest
// matching comment without needing to paginate through all of
// them. The default per_page is 30, which is too few for
// active issues.
const { data: comments } = await this.octokit.rest.issues.listComments({
owner,
repo,
issue_number: issueNumber,
per_page: 100,
direction: "desc",
});

const existingComment = comments
.reverse()
.find((comment: { body?: string }) =>
comment.body?.startsWith("Agent chat created:"),
);
const existingComment = comments.find((comment: { body?: string }) =>
comment.body?.startsWith("Agent chat created:"),
);

if (existingComment) {
await this.octokit.rest.issues.updateComment({
Expand Down
Loading