Skip to content

Add conditional footer control for PR review comments #15643

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
Feb 14, 2026
Merged

Add conditional footer control for PR review comments #15643

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
e219919
Initial plan
Copilot Feb 14, 2026
f62dbe0
Add footer configuration for PR review comments
Copilot Feb 14, 2026
84c58a6
Move boolean-to-string conversion to Go compiler
Copilot Feb 14, 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
53 changes: 40 additions & 13 deletions actions/setup/js/pr_review_buffer.cjs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -61,8 +61,8 @@ function createReviewBuffer() {
/** @type {{workflowName: string, runUrl: string, workflowSource: string, workflowSourceURL: string, triggeringIssueNumber: number|undefined, triggeringPRNumber: number|undefined, triggeringDiscussionNumber: number|undefined} | null} */
let footerContext = null;

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

/**
* Add a validated comment to the buffer.
Expand Down Expand Up @@ -120,13 +120,28 @@ function createReviewBuffer() {
}

/**
* Set whether to include footer in review body.
* Controlled by the `footer` config option (default: true).
* @param {boolean} value - Whether to include footer
* Set the footer mode for review body.
* Supported modes:
* - "always" (default): Always include footer
* - "none": Never include footer
* - "if-body": Only include footer if review body is non-empty
* Note: Boolean values are converted to strings in the Go compiler before reaching JavaScript.
* @param {string} value - Footer mode string
*/
function setIncludeFooter(value) {
includeFooter = value;
core.info(`PR review footer ${value ? "enabled" : "disabled"}`);
function setFooterMode(value) {
if (typeof value === "string") {
// Validate string mode
if (value === "always" || value === "none" || value === "if-body") {
footerMode = value;
core.info(`PR review footer mode set to "${footerMode}"`);
} else {
core.warning(`Invalid footer mode: "${value}". Using default "always". Valid values: "always", "none", "if-body"`);
footerMode = "always";
}
} else {
core.warning(`Invalid footer mode type: ${typeof value}. Using default "always".`);
footerMode = "always";
}
}
Comment on lines +123 to 145
Copy link

Copilot AI Feb 14, 2026

Choose a reason for hiding this comment

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

setIncludeFooter is labeled as a backward-compatibility alias, but it now points to setFooterMode and no longer supports the previous boolean contract. Any remaining callers passing false will hit the "invalid type" path and silently reset to the default "always" (re-enabling the footer). Consider keeping setIncludeFooter(value: boolean) semantics by mapping booleans to modes (false→"none", true→"always"), while keeping setFooterMode(value: string) as the new string-only API.

Copilot uses AI. Check for mistakes.

/**
Expand Down Expand Up @@ -182,9 +197,20 @@ function createReviewBuffer() {
const event = reviewMetadata ? reviewMetadata.event : "COMMENT";
let body = reviewMetadata ? reviewMetadata.body : "";

// Add footer to review body if enabled and we have footer context.
// Footer is always added (even for body-less reviews) to track which workflow submitted the review.
if (includeFooter && footerContext) {
// Determine if we should add footer based on footer mode
let shouldAddFooter = false;
if (footerMode === "always") {
shouldAddFooter = true;
} else if (footerMode === "none") {
shouldAddFooter = false;
} else if (footerMode === "if-body") {
// Only add footer if body is non-empty (has meaningful content)
shouldAddFooter = body.trim().length > 0;
core.info(`Footer mode "if-body": body is ${body.trim().length > 0 ? "non-empty" : "empty"}, ${shouldAddFooter ? "adding" : "skipping"} footer`);
}

// Add footer to review body if we should and we have footer context
if (shouldAddFooter && footerContext) {
body += generateFooterWithMessages(
footerContext.workflowName,
footerContext.runUrl,
Expand Down Expand Up @@ -275,7 +301,7 @@ function createReviewBuffer() {
reviewMetadata = null;
reviewContext = null;
footerContext = null;
includeFooter = true;
footerMode = "always";
}

return {
Expand All @@ -284,7 +310,8 @@ function createReviewBuffer() {
setReviewContext,
getReviewContext,
setFooterContext,
setIncludeFooter,
setFooterMode,
setIncludeFooter: setFooterMode, // Backward compatibility alias
hasBufferedComments,
hasReviewMetadata,
getBufferedCount,
Expand Down
212 changes: 206 additions & 6 deletions actions/setup/js/pr_review_buffer.test.cjs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -394,7 +394,7 @@ describe("pr_review_buffer (factory pattern)", () => {
expect(callArgs.body).toContain("test-workflow");
});

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

mockGithub.rest.pulls.createReview.mockResolvedValue({
data: {
Expand Down Expand Up @@ -503,7 +503,7 @@ describe("pr_review_buffer (factory pattern)", () => {
});

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

buffer.reset();

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

// After reset, footer should be re-enabled (default: true)
// After reset, footer should be "always" (default)
// Verify by submitting a review with footer context and checking body
buffer.addComment({ path: "test.js", line: 1, body: "comment" });
buffer.setReviewMetadata("Review after reset", "COMMENT");
Expand All @@ -545,9 +545,209 @@ describe("pr_review_buffer (factory pattern)", () => {
return buffer.submitReview().then(result => {
expect(result.success).toBe(true);
const callArgs = mockGithub.rest.pulls.createReview.mock.calls[0][0];
// Footer should be included since includeFooter was reset to true
// Footer should be included since footer mode was reset to "always"
expect(callArgs.body).toContain("test-workflow");
});
});
});

describe("footer mode", () => {
it("should support 'always' mode (default)", async () => {
buffer.addComment({ path: "test.js", line: 1, body: "comment" });
buffer.setReviewMetadata("", "APPROVE"); // Empty body
buffer.setReviewContext({
repo: "owner/repo",
repoParts: { owner: "owner", repo: "repo" },
pullRequestNumber: 42,
pullRequest: { head: { sha: "abc123" } },
});
buffer.setFooterContext({
workflowName: "test-workflow",
runUrl: "https://github.com/owner/repo/actions/runs/123",
workflowSource: "owner/repo/workflows/test.md@v1",
workflowSourceURL: "https://github.com/owner/repo/blob/main/test.md",
});
buffer.setFooterMode("always");

mockGithub.rest.pulls.createReview.mockResolvedValue({
data: {
id: 500,
html_url: "https://github.com/owner/repo/pull/42#pullrequestreview-500",
},
});

const result = await buffer.submitReview();

expect(result.success).toBe(true);
const callArgs = mockGithub.rest.pulls.createReview.mock.calls[0][0];
// Footer should be included even with empty body
expect(callArgs.body).toContain("test-workflow");
});

it("should support 'none' mode", async () => {
buffer.addComment({ path: "test.js", line: 1, body: "comment" });
buffer.setReviewMetadata("Review body", "COMMENT");
buffer.setReviewContext({
repo: "owner/repo",
repoParts: { owner: "owner", repo: "repo" },
pullRequestNumber: 42,
pullRequest: { head: { sha: "abc123" } },
});
buffer.setFooterContext({
workflowName: "test-workflow",
runUrl: "https://github.com/owner/repo/actions/runs/123",
workflowSource: "owner/repo/workflows/test.md@v1",
workflowSourceURL: "https://github.com/owner/repo/blob/main/test.md",
});
buffer.setFooterMode("none");

mockGithub.rest.pulls.createReview.mockResolvedValue({
data: {
id: 501,
html_url: "https://github.com/owner/repo/pull/42#pullrequestreview-501",
},
});

const result = await buffer.submitReview();

expect(result.success).toBe(true);
const callArgs = mockGithub.rest.pulls.createReview.mock.calls[0][0];
// Footer should not be included
expect(callArgs.body).toBe("Review body");
expect(callArgs.body).not.toContain("test-workflow");
});

it("should support 'if-body' mode with non-empty body", async () => {
buffer.addComment({ path: "test.js", line: 1, body: "comment" });
buffer.setReviewMetadata("Review body", "COMMENT");
buffer.setReviewContext({
repo: "owner/repo",
repoParts: { owner: "owner", repo: "repo" },
pullRequestNumber: 42,
pullRequest: { head: { sha: "abc123" } },
});
buffer.setFooterContext({
workflowName: "test-workflow",
runUrl: "https://github.com/owner/repo/actions/runs/123",
workflowSource: "owner/repo/workflows/test.md@v1",
workflowSourceURL: "https://github.com/owner/repo/blob/main/test.md",
});
buffer.setFooterMode("if-body");

mockGithub.rest.pulls.createReview.mockResolvedValue({
data: {
id: 502,
html_url: "https://github.com/owner/repo/pull/42#pullrequestreview-502",
},
});

const result = await buffer.submitReview();

expect(result.success).toBe(true);
const callArgs = mockGithub.rest.pulls.createReview.mock.calls[0][0];
// Footer should be included because body is non-empty
expect(callArgs.body).toContain("Review body");
expect(callArgs.body).toContain("test-workflow");
});

it("should support 'if-body' mode with empty body", async () => {
buffer.addComment({ path: "test.js", line: 1, body: "comment" });
buffer.setReviewMetadata("", "APPROVE"); // Empty body
buffer.setReviewContext({
repo: "owner/repo",
repoParts: { owner: "owner", repo: "repo" },
pullRequestNumber: 42,
pullRequest: { head: { sha: "abc123" } },
});
buffer.setFooterContext({
workflowName: "test-workflow",
runUrl: "https://github.com/owner/repo/actions/runs/123",
workflowSource: "owner/repo/workflows/test.md@v1",
workflowSourceURL: "https://github.com/owner/repo/blob/main/test.md",
});
buffer.setFooterMode("if-body");

mockGithub.rest.pulls.createReview.mockResolvedValue({
data: {
id: 503,
html_url: "https://github.com/owner/repo/pull/42#pullrequestreview-503",
},
});

const result = await buffer.submitReview();

expect(result.success).toBe(true);
const callArgs = mockGithub.rest.pulls.createReview.mock.calls[0][0];
// Footer should NOT be included because body is empty
// Body should be undefined (not included in API call) when empty
expect(callArgs.body).toBeUndefined();
expect(callArgs.body || "").not.toContain("test-workflow");
});

it("should support 'if-body' mode with whitespace-only body", async () => {
buffer.addComment({ path: "test.js", line: 1, body: "comment" });
buffer.setReviewMetadata(" \n ", "APPROVE"); // Whitespace-only body
buffer.setReviewContext({
repo: "owner/repo",
repoParts: { owner: "owner", repo: "repo" },
pullRequestNumber: 42,
pullRequest: { head: { sha: "abc123" } },
});
buffer.setFooterContext({
workflowName: "test-workflow",
runUrl: "https://github.com/owner/repo/actions/runs/123",
workflowSource: "owner/repo/workflows/test.md@v1",
workflowSourceURL: "https://github.com/owner/repo/blob/main/test.md",
});
buffer.setFooterMode("if-body");

mockGithub.rest.pulls.createReview.mockResolvedValue({
data: {
id: 504,
html_url: "https://github.com/owner/repo/pull/42#pullrequestreview-504",
},
});

const result = await buffer.submitReview();

expect(result.success).toBe(true);
const callArgs = mockGithub.rest.pulls.createReview.mock.calls[0][0];
// Footer should NOT be included because body is whitespace-only (trimmed length is 0)
// Original whitespace body is preserved in the API call
expect(callArgs.body).toBe(" \n ");
expect(callArgs.body).not.toContain("test-workflow");
});

it("should default to 'always' for invalid string mode", async () => {
buffer.addComment({ path: "test.js", line: 1, body: "comment" });
buffer.setReviewMetadata("", "APPROVE");
buffer.setReviewContext({
repo: "owner/repo",
repoParts: { owner: "owner", repo: "repo" },
pullRequestNumber: 42,
pullRequest: { head: { sha: "abc123" } },
});
buffer.setFooterContext({
workflowName: "test-workflow",
runUrl: "https://github.com/owner/repo/actions/runs/123",
workflowSource: "owner/repo/workflows/test.md@v1",
workflowSourceURL: "https://github.com/owner/repo/blob/main/test.md",
});
buffer.setFooterMode("invalid-mode");

mockGithub.rest.pulls.createReview.mockResolvedValue({
data: {
id: 507,
html_url: "https://github.com/owner/repo/pull/42#pullrequestreview-507",
},
});

const result = await buffer.submitReview();

expect(result.success).toBe(true);
const callArgs = mockGithub.rest.pulls.createReview.mock.calls[0][0];
// Should default to "always" and include footer
expect(callArgs.body).toContain("test-workflow");
});
});
});
18 changes: 15 additions & 3 deletions actions/setup/js/safe_output_handler_manager.cjs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -737,9 +737,21 @@ async function main() {
// Create the shared PR review buffer instance (no global state)
const prReviewBuffer = createReviewBuffer();

// Apply footer config from submit_pull_request_review (if configured)
if (config.submit_pull_request_review?.footer === false) {
prReviewBuffer.setIncludeFooter(false);
// Apply footer config with priority:
// 1. create_pull_request_review_comment.footer (highest priority)
// 2. submit_pull_request_review.footer (fallback)
// 3. Default: "always"
let footerConfig = undefined;
if (config.create_pull_request_review_comment?.footer !== undefined) {
footerConfig = config.create_pull_request_review_comment.footer;
core.info(`Using footer config from create_pull_request_review_comment: ${footerConfig}`);
} else if (config.submit_pull_request_review?.footer !== undefined) {
footerConfig = config.submit_pull_request_review.footer;
core.info(`Using footer config from submit_pull_request_review: ${footerConfig}`);
}

if (footerConfig !== undefined) {
prReviewBuffer.setFooterMode(footerConfig);
Copy link

Copilot AI Feb 14, 2026

Choose a reason for hiding this comment

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

submit_pull_request_review.footer is still emitted as a boolean by the Go compiler (via AddBoolPtr/getEffectiveFooter), but here it’s forwarded into prReviewBuffer.setFooterMode(), which only accepts strings. As a result, footer: false (or global safe-outputs.footer: false) will be treated as an invalid type and the buffer will fall back to the default "always", re-enabling the footer unexpectedly. Normalize boolean values to the corresponding string modes (false→"none", true→"always") before calling setFooterMode, or change the Go handler config to emit a string mode for submit_pull_request_review.footer as well.

Suggested change
prReviewBuffer.setFooterMode(footerConfig);
let normalizedFooterConfig = footerConfig;
if (typeof footerConfig === "boolean") {
normalizedFooterConfig = footerConfig ? "always" : "none";
}
prReviewBuffer.setFooterMode(normalizedFooterConfig);

Copilot uses AI. Check for mistakes.
}

// Load and initialize handlers based on configuration (factory pattern)
Expand Down
18 changes: 15 additions & 3 deletions actions/setup/js/safe_output_unified_handler_manager.cjs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -982,9 +982,21 @@ async function main() {
// Create the shared PR review buffer instance (no global state)
const prReviewBuffer = createReviewBuffer();

// Apply footer config from submit_pull_request_review (if configured)
if (configs.regular?.submit_pull_request_review?.footer === false) {
prReviewBuffer.setIncludeFooter(false);
// Apply footer config with priority:
// 1. create_pull_request_review_comment.footer (highest priority)
// 2. submit_pull_request_review.footer (fallback)
// 3. Default: "always"
let footerConfig = undefined;
if (configs.regular?.create_pull_request_review_comment?.footer !== undefined) {
footerConfig = configs.regular.create_pull_request_review_comment.footer;
core.info(`Using footer config from create_pull_request_review_comment: ${footerConfig}`);
} else if (configs.regular?.submit_pull_request_review?.footer !== undefined) {
footerConfig = configs.regular.submit_pull_request_review.footer;
core.info(`Using footer config from submit_pull_request_review: ${footerConfig}`);
}

if (footerConfig !== undefined) {
Copy link

Copilot AI Feb 14, 2026

Choose a reason for hiding this comment

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

Same issue as in safe_output_handler_manager.cjs: configs.regular.submit_pull_request_review.footer comes from Go as a boolean, but it’s passed into setFooterMode() which only accepts strings. This causes footer: false (and global footer disable) to be ignored and the footer mode to revert to the default "always". Convert boolean to string mode (false→"none", true→"always") before calling setFooterMode, or update the Go config emission for submit_pull_request_review to provide a string footer mode.

Suggested change
if (footerConfig !== undefined) {
if (footerConfig !== undefined) {
// Normalize boolean footer configs (from Go) to string modes expected by setFooterMode
if (typeof footerConfig === "boolean") {
footerConfig = footerConfig ? "always" : "none";
core.info(`Normalized boolean footer config to mode: ${footerConfig}`);
}

Copilot uses AI. Check for mistakes.
prReviewBuffer.setFooterMode(footerConfig);
}

// Load and initialize handlers based on configuration (factory pattern)
Expand Down
Loading
Loading