Skip to content
Closed
28 changes: 26 additions & 2 deletions actions/setup/js/create_issue.cjs
Original file line number Diff line number Diff line change
Expand Up @@ -511,6 +511,28 @@ async function shouldSkipRepoTitleDedupSearch(githubClient, owner, repo) {
return false;
}

/**
* Serialize calls to an async handler to prevent concurrent mutation of shared state.
* @template {(...args: any[]) => Promise<any>} T
* @param {T} handler
* @returns {T}
*/
function serializeAsyncHandler(handler) {
let queue = Promise.resolve();
function suppressQueueError() {
return undefined;
}

return /** @type {T} */ async (...args) => {
// Chain each invocation onto the previous one so calls run strictly in order.
// Keep the queue alive after failures so one rejected invocation does not block the next.
const resultPromise = queue.then(() => handler(...args));
// Intentionally suppress rejection in the queue chain only: callers still receive resultPromise errors.
queue = resultPromise.catch(suppressQueueError);
return resultPromise;
};
}

/**
* Main handler factory for create_issue
* Returns a message handler function that processes individual create_issue messages
Expand Down Expand Up @@ -646,7 +668,7 @@ async function main(config = {}) {
* @param {Object} resolvedTemporaryIds - Map of temporary IDs to {repo, number}
* @returns {Promise<Object>} Result with success/error status and issue details
*/
return async function handleCreateIssue(message, resolvedTemporaryIds) {
async function handleCreateIssue(message, resolvedTemporaryIds) {
// Merge external resolved temp IDs with our local map
if (resolvedTemporaryIds) {
for (const [tempId, resolved] of Object.entries(resolvedTemporaryIds)) {
Expand Down Expand Up @@ -1256,7 +1278,9 @@ async function main(config = {}) {
error: errorMessage,
};
}
};
}

return serializeAsyncHandler(handleCreateIssue);
}

module.exports = { main, createParentIssueTemplate, searchForExistingParent, getSubIssueCount };
78 changes: 73 additions & 5 deletions actions/setup/js/create_issue_new_arch.test.cjs
Original file line number Diff line number Diff line change
@@ -1,4 +1,13 @@
import { describe, it, expect, beforeEach, vi } from "vitest";
import { describe, it, expect, beforeEach, afterEach, vi } from "vitest";
import { mkdirSync, writeFileSync } from "fs";
import { join } from "path";
import { createRequire } from "module";

const require = createRequire(import.meta.url);
const { main } = require("./create_issue.cjs");
// Delay long enough for Promise.all-started calls to overlap and exercise concurrent interleaving paths.
const CONCURRENT_TEST_DELAY_MS = 15;
const delay = ms => new Promise(resolve => setTimeout(resolve, ms));

const mockCore = {
debug: vi.fn(),
Expand All @@ -15,6 +24,14 @@ const mockGithub = {
create: vi.fn(),
createComment: vi.fn(),
},
search: {
issuesAndPullRequests: vi.fn().mockResolvedValue({
data: {
total_count: 0,
items: [],
},
}),
},
},
graphql: vi.fn(),
};
Expand All @@ -33,21 +50,28 @@ global.context = mockContext;

describe("create_issue.cjs (New Handler Factory Architecture)", () => {
let handler;
let originalRunnerTemp;

beforeEach(async () => {
vi.clearAllMocks();
originalRunnerTemp = process.env.RUNNER_TEMP;

// Load the module and create handler
const { main } = require("./create_issue.cjs");
handler = await main({
max: 10,
labels: ["automation"],
title_prefix: "[AUTO] ",
});
});

afterEach(() => {
if (originalRunnerTemp === undefined) {
delete process.env.RUNNER_TEMP;
} else {
process.env.RUNNER_TEMP = originalRunnerTemp;
}
});

it("should return a function from main()", async () => {
const { main } = require("./create_issue.cjs");
const result = await main({});
expect(typeof result).toBe("function");
});
Expand Down Expand Up @@ -79,7 +103,6 @@ describe("create_issue.cjs (New Handler Factory Architecture)", () => {
});

it("should respect max count limit", async () => {
const { main } = require("./create_issue.cjs");
const limitedHandler = await main({ max: 1 });

const mockIssue = { number: 123, html_url: "https://github.com/testowner/testrepo/issues/123", node_id: "I_123" };
Expand Down Expand Up @@ -190,4 +213,49 @@ describe("create_issue.cjs (New Handler Factory Architecture)", () => {
// The error could be about format or allowed repos depending on validation order
expect(result.error).toMatch(/Invalid repository format|not in the allowed-repos list/);
});

it("should avoid duplicate parent creation for concurrent grouped issues", async () => {
process.env.RUNNER_TEMP = process.env.RUNNER_TEMP || "/tmp";
const promptsDir = join(process.env.RUNNER_TEMP, "gh-aw", "prompts");
mkdirSync(promptsDir, { recursive: true });
writeFileSync(join(promptsDir, "issue_group_parent.md"), "Group {{group_id}}");

const groupedHandler = await main({ group: true, max: 10 });

let nextIssueNumber = 200;
let parentCreateCount = 0;

mockGithub.rest.search.issuesAndPullRequests.mockImplementation(async () => {
await delay(CONCURRENT_TEST_DELAY_MS);
return { data: { total_count: 0, items: [] } };
});

mockGithub.rest.issues.create.mockImplementation(async ({ title }) => {
await delay(CONCURRENT_TEST_DELAY_MS);

if (String(title).includes("Issue Group")) {
parentCreateCount += 1;
return { data: { number: 999, html_url: "https://github.com/testowner/testrepo/issues/999", node_id: "I_999" } };
}

nextIssueNumber += 1;
return {
data: {
number: nextIssueNumber,
html_url: `https://github.com/testowner/testrepo/issues/${nextIssueNumber}`,
node_id: `I_${nextIssueNumber}`,
},
};
});

mockGithub.graphql.mockRejectedValue(new Error("GraphQL not required for this test"));

const firstMessage = { type: "create_issue", title: "Concurrent A", body: "A" };
const secondMessage = { type: "create_issue", title: "Concurrent B", body: "B" };
const [first, second] = await Promise.all([groupedHandler(firstMessage, {}), groupedHandler(secondMessage, {})]);

expect(first.success).toBe(true);
expect(second.success).toBe(true);
expect(parentCreateCount).toBe(1);
});
});