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..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. @@ -44,7 +45,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 +75,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 = buildWorkflowRunUrl(context, workflowRepo); // 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..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 @@ -29,7 +30,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 +42,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 = 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"); + }); +});