diff --git a/actions/setup/js/create_agent_session.cjs b/actions/setup/js/create_agent_session.cjs index daee324f47f..a1ea40d37be 100644 --- a/actions/setup/js/create_agent_session.cjs +++ b/actions/setup/js/create_agent_session.cjs @@ -7,14 +7,6 @@ const { getBaseBranch } = require("./get_base_branch.cjs"); const { isStagedMode } = require("./safe_output_helpers.cjs"); const { generateStagedPreview } = require("./staged_preview.cjs"); -/** - * Module-level state — populated by handleMessage(), read by the exported getters below. - * Using module-level variables (rather than closure-only state) allows the handler - * manager to read final output values after all messages have been processed. - * @type {Array<{id: string, url: string, success: boolean, error?: string}>} - */ -let _allResults = []; - /** * Create a dedicated GitHub client for create-agent-session operations. * @@ -48,8 +40,13 @@ async function createAgentSessionGitHubClient(config) { * @returns {Promise} Message processor function */ async function main(config = {}) { - // Reset module-level state for this run - _allResults = []; + // Per-invocation state — captured in a closure (rather than module scope) so that + // concurrent or repeated `main()` invocations in the same process never share or + // clobber each other's results. The accessor functions below are attached to the + // returned `handleMessage` function so the handler manager can read final output + // values after all messages have been processed for this specific invocation. + /** @type {Array<{id: string, url: string, success: boolean, error?: string}>} */ + const allResults = []; // Parse configuration const configuredBaseBranch = config.base ? String(config.base).trim() : null; @@ -68,12 +65,12 @@ async function main(config = {}) { * @param {Object} message - The agent output message * @returns {Promise<{success: boolean, id?: string, url?: string, error?: string, skipped?: boolean}>} */ - return async function handleMessage(message) { + const handleMessage = async function (message) { const taskDescription = message.body; if (!taskDescription || taskDescription.trim() === "") { core.warning("Agent task description is empty, skipping"); - _allResults.push({ id: "", url: "", success: false, error: "Empty task description" }); + allResults.push({ id: "", url: "", success: false, error: "Empty task description" }); return { success: false, error: "Empty task description" }; } @@ -82,7 +79,7 @@ async function main(config = {}) { if (!repoResult.success) { const errorMsg = `E004: ${repoResult.error}`; core.error(errorMsg); - _allResults.push({ id: "", url: "", success: false, error: repoResult.error }); + allResults.push({ id: "", url: "", success: false, error: repoResult.error }); return { success: false, error: repoResult.error }; } const { repo: effectiveRepo, repoParts } = repoResult; @@ -109,7 +106,7 @@ async function main(config = {}) { } try { - core.info(`Task ${_allResults.length + 1}: Creating agent session in ${effectiveRepo} on branch ${baseBranch}`); + core.info(`Task ${allResults.length + 1}: Creating agent session in ${effectiveRepo} on branch ${baseBranch}`); // Call the GitHub REST API to start a task // Reference: https://docs.github.com/en/rest/agent-tasks/agent-tasks?apiVersion=2026-03-10#start-a-task @@ -126,7 +123,7 @@ async function main(config = {}) { const taskUrl = task.html_url || task.url || ""; core.info(`✅ Successfully created agent session ${taskId}`); - _allResults.push({ id: taskId, url: taskUrl, success: true }); + allResults.push({ id: taskId, url: taskUrl, success: true }); return { success: true, id: taskId, url: taskUrl }; } catch (error) { const errorMessage = getErrorMessage(error); @@ -140,69 +137,71 @@ async function main(config = {}) { } else { core.error(`Error creating agent session: ${errorMessage}`); } - _allResults.push({ id: "", url: "", success: false, error: errorMessage }); + allResults.push({ id: "", url: "", success: false, error: errorMessage }); return { success: false, error: errorMessage }; } }; -} -/** - * Returns the session_number output: the ID of the first successfully created session. - * @returns {string} - */ -function getCreateAgentSessionNumber() { - const first = _allResults.find(r => r.success && r.id); - return first ? first.id : ""; -} + /** + * Returns the session_number output: the ID of the first successfully created session. + * @returns {string} + */ + handleMessage.getSessionNumber = () => { + const first = allResults.find(r => r.success && r.id); + return first ? first.id : ""; + }; -/** - * Returns the session_url output: the URL of the first successfully created session. - * @returns {string} - */ -function getCreateAgentSessionUrl() { - const first = _allResults.find(r => r.success && r.url); - return first ? first.url : ""; -} + /** + * Returns the session_url output: the URL of the first successfully created session. + * @returns {string} + */ + handleMessage.getSessionUrl = () => { + const first = allResults.find(r => r.success && r.url); + return first ? first.url : ""; + }; -/** - * Writes a step summary for agent session creation results. - * Called by the handler manager after all messages have been processed. - * @returns {Promise} - */ -async function writeCreateAgentSessionSummary() { - const successResults = _allResults.filter(r => r.success); - const failedResults = _allResults.filter(r => !r.success); - - if (_allResults.length === 0) return; - - let summaryContent = "## Agent Sessions\n\n"; - - if (successResults.length > 0) { - summaryContent += `✅ Successfully created ${successResults.length} agent session(s):\n\n`; - summaryContent += successResults - .map((r, i) => { - if (r.url && r.id) { - return `- [${r.id}](${r.url})`; - } else if (r.url) { - return `- [Session ${i + 1}](${r.url})`; - } - return `- Session ${i + 1}`; - }) - .join("\n"); - summaryContent += "\n\n"; - } + /** + * Writes a step summary for agent session creation results. + * Called by the handler manager after all messages have been processed. + * @returns {Promise} + */ + handleMessage.writeSummary = async () => { + const successResults = allResults.filter(r => r.success); + const failedResults = allResults.filter(r => !r.success); + + if (allResults.length === 0) return; + + let summaryContent = "## Agent Sessions\n\n"; + + if (successResults.length > 0) { + summaryContent += `✅ Successfully created ${successResults.length} agent session(s):\n\n`; + summaryContent += successResults + .map((r, i) => { + if (r.url && r.id) { + return `- [${r.id}](${r.url})`; + } else if (r.url) { + return `- [Session ${i + 1}](${r.url})`; + } + return `- Session ${i + 1}`; + }) + .join("\n"); + summaryContent += "\n\n"; + } - if (failedResults.length > 0) { - summaryContent += `❌ Failed to create ${failedResults.length} agent session(s):\n\n`; - summaryContent += failedResults.map(r => `- ${r.error || "Unknown error"}`).join("\n"); - summaryContent += "\n\n"; - } + if (failedResults.length > 0) { + summaryContent += `❌ Failed to create ${failedResults.length} agent session(s):\n\n`; + summaryContent += failedResults.map(r => `- ${r.error || "Unknown error"}`).join("\n"); + summaryContent += "\n\n"; + } - try { - await core.summary.addRaw(summaryContent).write(); - } catch (error) { - core.warning(`Failed to write agent session summary: ${getErrorMessage(error)}`); - } + try { + await core.summary.addRaw(summaryContent).write(); + } catch (error) { + core.warning(`Failed to write agent session summary: ${getErrorMessage(error)}`); + } + }; + + return handleMessage; } -module.exports = { main, getCreateAgentSessionNumber, getCreateAgentSessionUrl, writeCreateAgentSessionSummary }; +module.exports = { main }; diff --git a/actions/setup/js/create_agent_session.test.cjs b/actions/setup/js/create_agent_session.test.cjs index 7bb6bf45a09..9d5cf824201 100644 --- a/actions/setup/js/create_agent_session.test.cjs +++ b/actions/setup/js/create_agent_session.test.cjs @@ -253,8 +253,8 @@ describe("create_agent_session.cjs", () => { }); }); - describe("module-level getters", () => { - it("getCreateAgentSessionNumber() returns first successful session id", async () => { + describe("per-handler accessors", () => { + it("getSessionNumber() returns first successful session id", async () => { mockGithub.request.mockResolvedValue({ data: { id: "uuid-task-42", @@ -266,10 +266,10 @@ describe("create_agent_session.cjs", () => { const handler = await createAgentSessionModule.main({ base: "main" }); await handler({ type: "create_agent_session", body: "Task 1" }); - expect(createAgentSessionModule.getCreateAgentSessionNumber()).toBe("uuid-task-42"); + expect(handler.getSessionNumber()).toBe("uuid-task-42"); }); - it("getCreateAgentSessionUrl() returns first successful session URL", async () => { + it("getSessionUrl() returns first successful session URL", async () => { const expectedUrl = "https://github.com/test-owner/test-repo/copilot/tasks/uuid-task-42"; mockGithub.request.mockResolvedValue({ data: { @@ -282,21 +282,21 @@ describe("create_agent_session.cjs", () => { const handler = await createAgentSessionModule.main({ base: "main" }); await handler({ type: "create_agent_session", body: "Task 1" }); - expect(createAgentSessionModule.getCreateAgentSessionUrl()).toBe(expectedUrl); + expect(handler.getSessionUrl()).toBe(expectedUrl); }); - it("getCreateAgentSessionNumber() returns empty string when no sessions created", async () => { - await createAgentSessionModule.main({ base: "main" }); - expect(createAgentSessionModule.getCreateAgentSessionNumber()).toBe(""); + it("getSessionNumber() returns empty string when no sessions created", async () => { + const handler = await createAgentSessionModule.main({ base: "main" }); + expect(handler.getSessionNumber()).toBe(""); }); - it("getCreateAgentSessionUrl() returns empty string when no sessions created", async () => { - await createAgentSessionModule.main({ base: "main" }); - expect(createAgentSessionModule.getCreateAgentSessionUrl()).toBe(""); + it("getSessionUrl() returns empty string when no sessions created", async () => { + const handler = await createAgentSessionModule.main({ base: "main" }); + expect(handler.getSessionUrl()).toBe(""); }); }); - describe("writeCreateAgentSessionSummary()", () => { + describe("writeSummary()", () => { it("should write summary with successful sessions", async () => { mockGithub.request.mockResolvedValue({ data: { @@ -308,15 +308,15 @@ describe("create_agent_session.cjs", () => { const handler = await createAgentSessionModule.main({ base: "main" }); await handler({ type: "create_agent_session", body: "Task 1" }); - await createAgentSessionModule.writeCreateAgentSessionSummary(); + await handler.writeSummary(); expect(mockCore.summary.addRaw).toHaveBeenCalledWith(expect.stringContaining("Agent Sessions")); expect(mockCore.summary.addRaw).toHaveBeenCalledWith(expect.stringContaining("uuid-task-42")); }); it("should not write summary when no results", async () => { - await createAgentSessionModule.main({ base: "main" }); - await createAgentSessionModule.writeCreateAgentSessionSummary(); + const handler = await createAgentSessionModule.main({ base: "main" }); + await handler.writeSummary(); expect(mockCore.summary.addRaw).not.toHaveBeenCalled(); }); @@ -326,12 +326,51 @@ describe("create_agent_session.cjs", () => { const handler = await createAgentSessionModule.main({ base: "main" }); await handler({ type: "create_agent_session", body: "Task 1" }); - await createAgentSessionModule.writeCreateAgentSessionSummary(); + await handler.writeSummary(); expect(mockCore.summary.addRaw).toHaveBeenCalledWith(expect.stringContaining("❌ Failed")); }); }); + describe("concurrency safety - independent main() invocations", () => { + it("should not cross-contaminate results between two concurrently-processed handler instances", async () => { + mockGithub.request.mockResolvedValueOnce({ + data: { + id: "session-A", + html_url: "https://github.com/test-owner/test-repo/copilot/tasks/session-A", + state: "queued", + }, + }); + + const mockTokenClient = { request: vi.fn() }; + mockTokenClient.request.mockResolvedValueOnce({ + data: { + id: "session-B", + html_url: "https://github.com/test-owner/test-repo/copilot/tasks/session-B", + state: "queued", + }, + }); + mockGetOctokit.mockReturnValueOnce(mockTokenClient); + + // Two independent handler instances created from separate main() invocations. + const handlerA = await createAgentSessionModule.main({ base: "main" }); + const handlerB = await createAgentSessionModule.main({ base: "main", "github-token": "handler-b-token" }); + + // Interleave processing to simulate concurrent/parallel dispatch. + const resultA = await handlerA({ type: "create_agent_session", body: "Task A" }); + const resultB = await handlerB({ type: "create_agent_session", body: "Task B" }); + + expect(resultA.id).toBe("session-A"); + expect(resultB.id).toBe("session-B"); + + // Each handler's accessors must only reflect its own results. + expect(handlerA.getSessionNumber()).toBe("session-A"); + expect(handlerB.getSessionNumber()).toBe("session-B"); + expect(handlerA.getSessionUrl()).toContain("session-A"); + expect(handlerB.getSessionUrl()).toContain("session-B"); + }); + }); + describe("cross-repository allowlist validation", () => { it("should allow target repository in allowlist", async () => { process.env.GH_AW_ALLOWED_REPOS = "allowed-owner/allowed-repo"; diff --git a/actions/setup/js/safe_output_handler_manager.cjs b/actions/setup/js/safe_output_handler_manager.cjs index 6335ecd15fb..6caa6598277 100644 --- a/actions/setup/js/safe_output_handler_manager.cjs +++ b/actions/setup/js/safe_output_handler_manager.cjs @@ -18,7 +18,6 @@ const { generateMissingInfoSections } = require("./missing_info_formatter.cjs"); const { setCollectedMissings } = require("./missing_messages_helper.cjs"); const { writeSafeOutputSummaries } = require("./safe_output_summary.cjs"); const { getAssignToAgentAssigned, getAssignToAgentErrors, getAssignToAgentErrorCount, writeAssignToAgentSummary } = require("./assign_to_agent.cjs"); -const { getCreateAgentSessionNumber, getCreateAgentSessionUrl, writeCreateAgentSessionSummary } = require("./create_agent_session.cjs"); const { createPrReviewBufferRegistry } = require("./pr_review_buffer.cjs"); const { sanitizeContent } = require("./sanitize_content.cjs"); const { resolveAllowedMentionsFromPayload } = require("./resolve_mentions_from_payload.cjs"); @@ -1746,12 +1745,14 @@ async function main() { // Export create_agent_session outputs when the handler was loaded if (messageHandlers.has("create_agent_session")) { - const sessionNumber = getCreateAgentSessionNumber(); - const sessionUrl = getCreateAgentSessionUrl(); + /** @type {any} */ + const createAgentSessionHandler = messageHandlers.get("create_agent_session"); + const sessionNumber = createAgentSessionHandler.getSessionNumber(); + const sessionUrl = createAgentSessionHandler.getSessionUrl(); core.setOutput("session_number", sessionNumber); core.setOutput("session_url", sessionUrl); core.info(`Exported create_agent_session outputs (session_number=${sessionNumber})`); - await writeCreateAgentSessionSummary(); + await createAgentSessionHandler.writeSummary(); } // Export create_discussion errors for conclusion job