From 55eb57ac784502f396f6dc2b017512b77bc020b8 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Tue, 24 Mar 2026 05:01:49 +0000 Subject: [PATCH 1/3] Initial plan From c3bc1f1070e0febf69b3d01065a1aa26fe811518 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Tue, 24 Mar 2026 05:34:37 +0000 Subject: [PATCH 2/3] fix: prevent double-posting when discussion creation partially succeeds MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When the GitHub GraphQL API returns both `data` and `errors` in a `createDiscussion` mutation response (a GraphqlResponseError from @octokit/graphql), the discussion is already persisted in GitHub's database but the code previously fell through to the fallback-to-issue path — producing both a discussion and a fallback issue with the same title. Two guards are added in the outer catch block: 1. A `createdDiscussion` flag set after the mutation is confirmed valid, protecting against any post-creation operation error that unexpectedly escapes its inner try-catch. 2. A check of `error.data?.createDiscussion?.discussion` for partial success data attached to a GraphqlResponseError, covering the confirmed root cause. In either case the handler returns success with the discussion details and a warning, without creating a fallback issue. Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> Agent-Logs-Url: https://github.com/github/gh-aw/sessions/26ef7c45-6bba-4d0e-826e-7af38ed22870 --- actions/setup/js/create_discussion.cjs | 40 ++++ .../js/create_discussion_fallback.test.cjs | 197 ++++++++++++++++++ 2 files changed, 237 insertions(+) diff --git a/actions/setup/js/create_discussion.cjs b/actions/setup/js/create_discussion.cjs index dad7439f262..e51765693e4 100644 --- a/actions/setup/js/create_discussion.cjs +++ b/actions/setup/js/create_discussion.cjs @@ -578,6 +578,13 @@ async function main(config = {}) { }; } + // Track whether the discussion was successfully created to guard against + // double-posting: if the discussion is created but a subsequent operation + // (e.g., close-older-discussions search) unexpectedly escapes its own + // try-catch and reaches the outer catch, we must NOT fall back to creating + // an issue — the discussion already exists. + let createdDiscussion = null; + try { const createDiscussionMutation = ` mutation($repositoryId: ID!, $categoryId: ID!, $title: String!, $body: String!) { @@ -614,6 +621,9 @@ async function main(config = {}) { }; } + // Mark the discussion as created so the outer catch won't trigger a + // fallback issue even if a post-creation operation fails unexpectedly. + createdDiscussion = discussion; core.info(`Created discussion ${qualifiedItemRepo}#${discussion.number}: ${discussion.url}`); // Close older discussions if enabled @@ -670,6 +680,36 @@ async function main(config = {}) { } catch (error) { const errorMessage = getErrorMessage(error); + // Guard against double-posting: detect cases where the discussion was + // already persisted even though an error was thrown. + // + // Two scenarios can cause this: + // + // 1. A post-creation operation (close-older-discussions, label application) + // unexpectedly escaped its inner try-catch and reached this outer catch + // after `createdDiscussion` was set. + // + // 2. @octokit/graphql threw a GraphqlResponseError that contains BOTH the + // created discussion (partial success) AND errors — for example, when + // GitHub returns `{"data": {"createDiscussion": {...}}, "errors": [...]}`. + // In that case `createdDiscussion` is still null but the discussion was + // already persisted in GitHub's database. + // + // In either case we must NOT fall back to creating an issue, as that would + // result in both a discussion and a fallback issue existing at the same time. + /** @type {{id: string, number: number, title: string, url: string} | null | undefined} */ + const partialDiscussion = /** @type {any} */ error?.data?.createDiscussion?.discussion; + const resolvedDiscussion = createdDiscussion || partialDiscussion; + if (resolvedDiscussion) { + core.warning(`Discussion ${qualifiedItemRepo}#${resolvedDiscussion.number} was created but a post-creation operation failed: ${errorMessage}`); + return { + success: true, + repo: qualifiedItemRepo, + number: resolvedDiscussion.number, + url: resolvedDiscussion.url, + }; + } + // Check if this is a permissions error and fallback is enabled if (fallbackToIssue && createIssueHandler && isPermissionsError(errorMessage)) { core.warning(`Discussion creation failed due to permissions: ${errorMessage}`); diff --git a/actions/setup/js/create_discussion_fallback.test.cjs b/actions/setup/js/create_discussion_fallback.test.cjs index 7623550fe2b..39f5c24b1e0 100644 --- a/actions/setup/js/create_discussion_fallback.test.cjs +++ b/actions/setup/js/create_discussion_fallback.test.cjs @@ -248,3 +248,200 @@ describe("create_discussion fallback with close_older_discussions", () => { ); }); }); + +describe("create_discussion double-posting prevention", () => { + let mockGithub; + let mockCore; + let mockContext; + let mockExec; + let originalEnv; + + beforeEach(() => { + originalEnv = { ...process.env }; + + resetIssuesToAssignCopilot(); + + // Base mock — each test overrides graphql as needed. + mockGithub = { + rest: { + issues: { + create: vi.fn().mockResolvedValue({ + data: { + number: 789, + html_url: "https://github.com/owner/repo/issues/789", + title: "Fallback Issue (should NOT be created)", + }, + }), + }, + search: { + issuesAndPullRequests: vi.fn().mockResolvedValue({ + data: { total_count: 0, items: [] }, + }), + }, + }, + graphql: vi.fn().mockRejectedValue(new Error("graphql not configured for this test")), + }; + + mockCore = { + info: vi.fn(), + warning: vi.fn(), + error: vi.fn(), + setOutput: vi.fn(), + }; + + mockContext = { + repo: { owner: "test-owner", repo: "test-repo" }, + runId: 12345, + payload: { + repository: { + html_url: "https://github.com/test-owner/test-repo", + }, + }, + }; + + mockExec = { exec: vi.fn().mockResolvedValue(0) }; + + global.github = mockGithub; + global.core = mockCore; + global.context = mockContext; + global.exec = mockExec; + + process.env.GH_AW_WORKFLOW_NAME = "Daily Chronicle"; + process.env.GH_AW_WORKFLOW_ID = "daily-chronicle"; + process.env.GH_AW_WORKFLOW_SOURCE_URL = "https://github.com/owner/repo/blob/main/workflow.md"; + process.env.GITHUB_SERVER_URL = "https://github.com"; + }); + + afterEach(() => { + for (const key of Object.keys(process.env)) { + if (!(key in originalEnv)) { + delete process.env[key]; + } + } + Object.assign(process.env, originalEnv); + vi.clearAllMocks(); + }); + + it("should NOT create a fallback issue when the discussion was already created", async () => { + // This is the double-posting regression test. The createDiscussion mutation + // returns BOTH discussion data AND errors (a partial-success GraphqlResponseError + // from @octokit/graphql). The handler must return success for the created + // discussion and must NOT fall back to creating an issue. + mockGithub.graphql = vi.fn().mockImplementation(query => { + if (query.includes("discussionCategories")) { + return Promise.resolve({ + repository: { + id: "R_test123", + discussionCategories: { + nodes: [ + { + id: "DIC_announcements", + name: "Announcements", + slug: "announcements", + description: "Announcements", + }, + ], + }, + }, + }); + } + if (query.includes("createDiscussion")) { + // Simulate @octokit/graphql GraphqlResponseError: discussion was + // persisted but the API also returned an error in the response. + const err = new Error("Request failed due to following response errors:\n - Resource not accessible by integration"); + // Attach partial data the way @octokit/graphql does: + // err.data = response.data (the full response body's "data" field) + /** @type {{ createDiscussion: { discussion: { id: string, number: number, title: string, url: string } } }} */ /** @type {any} */ err.data = { + createDiscussion: { + discussion: { + id: "D_test456", + number: 42, + title: "📰 Daily Report", + url: "https://github.com/test-owner/test-repo/discussions/42", + }, + }, + }; + return Promise.reject(err); + } + return Promise.reject(new Error("Unexpected GraphQL query")); + }); + + const handler = await createDiscussionMain({ + max: 1, + fallback_to_issue: true, + close_older_discussions: true, + category: "announcements", + title_prefix: "📰 ", + }); + + const result = await handler( + { + title: "Daily Report", + body: "Today's report content.", + }, + {} + ); + + // The discussion was created — handler must succeed + expect(result.success).toBe(true); + expect(result.number).toBe(42); + expect(result.url).toBe("https://github.com/test-owner/test-repo/discussions/42"); + + // Must NOT have fallen back to an issue + expect(result.fallback).toBeUndefined(); + expect(mockGithub.rest.issues.create).not.toHaveBeenCalled(); + + // A warning about the post-creation failure should have been emitted + expect(mockCore.warning).toHaveBeenCalledWith(expect.stringContaining("post-creation operation failed")); + }); + + it("should still fall back to an issue when discussion creation itself fails", async () => { + // Override graphql so that createDiscussion also fails with a permissions error. + mockGithub.graphql = vi.fn().mockImplementation(query => { + if (query.includes("discussionCategories")) { + return Promise.resolve({ + repository: { + id: "R_test123", + discussionCategories: { + nodes: [ + { + id: "DIC_announcements", + name: "Announcements", + slug: "announcements", + description: "Announcements", + }, + ], + }, + }, + }); + } + // createDiscussion mutation fails — discussion is NOT created + throw new Error("Resource not accessible by integration"); + }); + + const handler = await createDiscussionMain({ + max: 1, + fallback_to_issue: true, + close_older_discussions: true, + category: "announcements", + title_prefix: "📰 ", + }); + + const result = await handler( + { + title: "Daily Report", + body: "Today's report content.", + }, + {} + ); + + // Fallback to issue should still work when the mutation itself failed + expect(result.success).toBe(true); + expect(result.fallback).toBe("issue"); + expect(mockGithub.rest.issues.create).toHaveBeenCalled(); + + // The fallback note must be present in the issue body + const createCallArgs = mockGithub.rest.issues.create.mock.calls[0][0]; + expect(createCallArgs.body).toContain("This was intended to be a discussion"); + }); +}); From 9e9e7bfdb7d99a5e5b14e6d668586cf3dfb276a5 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Tue, 24 Mar 2026 06:03:10 +0000 Subject: [PATCH 3/3] fix: add prettier-ignore to preserve JSDoc cast parens for TypeScript MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The @type {any} JSDoc cast requires parentheses: `/** @type {any} */ (expr)`. Without them TypeScript does not treat it as a cast and reports a type error. Prettier was stripping the parens — add `// prettier-ignore` (same pattern used in assign_agent_helpers.cjs) to preserve them so `tsc --noEmit` passes. Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> Agent-Logs-Url: https://github.com/github/gh-aw/sessions/6b0f8428-4bb0-4473-9541-b9cf98602b4d --- actions/setup/js/create_discussion.cjs | 4 +++- actions/setup/js/create_discussion_fallback.test.cjs | 4 ++-- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/actions/setup/js/create_discussion.cjs b/actions/setup/js/create_discussion.cjs index e51765693e4..3f43cf4de2c 100644 --- a/actions/setup/js/create_discussion.cjs +++ b/actions/setup/js/create_discussion.cjs @@ -697,8 +697,10 @@ async function main(config = {}) { // // In either case we must NOT fall back to creating an issue, as that would // result in both a discussion and a fallback issue existing at the same time. + // prettier-ignore + const errorAny = /** @type {any} */ (error); /** @type {{id: string, number: number, title: string, url: string} | null | undefined} */ - const partialDiscussion = /** @type {any} */ error?.data?.createDiscussion?.discussion; + const partialDiscussion = errorAny?.data?.createDiscussion?.discussion; const resolvedDiscussion = createdDiscussion || partialDiscussion; if (resolvedDiscussion) { core.warning(`Discussion ${qualifiedItemRepo}#${resolvedDiscussion.number} was created but a post-creation operation failed: ${errorMessage}`); diff --git a/actions/setup/js/create_discussion_fallback.test.cjs b/actions/setup/js/create_discussion_fallback.test.cjs index 39f5c24b1e0..81cb74af8da 100644 --- a/actions/setup/js/create_discussion_fallback.test.cjs +++ b/actions/setup/js/create_discussion_fallback.test.cjs @@ -348,10 +348,10 @@ describe("create_discussion double-posting prevention", () => { if (query.includes("createDiscussion")) { // Simulate @octokit/graphql GraphqlResponseError: discussion was // persisted but the API also returned an error in the response. - const err = new Error("Request failed due to following response errors:\n - Resource not accessible by integration"); + const err = /** @type {any} */ new Error("Request failed due to following response errors:\n - Resource not accessible by integration"); // Attach partial data the way @octokit/graphql does: // err.data = response.data (the full response body's "data" field) - /** @type {{ createDiscussion: { discussion: { id: string, number: number, title: string, url: string } } }} */ /** @type {any} */ err.data = { + err.data = { createDiscussion: { discussion: { id: "D_test456",