From 02adac04d4aeac12f1ba29192b9424b96867cbfc Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Tue, 3 Mar 2026 00:09:45 +0000 Subject: [PATCH 1/4] Initial plan From f2e0c276a22d8ed96383e1da8364a3813ddc2ede Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Tue, 3 Mar 2026 00:26:23 +0000 Subject: [PATCH 2/4] Initial plan Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> --- actions/setup/js/cross_repo_test.test.cjs | 141 ++++++++++++++++++++++ 1 file changed, 141 insertions(+) create mode 100644 actions/setup/js/cross_repo_test.test.cjs diff --git a/actions/setup/js/cross_repo_test.test.cjs b/actions/setup/js/cross_repo_test.test.cjs new file mode 100644 index 00000000000..cb19c8230dc --- /dev/null +++ b/actions/setup/js/cross_repo_test.test.cjs @@ -0,0 +1,141 @@ +import { describe, it, expect, beforeEach, vi } from "vitest"; + +const mockCore = { + debug: vi.fn(), + info: vi.fn(), + notice: vi.fn(), + warning: vi.fn(), + error: vi.fn(), + setFailed: vi.fn(), + setOutput: vi.fn(), +}; + +const mockGithub = { + rest: { + issues: { + get: vi.fn(), + update: vi.fn(), + }, + }, +}; + +const mockContext = { + eventName: "issues", + repo: { + owner: "current-owner", + repo: "current-repo", + }, + serverUrl: "https://github.com", + runId: 12345, + payload: { + issue: { + number: 100, + }, + }, +}; + +global.core = mockCore; +global.github = mockGithub; +global.context = mockContext; + +describe("update_issue cross-repo + operation integration", () => { + beforeEach(async () => { + vi.clearAllMocks(); + vi.resetModules(); + process.env.GH_AW_WORKFLOW_NAME = "Test Workflow"; + process.env.GH_AW_WORKFLOW_ID = "test-workflow"; + }); + + it("should route API calls to the correct cross-repo target", async () => { + let capturedGetOwner, capturedGetRepo; + let capturedUpdateOwner, capturedUpdateRepo; + let capturedBody; + + mockGithub.rest.issues.get.mockImplementation(async ({ owner, repo, issue_number }) => { + capturedGetOwner = owner; + capturedGetRepo = repo; + return { + data: { + number: issue_number, + title: "Test Issue", + body: "Original body", + html_url: `https://github.com/${owner}/${repo}/issues/${issue_number}`, + }, + }; + }); + + mockGithub.rest.issues.update.mockImplementation(async ({ owner, repo, issue_number, ...data }) => { + capturedUpdateOwner = owner; + capturedUpdateRepo = repo; + capturedBody = data.body; + return { + data: { + number: issue_number, + title: "Test Issue", + body: data.body || "Updated body", + html_url: `https://github.com/${owner}/${repo}/issues/${issue_number}`, + }, + }; + }); + + const { main } = await import("/home/runner/work/gh-aw/gh-aw/actions/setup/js/update_issue.cjs"); + const handler = await main({ + "target-repo": "cross-owner/cross-repo", + allowed_repos: ["cross-owner/cross-repo"], + target: "*", + }); + + const message = { + type: "update_issue", + issue_number: 42, + repo: "cross-owner/cross-repo", + operation: "replace", + body: "New body content", + }; + + const result = await handler(message, {}); + + console.log("Result:", JSON.stringify(result)); + + // API should go to cross-repo target + expect(capturedGetOwner).toBe("cross-owner"); + expect(capturedGetRepo).toBe("cross-repo"); + expect(capturedUpdateOwner).toBe("cross-owner"); + expect(capturedUpdateRepo).toBe("cross-repo"); + + // Body should contain the run URL - check if it's correct + console.log("Captured body:", capturedBody); + // The attribution URL should point to the CURRENT workflow's repo, not the cross-repo target + if (capturedBody.includes("cross-owner/cross-repo/actions")) { + console.error("BUG: Attribution URL points to cross-repo target instead of current workflow!"); + console.error("Body:", capturedBody); + } else if (capturedBody.includes("current-owner/current-repo/actions")) { + console.log("CORRECT: Attribution URL points to current workflow's repo"); + } else { + console.log("INFO: Body does not contain expected run URL pattern"); + } + + expect(result.success).toBe(true); + }); + + it("should apply 'replace' operation correctly", async () => { + mockGithub.rest.issues.get.mockResolvedValue({ + data: { number: 100, title: "Test", body: "Old body", html_url: "https://example.com/issues/100" }, + }); + mockGithub.rest.issues.update.mockResolvedValue({ + data: { number: 100, title: "Test", body: "New body", html_url: "https://example.com/issues/100" }, + }); + + const { main } = await import("/home/runner/work/gh-aw/gh-aw/actions/setup/js/update_issue.cjs"); + const handler = await main({ target: "*" }); + + const result = await handler({ issue_number: 100, operation: "replace", body: "New body" }, {}); + + expect(result.success).toBe(true); + const updateCall = mockGithub.rest.issues.update.mock.calls[0][0]; + console.log("Update called with body:", updateCall.body); + // Replace should NOT include old body + expect(updateCall.body).not.toContain("Old body"); + expect(updateCall.body).toContain("New body"); + }); +}); From e6acfb04336db9d21291eb96c9bb680e7c619c37 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Tue, 3 Mar 2026 00:32:50 +0000 Subject: [PATCH 3/4] Fix cross-repo attribution URL and improve update logging in safe-outputs handlers - Inject _workflowRepo into updateData so executeIssueUpdate and executePRUpdate always reference the current workflow's repo in attribution URLs, not the cross-repo target - Improve update log to show 'body' field when _rawBody is present (was showing empty [] for body-only updates) - Include target repo in the Updating log message for easier diagnosability - Add integration tests for cross-repo routing, attribution URL correctness, and all four operation modes (append, replace, prepend, replace-island) - Add factory tests for _workflowRepo injection and body field logging Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> --- actions/setup/js/cross_repo_test.test.cjs | 141 ----------------- actions/setup/js/update_handler_factory.cjs | 11 +- .../setup/js/update_handler_factory.test.cjs | 46 ++++++ actions/setup/js/update_issue.cjs | 9 +- actions/setup/js/update_issue.test.cjs | 143 ++++++++++++++++++ actions/setup/js/update_pull_request.cjs | 9 +- 6 files changed, 210 insertions(+), 149 deletions(-) delete mode 100644 actions/setup/js/cross_repo_test.test.cjs diff --git a/actions/setup/js/cross_repo_test.test.cjs b/actions/setup/js/cross_repo_test.test.cjs deleted file mode 100644 index cb19c8230dc..00000000000 --- a/actions/setup/js/cross_repo_test.test.cjs +++ /dev/null @@ -1,141 +0,0 @@ -import { describe, it, expect, beforeEach, vi } from "vitest"; - -const mockCore = { - debug: vi.fn(), - info: vi.fn(), - notice: vi.fn(), - warning: vi.fn(), - error: vi.fn(), - setFailed: vi.fn(), - setOutput: vi.fn(), -}; - -const mockGithub = { - rest: { - issues: { - get: vi.fn(), - update: vi.fn(), - }, - }, -}; - -const mockContext = { - eventName: "issues", - repo: { - owner: "current-owner", - repo: "current-repo", - }, - serverUrl: "https://github.com", - runId: 12345, - payload: { - issue: { - number: 100, - }, - }, -}; - -global.core = mockCore; -global.github = mockGithub; -global.context = mockContext; - -describe("update_issue cross-repo + operation integration", () => { - beforeEach(async () => { - vi.clearAllMocks(); - vi.resetModules(); - process.env.GH_AW_WORKFLOW_NAME = "Test Workflow"; - process.env.GH_AW_WORKFLOW_ID = "test-workflow"; - }); - - it("should route API calls to the correct cross-repo target", async () => { - let capturedGetOwner, capturedGetRepo; - let capturedUpdateOwner, capturedUpdateRepo; - let capturedBody; - - mockGithub.rest.issues.get.mockImplementation(async ({ owner, repo, issue_number }) => { - capturedGetOwner = owner; - capturedGetRepo = repo; - return { - data: { - number: issue_number, - title: "Test Issue", - body: "Original body", - html_url: `https://github.com/${owner}/${repo}/issues/${issue_number}`, - }, - }; - }); - - mockGithub.rest.issues.update.mockImplementation(async ({ owner, repo, issue_number, ...data }) => { - capturedUpdateOwner = owner; - capturedUpdateRepo = repo; - capturedBody = data.body; - return { - data: { - number: issue_number, - title: "Test Issue", - body: data.body || "Updated body", - html_url: `https://github.com/${owner}/${repo}/issues/${issue_number}`, - }, - }; - }); - - const { main } = await import("/home/runner/work/gh-aw/gh-aw/actions/setup/js/update_issue.cjs"); - const handler = await main({ - "target-repo": "cross-owner/cross-repo", - allowed_repos: ["cross-owner/cross-repo"], - target: "*", - }); - - const message = { - type: "update_issue", - issue_number: 42, - repo: "cross-owner/cross-repo", - operation: "replace", - body: "New body content", - }; - - const result = await handler(message, {}); - - console.log("Result:", JSON.stringify(result)); - - // API should go to cross-repo target - expect(capturedGetOwner).toBe("cross-owner"); - expect(capturedGetRepo).toBe("cross-repo"); - expect(capturedUpdateOwner).toBe("cross-owner"); - expect(capturedUpdateRepo).toBe("cross-repo"); - - // Body should contain the run URL - check if it's correct - console.log("Captured body:", capturedBody); - // The attribution URL should point to the CURRENT workflow's repo, not the cross-repo target - if (capturedBody.includes("cross-owner/cross-repo/actions")) { - console.error("BUG: Attribution URL points to cross-repo target instead of current workflow!"); - console.error("Body:", capturedBody); - } else if (capturedBody.includes("current-owner/current-repo/actions")) { - console.log("CORRECT: Attribution URL points to current workflow's repo"); - } else { - console.log("INFO: Body does not contain expected run URL pattern"); - } - - expect(result.success).toBe(true); - }); - - it("should apply 'replace' operation correctly", async () => { - mockGithub.rest.issues.get.mockResolvedValue({ - data: { number: 100, title: "Test", body: "Old body", html_url: "https://example.com/issues/100" }, - }); - mockGithub.rest.issues.update.mockResolvedValue({ - data: { number: 100, title: "Test", body: "New body", html_url: "https://example.com/issues/100" }, - }); - - const { main } = await import("/home/runner/work/gh-aw/gh-aw/actions/setup/js/update_issue.cjs"); - const handler = await main({ target: "*" }); - - const result = await handler({ issue_number: 100, operation: "replace", body: "New body" }, {}); - - expect(result.success).toBe(true); - const updateCall = mockGithub.rest.issues.update.mock.calls[0][0]; - console.log("Update called with body:", updateCall.body); - // Replace should NOT include old body - expect(updateCall.body).not.toContain("Old body"); - expect(updateCall.body).toContain("New body"); - }); -}); diff --git a/actions/setup/js/update_handler_factory.cjs b/actions/setup/js/update_handler_factory.cjs index c007af14d14..80e8a3ced33 100644 --- a/actions/setup/js/update_handler_factory.cjs +++ b/actions/setup/js/update_handler_factory.cjs @@ -202,6 +202,11 @@ function createUpdateHandlerFactory(handlerConfig) { const updateData = updateDataResult.data; + // Store the original workflow repo in updateData so that executeUpdate functions + // can build correct attribution URLs. effectiveContext.repo may be overridden to + // the cross-repo target, but the run URL must always reference the current workflow. + updateData._workflowRepo = context.repo; + // Validate that we have something to update // Note: Fields starting with "_" are internal (e.g., _rawBody, _operation) // and will be processed by executeUpdate. We should NOT skip if _rawBody exists. @@ -217,11 +222,13 @@ function createUpdateHandlerFactory(handlerConfig) { }; } - core.info(`Updating ${itemTypeName} #${itemNumber} with: ${JSON.stringify(updateFields)}`); + // Include "body" in logged fields when a body update is queued (stored as internal _rawBody) + const logFields = hasRawBody ? [...updateFields, "body"] : updateFields; + core.info(`Updating ${itemTypeName} #${itemNumber} in ${effectiveContext.repo.owner}/${effectiveContext.repo.repo} with: ${JSON.stringify(logFields)}`); // If in staged mode, preview the update without applying it if (isStaged) { - logStagedPreviewInfo(`Would update ${itemTypeName} #${itemNumber} with fields: ${JSON.stringify(updateFields)}`); + logStagedPreviewInfo(`Would update ${itemTypeName} #${itemNumber} with fields: ${JSON.stringify(logFields)}`); return { success: true, staged: true, diff --git a/actions/setup/js/update_handler_factory.test.cjs b/actions/setup/js/update_handler_factory.test.cjs index 10c72356329..7a2edfbfc71 100644 --- a/actions/setup/js/update_handler_factory.test.cjs +++ b/actions/setup/js/update_handler_factory.test.cjs @@ -589,5 +589,51 @@ describe("update_handler_factory.cjs", () => { // executeUpdate should not have been called expect(mockExecuteUpdate).not.toHaveBeenCalled(); }); + + it("should inject _workflowRepo into updateData before calling executeUpdate", async () => { + let capturedUpdateData = null; + const mockExecuteUpdate = vi.fn().mockImplementation(async (githubClient, context, num, data) => { + capturedUpdateData = data; + return { html_url: "https://example.com", title: "Updated" }; + }); + + const handlerFactory = factoryModule.createUpdateHandlerFactory({ + itemType: "update_test", + itemTypeName: "test item", + supportsPR: false, + resolveItemNumber: vi.fn().mockReturnValue({ success: true, number: 42 }), + buildUpdateData: vi.fn().mockReturnValue({ success: true, data: { title: "Test" } }), + executeUpdate: mockExecuteUpdate, + formatSuccessResult: vi.fn().mockReturnValue({ success: true }), + }); + + const handler = await handlerFactory({ "target-repo": "other-owner/side-repo", allowed_repos: ["other-owner/side-repo"] }); + await handler({ issue_number: 42, repo: "other-owner/side-repo" }); + + // _workflowRepo must reference the original context.repo (the current workflow) + expect(capturedUpdateData._workflowRepo).toBeDefined(); + expect(capturedUpdateData._workflowRepo.owner).toBe(mockContext.repo.owner); + expect(capturedUpdateData._workflowRepo.repo).toBe(mockContext.repo.repo); + }); + + it("should include body in log fields when _rawBody is present", async () => { + const mockExecuteUpdate = vi.fn().mockResolvedValue({ html_url: "https://example.com", title: "Updated" }); + + const handlerFactory = factoryModule.createUpdateHandlerFactory({ + itemType: "update_test", + itemTypeName: "test item", + supportsPR: false, + resolveItemNumber: vi.fn().mockReturnValue({ success: true, number: 42 }), + buildUpdateData: vi.fn().mockReturnValue({ success: true, data: { _rawBody: "new body content" } }), + executeUpdate: mockExecuteUpdate, + formatSuccessResult: vi.fn().mockReturnValue({ success: true }), + }); + + const handler = await handlerFactory({}); + await handler({ body: "new body content" }); + + // The log should mention "body" even though _rawBody starts with underscore + expect(mockCore.info).toHaveBeenCalledWith(expect.stringContaining('"body"')); + }); }); }); diff --git a/actions/setup/js/update_issue.cjs b/actions/setup/js/update_issue.cjs index 7a99890b347..e9ec0216a6f 100644 --- a/actions/setup/js/update_issue.cjs +++ b/actions/setup/js/update_issue.cjs @@ -44,7 +44,7 @@ async function executeIssueUpdate(github, context, issueNumber, updateData) { const titlePrefix = updateData._titlePrefix || ""; // Remove internal fields - const { _operation, _rawBody, _includeFooter, _titlePrefix, ...apiData } = updateData; + const { _operation, _rawBody, _includeFooter, _titlePrefix, _workflowRepo, ...apiData } = updateData; // Fetch current issue if needed (title prefix validation or body update) if (titlePrefix || rawBody !== undefined) { @@ -74,10 +74,13 @@ async function executeIssueUpdate(github, context, issueNumber, updateData) { const currentBody = currentIssue.body || ""; - // Get workflow run URL for AI attribution + // Get workflow run URL for AI attribution. + // Use the original workflow repo (_workflowRepo) rather than context.repo, because + // context may be effectiveContext with repo overridden to a cross-repo target. const workflowName = process.env.GH_AW_WORKFLOW_NAME || "GitHub Agentic Workflow"; const workflowId = process.env.GH_AW_WORKFLOW_ID || ""; - const runUrl = `${context.serverUrl}/${context.repo.owner}/${context.repo.repo}/actions/runs/${context.runId}`; + const workflowRepo = _workflowRepo || context.repo; + const runUrl = `${context.serverUrl}/${workflowRepo.owner}/${workflowRepo.repo}/actions/runs/${context.runId}`; // Use helper to update body (handles all operations including replace) apiData.body = updateBody({ diff --git a/actions/setup/js/update_issue.test.cjs b/actions/setup/js/update_issue.test.cjs index 6ebdca8735a..aa6b33bc98f 100644 --- a/actions/setup/js/update_issue.test.cjs +++ b/actions/setup/js/update_issue.test.cjs @@ -741,3 +741,146 @@ describe("update_issue.cjs - title_prefix configuration", () => { expect(mockGithub.rest.issues.update).not.toHaveBeenCalled(); }); }); + +describe("update_issue.cjs - cross-repo and operation integration", () => { + beforeEach(async () => { + vi.clearAllMocks(); + vi.resetModules(); + process.env.GH_AW_WORKFLOW_NAME = "Test Workflow"; + process.env.GH_AW_WORKFLOW_ID = "test-workflow"; + }); + + it("should route API calls to the cross-repo target when repo field is provided", async () => { + let capturedOwner, capturedRepo; + + mockGithub.rest.issues.get.mockImplementation(async ({ owner, repo, issue_number }) => ({ + data: { number: issue_number, title: "Cross-repo Issue", body: "Original", html_url: `https://github.com/${owner}/${repo}/issues/${issue_number}` }, + })); + mockGithub.rest.issues.update.mockImplementation(async ({ owner, repo, issue_number, ...data }) => { + capturedOwner = owner; + capturedRepo = repo; + return { data: { number: issue_number, title: "Cross-repo Issue", body: data.body, html_url: `https://github.com/${owner}/${repo}/issues/${issue_number}` } }; + }); + + const { main } = await import("./update_issue.cjs"); + const handler = await main({ + "target-repo": "other-owner/other-repo", + allowed_repos: ["other-owner/other-repo"], + target: "*", + }); + + const result = await handler({ issue_number: 42, repo: "other-owner/other-repo", body: "New content" }, {}); + + expect(result.success).toBe(true); + expect(capturedOwner).toBe("other-owner"); + expect(capturedRepo).toBe("other-repo"); + }); + + it("should use current workflow repo in attribution URL for cross-repo updates", async () => { + let capturedBody; + + mockGithub.rest.issues.get.mockResolvedValue({ + data: { number: 42, title: "Cross-repo Issue", body: "Original", html_url: "https://github.com/other-owner/other-repo/issues/42" }, + }); + mockGithub.rest.issues.update.mockImplementation(async ({ body }) => { + capturedBody = body; + return { data: { number: 42, title: "Cross-repo Issue", body, html_url: "https://github.com/other-owner/other-repo/issues/42" } }; + }); + + const { main } = await import("./update_issue.cjs"); + const handler = await main({ + "target-repo": "other-owner/other-repo", + allowed_repos: ["other-owner/other-repo"], + target: "*", + }); + + await handler({ issue_number: 42, repo: "other-owner/other-repo", operation: "replace", body: "New content" }, {}); + + // Attribution URL must reference the current workflow's repo, not the cross-repo target + expect(capturedBody).toContain("testowner/testrepo/actions/runs/12345"); + expect(capturedBody).not.toContain("other-owner/other-repo/actions/runs/12345"); + }); + + it("should apply 'append' operation: new content added after existing body", async () => { + let capturedBody; + + mockGithub.rest.issues.get.mockResolvedValue({ + data: { number: 100, title: "Test", body: "Existing body", html_url: "https://github.com/testowner/testrepo/issues/100" }, + }); + mockGithub.rest.issues.update.mockImplementation(async ({ body }) => { + capturedBody = body; + return { data: { number: 100, title: "Test", body, html_url: "https://github.com/testowner/testrepo/issues/100" } }; + }); + + const { main } = await import("./update_issue.cjs"); + const handler = await main({ target: "*" }); + await handler({ issue_number: 100, operation: "append", body: "Appended content" }, {}); + + expect(capturedBody).toContain("Existing body"); + expect(capturedBody).toContain("Appended content"); + expect(capturedBody.indexOf("Existing body")).toBeLessThan(capturedBody.indexOf("Appended content")); + }); + + it("should apply 'replace' operation: old content replaced by new content", async () => { + let capturedBody; + + mockGithub.rest.issues.get.mockResolvedValue({ + data: { number: 100, title: "Test", body: "Old body to replace", html_url: "https://github.com/testowner/testrepo/issues/100" }, + }); + mockGithub.rest.issues.update.mockImplementation(async ({ body }) => { + capturedBody = body; + return { data: { number: 100, title: "Test", body, html_url: "https://github.com/testowner/testrepo/issues/100" } }; + }); + + const { main } = await import("./update_issue.cjs"); + const handler = await main({ target: "*" }); + await handler({ issue_number: 100, operation: "replace", body: "Replacement content" }, {}); + + expect(capturedBody).not.toContain("Old body to replace"); + expect(capturedBody).toContain("Replacement content"); + }); + + it("should apply 'prepend' operation: new content placed before existing body", async () => { + let capturedBody; + + mockGithub.rest.issues.get.mockResolvedValue({ + data: { number: 100, title: "Test", body: "Existing body", html_url: "https://github.com/testowner/testrepo/issues/100" }, + }); + mockGithub.rest.issues.update.mockImplementation(async ({ body }) => { + capturedBody = body; + return { data: { number: 100, title: "Test", body, html_url: "https://github.com/testowner/testrepo/issues/100" } }; + }); + + const { main } = await import("./update_issue.cjs"); + const handler = await main({ target: "*" }); + await handler({ issue_number: 100, operation: "prepend", body: "Prepended content" }, {}); + + expect(capturedBody).toContain("Existing body"); + expect(capturedBody).toContain("Prepended content"); + expect(capturedBody.indexOf("Prepended content")).toBeLessThan(capturedBody.indexOf("Existing body")); + }); + + it("should apply 'replace-island' operation: only the island section is updated", async () => { + const existingBody = "Header\n\nOld island\n\nFooter"; + let capturedBody; + + mockGithub.rest.issues.get.mockResolvedValue({ + data: { number: 100, title: "Test", body: existingBody, html_url: "https://github.com/testowner/testrepo/issues/100" }, + }); + mockGithub.rest.issues.update.mockImplementation(async ({ body }) => { + capturedBody = body; + return { data: { number: 100, title: "Test", body, html_url: "https://github.com/testowner/testrepo/issues/100" } }; + }); + + const { main } = await import("./update_issue.cjs"); + const handler = await main({ target: "*" }); + await handler({ issue_number: 100, operation: "replace-island", body: "New island content" }, {}); + + expect(capturedBody).toContain("Header"); + expect(capturedBody).toContain("Footer"); + expect(capturedBody).toContain("New island content"); + expect(capturedBody).not.toContain("Old island"); + expect(capturedBody).toContain(""); + expect(capturedBody).toContain(""); + }); +}); diff --git a/actions/setup/js/update_pull_request.cjs b/actions/setup/js/update_pull_request.cjs index 3cde54dec0c..b5cc1e79b27 100644 --- a/actions/setup/js/update_pull_request.cjs +++ b/actions/setup/js/update_pull_request.cjs @@ -29,7 +29,7 @@ async function executePRUpdate(github, context, prNumber, updateData) { const includeFooter = updateData._includeFooter !== false; // Default to true // Remove internal fields - const { _operation, _rawBody, _includeFooter, ...apiData } = updateData; + const { _operation, _rawBody, _includeFooter, _workflowRepo, ...apiData } = updateData; // If we have a body, process it with the appropriate operation if (rawBody !== undefined) { @@ -41,10 +41,13 @@ async function executePRUpdate(github, context, prNumber, updateData) { }); const currentBody = currentPR.body || ""; - // Get workflow run URL for AI attribution + // Get workflow run URL for AI attribution. + // Use the original workflow repo (_workflowRepo) rather than context.repo, because + // context may be effectiveContext with repo overridden to a cross-repo target. const workflowName = process.env.GH_AW_WORKFLOW_NAME || "GitHub Agentic Workflow"; const workflowId = process.env.GH_AW_WORKFLOW_ID || ""; - const runUrl = `${context.serverUrl}/${context.repo.owner}/${context.repo.repo}/actions/runs/${context.runId}`; + const workflowRepo = _workflowRepo || context.repo; + const runUrl = `${context.serverUrl}/${workflowRepo.owner}/${workflowRepo.repo}/actions/runs/${context.runId}`; // Use helper to update body (handles all operations including replace) apiData.body = updateBody({ From c6a405a49aadd6c510b945f341b6c6b8e2720ac3 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Tue, 3 Mar 2026 01:06:30 +0000 Subject: [PATCH 4/4] Refactor workflow run URL construction into workflow_metadata_helpers Add buildWorkflowRunUrl(ctx, workflowRepo) to workflow_metadata_helpers.cjs and update update_issue.cjs and update_pull_request.cjs to use it instead of building the URL inline. This centralises the logic for cross-repo attribution URL generation and avoids duplication between the two handlers. Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> --- actions/setup/js/update_issue.cjs | 3 +- actions/setup/js/update_pull_request.cjs | 3 +- .../setup/js/workflow_metadata_helpers.cjs | 18 ++++++++++ .../js/workflow_metadata_helpers.test.cjs | 33 ++++++++++++++++++- 4 files changed, 54 insertions(+), 3 deletions(-) diff --git a/actions/setup/js/update_issue.cjs b/actions/setup/js/update_issue.cjs index e9ec0216a6f..f79cc6e1705 100644 --- a/actions/setup/js/update_issue.cjs +++ b/actions/setup/js/update_issue.cjs @@ -16,6 +16,7 @@ const { sanitizeTitle } = require("./sanitize_title.cjs"); const { tryEnforceArrayLimit } = require("./limit_enforcement_helpers.cjs"); const { ERR_VALIDATION } = require("./error_codes.cjs"); const { parseBoolTemplatable } = require("./templatable.cjs"); +const { buildWorkflowRunUrl } = require("./workflow_metadata_helpers.cjs"); /** * Maximum limits for issue update parameters to prevent resource exhaustion. @@ -80,7 +81,7 @@ async function executeIssueUpdate(github, context, issueNumber, updateData) { const workflowName = process.env.GH_AW_WORKFLOW_NAME || "GitHub Agentic Workflow"; const workflowId = process.env.GH_AW_WORKFLOW_ID || ""; const workflowRepo = _workflowRepo || context.repo; - const runUrl = `${context.serverUrl}/${workflowRepo.owner}/${workflowRepo.repo}/actions/runs/${context.runId}`; + const runUrl = buildWorkflowRunUrl(context, workflowRepo); // Use helper to update body (handles all operations including replace) apiData.body = updateBody({ diff --git a/actions/setup/js/update_pull_request.cjs b/actions/setup/js/update_pull_request.cjs index b5cc1e79b27..729f3111967 100644 --- a/actions/setup/js/update_pull_request.cjs +++ b/actions/setup/js/update_pull_request.cjs @@ -13,6 +13,7 @@ const { resolveTarget } = require("./safe_output_helpers.cjs"); const { createUpdateHandlerFactory, createStandardResolveNumber, createStandardFormatResult } = require("./update_handler_factory.cjs"); const { sanitizeTitle } = require("./sanitize_title.cjs"); const { parseBoolTemplatable } = require("./templatable.cjs"); +const { buildWorkflowRunUrl } = require("./workflow_metadata_helpers.cjs"); /** * Execute the pull request update API call @@ -47,7 +48,7 @@ async function executePRUpdate(github, context, prNumber, updateData) { const workflowName = process.env.GH_AW_WORKFLOW_NAME || "GitHub Agentic Workflow"; const workflowId = process.env.GH_AW_WORKFLOW_ID || ""; const workflowRepo = _workflowRepo || context.repo; - const runUrl = `${context.serverUrl}/${workflowRepo.owner}/${workflowRepo.repo}/actions/runs/${context.runId}`; + const runUrl = buildWorkflowRunUrl(context, workflowRepo); // Use helper to update body (handles all operations including replace) apiData.body = updateBody({ diff --git a/actions/setup/js/workflow_metadata_helpers.cjs b/actions/setup/js/workflow_metadata_helpers.cjs index e2664cda4c9..58491930dc0 100644 --- a/actions/setup/js/workflow_metadata_helpers.cjs +++ b/actions/setup/js/workflow_metadata_helpers.cjs @@ -25,6 +25,24 @@ function getWorkflowMetadata(owner, repo) { }; } +/** + * Build a workflow run URL from the provided context and workflow repository. + * + * This is the canonical helper for constructing attribution run URLs in safe-output + * handlers. It must always receive the **original** workflow repo (not an overridden + * cross-repo effectiveContext) so that footer links point back to the actual workflow + * run regardless of which repository the output action targets. + * + * @param {any} ctx - GitHub Actions context (provides serverUrl and runId) + * @param {{ owner: string, repo: string }} workflowRepo - The repository that owns the workflow run + * @returns {string} The full workflow run URL + */ +function buildWorkflowRunUrl(ctx, workflowRepo) { + const server = ctx.serverUrl || process.env.GITHUB_SERVER_URL || "https://github.com"; + return `${server}/${workflowRepo.owner}/${workflowRepo.repo}/actions/runs/${ctx.runId}`; +} + module.exports = { getWorkflowMetadata, + buildWorkflowRunUrl, }; diff --git a/actions/setup/js/workflow_metadata_helpers.test.cjs b/actions/setup/js/workflow_metadata_helpers.test.cjs index d9db2a8ac96..7c2af2955ed 100644 --- a/actions/setup/js/workflow_metadata_helpers.test.cjs +++ b/actions/setup/js/workflow_metadata_helpers.test.cjs @@ -1,6 +1,6 @@ // @ts-check -const { getWorkflowMetadata } = require("./workflow_metadata_helpers.cjs"); +const { getWorkflowMetadata, buildWorkflowRunUrl } = require("./workflow_metadata_helpers.cjs"); describe("getWorkflowMetadata", () => { let originalEnv; @@ -95,3 +95,34 @@ describe("getWorkflowMetadata", () => { expect(metadata.runUrl).toBe("https://github.com/test-owner/test-repo/actions/runs/0"); }); }); + +describe("buildWorkflowRunUrl", () => { + it("should build run URL from context.serverUrl and explicit workflowRepo", () => { + const ctx = { serverUrl: "https://github.com", runId: 42000 }; + const url = buildWorkflowRunUrl(ctx, { owner: "wf-owner", repo: "wf-repo" }); + expect(url).toBe("https://github.com/wf-owner/wf-repo/actions/runs/42000"); + }); + + it("should fall back to GITHUB_SERVER_URL when context.serverUrl is absent", () => { + const originalEnv = process.env.GITHUB_SERVER_URL; + process.env.GITHUB_SERVER_URL = "https://ghes.example.com"; + const ctx = { runId: 99 }; + const url = buildWorkflowRunUrl(ctx, { owner: "ent-owner", repo: "ent-repo" }); + expect(url).toBe("https://ghes.example.com/ent-owner/ent-repo/actions/runs/99"); + if (originalEnv === undefined) { + delete process.env.GITHUB_SERVER_URL; + } else { + process.env.GITHUB_SERVER_URL = originalEnv; + } + }); + + it("should use the workflowRepo, not a cross-repo target", () => { + // Simulates the cross-repo case: context.repo is the target but workflowRepo is the workflow owner + const ctx = { serverUrl: "https://github.com", runId: 7777, repo: { owner: "cross-owner", repo: "cross-repo" } }; + const workflowRepo = { owner: "wf-owner", repo: "wf-repo" }; + const url = buildWorkflowRunUrl(ctx, workflowRepo); + expect(url).toBe("https://github.com/wf-owner/wf-repo/actions/runs/7777"); + expect(url).not.toContain("cross-owner"); + expect(url).not.toContain("cross-repo"); + }); +});