From cb38db183dfdde84c23e30950304b8a0b6bce890 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Sat, 15 Aug 2026 20:11:46 +0000 Subject: [PATCH 1/2] Fix centralized PR review comments Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> --- actions/setup/js/create_pr_review_comment.cjs | 30 ++++++++------ .../js/create_pr_review_comment.test.cjs | 40 +++++++++++++++++++ 2 files changed, 57 insertions(+), 13 deletions(-) diff --git a/actions/setup/js/create_pr_review_comment.cjs b/actions/setup/js/create_pr_review_comment.cjs index 41915f959e0..0983e32cb33 100644 --- a/actions/setup/js/create_pr_review_comment.cjs +++ b/actions/setup/js/create_pr_review_comment.cjs @@ -12,6 +12,7 @@ const { createAuthenticatedGitHubClient } = require("./handler_auth.cjs"); const { buildWorkflowRunUrl } = require("./workflow_metadata_helpers.cjs"); const { isTemplatableTrue, isStagedMode, logStagedPreviewInfo, checkRequiredFilter } = require("./safe_output_helpers.cjs"); const { resolveAllowedMentionsFromPayload } = require("./resolve_mentions_from_payload.cjs"); +const { resolveInvocationContext } = require("./invocation_context_helpers.cjs"); /** @type {string} Safe output type handled by this module */ const HANDLER_TYPE = "create_pull_request_review_comment"; @@ -38,6 +39,9 @@ async function main(config = {}) { const legacyBuffer = registry ? null : config._prReviewBuffer || null; const { defaultTargetRepo, allowedRepos } = resolveTargetRepoConfig(config); const githubClient = await createAuthenticatedGitHubClient(config); + const invocationContext = resolveInvocationContext(context); + const effectiveEventName = invocationContext.eventName || context.eventName; + const effectivePayload = invocationContext.eventPayload || context.payload; const requiredLabels = Array.isArray(config.required_labels) ? config.required_labels : []; const requiredTitlePrefix = config.required_title_prefix || ""; if (requiredLabels.length > 0) core.info(`Required labels (all): ${requiredLabels.join(", ")}`); @@ -85,9 +89,9 @@ async function main(config = {}) { } // Extract triggering context for footer generation - const triggeringIssueNumber = context.payload?.issue?.number && !context.payload?.issue?.pull_request ? context.payload.issue.number : undefined; - const triggeringPRNumber = context.payload?.pull_request?.number || (context.payload?.issue?.pull_request ? context.payload.issue.number : undefined); - const triggeringDiscussionNumber = context.payload?.discussion?.number; + const triggeringIssueNumber = effectivePayload?.issue?.number && !effectivePayload?.issue?.pull_request ? effectivePayload.issue.number : undefined; + const triggeringPRNumber = effectivePayload?.pull_request?.number || (effectivePayload?.issue?.pull_request ? effectivePayload.issue.number : undefined); + const triggeringDiscussionNumber = effectivePayload?.discussion?.number; const workflowName = process.env.GH_AW_WORKFLOW_NAME || "Workflow"; const workflowSource = process.env.GH_AW_WORKFLOW_SOURCE || ""; @@ -154,11 +158,11 @@ async function main(config = {}) { // Check if we're in a pull request context, or an issue comment context on a PR const isPRContext = - context.eventName === "pull_request" || - context.eventName === "pull_request_target" || - context.eventName === "pull_request_review" || - context.eventName === "pull_request_review_comment" || - (context.eventName === "issue_comment" && context.payload.issue && context.payload.issue.pull_request); + effectiveEventName === "pull_request" || + effectiveEventName === "pull_request_target" || + effectiveEventName === "pull_request_review" || + effectiveEventName === "pull_request_review_comment" || + (effectiveEventName === "issue_comment" && effectivePayload.issue && effectivePayload.issue.pull_request); // Validate context based on target configuration if (commentTarget === "triggering" && !isPRContext) { @@ -229,11 +233,11 @@ async function main(config = {}) { } } else { // Default behavior: use triggering PR - if (context.payload.pull_request) { - pullRequestNumber = context.payload.pull_request.number; - pullRequest = context.payload.pull_request; - } else if (context.payload.issue && context.payload.issue.pull_request) { - pullRequestNumber = context.payload.issue.number; + if (effectivePayload.pull_request) { + pullRequestNumber = effectivePayload.pull_request.number; + pullRequest = effectivePayload.pull_request; + } else if (effectivePayload.issue && effectivePayload.issue.pull_request) { + pullRequestNumber = effectivePayload.issue.number; } else { core.warning("Pull request context detected but no pull request found in payload"); return { diff --git a/actions/setup/js/create_pr_review_comment.test.cjs b/actions/setup/js/create_pr_review_comment.test.cjs index 694cba41c66..cd1eb4ad7a5 100644 --- a/actions/setup/js/create_pr_review_comment.test.cjs +++ b/actions/setup/js/create_pr_review_comment.test.cjs @@ -463,6 +463,46 @@ describe("create_pr_review_comment.cjs", () => { expect(buffer.getBufferedCount()).toBe(1); }); + it("should use the original PR context for centralized slash-command dispatches", async () => { + global.context = { + eventName: "workflow_dispatch", + runId: 12345, + repo: { owner: "testowner", repo: "testrepo" }, + payload: { + inputs: { + event_name: "issue_comment", + event_payload: JSON.stringify({ + issue: { number: 456, pull_request: {} }, + repository: mockContext.payload.repository, + }), + }, + }, + }; + mockGithub.rest.pulls.get.mockResolvedValue({ + data: { number: 456, head: { sha: "dispatch123abc" } }, + }); + const handler = await createHandler({ target: "triggering" }); + const message = { + type: "create_pull_request_review_comment", + pull_request_number: 456, + path: "src/main.js", + line: 5, + body: "Review comment from centralized slash command", + }; + + const result = await handler(message, {}); + + expect(result.success).toBe(true); + expect(result.buffered).toBe(true); + expect(result.pull_request_number).toBe(456); + expect(mockGithub.rest.pulls.get).toHaveBeenCalledWith({ + owner: "testowner", + repo: "testrepo", + pull_number: 456, + }); + expect(buffer.getBufferedCount()).toBe(1); + }); + it("should reject comments targeting a different PR than the first comment", async () => { // First comment sets context to PR #123 const handler = await createHandler(); From ec00158ddf32a318600f43d3767d2482a44a477f Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Sat, 15 Aug 2026 22:44:27 +0000 Subject: [PATCH 2/2] Handle invocation context fallback for PR review comments Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> --- actions/setup/js/create_pr_review_comment.cjs | 14 ++++- .../js/create_pr_review_comment.test.cjs | 53 +++++++++++++++++++ .../setup/js/invocation_context_helpers.cjs | 2 +- 3 files changed, 67 insertions(+), 2 deletions(-) diff --git a/actions/setup/js/create_pr_review_comment.cjs b/actions/setup/js/create_pr_review_comment.cjs index 0983e32cb33..89aa3b25022 100644 --- a/actions/setup/js/create_pr_review_comment.cjs +++ b/actions/setup/js/create_pr_review_comment.cjs @@ -13,6 +13,7 @@ const { buildWorkflowRunUrl } = require("./workflow_metadata_helpers.cjs"); const { isTemplatableTrue, isStagedMode, logStagedPreviewInfo, checkRequiredFilter } = require("./safe_output_helpers.cjs"); const { resolveAllowedMentionsFromPayload } = require("./resolve_mentions_from_payload.cjs"); const { resolveInvocationContext } = require("./invocation_context_helpers.cjs"); +const { ERR_VALIDATION } = require("./error_codes.cjs"); /** @type {string} Safe output type handled by this module */ const HANDLER_TYPE = "create_pull_request_review_comment"; @@ -39,7 +40,18 @@ async function main(config = {}) { const legacyBuffer = registry ? null : config._prReviewBuffer || null; const { defaultTargetRepo, allowedRepos } = resolveTargetRepoConfig(config); const githubClient = await createAuthenticatedGitHubClient(config); - const invocationContext = resolveInvocationContext(context); + let invocationContext = {}; + try { + invocationContext = resolveInvocationContext(context); + } catch (error) { + const typedError = /** @type {Error & { code?: string }} */ (error); + const isValidationError = typedError.code === ERR_VALIDATION; + if (isValidationError) { + throw error; + } + const errorMessage = getErrorMessage(error); + core.warning(`create_pull_request_review_comment: failed to resolve invocation context, using raw context: ${errorMessage}`); + } const effectiveEventName = invocationContext.eventName || context.eventName; const effectivePayload = invocationContext.eventPayload || context.payload; const requiredLabels = Array.isArray(config.required_labels) ? config.required_labels : []; diff --git a/actions/setup/js/create_pr_review_comment.test.cjs b/actions/setup/js/create_pr_review_comment.test.cjs index cd1eb4ad7a5..7ef2d0a7382 100644 --- a/actions/setup/js/create_pr_review_comment.test.cjs +++ b/actions/setup/js/create_pr_review_comment.test.cjs @@ -503,6 +503,59 @@ describe("create_pr_review_comment.cjs", () => { expect(buffer.getBufferedCount()).toBe(1); }); + it("falls back to raw context when resolveInvocationContext returns empty object", async () => { + const invocationHelpersPath = path.join(__dirname, "invocation_context_helpers.cjs"); + const invocationHelpers = require(invocationHelpersPath); + const resolveInvocationContextSpy = vi.spyOn(invocationHelpers, "resolveInvocationContext").mockReturnValue({}); + try { + const handler = await createHandler({ target: "triggering" }); + const message = { + type: "create_pull_request_review_comment", + path: "src/main.js", + line: 5, + body: "Review comment from fallback context", + }; + + const result = await handler(message, {}); + + expect(result.success).toBe(true); + expect(result.buffered).toBe(true); + expect(result.pull_request_number).toBe(123); + expect(buffer.getBufferedCount()).toBe(1); + expect(resolveInvocationContextSpy).toHaveBeenCalled(); + } finally { + resolveInvocationContextSpy.mockRestore(); + } + }); + + it("falls back to raw context when resolveInvocationContext throws a non-validation error", async () => { + const invocationHelpersPath = path.join(__dirname, "invocation_context_helpers.cjs"); + const invocationHelpers = require(invocationHelpersPath); + const resolveInvocationContextSpy = vi.spyOn(invocationHelpers, "resolveInvocationContext").mockImplementation(() => { + throw new Error("boom"); + }); + try { + const handler = await createHandler({ target: "triggering" }); + const message = { + type: "create_pull_request_review_comment", + path: "src/main.js", + line: 5, + body: "Review comment from thrown context resolver", + }; + + const result = await handler(message, {}); + + expect(result.success).toBe(true); + expect(result.buffered).toBe(true); + expect(result.pull_request_number).toBe(123); + expect(buffer.getBufferedCount()).toBe(1); + expect(resolveInvocationContextSpy).toHaveBeenCalled(); + expect(mockCore.warning).toHaveBeenCalledWith(expect.stringContaining("failed to resolve invocation context")); + } finally { + resolveInvocationContextSpy.mockRestore(); + } + }); + it("should reject comments targeting a different PR than the first comment", async () => { // First comment sets context to PR #123 const handler = await createHandler(); diff --git a/actions/setup/js/invocation_context_helpers.cjs b/actions/setup/js/invocation_context_helpers.cjs index 7c01161633f..8c4bb023343 100644 --- a/actions/setup/js/invocation_context_helpers.cjs +++ b/actions/setup/js/invocation_context_helpers.cjs @@ -190,7 +190,7 @@ function checkAllowedRepo(workflowRepo, targetRepo) { const validation = validateTargetRepo(targetRepoSlug, defaultRepo, allowedRepos); if (!validation.valid) { - throw new Error(`${ERR_VALIDATION}: ${validation.error}`); + throw Object.assign(new Error(`${ERR_VALIDATION}: ${validation.error}`), { code: ERR_VALIDATION }); } }