Skip to content

add-comment: support hide-older-comments.match for exact multi-workflow comment minimization #37977

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 7 commits into from
Jun 9, 2026
Merged

add-comment: support hide-older-comments.match for exact multi-workflow comment minimization #37977

Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
bb81f15
Initial plan
Copilot Jun 9, 2026
c3fd001
Add hide-older-comments match handling
Copilot Jun 9, 2026
93c08cb
Support hide-older-comments match list
Copilot Jun 9, 2026
c92f3a8
Refactor hide-older-comments match normalization
Copilot Jun 9, 2026
e4552c0
Polish hide-older-comments messaging
Copilot Jun 9, 2026
0f9c050
Add draft ADR for hide-older-comments match list (Design Decision Gate)
github-actions[bot] Jun 9, 2026
1125206
test: cover add-comment workflow match review feedback
Copilot Jun 9, 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
65 changes: 49 additions & 16 deletions actions/setup/js/add_comment.cjs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,29 @@ function deduplicateCaseInsensitive(aliases) {
});
}

/**
* @param {unknown} value
* @returns {value is { enabled?: boolean | string, match?: unknown[] }}
*/
function isHideOlderCommentsObject(value) {
return Boolean(value && typeof value === "object" && !Array.isArray(value));
}

/**
* @param {unknown[]} ids
* @returns {string[]}
*/
function normalizeWorkflowIdList(ids) {
return [
...new Set(
ids
.filter(id => typeof id === "string")
.map(id => id.trim())
.filter(Boolean)

@github-actions github-actions Bot Jun 9, 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] normalizeWorkflowIdList trims whitespace and deduplicates entries, but neither behaviour is exercised by any test. A user authoring match: [" wf-a ", "wf-a"] should get ["wf-a"] — both the dedup and the trim are load-bearing for correctness.

💡 Suggested unit-level assertions

Since this is a pure function, it can be tested directly (or via a small dedicated test block without needing a full mock setup):

// trim
normalizeWorkflowIdList(["  wf-a  ", "wf-b "]) // → ["wf-a", "wf-b"]
// dedup
normalizeWorkflowIdList(["wf-a", "wf-a", "wf-b"]) // → ["wf-a", "wf-b"]
// empty-string / whitespace-only filtered
normalizeWorkflowIdList(["", "  ", "wf-a"]) // → ["wf-a"]

),
];
}

/**
* Resolve effective event name/payload for native and forwarded contexts.
* Supports:
Expand Down Expand Up @@ -149,10 +172,10 @@ async function minimizeComment(github, nodeId, reason = "outdated") {
* @param {string} owner - Repository owner
* @param {string} repo - Repository name
* @param {number} issueNumber - Issue/PR number
* @param {string} workflowId - Workflow ID to search for
* @param {string[]} workflowIds - Workflow IDs to search for
* @returns {Promise<Array<{id: number, node_id: string, body: string}>>}
*/
async function findCommentsWithTrackerId(github, owner, repo, issueNumber, workflowId) {
async function findCommentsWithTrackerId(github, owner, repo, issueNumber, workflowIds) {
Comment on lines 174 to +178
const comments = [];
let page = 1;
const perPage = 100;
Expand All @@ -171,7 +194,7 @@ async function findCommentsWithTrackerId(github, owner, repo, issueNumber, workf
break;
}

const filteredComments = data.filter(comment => matchesWorkflowId(comment.body, workflowId)).map(({ id, node_id, body }) => ({ id, node_id, body }));
const filteredComments = data.filter(comment => workflowIds.some(id => matchesWorkflowId(comment.body, id))).map(({ id, node_id, body }) => ({ id, node_id, body }));

comments.push(...filteredComments);

Expand All @@ -191,10 +214,10 @@ async function findCommentsWithTrackerId(github, owner, repo, issueNumber, workf
* @param {string} owner - Repository owner
* @param {string} repo - Repository name
* @param {number} discussionNumber - Discussion number
* @param {string} workflowId - Workflow ID to search for
* @param {string[]} workflowIds - Workflow IDs to search for
* @returns {Promise<Array<{id: string, body: string}>>}
*/
async function findDiscussionCommentsWithTrackerId(github, owner, repo, discussionNumber, workflowId) {
async function findDiscussionCommentsWithTrackerId(github, owner, repo, discussionNumber, workflowIds) {
Comment on lines 216 to +220
const query = /* GraphQL */ `
query ($owner: String!, $repo: String!, $num: Int!, $cursor: String) {
repository(owner: $owner, name: $repo) {
Expand Down Expand Up @@ -224,7 +247,7 @@ async function findDiscussionCommentsWithTrackerId(github, owner, repo, discussi
break;
}

const filteredComments = result.repository.discussion.comments.nodes.filter(comment => matchesWorkflowId(comment.body, workflowId)).map(({ id, body }) => ({ id, body }));
const filteredComments = result.repository.discussion.comments.nodes.filter(comment => workflowIds.some(id => matchesWorkflowId(comment.body, id))).map(({ id, body }) => ({ id, body }));

comments.push(...filteredComments);

Expand All @@ -244,15 +267,15 @@ async function findDiscussionCommentsWithTrackerId(github, owner, repo, discussi
* @param {string} owner - Repository owner
* @param {string} repo - Repository name
* @param {number} itemNumber - Issue/PR/Discussion number
* @param {string} workflowId - Workflow ID to match
* @param {string[]} workflowIds - Workflow IDs to match
* @param {boolean} isDiscussion - Whether this is a discussion
* @param {string} reason - Reason for hiding (default: outdated)
* @param {string[] | null} allowedReasons - List of allowed reasons (default: null for all)
* @returns {Promise<number>} Number of comments hidden
*/
async function hideOlderComments(github, owner, repo, itemNumber, workflowId, isDiscussion, reason = "outdated", allowedReasons = null) {
if (!workflowId) {
core.info("No workflow ID available, skipping hide-older-comments");
async function hideOlderComments(github, owner, repo, itemNumber, workflowIds, isDiscussion, reason = "outdated", allowedReasons = null) {
if (!Array.isArray(workflowIds) || workflowIds.length === 0) {
core.info("No workflow IDs provided, skipping hide-older-comments");
return 0;
}

Expand All @@ -268,13 +291,13 @@ async function hideOlderComments(github, owner, repo, itemNumber, workflowId, is
}
}

core.info(`Searching for previous comments with workflow ID: ${workflowId}`);
core.info(`Searching for previous comments with workflow IDs: ${workflowIds.join(", ")}`);

let comments;
if (isDiscussion) {
comments = await findDiscussionCommentsWithTrackerId(github, owner, repo, itemNumber, workflowId);
comments = await findDiscussionCommentsWithTrackerId(github, owner, repo, itemNumber, workflowIds);
} else {
comments = await findCommentsWithTrackerId(github, owner, repo, itemNumber, workflowId);
comments = await findCommentsWithTrackerId(github, owner, repo, itemNumber, workflowIds);
}

if (comments.length === 0) {
Expand Down Expand Up @@ -375,7 +398,13 @@ async function commentOnDiscussion(github, owner, repo, discussionNumber, messag
*/
async function main(config = {}) {
// Extract configuration
const hideOlderCommentsEnabled = parseBoolTemplatable(config.hide_older_comments, false);
const hideOlderCommentsConfig = isHideOlderCommentsObject(config.hide_older_comments) ? config.hide_older_comments : null;
const hideOlderCommentsEnabled = parseBoolTemplatable(hideOlderCommentsConfig ? (hideOlderCommentsConfig.enabled ?? true) : config.hide_older_comments, false);
const hideOlderCommentsMatch = Array.isArray(hideOlderCommentsConfig?.match)
? normalizeWorkflowIdList(hideOlderCommentsConfig.match)
: Array.isArray(config.hide_older_comments_match)
? normalizeWorkflowIdList(config.hide_older_comments_match)

@github-actions github-actions Bot Jun 9, 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] The config.hide_older_comments_match branch (lines 405–406) — the actual compiled production path — has no test coverage. When Go compiles a workflow, the registry sends hide_older_comments_match as a flat top-level key with hide_older_comments as the string "true"; this else-branch is what runs in production. The existing tests only exercise the object-direct path (hide_older_comments is an object), leaving the live code path untested.

💡 Suggested test (compiled config form) 
// mirrors what the Go registry actually sends to the JS handler
const handler = await eval(`(async () => {
  ${addCommentScript};
  return await main({
    hide_older_comments: "true",
    hide_older_comments_match: ["other_workflow"]
  });
})()`);
const result = await handler({ type: "add_comment", body: "compiled path test" }, {});
expect(hiddenNodeIds).toContain("IC_kwDOTest11"); // other_workflow

This mirrors the actual runtime values set by AddTemplatableBool + AddStringSlice in safe_outputs_handler_registry.go.

: [];
const commentTarget = config.target || "triggering";
const maxCount = config.max || 20;
const { defaultTargetRepo, allowedRepos } = resolveTargetRepoConfig(config);
Expand Down Expand Up @@ -406,6 +435,9 @@ async function main(config = {}) {
if (requiredTitlePrefix) core.info(`Required title prefix: ${requiredTitlePrefix}`);
if (hideOlderCommentsEnabled) {
core.info("Hide-older-comments is enabled");
if (hideOlderCommentsMatch.length > 0) {
core.info(`Hide-older-comments additional workflow matches: ${hideOlderCommentsMatch.join(", ")}`);
}
}
if (appendOnlyComments) {
core.info("Append-only-comments is enabled - will not hide older comments");
Expand Down Expand Up @@ -776,8 +808,9 @@ async function main(config = {}) {
core.info("Skipping hide-older-comments because an existing comment is being updated");
} else if (appendOnlyComments) {
core.info("Skipping hide-older-comments because append-only-comments is enabled");
} else if (workflowId) {
await hideOlderComments(githubClient, repoParts.owner, repoParts.repo, itemNumber, workflowId, isDiscussion);
} else {
const hideWorkflowIds = normalizeWorkflowIdList([workflowId, ...hideOlderCommentsMatch]);

@github-actions github-actions Bot Jun 9, 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] When GH_AW_WORKFLOW_ID is not set, workflowId is "" (line 452) and is correctly filtered out by normalizeWorkflowIdList. This means hiding is driven entirely by hideOlderCommentsMatch. There is no test for this path — confirming that { match: ["target-wf"] } alone hides the right comments when the env var is absent.

💡 Suggested test 
it("should hide match-list comments when GH_AW_WORKFLOW_ID is not set", async () => {
  delete process.env.GH_AW_WORKFLOW_ID;
  // ... setup mocks ...
  const handler = await eval(`... main({ hide_older_comments: { match: ["target-wf"] } }) ...`);
  const result = await handler({ type: "add_comment", body: "test" }, {});
  expect(hiddenNodeIds).toContain(/* target-wf comment node */);
  expect(hiddenNodeIds).not.toContain(/* unrelated comment node */);
});

await hideOlderComments(githubClient, repoParts.owner, repoParts.repo, itemNumber, hideWorkflowIds, isDiscussion);
}
}

Expand Down
81 changes: 81 additions & 0 deletions actions/setup/js/add_comment.test.cjs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3248,6 +3248,87 @@ describe("add_comment", () => {
}
}
});

it("should hide comments matching hide-older-comments.match values", async () => {
const addCommentScript = fs.readFileSync(path.join(__dirname, "add_comment.cjs"), "utf8");
const originalWorkflowId = process.env.GH_AW_WORKFLOW_ID;
process.env.GH_AW_WORKFLOW_ID = "current-workflow";

try {
let hiddenNodeIds = [];
mockGithub.rest.issues.listComments = async () => ({
data: [
{
id: 10,
node_id: "IC_kwDOTest10",
body: "Current\n\n<!-- gh-aw-workflow-id: current-workflow -->",
},
{
id: 11,
node_id: "IC_kwDOTest11",
body: "Other\n\n<!-- gh-aw-workflow-id: other_workflow -->",
},
{
id: 12,
node_id: "IC_kwDOTest12",
body: "Unrelated\n\n<!-- gh-aw-workflow-id: unrelated -->",
},
],
});
mockGithub.graphql = async (query, variables) => {
if (query.includes("minimizeComment")) hiddenNodeIds.push(variables.nodeId);
return { minimizeComment: { minimizedComment: { isMinimized: true } } };
};
mockGithub.rest.issues.createComment = async () => ({
data: { id: 1, html_url: "https://github.com/owner/repo/issues/8535#issuecomment-1" },
});

const handler = await eval(`(async () => { ${addCommentScript}; return await main({ hide_older_comments: { match: ["other_workflow"] } }); })()`);

const result = await handler({ type: "add_comment", body: "Comment with match list" }, {});

expect(result.success).toBe(true);
expect(hiddenNodeIds).toContain("IC_kwDOTest10");
expect(hiddenNodeIds).toContain("IC_kwDOTest11");
expect(hiddenNodeIds).not.toContain("IC_kwDOTest12");
} finally {
if (originalWorkflowId === undefined) {
delete process.env.GH_AW_WORKFLOW_ID;
} else {
process.env.GH_AW_WORKFLOW_ID = originalWorkflowId;
}
}
});

it("should respect hide_older_comments.enabled when object config is used", async () => {
const addCommentScript = fs.readFileSync(path.join(__dirname, "add_comment.cjs"), "utf8");
const originalWorkflowId = process.env.GH_AW_WORKFLOW_ID;
process.env.GH_AW_WORKFLOW_ID = "current-workflow";

try {
let listCommentsCalled = false;
mockGithub.rest.issues.listComments = async () => {
listCommentsCalled = true;
return { data: [] };
};
mockGithub.rest.issues.createComment = async () => ({
data: { id: 1, html_url: "https://github.com/owner/repo/issues/8535#issuecomment-1" },
});

const handler = await eval(`(async () => { ${addCommentScript}; return await main({ hide_older_comments: { enabled: false, match: ["other_workflow"] } }); })()`);

const result = await handler({ type: "add_comment", body: "Comment with disabled hide" }, {});

expect(result.success).toBe(true);
expect(listCommentsCalled).toBe(false);
} finally {
if (originalWorkflowId === undefined) {
delete process.env.GH_AW_WORKFLOW_ID;
} else {
process.env.GH_AW_WORKFLOW_ID = originalWorkflowId;
}
}
});
});

let enforceCommentLimits;
Expand Down
25 changes: 23 additions & 2 deletions pkg/parser/schemas/main_workflow_schema.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6266,8 +6266,29 @@
]
},
"hide-older-comments": {
"$ref": "#/$defs/templatable_boolean",
"description": "When true, minimizes/hides all previous comments from the same agentic workflow (identified by tracker-id) before creating the new comment. Supports literal boolean or GitHub Actions expression (e.g. '${{ inputs.hide-older-comments }}'). Default: false."
"description": "When true, minimizes/hides all previous comments from the same agentic workflow (identified by tracker-id) before creating the new comment. Supports literal boolean, GitHub Actions expression (e.g. '${{ inputs.hide-older-comments }}'), or object form for advanced matching. Default: false.",
"oneOf": [
{
"$ref": "#/$defs/templatable_boolean"
},
{
"type": "object",
"additionalProperties": false,
"properties": {
"enabled": {
"$ref": "#/$defs/templatable_boolean",
"description": "Enable or disable hide-older-comments when using object form. Defaults to true when omitted."
},
"match": {
"type": "array",
"items": {
"type": "string"
},
"description": "Additional workflow-id values to fully match when selecting older comments to hide."
}
}
}
]
},
"allowed-reasons": {
"type": "array",
Expand Down
56 changes: 51 additions & 5 deletions pkg/workflow/add_comment.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
package workflow

import (
"fmt"

"github.com/github/gh-aw/pkg/logger"
)

Expand All @@ -17,11 +19,12 @@ type AddCommentsConfig struct {
TargetRepoSlug string `yaml:"target-repo,omitempty"` // Target repository in format "owner/repo" for cross-repository comments
AllowedRepos []string `yaml:"allowed-repos,omitempty"` // List of additional repositories that comments can be added to (additionally to the target-repo)
HideOlderComments *string `yaml:"hide-older-comments,omitempty"` // When true, minimizes/hides all previous comments from the same workflow before creating the new comment
AllowedReasons []string `yaml:"allowed-reasons,omitempty"` // List of allowed reasons for hiding older comments (default: all reasons allowed)
Issues *bool `yaml:"issues,omitempty"` // When false, excludes issues:write permission and issues from event condition. Default (nil or true) includes issues:write.
PullRequests *bool `yaml:"pull-requests,omitempty"` // When false, excludes pull-requests:write permission and PRs from event condition. Default (nil or true) includes pull-requests:write.
Discussions *bool `yaml:"discussions,omitempty"` // When false, excludes discussions:write permission. Default (nil or true) includes discussions:write.
Footer *string `yaml:"footer,omitempty"` // Controls whether AI-generated footer is added. When false, visible footer is omitted but XML markers are kept.
HideOlderCommentsMatch []string `yaml:"hide-older-comments-match,omitempty"`

@github-actions github-actions Bot Jun 9, 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.

[/grill-with-docs] The yaml:"hide-older-comments-match" tag looks like a user-facing YAML key, but this field is an internal intermediate value injected by preprocessHideOlderCommentsConfig — it does not appear in the schema and cannot be set directly in workflow YAML. Future contributors may be confused about why it exists as a struct tag vs. a typed nested field.

💡 Suggested comment 
// HideOlderCommentsMatch is an internal field populated by preprocessHideOlderCommentsConfig
// from the hide-older-comments.match array. Not a user-facing YAML key; the schema exposes
// it only via the hide-older-comments object form.
HideOlderCommentsMatch []string `yaml:"hide-older-comments-match,omitempty"`

AllowedReasons []string `yaml:"allowed-reasons,omitempty"` // List of allowed reasons for hiding older comments (default: all reasons allowed)
Issues *bool `yaml:"issues,omitempty"` // When false, excludes issues:write permission and issues from event condition. Default (nil or true) includes issues:write.
PullRequests *bool `yaml:"pull-requests,omitempty"` // When false, excludes pull-requests:write permission and PRs from event condition. Default (nil or true) includes pull-requests:write.
Discussions *bool `yaml:"discussions,omitempty"` // When false, excludes discussions:write permission. Default (nil or true) includes discussions:write.
Footer *string `yaml:"footer,omitempty"` // Controls whether AI-generated footer is added. When false, visible footer is omitted but XML markers are kept.
}

// parseCommentsConfig handles add-comment configuration
Expand All @@ -34,6 +37,11 @@ func (c *Compiler) parseCommentsConfig(outputMap map[string]any) *AddCommentsCon
// Get config data for pre-processing before YAML unmarshaling
configData, _ := outputMap["add-comment"].(map[string]any)

if err := preprocessHideOlderCommentsConfig(configData, addCommentLog); err != nil {
addCommentLog.Printf("Invalid hide-older-comments configuration: %v", err)
return nil

@github-actions github-actions Bot Jun 9, 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.

[/diagnose] An invalid hide-older-comments.enabled value causes parseCommentsConfig to return nil, silently dropping the entire add-comment configuration. The error is only logged internally; the workflow author gets no compile-time diagnostic explaining why their add-comment handler disappeared.

💡 Suggestion

Consider following the existing preprocessBoolFieldAsString pattern more closely — those also return nil on error, but the enabled case is harder to diagnose because it removes the whole handler rather than one field. At minimum, upgrade the log to a user-visible error (e.g. via the compiler error collection) or add a compile step that rejects the workflow with a descriptive message like "hide-older-comments.enabled must be a boolean or ${{ expr }}".

}

// Pre-process templatable bool fields
if err := preprocessBoolFieldAsString(configData, "hide-older-comments", addCommentLog); err != nil {
addCommentLog.Printf("Invalid hide-older-comments value: %v", err)
Expand Down Expand Up @@ -73,6 +81,44 @@ func (c *Compiler) parseCommentsConfig(outputMap map[string]any) *AddCommentsCon
return config
}

func preprocessHideOlderCommentsConfig(configData map[string]any, debugLog *logger.Logger) error {
if configData == nil {
return nil
}

raw, exists := configData["hide-older-comments"]
if !exists || raw == nil {
return nil
}

objectConfig, ok := raw.(map[string]any)
if !ok {
return nil
}

if enabledRaw, hasEnabled := objectConfig["enabled"]; hasEnabled {
switch enabled := enabledRaw.(type) {
case bool:
configData["hide-older-comments"] = enabled
case string:
if !isExpression(enabled) {
return fmt.Errorf("field %q must be a boolean or a GitHub Actions expression", "hide-older-comments.enabled")
}
configData["hide-older-comments"] = enabled
default:
return fmt.Errorf("field %q must be a boolean or a GitHub Actions expression", "hide-older-comments.enabled")
}
} else {
configData["hide-older-comments"] = true
}

if matchRaw, hasMatch := objectConfig["match"]; hasMatch {
configData["hide-older-comments-match"] = parseStringSliceAny(matchRaw, debugLog)
}

return nil
}

// buildAddCommentPermissions computes the permissions for the add_comment job based on config.
// Issues: nil or true → issues:write (default: true)
// PullRequests: nil or true → pull-requests:write (default: true)
Expand Down
Loading
Loading