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 all 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
69 changes: 51 additions & 18 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 @@ -144,15 +167,15 @@ async function minimizeComment(github, nodeId, reason = "outdated") {
}

/**
* Find comments on an issue/PR with a specific tracker-id
* Find comments on an issue/PR with any matching workflow ID marker
* @param {any} github - GitHub REST API instance
* @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 @@ -186,15 +209,15 @@ async function findCommentsWithTrackerId(github, owner, repo, issueNumber, workf
}

/**
* Find comments on a discussion with a specific workflow ID
* Find comments on a discussion with any matching workflow ID marker
* @param {any} github - GitHub GraphQL instance
* @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
183 changes: 183 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 @@ -3185,6 +3185,13 @@ describe("add_comment", () => {
});

describe("hide-older-comments behavior", () => {
it("should normalize workflow ID match lists", async () => {
const addCommentScript = fs.readFileSync(path.join(__dirname, "add_comment.cjs"), "utf8");
const exports = await eval(`(async () => { ${addCommentScript}; return { normalizeWorkflowIdList }; })()`);

expect(exports.normalizeWorkflowIdList([" wf-a ", "wf-a", "", " ", "wf-b "])).toEqual(["wf-a", "wf-b"]);
});

it("should skip hiding when no workflow ID is set", async () => {
const addCommentScript = fs.readFileSync(path.join(__dirname, "add_comment.cjs"), "utf8");

Expand Down Expand Up @@ -3248,6 +3255,182 @@ describe("add_comment", () => {
}
}
});

it("should hide comments matching compiled 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: "true", hide_older_comments_match: [" other_workflow ", "other_workflow", "", " "] }); })()`);

const result = await handler({ type: "add_comment", body: "Comment with compiled 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 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 hide match-list comments when workflow ID is not set", async () => {
const addCommentScript = fs.readFileSync(path.join(__dirname, "add_comment.cjs"), "utf8");
const originalWorkflowId = process.env.GH_AW_WORKFLOW_ID;
delete process.env.GH_AW_WORKFLOW_ID;

try {
let hiddenNodeIds = [];
mockGithub.rest.issues.listComments = async () => ({
data: [
{
id: 11,
node_id: "IC_kwDOTest11",
body: "Target\n\n<!-- gh-aw-workflow-id: target-wf -->",
},
{
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: ["target-wf"] } }); })()`);

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

expect(result.success).toBe(true);
expect(hiddenNodeIds).toEqual(["IC_kwDOTest11"]);
} 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
40 changes: 40 additions & 0 deletions docs/adr/37977-object-form-hide-older-comments-with-match-list.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
# ADR-37977: Object Form for `hide-older-comments` with Multi-Workflow Match List

**Date**: 2026-06-09
**Status**: Draft

## Context

The `add-comment` safe output supports a `hide-older-comments` option that minimizes previous comments from the *same* agentic workflow before posting a new one, identified by a tracker-id embedded in the comment body. Some workflows emit multiple distinct comments per run, and other workflows post comments that conceptually belong to the same logical cleanup set. Because the existing boolean form only ever matches the currently-running workflow's ID, those additional comments cannot be minimized in the same pass, forcing brittle downstream cleanup logic. We need a way to declare additional workflow IDs whose older comments should also be hidden, without breaking the widely-used boolean configuration.

## Decision

We will extend `hide-older-comments` to accept an **object form** alongside the existing boolean form. The object supports two keys: `enabled` (templatable boolean, defaulting to `true` when omitted) and `match` (an array of workflow-id strings). At runtime the handler hides older comments whose tracker-id exactly matches any ID in the set composed of the current workflow ID plus the configured `match` entries. Match entries are normalized (trimmed, empty-filtered, de-duplicated) before use. The existing boolean form is preserved unchanged for backward compatibility, and the JSON schema becomes a `oneOf` of the templatable boolean and the object. Matching remains **exact, full-string** — not substring or pattern based — so only intentionally named workflows are affected.

## Alternatives Considered

### Alternative 1: Flat top-level `hide-older-comments-match` field
Expose the match list as a sibling top-level field rather than nesting it inside an object. This avoids the `oneOf` schema and keeps the boolean form untouched, but it scatters related configuration across two keys, allows nonsensical states (a match list with hiding disabled), and reads less cohesively. The object form groups the toggle and its modifiers together, so it was preferred. (Internally the compiler still flattens `match` to a `hide-older-comments-match` field for handler plumbing, but the authored surface stays grouped.)

### Alternative 2: Substring or pattern matching of workflow IDs
Allow `match` entries to match workflow IDs by prefix, substring, or glob. This would be more flexible for families of related workflows, but it risks silently minimizing unrelated comments whose IDs happen to overlap, which is hard to debug and irreversible from the user's view. Exact full-string matching was chosen to keep the behavior predictable and safe.

## Consequences

### Positive
- A single `add-comment` run can minimize older comments from multiple explicitly-named workflows, removing brittle downstream cleanup.
- Fully backward compatible: existing boolean `hide-older-comments` configurations are unchanged in meaning and schema validity.
- Exact full-string matching keeps hiding behavior predictable and limits blast radius to intentionally listed workflows.

### Negative
- Configuration and parsing complexity increases: the compiler must preprocess an object-or-boolean union, and the JS handler must branch on object vs. boolean shape.
- The schema is now a `oneOf`, which produces less precise validation error messages than a single concrete type.
- `match` values are maintained by hand and can drift if a referenced workflow is renamed, silently reducing what gets hidden.

### Neutral
- The compiled handler config gains a `hide_older_comments_match` string-slice field, and the search functions now accept a list of workflow IDs instead of a single ID.
- Normalization (trim/dedup) happens both at compile time (Go) and runtime (JS), so behavior is consistent regardless of which path supplies the list.

---

*This is a DRAFT ADR generated by the [Design Decision Gate](https://github.com/github/gh-aw/actions/runs/27177131882) workflow. The PR author must review, complete, and finalize this document before the PR can merge.*
Loading
Loading