Skip to content

Commit bdd5dd4

Browse files
CopilotCopilot
authored
Add conditional footer control for PR review comments (#15643)
1 parent 354d436 commit bdd5dd4

7 files changed

Lines changed: 478 additions & 25 deletions

‎actions/setup/js/pr_review_buffer.cjs‎

Lines changed: 40 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -61,8 +61,8 @@ function createReviewBuffer() {
6161
/** @type {{workflowName: string, runUrl: string, workflowSource: string, workflowSourceURL: string, triggeringIssueNumber: number|undefined, triggeringPRNumber: number|undefined, triggeringDiscussionNumber: number|undefined} | null} */
6262
let footerContext = null;
6363

64-
/** @type {boolean} Whether to include footer in review body (default: true, controlled by config.footer) */
65-
let includeFooter = true;
64+
/** @type {string} Footer mode: "always" (default), "none", or "if-body" */
65+
let footerMode = "always";
6666

6767
/**
6868
* Add a validated comment to the buffer.
@@ -120,13 +120,28 @@ function createReviewBuffer() {
120120
}
121121

122122
/**
123-
* Set whether to include footer in review body.
124-
* Controlled by the `footer` config option (default: true).
125-
* @param {boolean} value - Whether to include footer
123+
* Set the footer mode for review body.
124+
* Supported modes:
125+
* - "always" (default): Always include footer
126+
* - "none": Never include footer
127+
* - "if-body": Only include footer if review body is non-empty
128+
* Note: Boolean values are converted to strings in the Go compiler before reaching JavaScript.
129+
* @param {string} value - Footer mode string
126130
*/
127-
function setIncludeFooter(value) {
128-
includeFooter = value;
129-
core.info(`PR review footer ${value ? "enabled" : "disabled"}`);
131+
function setFooterMode(value) {
132+
if (typeof value === "string") {
133+
// Validate string mode
134+
if (value === "always" || value === "none" || value === "if-body") {
135+
footerMode = value;
136+
core.info(`PR review footer mode set to "${footerMode}"`);
137+
} else {
138+
core.warning(`Invalid footer mode: "${value}". Using default "always". Valid values: "always", "none", "if-body"`);
139+
footerMode = "always";
140+
}
141+
} else {
142+
core.warning(`Invalid footer mode type: ${typeof value}. Using default "always".`);
143+
footerMode = "always";
144+
}
130145
}
131146

132147
/**
@@ -182,9 +197,20 @@ function createReviewBuffer() {
182197
const event = reviewMetadata ? reviewMetadata.event : "COMMENT";
183198
let body = reviewMetadata ? reviewMetadata.body : "";
184199

185-
// Add footer to review body if enabled and we have footer context.
186-
// Footer is always added (even for body-less reviews) to track which workflow submitted the review.
187-
if (includeFooter && footerContext) {
200+
// Determine if we should add footer based on footer mode
201+
let shouldAddFooter = false;
202+
if (footerMode === "always") {
203+
shouldAddFooter = true;
204+
} else if (footerMode === "none") {
205+
shouldAddFooter = false;
206+
} else if (footerMode === "if-body") {
207+
// Only add footer if body is non-empty (has meaningful content)
208+
shouldAddFooter = body.trim().length > 0;
209+
core.info(`Footer mode "if-body": body is ${body.trim().length > 0 ? "non-empty" : "empty"}, ${shouldAddFooter ? "adding" : "skipping"} footer`);
210+
}
211+
212+
// Add footer to review body if we should and we have footer context
213+
if (shouldAddFooter && footerContext) {
188214
body += generateFooterWithMessages(
189215
footerContext.workflowName,
190216
footerContext.runUrl,
@@ -275,7 +301,7 @@ function createReviewBuffer() {
275301
reviewMetadata = null;
276302
reviewContext = null;
277303
footerContext = null;
278-
includeFooter = true;
304+
footerMode = "always";
279305
}
280306

281307
return {
@@ -284,7 +310,8 @@ function createReviewBuffer() {
284310
setReviewContext,
285311
getReviewContext,
286312
setFooterContext,
287-
setIncludeFooter,
313+
setFooterMode,
314+
setIncludeFooter: setFooterMode, // Backward compatibility alias
288315
hasBufferedComments,
289316
hasReviewMetadata,
290317
getBufferedCount,

‎actions/setup/js/pr_review_buffer.test.cjs‎

Lines changed: 206 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -394,7 +394,7 @@ describe("pr_review_buffer (factory pattern)", () => {
394394
expect(callArgs.body).toContain("test-workflow");
395395
});
396396

397-
it("should skip footer when setIncludeFooter(false) is called", async () => {
397+
it("should skip footer when setIncludeFooter('none') is called", async () => {
398398
buffer.addComment({ path: "test.js", line: 1, body: "comment" });
399399
buffer.setReviewMetadata("Review body", "COMMENT");
400400
buffer.setReviewContext({
@@ -409,7 +409,7 @@ describe("pr_review_buffer (factory pattern)", () => {
409409
workflowSource: "owner/repo/workflows/test.md@v1",
410410
workflowSourceURL: "https://github.com/owner/repo/blob/main/test.md",
411411
});
412-
buffer.setIncludeFooter(false);
412+
buffer.setIncludeFooter("none");
413413

414414
mockGithub.rest.pulls.createReview.mockResolvedValue({
415415
data: {
@@ -503,7 +503,7 @@ describe("pr_review_buffer (factory pattern)", () => {
503503
});
504504

505505
describe("reset", () => {
506-
it("should clear all state including includeFooter", () => {
506+
it("should clear all state including footer mode", () => {
507507
buffer.addComment({ path: "test.js", line: 1, body: "comment" });
508508
buffer.setReviewMetadata("body", "APPROVE");
509509
buffer.setReviewContext({
@@ -512,7 +512,7 @@ describe("pr_review_buffer (factory pattern)", () => {
512512
pullRequestNumber: 1,
513513
pullRequest: { head: { sha: "abc" } },
514514
});
515-
buffer.setIncludeFooter(false);
515+
buffer.setFooterMode("none");
516516

517517
buffer.reset();
518518

@@ -521,7 +521,7 @@ describe("pr_review_buffer (factory pattern)", () => {
521521
expect(buffer.hasReviewMetadata()).toBe(false);
522522
expect(buffer.getReviewContext()).toBeNull();
523523

524-
// After reset, footer should be re-enabled (default: true)
524+
// After reset, footer should be "always" (default)
525525
// Verify by submitting a review with footer context and checking body
526526
buffer.addComment({ path: "test.js", line: 1, body: "comment" });
527527
buffer.setReviewMetadata("Review after reset", "COMMENT");
@@ -545,9 +545,209 @@ describe("pr_review_buffer (factory pattern)", () => {
545545
return buffer.submitReview().then(result => {
546546
expect(result.success).toBe(true);
547547
const callArgs = mockGithub.rest.pulls.createReview.mock.calls[0][0];
548-
// Footer should be included since includeFooter was reset to true
548+
// Footer should be included since footer mode was reset to "always"
549549
expect(callArgs.body).toContain("test-workflow");
550550
});
551551
});
552552
});
553+
554+
describe("footer mode", () => {
555+
it("should support 'always' mode (default)", async () => {
556+
buffer.addComment({ path: "test.js", line: 1, body: "comment" });
557+
buffer.setReviewMetadata("", "APPROVE"); // Empty body
558+
buffer.setReviewContext({
559+
repo: "owner/repo",
560+
repoParts: { owner: "owner", repo: "repo" },
561+
pullRequestNumber: 42,
562+
pullRequest: { head: { sha: "abc123" } },
563+
});
564+
buffer.setFooterContext({
565+
workflowName: "test-workflow",
566+
runUrl: "https://github.com/owner/repo/actions/runs/123",
567+
workflowSource: "owner/repo/workflows/test.md@v1",
568+
workflowSourceURL: "https://github.com/owner/repo/blob/main/test.md",
569+
});
570+
buffer.setFooterMode("always");
571+
572+
mockGithub.rest.pulls.createReview.mockResolvedValue({
573+
data: {
574+
id: 500,
575+
html_url: "https://github.com/owner/repo/pull/42#pullrequestreview-500",
576+
},
577+
});
578+
579+
const result = await buffer.submitReview();
580+
581+
expect(result.success).toBe(true);
582+
const callArgs = mockGithub.rest.pulls.createReview.mock.calls[0][0];
583+
// Footer should be included even with empty body
584+
expect(callArgs.body).toContain("test-workflow");
585+
});
586+
587+
it("should support 'none' mode", async () => {
588+
buffer.addComment({ path: "test.js", line: 1, body: "comment" });
589+
buffer.setReviewMetadata("Review body", "COMMENT");
590+
buffer.setReviewContext({
591+
repo: "owner/repo",
592+
repoParts: { owner: "owner", repo: "repo" },
593+
pullRequestNumber: 42,
594+
pullRequest: { head: { sha: "abc123" } },
595+
});
596+
buffer.setFooterContext({
597+
workflowName: "test-workflow",
598+
runUrl: "https://github.com/owner/repo/actions/runs/123",
599+
workflowSource: "owner/repo/workflows/test.md@v1",
600+
workflowSourceURL: "https://github.com/owner/repo/blob/main/test.md",
601+
});
602+
buffer.setFooterMode("none");
603+
604+
mockGithub.rest.pulls.createReview.mockResolvedValue({
605+
data: {
606+
id: 501,
607+
html_url: "https://github.com/owner/repo/pull/42#pullrequestreview-501",
608+
},
609+
});
610+
611+
const result = await buffer.submitReview();
612+
613+
expect(result.success).toBe(true);
614+
const callArgs = mockGithub.rest.pulls.createReview.mock.calls[0][0];
615+
// Footer should not be included
616+
expect(callArgs.body).toBe("Review body");
617+
expect(callArgs.body).not.toContain("test-workflow");
618+
});
619+
620+
it("should support 'if-body' mode with non-empty body", async () => {
621+
buffer.addComment({ path: "test.js", line: 1, body: "comment" });
622+
buffer.setReviewMetadata("Review body", "COMMENT");
623+
buffer.setReviewContext({
624+
repo: "owner/repo",
625+
repoParts: { owner: "owner", repo: "repo" },
626+
pullRequestNumber: 42,
627+
pullRequest: { head: { sha: "abc123" } },
628+
});
629+
buffer.setFooterContext({
630+
workflowName: "test-workflow",
631+
runUrl: "https://github.com/owner/repo/actions/runs/123",
632+
workflowSource: "owner/repo/workflows/test.md@v1",
633+
workflowSourceURL: "https://github.com/owner/repo/blob/main/test.md",
634+
});
635+
buffer.setFooterMode("if-body");
636+
637+
mockGithub.rest.pulls.createReview.mockResolvedValue({
638+
data: {
639+
id: 502,
640+
html_url: "https://github.com/owner/repo/pull/42#pullrequestreview-502",
641+
},
642+
});
643+
644+
const result = await buffer.submitReview();
645+
646+
expect(result.success).toBe(true);
647+
const callArgs = mockGithub.rest.pulls.createReview.mock.calls[0][0];
648+
// Footer should be included because body is non-empty
649+
expect(callArgs.body).toContain("Review body");
650+
expect(callArgs.body).toContain("test-workflow");
651+
});
652+
653+
it("should support 'if-body' mode with empty body", async () => {
654+
buffer.addComment({ path: "test.js", line: 1, body: "comment" });
655+
buffer.setReviewMetadata("", "APPROVE"); // Empty body
656+
buffer.setReviewContext({
657+
repo: "owner/repo",
658+
repoParts: { owner: "owner", repo: "repo" },
659+
pullRequestNumber: 42,
660+
pullRequest: { head: { sha: "abc123" } },
661+
});
662+
buffer.setFooterContext({
663+
workflowName: "test-workflow",
664+
runUrl: "https://github.com/owner/repo/actions/runs/123",
665+
workflowSource: "owner/repo/workflows/test.md@v1",
666+
workflowSourceURL: "https://github.com/owner/repo/blob/main/test.md",
667+
});
668+
buffer.setFooterMode("if-body");
669+
670+
mockGithub.rest.pulls.createReview.mockResolvedValue({
671+
data: {
672+
id: 503,
673+
html_url: "https://github.com/owner/repo/pull/42#pullrequestreview-503",
674+
},
675+
});
676+
677+
const result = await buffer.submitReview();
678+
679+
expect(result.success).toBe(true);
680+
const callArgs = mockGithub.rest.pulls.createReview.mock.calls[0][0];
681+
// Footer should NOT be included because body is empty
682+
// Body should be undefined (not included in API call) when empty
683+
expect(callArgs.body).toBeUndefined();
684+
expect(callArgs.body || "").not.toContain("test-workflow");
685+
});
686+
687+
it("should support 'if-body' mode with whitespace-only body", async () => {
688+
buffer.addComment({ path: "test.js", line: 1, body: "comment" });
689+
buffer.setReviewMetadata(" \n ", "APPROVE"); // Whitespace-only body
690+
buffer.setReviewContext({
691+
repo: "owner/repo",
692+
repoParts: { owner: "owner", repo: "repo" },
693+
pullRequestNumber: 42,
694+
pullRequest: { head: { sha: "abc123" } },
695+
});
696+
buffer.setFooterContext({
697+
workflowName: "test-workflow",
698+
runUrl: "https://github.com/owner/repo/actions/runs/123",
699+
workflowSource: "owner/repo/workflows/test.md@v1",
700+
workflowSourceURL: "https://github.com/owner/repo/blob/main/test.md",
701+
});
702+
buffer.setFooterMode("if-body");
703+
704+
mockGithub.rest.pulls.createReview.mockResolvedValue({
705+
data: {
706+
id: 504,
707+
html_url: "https://github.com/owner/repo/pull/42#pullrequestreview-504",
708+
},
709+
});
710+
711+
const result = await buffer.submitReview();
712+
713+
expect(result.success).toBe(true);
714+
const callArgs = mockGithub.rest.pulls.createReview.mock.calls[0][0];
715+
// Footer should NOT be included because body is whitespace-only (trimmed length is 0)
716+
// Original whitespace body is preserved in the API call
717+
expect(callArgs.body).toBe(" \n ");
718+
expect(callArgs.body).not.toContain("test-workflow");
719+
});
720+
721+
it("should default to 'always' for invalid string mode", async () => {
722+
buffer.addComment({ path: "test.js", line: 1, body: "comment" });
723+
buffer.setReviewMetadata("", "APPROVE");
724+
buffer.setReviewContext({
725+
repo: "owner/repo",
726+
repoParts: { owner: "owner", repo: "repo" },
727+
pullRequestNumber: 42,
728+
pullRequest: { head: { sha: "abc123" } },
729+
});
730+
buffer.setFooterContext({
731+
workflowName: "test-workflow",
732+
runUrl: "https://github.com/owner/repo/actions/runs/123",
733+
workflowSource: "owner/repo/workflows/test.md@v1",
734+
workflowSourceURL: "https://github.com/owner/repo/blob/main/test.md",
735+
});
736+
buffer.setFooterMode("invalid-mode");
737+
738+
mockGithub.rest.pulls.createReview.mockResolvedValue({
739+
data: {
740+
id: 507,
741+
html_url: "https://github.com/owner/repo/pull/42#pullrequestreview-507",
742+
},
743+
});
744+
745+
const result = await buffer.submitReview();
746+
747+
expect(result.success).toBe(true);
748+
const callArgs = mockGithub.rest.pulls.createReview.mock.calls[0][0];
749+
// Should default to "always" and include footer
750+
expect(callArgs.body).toContain("test-workflow");
751+
});
752+
});
553753
});

‎actions/setup/js/safe_output_handler_manager.cjs‎

Lines changed: 15 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -737,9 +737,21 @@ async function main() {
737737
// Create the shared PR review buffer instance (no global state)
738738
const prReviewBuffer = createReviewBuffer();
739739

740-
// Apply footer config from submit_pull_request_review (if configured)
741-
if (config.submit_pull_request_review?.footer === false) {
742-
prReviewBuffer.setIncludeFooter(false);
740+
// Apply footer config with priority:
741+
// 1. create_pull_request_review_comment.footer (highest priority)
742+
// 2. submit_pull_request_review.footer (fallback)
743+
// 3. Default: "always"
744+
let footerConfig = undefined;
745+
if (config.create_pull_request_review_comment?.footer !== undefined) {
746+
footerConfig = config.create_pull_request_review_comment.footer;
747+
core.info(`Using footer config from create_pull_request_review_comment: ${footerConfig}`);
748+
} else if (config.submit_pull_request_review?.footer !== undefined) {
749+
footerConfig = config.submit_pull_request_review.footer;
750+
core.info(`Using footer config from submit_pull_request_review: ${footerConfig}`);
751+
}
752+
753+
if (footerConfig !== undefined) {
754+
prReviewBuffer.setFooterMode(footerConfig);
743755
}
744756

745757
// Load and initialize handlers based on configuration (factory pattern)

‎actions/setup/js/safe_output_unified_handler_manager.cjs‎

Lines changed: 15 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -982,9 +982,21 @@ async function main() {
982982
// Create the shared PR review buffer instance (no global state)
983983
const prReviewBuffer = createReviewBuffer();
984984

985-
// Apply footer config from submit_pull_request_review (if configured)
986-
if (configs.regular?.submit_pull_request_review?.footer === false) {
987-
prReviewBuffer.setIncludeFooter(false);
985+
// Apply footer config with priority:
986+
// 1. create_pull_request_review_comment.footer (highest priority)
987+
// 2. submit_pull_request_review.footer (fallback)
988+
// 3. Default: "always"
989+
let footerConfig = undefined;
990+
if (configs.regular?.create_pull_request_review_comment?.footer !== undefined) {
991+
footerConfig = configs.regular.create_pull_request_review_comment.footer;
992+
core.info(`Using footer config from create_pull_request_review_comment: ${footerConfig}`);
993+
} else if (configs.regular?.submit_pull_request_review?.footer !== undefined) {
994+
footerConfig = configs.regular.submit_pull_request_review.footer;
995+
core.info(`Using footer config from submit_pull_request_review: ${footerConfig}`);
996+
}
997+
998+
if (footerConfig !== undefined) {
999+
prReviewBuffer.setFooterMode(footerConfig);
9881000
}
9891001

9901002
// Load and initialize handlers based on configuration (factory pattern)

0 commit comments

Comments
 (0)