From 02ac89980d4382cd35774b1163296aea6e293f62 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Tue, 21 Apr 2026 10:41:16 +0000 Subject: [PATCH 1/4] Initial plan From b1ff3720ee38e3ba7265799b29a08884864b7d34 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Tue, 21 Apr 2026 10:58:18 +0000 Subject: [PATCH 2/4] fix: make create_issue max-count reservation concurrency-safe Agent-Logs-Url: https://github.com/github/gh-aw/sessions/d625376d-a36f-47d0-867d-4070fa06b91d Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> --- actions/setup/js/create_issue.cjs | 53 ++++++++++---------------- actions/setup/js/create_issue.test.cjs | 18 +++++++++ 2 files changed, 38 insertions(+), 33 deletions(-) diff --git a/actions/setup/js/create_issue.cjs b/actions/setup/js/create_issue.cjs index ed66c1f0250..ca304e96830 100644 --- a/actions/setup/js/create_issue.cjs +++ b/actions/setup/js/create_issue.cjs @@ -1,34 +1,22 @@ // @ts-check /// -/** - * Module-level storage retained for backward compatibility with the - * `assign_copilot_to_created_issues` step. The create_issue handler now assigns - * copilot inline immediately after issue creation (via assign_agent_helpers.cjs), - * so this list is never populated during normal operation and the downstream step - * is a no-op. It is exposed via getIssuesToAssignCopilot/resetIssuesToAssignCopilot - * for unit tests. - * @type {Array} - */ -let issuesToAssignCopilotGlobal = []; - /** * Get the list of issues that need copilot assignment. - * Returns a defensive copy so callers cannot accidentally mutate global state. - * In practice this list is always empty because assignment is now done inline. - * @returns {Array} Copy of the "repo:number" strings array + * Assignment now happens inline, so this queue is always empty. + * Kept for backward-compatible tests and no-op downstream wiring. + * @returns {Array} Always empty array */ function getIssuesToAssignCopilot() { - return [...issuesToAssignCopilotGlobal]; + return []; } /** * Reset the list of issues that need copilot assignment. - * Clears the internal array in-place. Previously returned snapshots (copies) - * are not affected. Used for testing. + * No-op kept for backward-compatible tests. */ function resetIssuesToAssignCopilot() { - issuesToAssignCopilotGlobal.length = 0; + // Intentionally empty: there is no mutable queue to reset. } const { sanitizeLabelContent } = require("./sanitize_label_content.cjs"); @@ -322,15 +310,6 @@ async function main(config = {}) { * @returns {Promise} Result with success/error status and issue details */ return async function handleCreateIssue(message, resolvedTemporaryIds) { - // Check if we've hit the max limit - if (processedCount >= maxCount) { - core.warning(`Skipping create_issue: max count of ${maxCount} reached`); - return { - success: false, - error: `Max count of ${maxCount} reached`, - }; - } - // Merge external resolved temp IDs with our local map if (resolvedTemporaryIds) { for (const [tempId, resolved] of Object.entries(resolvedTemporaryIds)) { @@ -529,11 +508,22 @@ async function main(config = {}) { bodyLines.push(""); const body = bodyLines.join("\n").trim(); + // Atomically reserve a max-count slot before any async pre-creation work. + // There is no await between check and increment, so concurrent invocations + // cannot pass the limit check simultaneously. + if (processedCount >= maxCount) { + core.warning(`Skipping create_issue: max count of ${maxCount} reached`); + return { + success: false, + error: `Max count of ${maxCount} reached`, + }; + } + processedCount++; + // Group-by-day check: if enabled, search for an existing open issue created today. // When found, post the new content as a comment on the existing issue instead of // creating a duplicate. This groups multiple same-day runs into a single issue. - // The max-count slot is NOT consumed when posting as a comment (processedCount is - // only incremented below, just before actual issue creation). + // The reserved max-count slot is released when posting as a comment. if (groupByDayEnabled && (closeOlderKey || workflowId)) { const today = new Date().toISOString().split("T")[0]; // YYYY-MM-DD (UTC) try { @@ -554,6 +544,7 @@ async function main(config = {}) { core.info(`Group-by-day: found open issue #${todayIssue.number} created today (${today}) — posting new content as a comment`); const comment = await addIssueComment(githubClient, repoParts.owner, repoParts.repo, todayIssue.number, body); core.info(`Posted content as comment ${comment.html_url} on issue #${todayIssue.number}`); + processedCount--; return { success: true, grouped: true, @@ -568,10 +559,6 @@ async function main(config = {}) { } } - // Increment processed count only when we are about to create an issue - // (group-by-day comment paths return above without consuming a slot) - processedCount++; - core.info(`Creating issue in ${qualifiedItemRepo} with title: ${title}`); core.info(`Labels: ${labels.join(", ")}`); if (assignees.length > 0) { diff --git a/actions/setup/js/create_issue.test.cjs b/actions/setup/js/create_issue.test.cjs index d936961fb19..6340a417dfb 100644 --- a/actions/setup/js/create_issue.test.cjs +++ b/actions/setup/js/create_issue.test.cjs @@ -300,6 +300,24 @@ describe("create_issue", () => { expect(result3.success).toBe(false); expect(result3.error).toContain("Max count of 2 reached"); }); + + it("should respect max count limit under concurrent calls with group-by-day pre-check", async () => { + const handler = await main({ + max: 1, + group_by_day: true, + close_older_key: "concurrency-key", + }); + + const [result1, result2] = await Promise.all([handler({ title: "Issue 1" }), handler({ title: "Issue 2" })]); + const results = [result1, result2]; + const successes = results.filter(result => result.success); + const failures = results.filter(result => !result.success); + + expect(successes).toHaveLength(1); + expect(failures).toHaveLength(1); + expect(failures[0].error).toContain("Max count of 1 reached"); + expect(mockGithub.rest.issues.create).toHaveBeenCalledTimes(1); + }); }); describe("title prefix", () => { From 3f1f51843d489e57bd46e4414b1bbe54ecb64875 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Tue, 21 Apr 2026 10:59:50 +0000 Subject: [PATCH 3/4] docs: clarify synchronous max-slot reservation wording Agent-Logs-Url: https://github.com/github/gh-aw/sessions/d625376d-a36f-47d0-867d-4070fa06b91d Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> --- actions/setup/js/create_issue.cjs | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/actions/setup/js/create_issue.cjs b/actions/setup/js/create_issue.cjs index ca304e96830..0eac93eebb2 100644 --- a/actions/setup/js/create_issue.cjs +++ b/actions/setup/js/create_issue.cjs @@ -508,9 +508,9 @@ async function main(config = {}) { bodyLines.push(""); const body = bodyLines.join("\n").trim(); - // Atomically reserve a max-count slot before any async pre-creation work. + // Reserve a max-count slot synchronously before any async pre-creation work. // There is no await between check and increment, so concurrent invocations - // cannot pass the limit check simultaneously. + // cannot interleave between these two operations. if (processedCount >= maxCount) { core.warning(`Skipping create_issue: max count of ${maxCount} reached`); return { @@ -544,6 +544,8 @@ async function main(config = {}) { core.info(`Group-by-day: found open issue #${todayIssue.number} created today (${today}) — posting new content as a comment`); const comment = await addIssueComment(githubClient, repoParts.owner, repoParts.repo, todayIssue.number, body); core.info(`Posted content as comment ${comment.html_url} on issue #${todayIssue.number}`); + // No issue was created (content was grouped into a comment), so free + // the reserved slot for subsequent create_issue calls. processedCount--; return { success: true, From f5563aa26c5a62762ff8db423eaed3f4438c83b7 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Tue, 21 Apr 2026 13:45:52 +0000 Subject: [PATCH 4/4] refactor: remove dead create_issue copilot queue compatibility code Agent-Logs-Url: https://github.com/github/gh-aw/sessions/16b1e99a-00e9-4025-a027-2a8c120e2315 Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> --- .../js/create_discussion_fallback.test.cjs | 6 ---- actions/setup/js/create_issue.cjs | 20 +----------- actions/setup/js/create_issue.test.cjs | 32 +------------------ .../setup/js/safe_output_handler_manager.cjs | 16 ---------- 4 files changed, 2 insertions(+), 72 deletions(-) diff --git a/actions/setup/js/create_discussion_fallback.test.cjs b/actions/setup/js/create_discussion_fallback.test.cjs index 81cb74af8da..1f6a181b1b5 100644 --- a/actions/setup/js/create_discussion_fallback.test.cjs +++ b/actions/setup/js/create_discussion_fallback.test.cjs @@ -4,7 +4,6 @@ import { createRequire } from "module"; const require = createRequire(import.meta.url); const { main: createDiscussionMain } = require("./create_discussion.cjs"); -const { resetIssuesToAssignCopilot } = require("./create_issue.cjs"); describe("create_discussion fallback with close_older_discussions", () => { let mockGithub; @@ -17,9 +16,6 @@ describe("create_discussion fallback with close_older_discussions", () => { // Save original environment originalEnv = { ...process.env }; - // Reset copilot assignment tracking - resetIssuesToAssignCopilot(); - // Mock GitHub API mockGithub = { rest: { @@ -259,8 +255,6 @@ describe("create_discussion double-posting prevention", () => { beforeEach(() => { originalEnv = { ...process.env }; - resetIssuesToAssignCopilot(); - // Base mock — each test overrides graphql as needed. mockGithub = { rest: { diff --git a/actions/setup/js/create_issue.cjs b/actions/setup/js/create_issue.cjs index 0eac93eebb2..637b41a989a 100644 --- a/actions/setup/js/create_issue.cjs +++ b/actions/setup/js/create_issue.cjs @@ -1,24 +1,6 @@ // @ts-check /// -/** - * Get the list of issues that need copilot assignment. - * Assignment now happens inline, so this queue is always empty. - * Kept for backward-compatible tests and no-op downstream wiring. - * @returns {Array} Always empty array - */ -function getIssuesToAssignCopilot() { - return []; -} - -/** - * Reset the list of issues that need copilot assignment. - * No-op kept for backward-compatible tests. - */ -function resetIssuesToAssignCopilot() { - // Intentionally empty: there is no mutable queue to reset. -} - const { sanitizeLabelContent } = require("./sanitize_label_content.cjs"); const { sanitizeTitle, applyTitlePrefix } = require("./sanitize_title.cjs"); const { sanitizeContent } = require("./sanitize_content.cjs"); @@ -804,4 +786,4 @@ async function main(config = {}) { }; } -module.exports = { main, createParentIssueTemplate, searchForExistingParent, getSubIssueCount, getIssuesToAssignCopilot, resetIssuesToAssignCopilot }; +module.exports = { main, createParentIssueTemplate, searchForExistingParent, getSubIssueCount }; diff --git a/actions/setup/js/create_issue.test.cjs b/actions/setup/js/create_issue.test.cjs index 6340a417dfb..fb46e536c80 100644 --- a/actions/setup/js/create_issue.test.cjs +++ b/actions/setup/js/create_issue.test.cjs @@ -2,7 +2,7 @@ import { describe, it, expect, beforeEach, afterEach, vi } from "vitest"; import { createRequire } from "module"; const require = createRequire(import.meta.url); -const { main, getIssuesToAssignCopilot, resetIssuesToAssignCopilot } = require("./create_issue.cjs"); +const { main } = require("./create_issue.cjs"); describe("create_issue", () => { let mockGithub; @@ -15,9 +15,6 @@ describe("create_issue", () => { // Save original environment originalEnv = { ...process.env }; - // Reset copilot assignment tracking - resetIssuesToAssignCopilot(); - // Mock GitHub API mockGithub = { rest: { @@ -257,33 +254,6 @@ describe("create_issue", () => { // Verify graphql was called three times (findAgent, getIssueDetails, assignAgentToIssue) expect(mockGithub.graphql).toHaveBeenCalledTimes(3); - // Global queue should remain empty (assignment done directly, not queued) - expect(getIssuesToAssignCopilot()).toHaveLength(0); - }); - }); - - describe("global state safety", () => { - it("getIssuesToAssignCopilot returns a copy, not the live reference", () => { - const snapshot = getIssuesToAssignCopilot(); - const lengthBefore = snapshot.length; - - // Mutating the returned copy must NOT affect internal state - snapshot.push("external:999"); - expect(getIssuesToAssignCopilot().length).toBe(lengthBefore); - }); - - it("resetIssuesToAssignCopilot clears internal state but does not affect held snapshots", () => { - const snapshot = getIssuesToAssignCopilot(); - - // Make the held copy observably different from internal state - snapshot.push("external:999"); - - resetIssuesToAssignCopilot(); - - // Fresh reads reflect cleared internal state - expect(getIssuesToAssignCopilot()).toHaveLength(0); - // Previously returned snapshots remain unchanged because getter returns a copy - expect(snapshot).toEqual(expect.arrayContaining(["external:999"])); }); }); diff --git a/actions/setup/js/safe_output_handler_manager.cjs b/actions/setup/js/safe_output_handler_manager.cjs index 1aa5cd9c504..93f376c9bc7 100644 --- a/actions/setup/js/safe_output_handler_manager.cjs +++ b/actions/setup/js/safe_output_handler_manager.cjs @@ -16,7 +16,6 @@ const { hasUnresolvedTemporaryIds, replaceTemporaryIdReferences, replaceArtifact const { generateMissingInfoSections } = require("./missing_info_formatter.cjs"); const { setCollectedMissings } = require("./missing_messages_helper.cjs"); const { writeSafeOutputSummaries } = require("./safe_output_summary.cjs"); -const { getIssuesToAssignCopilot } = require("./create_issue.cjs"); const { getAssignToAgentAssigned, getAssignToAgentErrors, getAssignToAgentErrorCount, writeAssignToAgentSummary } = require("./assign_to_agent.cjs"); const { getCreateAgentSessionNumber, getCreateAgentSessionUrl, writeCreateAgentSessionSummary } = require("./create_agent_session.cjs"); const { createReviewBuffer } = require("./pr_review_buffer.cjs"); @@ -1048,11 +1047,6 @@ async function main() { try { core.info("Safe Output Handler Manager starting..."); - // Reset create_issue handler's global state to ensure clean state for this run - // This prevents stale data accumulation if the module is reused - const { resetIssuesToAssignCopilot } = require("./create_issue.cjs"); - resetIssuesToAssignCopilot(); - // Load configuration const config = loadConfig(); core.debug(`Configuration: ${JSON.stringify(Object.keys(config))}`); @@ -1224,16 +1218,6 @@ async function main() { // Export processed count for consistency with project handler core.setOutput("processed_count", successCount); - // Export issues that need copilot assignment (if any) - const issuesToAssignCopilot = getIssuesToAssignCopilot(); - if (issuesToAssignCopilot.length > 0) { - const issuesToAssignStr = issuesToAssignCopilot.join(","); - core.setOutput("issues_to_assign_copilot", issuesToAssignStr); - core.info(`Exported ${issuesToAssignCopilot.length} issue(s) for copilot assignment: ${issuesToAssignStr}`); - } else { - core.setOutput("issues_to_assign_copilot", ""); - } - // Export assign_to_agent outputs when the handler was loaded if (messageHandlers.has("assign_to_agent")) { const assignToAgentAssigned = getAssignToAgentAssigned();