Skip to content

Deduplicate safe-output body attribution handling #51648

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
pelikhan merged 3 commits into from
Aug 10, 2026
+78 −59
Merged

Deduplicate safe-output body attribution handling #51648

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
a22eb89
Initial plan
Copilot Aug 9, 2026
caab216
Deduplicate safe-output body attribution flow
Copilot Aug 9, 2026
5aa1acc
Merge branch 'main' into copilot/duplicate-code-fix-body-update
github-actions[bot] Aug 10, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
35 changes: 6 additions & 29 deletions actions/setup/js/update_issue.cjs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,13 +10,11 @@ const HANDLER_TYPE = "update_issue";

const { resolveTarget, checkRequiredFilter } = require("./safe_output_helpers.cjs");
const { createUpdateHandlerFactory, createStandardResolveNumber, createStandardFormatResult } = require("./update_handler_factory.cjs");
const { updateBody } = require("./update_pr_description_helpers.cjs");
const { buildUpdatedBody } = require("./update_pr_description_helpers.cjs");
const { buildCommonEntityUpdateData } = require("./update_entity_helpers.cjs");
const { loadTemporaryProjectMap, replaceTemporaryProjectReferences } = require("./temporary_id.cjs");
const { tryEnforceArrayLimit } = require("./limit_enforcement_helpers.cjs");
const { ERR_VALIDATION } = require("./error_codes.cjs");
const { buildWorkflowRunUrl } = require("./workflow_metadata_helpers.cjs");
const { generateHistoryUrl } = require("./generate_history_link.cjs");
const { fetchIssueState, mergeIssueState } = require("./safe_output_execution_metadata.cjs");
const { MAX_LABELS, MAX_ASSIGNEES } = require("./constants.cjs");
const { fetchAllRepoLabels } = require("./github_api_helpers.cjs");
Expand Down Expand Up @@ -82,35 +80,14 @@ async function executeIssueUpdate(github, context, issueNumber, updateData) {

const currentBody = currentIssue.body || "";

// 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 callerWorkflowId = process.env.GH_AW_CALLER_WORKFLOW_ID || "";
const workflowRepo = _workflowRepo || context.repo;
const runUrl = buildWorkflowRunUrl(context, workflowRepo);

const historyUrl =
generateHistoryUrl({
owner: context.repo.owner,
repo: context.repo.repo,
itemType: "issue",
workflowCallId: callerWorkflowId,
workflowId,
serverUrl: context.serverUrl,
}) || undefined;

// Use helper to update body (handles all operations including replace)
apiData.body = updateBody({
apiData.body = buildUpdatedBody({
context,
currentBody,
newContent: rawBody,
operation,
workflowName,
runUrl,
workflowId,
includeFooter, // Pass footer flag to helper
historyUrl,
includeFooter,
workflowRepo: _workflowRepo,
itemType: "issue",
});

core.info(`Will update body (length: ${apiData.body.length})`);
Expand Down
42 changes: 42 additions & 0 deletions actions/setup/js/update_pr_description_helpers.cjs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,8 @@
const { assembleMarkdownBodyParts, buildGeneratedFooter } = require("./markdown_body_helpers.cjs");
const { generateWorkflowIdMarker } = require("./generate_footer.cjs");
const { sanitizeContent } = require("./sanitize_content.cjs");
const { buildWorkflowRunUrl } = require("./workflow_metadata_helpers.cjs");
const { generateHistoryUrl } = require("./generate_history_link.cjs");

/**
* Build the AI footer with workflow attribution
Expand Down Expand Up @@ -149,8 +151,48 @@ function updateBody(params) {
return currentBody + appendSection;
}

/**
* Build an updated entity body with workflow attribution.
* @param {Object} params - Body update parameters
* @param {any} params.context - GitHub Actions context for the target repository
* @param {string} params.currentBody - Current body content
* @param {string} params.newContent - New content to add or replace
* @param {string} params.operation - Body update operation
* @param {boolean} params.includeFooter - Whether to include the generated footer
* @param {any} [params.workflowRepo] - Original workflow repository for run attribution
* @param {"issue" | "pull_request"} params.itemType - Updated entity type
* @returns {string} Updated body content
*/
function buildUpdatedBody({ context, currentBody, newContent, operation, includeFooter, workflowRepo, itemType }) {
const workflowName = process.env.GH_AW_WORKFLOW_NAME || "GitHub Agentic Workflow";
const workflowId = process.env.GH_AW_WORKFLOW_ID || "";
const workflowCallId = process.env.GH_AW_CALLER_WORKFLOW_ID || "";
const runUrl = buildWorkflowRunUrl(context, workflowRepo || context.repo);
const historyUrl =
generateHistoryUrl({
owner: context.repo.owner,
repo: context.repo.repo,
itemType,

@github-actions github-actions Bot Aug 10, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[/codebase-design] The critical cross-repo invariant — "use workflowRepo for the run URL, but context.repo for the history link" — was documented in both callers but is absent from buildUpdatedBody. Callers can no longer see this distinction; it should live here.

💡 Suggested JSDoc addition

Add to the function's JSDoc (above @param params):

 * `@remarks`
 * Cross-repository attribution contract:
 *   - Run URL uses `workflowRepo` (falls back to `context.repo`) so attribution
 *     always points to the originating workflow, not the target repository.
 *   - History URL uses `context.repo` so links target the item being updated.

@copilot please address this.

workflowCallId,
workflowId,
serverUrl: context.serverUrl,
}) || undefined;

return updateBody({
currentBody,
newContent,
operation,
workflowName,
runUrl,
workflowId,
includeFooter,
historyUrl,
});
}

module.exports = {
buildAIFooter,
buildUpdatedBody,
buildIslandStartMarker,
buildIslandEndMarker,
findIsland,
Expand Down
25 changes: 24 additions & 1 deletion actions/setup/js/update_pr_description_helpers.test.cjs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ const mockCore = {
global.core = mockCore;

// Import the module
const { buildAIFooter, buildIslandStartMarker, buildIslandEndMarker, findIsland, updateBody } = await import("./update_pr_description_helpers.cjs");
const { buildAIFooter, buildUpdatedBody, buildIslandStartMarker, buildIslandEndMarker, findIsland, updateBody } = await import("./update_pr_description_helpers.cjs");

describe("update_pr_description_helpers.cjs", () => {
beforeEach(() => {
Expand Down Expand Up @@ -45,6 +45,29 @@ describe("update_pr_description_helpers.cjs", () => {
});
});

describe("buildUpdatedBody", () => {
it("uses the workflow repository for attribution and target repository for history", () => {
process.env.GH_AW_CALLER_WORKFLOW_ID = "caller-workflow";
const result = buildUpdatedBody({
context: {
repo: { owner: "target", repo: "repository" },
serverUrl: "https://github.example",
runId: 123,
},
currentBody: "Existing body",
newContent: "New body",
operation: "append",
includeFooter: true,
workflowRepo: { owner: "workflow", repo: "repository" },
itemType: "issue",
});

expect(result).toContain("https://github.example/workflow/repository/actions/runs/123");

@github-actions github-actions Bot Aug 10, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[/tdd] delete process.env.GH_AW_CALLER_WORKFLOW_ID runs inline — if an assertion throws first, the env var leaks into subsequent tests and can cause false passes or spurious failures.

💡 Use afterEach to guarantee cleanup

Move env setup/teardown to beforeEach/afterEach at the describe block level:

describe('buildUpdatedBody', () => {
  const originalCallerId = process.env.GH_AW_CALLER_WORKFLOW_ID;
  afterEach(() => {
    if (originalCallerId === undefined) delete process.env.GH_AW_CALLER_WORKFLOW_ID;
    else process.env.GH_AW_CALLER_WORKFLOW_ID = originalCallerId;
  });
  // ...
});

This guarantees cleanup even if assertions throw.

@copilot please address this.

expect(result).toContain("repo%3Atarget%2Frepository");
delete process.env.GH_AW_CALLER_WORKFLOW_ID;
});
});

describe("buildIslandStartMarker", () => {
it("should build island start marker with workflow ID", () => {
const marker = buildIslandStartMarker("test-workflow");

@github-actions github-actions Bot Aug 10, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[/tdd] No test for the workflowRepo omitted/undefined case — this is the default path for non-cross-repo calls and is the most common usage.

💡 Suggested test 
it('falls back to context.repo when workflowRepo is omitted', () => {
  const result = buildUpdatedBody({
    context: { repo: { owner: 'myorg', repo: 'myrepo' }, serverUrl: 'https://github.com', runId: 1 },
    currentBody: '',
    newContent: 'Body',
    operation: 'append',
    includeFooter: true,
    itemType: 'issue',
    // workflowRepo intentionally omitted
  });
  // run URL should use context.repo
  expect(result).toContain('https://github.com/myorg/myrepo/actions/runs/1');
});

This guards against a regression in buildWorkflowRunUrl(context, workflowRepo || context.repo) where undefined isn't handled the same way in all environments.

@copilot please address this.

Expand Down
35 changes: 6 additions & 29 deletions actions/setup/js/update_pull_request.cjs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,12 +9,10 @@
/** @type {string} Safe output type handled by this module */
const HANDLER_TYPE = "update_pull_request";

const { updateBody } = require("./update_pr_description_helpers.cjs");
const { buildUpdatedBody } = require("./update_pr_description_helpers.cjs");
const { resolveTarget, checkRequiredFilter } = require("./safe_output_helpers.cjs");
const { createUpdateHandlerFactory, createStandardResolveNumber, createStandardFormatResult } = require("./update_handler_factory.cjs");
const { buildCommonEntityUpdateData } = require("./update_entity_helpers.cjs");
const { buildWorkflowRunUrl } = require("./workflow_metadata_helpers.cjs");
const { generateHistoryUrl } = require("./generate_history_link.cjs");
const { getErrorMessage } = require("./error_helpers.cjs");
const { fetchPullRequestState, mergePullRequestState } = require("./safe_output_execution_metadata.cjs");
const { withRetry, isTransientError } = require("./error_recovery.cjs");
Expand Down Expand Up @@ -132,35 +130,14 @@ async function executePRUpdate(github, context, prNumber, updateData) {
});
const currentBody = currentPR.body || "";

// 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 callerWorkflowId = process.env.GH_AW_CALLER_WORKFLOW_ID || "";
const workflowRepo = _workflowRepo || context.repo;
const runUrl = buildWorkflowRunUrl(context, workflowRepo);

const historyUrl =
generateHistoryUrl({
owner: context.repo.owner,
repo: context.repo.repo,
itemType: "pull_request",
workflowCallId: callerWorkflowId,
workflowId,
serverUrl: context.serverUrl,
}) || undefined;

// Use helper to update body (handles all operations including replace)
apiData.body = updateBody({
apiData.body = buildUpdatedBody({
context,
currentBody,
newContent: rawBody,
operation,
workflowName,
runUrl,
workflowId,
includeFooter, // Pass footer flag to helper
historyUrl,
includeFooter,
workflowRepo: _workflowRepo,
itemType: "pull_request",
});

core.info(`Will update body (length: ${apiData.body.length})`);
Expand Down
Loading