From 94b6dccc32b89d3082431279c99f2130e9da9168 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Mon, 13 Apr 2026 12:08:28 +0000 Subject: [PATCH 1/3] Initial plan From 60e7474fd17635699c260c5e0697ae9929cddf71 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Mon, 13 Apr 2026 12:31:29 +0000 Subject: [PATCH 2/3] Extract shared close-older marker search/filter logic into close_older_search_helpers.cjs Refactors the duplicated marker-based query construction and exact-marker filtering logic from close_older_issues.cjs and close_older_discussions.cjs into a shared close_older_search_helpers.cjs module with three functions: - buildMarkerSearchQuery: Builds search query and exact marker from closeOlderKey / workflowId / callerWorkflowId parameters - filterByMarker: Filters search results by excluding newly created entities and verifying exact marker presence, with entity-specific additional filters - logFilterSummary: Logs filtering counters in a consistent format Includes 22 new tests with parity tests ensuring issue and discussion paths produce equivalent results. All 69 tests pass (47 existing + 22 new). Agent-Logs-Url: https://github.com/github/gh-aw/sessions/d6fb4b1e-560e-434e-af18-c8e3c048bf30 Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> --- actions/setup/js/close_older_discussions.cjs | 122 ++---- actions/setup/js/close_older_issues.cjs | 99 ++--- .../setup/js/close_older_search_helpers.cjs | 141 +++++++ .../js/close_older_search_helpers.test.cjs | 380 ++++++++++++++++++ 4 files changed, 589 insertions(+), 153 deletions(-) create mode 100644 actions/setup/js/close_older_search_helpers.cjs create mode 100644 actions/setup/js/close_older_search_helpers.test.cjs diff --git a/actions/setup/js/close_older_discussions.cjs b/actions/setup/js/close_older_discussions.cjs index 1af26e6ed59..a79ae78e293 100644 --- a/actions/setup/js/close_older_discussions.cjs +++ b/actions/setup/js/close_older_discussions.cjs @@ -3,9 +3,9 @@ const { getCloseOlderDiscussionMessage } = require("./messages_close_discussion.cjs"); const { getErrorMessage } = require("./error_helpers.cjs"); -const { getWorkflowIdMarkerContent, generateWorkflowIdMarker, generateWorkflowCallIdMarker, generateCloseKeyMarker, getCloseKeyMarkerContent } = require("./generate_footer.cjs"); const { sanitizeContent } = require("./sanitize_content.cjs"); const { closeOlderEntities, MAX_CLOSE_COUNT: SHARED_MAX_CLOSE_COUNT } = require("./close_older_entities.cjs"); +const { buildMarkerSearchQuery, filterByMarker, logFilterSummary } = require("./close_older_search_helpers.cjs"); /** * Maximum number of older discussions to close @@ -44,27 +44,13 @@ async function searchOlderDiscussions(github, owner, repo, workflowId, categoryI return []; } - // Build GraphQL search query. - // When a close-older-key is provided it becomes the primary search term; otherwise - // fall back to the workflow-id marker. - let searchQuery; - let exactMarker; - if (closeOlderKey) { - const closeKeyMarkerContent = getCloseKeyMarkerContent(closeOlderKey); - const escapedMarker = closeKeyMarkerContent.replace(/"/g, '\\"'); - searchQuery = `repo:${owner}/${repo} is:open "${escapedMarker}" in:body`; - exactMarker = generateCloseKeyMarker(closeOlderKey); - core.info(` Using close-older-key for search: "${escapedMarker}" in:body`); - } else { - // Build GraphQL search query - // Search for open discussions with the workflow-id marker in the body - const workflowIdMarker = getWorkflowIdMarkerContent(workflowId); - // Escape quotes in workflow ID to prevent query injection - const escapedMarker = workflowIdMarker.replace(/"/g, '\\"'); - searchQuery = `repo:${owner}/${repo} is:open "${escapedMarker}" in:body`; - exactMarker = callerWorkflowId ? generateWorkflowCallIdMarker(callerWorkflowId) : generateWorkflowIdMarker(workflowId); - core.info(` Added workflow ID marker filter to query: "${escapedMarker}" in:body`); - } + const { searchQuery, exactMarker } = buildMarkerSearchQuery({ + owner, + repo, + workflowId, + callerWorkflowId, + closeOlderKey, + }); core.info(`Executing GitHub search with query: ${searchQuery}`); const result = await github.graphql( @@ -96,73 +82,39 @@ async function searchOlderDiscussions(github, owner, repo, workflowId, categoryI return []; } - // Filter results: - // 1. Must not be the excluded discussion (newly created one) - // 2. Must not be already closed - // 3. If categoryId is specified, must match - // 4. Body must contain the exact marker. When closeOlderKey is set the close-key marker - // is used. Otherwise, when callerWorkflowId is set, match `gh-aw-workflow-call-id` so - // that callers sharing the same reusable workflow do not close each other's discussions. - // Fall back to `gh-aw-workflow-id` for backward compat with older discussions. core.info("Filtering search results..."); - let filteredCount = 0; - let excludedCount = 0; - let closedCount = 0; - let markerMismatchCount = 0; - const filtered = result.search.nodes - .filter( - /** @param {any} d */ d => { - if (!d) { - return false; - } - - // Exclude the newly created discussion - if (d.number === excludeNumber) { - excludedCount++; - core.info(` Excluding discussion #${d.number} (the newly created discussion)`); - return false; - } - - // Exclude already closed discussions - if (d.closed) { - closedCount++; - return false; - } - - // Check category if specified - if (categoryId && (!d.category || d.category.id !== categoryId)) { - return false; - } - - // Exact-match the marker in the discussion body to prevent GitHub search - // substring tokenization from matching related workflow IDs - // (e.g. "foo" would otherwise match discussions from "foo-bar") - if (!d.body?.includes(exactMarker)) { - markerMismatchCount++; - core.info(` Excluding discussion #${d.number} (body does not contain exact marker)`); - return false; - } - - filteredCount++; - core.info(` ✓ Discussion #${d.number} matches criteria: ${d.title}`); - return true; + const { filtered: filteredItems, counters } = filterByMarker({ + items: result.search.nodes, + excludeNumber, + exactMarker, + entityType: "discussion", + additionalFilter: (d, extra) => { + if (d.closed) { + extra.closedCount = (extra.closedCount || 0) + 1; + return false; + } + if (categoryId && (!d.category || d.category.id !== categoryId)) { + return false; } - ) - .map( - /** @param {any} d */ d => ({ - id: d.id, - number: d.number, - title: d.title, - url: d.url, - }) - ); + return true; + }, + }); + + const filtered = filteredItems.map( + /** @param {any} d */ d => ({ + id: d.id, + number: d.number, + title: d.title, + url: d.url, + }) + ); - core.info(`Filtering complete:`); - core.info(` - Matched discussions: ${filteredCount}`); - core.info(` - Excluded new discussion: ${excludedCount}`); - core.info(` - Excluded closed discussions: ${closedCount}`); - core.info(` - Excluded marker mismatch: ${markerMismatchCount}`); + logFilterSummary({ + entityTypePlural: "discussions", + counters, + extraLabels: [["closedCount", "Excluded closed discussions"]], + }); return filtered; } diff --git a/actions/setup/js/close_older_issues.cjs b/actions/setup/js/close_older_issues.cjs index d225684df54..e7b51be8d98 100644 --- a/actions/setup/js/close_older_issues.cjs +++ b/actions/setup/js/close_older_issues.cjs @@ -1,9 +1,9 @@ // @ts-check /// -const { getWorkflowIdMarkerContent, generateWorkflowIdMarker, generateWorkflowCallIdMarker, generateCloseKeyMarker, getCloseKeyMarkerContent } = require("./generate_footer.cjs"); const { sanitizeContent } = require("./sanitize_content.cjs"); const { closeOlderEntities, MAX_CLOSE_COUNT: SHARED_MAX_CLOSE_COUNT } = require("./close_older_entities.cjs"); +const { buildMarkerSearchQuery, filterByMarker, logFilterSummary } = require("./close_older_search_helpers.cjs"); /** * Maximum number of older issues to close @@ -41,26 +41,14 @@ async function searchOlderIssues(github, owner, repo, workflowId, excludeNumber, return []; } - // Build REST API search query. - // When a close-older-key is provided it becomes the primary search term; otherwise - // fall back to the workflow-id marker. - let searchQuery; - let exactMarker; - if (closeOlderKey) { - const closeKeyMarkerContent = getCloseKeyMarkerContent(closeOlderKey); - const escapedMarker = closeKeyMarkerContent.replace(/"/g, '\\"'); - searchQuery = `repo:${owner}/${repo} is:issue is:open "${escapedMarker}" in:body`; - exactMarker = generateCloseKeyMarker(closeOlderKey); - core.info(` Using close-older-key for search: "${escapedMarker}" in:body`); - } else { - // Search for open issues with the workflow-id marker in the body - const workflowIdMarker = getWorkflowIdMarkerContent(workflowId); - // Escape quotes in workflow ID to prevent query injection - const escapedMarker = workflowIdMarker.replace(/"/g, '\\"'); - searchQuery = `repo:${owner}/${repo} is:issue is:open "${escapedMarker}" in:body`; - exactMarker = callerWorkflowId ? generateWorkflowCallIdMarker(callerWorkflowId) : generateWorkflowIdMarker(workflowId); - core.info(` Added workflow-id marker filter to query: "${escapedMarker}" in:body`); - } + const { searchQuery, exactMarker } = buildMarkerSearchQuery({ + owner, + repo, + workflowId, + callerWorkflowId, + closeOlderKey, + entityQualifier: "is:issue", + }); core.info(`Executing GitHub search with query: ${searchQuery}`); const result = await github.rest.search.issuesAndPullRequests({ @@ -75,60 +63,35 @@ async function searchOlderIssues(github, owner, repo, workflowId, excludeNumber, return []; } - // Filter results: - // 1. Must not be the excluded issue (newly created one) - // 2. Must not be a pull request - // 3. Body must contain the exact marker. When closeOlderKey is set the close-key marker - // is used. Otherwise, when callerWorkflowId is set, match `gh-aw-workflow-call-id` so - // that callers sharing the same reusable workflow do not close each other's issues. - // Fall back to `gh-aw-workflow-id` for backward compat with older issues. core.info("Filtering search results..."); - let filteredCount = 0; - let pullRequestCount = 0; - let excludedCount = 0; - let markerMismatchCount = 0; - const filtered = result.data.items - .filter(item => { - // Exclude pull requests + const { filtered: filteredItems, counters } = filterByMarker({ + items: result.data.items, + excludeNumber, + exactMarker, + entityType: "issue", + additionalFilter: (item, extra) => { if (item.pull_request) { - pullRequestCount++; - return false; - } - - // Exclude the newly created issue - if (item.number === excludeNumber) { - excludedCount++; - core.info(` Excluding issue #${item.number} (the newly created issue)`); - return false; - } - - // Exact-match the marker in the issue body to prevent GitHub search - // substring tokenization from matching related workflow IDs - // (e.g. "foo" would otherwise match issues from "foo-bar") - if (!item.body?.includes(exactMarker)) { - markerMismatchCount++; - core.info(` Excluding issue #${item.number} (body does not contain exact marker)`); + extra.pullRequestCount = (extra.pullRequestCount || 0) + 1; return false; } - - filteredCount++; - core.info(` ✓ Issue #${item.number} matches criteria: ${item.title}`); return true; - }) - .map(item => ({ - number: item.number, - title: item.title, - html_url: item.html_url, - labels: item.labels || [], - created_at: item.created_at, - })); + }, + }); - core.info(`Filtering complete:`); - core.info(` - Matched issues: ${filteredCount}`); - core.info(` - Excluded pull requests: ${pullRequestCount}`); - core.info(` - Excluded new issue: ${excludedCount}`); - core.info(` - Excluded marker mismatch: ${markerMismatchCount}`); + const filtered = filteredItems.map(item => ({ + number: item.number, + title: item.title, + html_url: item.html_url, + labels: item.labels || [], + created_at: item.created_at, + })); + + logFilterSummary({ + entityTypePlural: "issues", + counters, + extraLabels: [["pullRequestCount", "Excluded pull requests"]], + }); return filtered; } diff --git a/actions/setup/js/close_older_search_helpers.cjs b/actions/setup/js/close_older_search_helpers.cjs new file mode 100644 index 00000000000..61c43f87ce3 --- /dev/null +++ b/actions/setup/js/close_older_search_helpers.cjs @@ -0,0 +1,141 @@ +// @ts-check +/// + +const { getWorkflowIdMarkerContent, generateWorkflowIdMarker, generateWorkflowCallIdMarker, generateCloseKeyMarker, getCloseKeyMarkerContent } = require("./generate_footer.cjs"); + +/** + * Build the search query string and the exact marker used for body-level + * filtering after the GitHub search API returns results. + * + * The logic is shared between the REST (issues) and GraphQL (discussions) + * search paths – only the `entityQualifier` differs (e.g. `"is:issue"` for + * issues, or `""` for discussions which don't need one). + * + * @param {object} params + * @param {string} params.owner - Repository owner + * @param {string} params.repo - Repository name + * @param {string} params.workflowId - Workflow ID to match in the marker + * @param {string} [params.callerWorkflowId] - Optional calling workflow identity + * @param {string} [params.closeOlderKey] - Optional explicit deduplication key + * @param {string} [params.entityQualifier] - Extra qualifier appended to the query (e.g. "is:issue") + * @returns {{ searchQuery: string, exactMarker: string }} + */ +function buildMarkerSearchQuery({ owner, repo, workflowId, callerWorkflowId, closeOlderKey, entityQualifier }) { + const qualifierSegment = entityQualifier ? ` ${entityQualifier}` : ""; + + if (closeOlderKey) { + const closeKeyMarkerContent = getCloseKeyMarkerContent(closeOlderKey); + const escapedMarker = closeKeyMarkerContent.replace(/"/g, '\\"'); + const searchQuery = `repo:${owner}/${repo}${qualifierSegment} is:open "${escapedMarker}" in:body`; + const exactMarker = generateCloseKeyMarker(closeOlderKey); + core.info(` Using close-older-key for search: "${escapedMarker}" in:body`); + return { searchQuery, exactMarker }; + } + + const workflowIdMarker = getWorkflowIdMarkerContent(workflowId); + const escapedMarker = workflowIdMarker.replace(/"/g, '\\"'); + const searchQuery = `repo:${owner}/${repo}${qualifierSegment} is:open "${escapedMarker}" in:body`; + const exactMarker = callerWorkflowId ? generateWorkflowCallIdMarker(callerWorkflowId) : generateWorkflowIdMarker(workflowId); + core.info(` Added workflow-id marker filter to query: "${escapedMarker}" in:body`); + return { searchQuery, exactMarker }; +} + +/** + * @typedef {Object} SearchResultItem + * @property {number} number - Entity number + * @property {string} [body] - Entity body text + */ + +/** + * @typedef {Object} FilterCounters + * @property {number} filteredCount - Number of entities that passed all filters + * @property {number} excludedCount - Number of entities excluded as the "new" entity + * @property {number} markerMismatchCount - Number of entities excluded due to marker mismatch + */ + +/** + * Filter search results by excluding the newly created entity and verifying + * the exact marker is present in the body. Entity-specific additional filters + * (e.g. pull-request exclusion for issues, closed/category checks for + * discussions) are handled by the optional `additionalFilter` callback. + * + * @param {object} params + * @param {Array} params.items - Raw search result items + * @param {number} params.excludeNumber - Entity number to exclude (the newly created one) + * @param {string} params.exactMarker - Exact marker string that must appear in the body + * @param {string} params.entityType - Entity type name for logging (e.g. "issue", "discussion") + * @param {(item: any, counters: Record) => boolean} [params.additionalFilter] - + * Optional callback for entity-specific filtering. Return `true` to keep the item. + * The `counters` object can be mutated to track extra exclusion reasons. + * @returns {{ filtered: Array, counters: FilterCounters & Record }} + */ +function filterByMarker({ items, excludeNumber, exactMarker, entityType, additionalFilter }) { + let filteredCount = 0; + let excludedCount = 0; + let markerMismatchCount = 0; + /** @type {Record} */ + const extraCounters = {}; + + const filtered = items.filter(item => { + if (!item) { + return false; + } + + // Run entity-specific filters first (e.g. pull_request, closed, category) + if (additionalFilter && !additionalFilter(item, extraCounters)) { + return false; + } + + // Exclude the newly created entity + if (item.number === excludeNumber) { + excludedCount++; + core.info(` Excluding ${entityType} #${item.number} (the newly created ${entityType})`); + return false; + } + + // Exact-match the marker in the body to prevent GitHub search + // substring tokenization from matching related workflow IDs + // (e.g. "foo" would otherwise match entities from "foo-bar") + if (!item.body?.includes(exactMarker)) { + markerMismatchCount++; + core.info(` Excluding ${entityType} #${item.number} (body does not contain exact marker)`); + return false; + } + + filteredCount++; + core.info(` ✓ ${entityType.charAt(0).toUpperCase() + entityType.slice(1)} #${item.number} matches criteria: ${item.title}`); + return true; + }); + + return { + filtered, + counters: { filteredCount, excludedCount, markerMismatchCount, ...extraCounters }, + }; +} + +/** + * Log the filtering summary counters. + * + * @param {object} params + * @param {string} params.entityTypePlural - Plural entity name (e.g. "issues") + * @param {FilterCounters & Record} params.counters - Counters from filterByMarker + * @param {Array<[string, string]>} [params.extraLabels] - Additional counter labels to log + * as `[counterKey, label]` pairs (e.g. `[["pullRequestCount", "Excluded pull requests"]]`) + */ +function logFilterSummary({ entityTypePlural, counters, extraLabels }) { + core.info(`Filtering complete:`); + core.info(` - Matched ${entityTypePlural}: ${counters.filteredCount}`); + if (extraLabels) { + for (const [key, label] of extraLabels) { + core.info(` - ${label}: ${counters[key] || 0}`); + } + } + core.info(` - Excluded new ${entityTypePlural.slice(0, -1)}: ${counters.excludedCount}`); + core.info(` - Excluded marker mismatch: ${counters.markerMismatchCount}`); +} + +module.exports = { + buildMarkerSearchQuery, + filterByMarker, + logFilterSummary, +}; diff --git a/actions/setup/js/close_older_search_helpers.test.cjs b/actions/setup/js/close_older_search_helpers.test.cjs new file mode 100644 index 00000000000..b79ef3a935f --- /dev/null +++ b/actions/setup/js/close_older_search_helpers.test.cjs @@ -0,0 +1,380 @@ +// @ts-check + +import { describe, it, expect, beforeEach, vi } from "vitest"; +import { buildMarkerSearchQuery, filterByMarker, logFilterSummary } from "./close_older_search_helpers.cjs"; + +// Mock globals +global.core = { + info: vi.fn(), + warning: vi.fn(), + error: vi.fn(), +}; + +describe("close_older_search_helpers", () => { + beforeEach(() => { + vi.clearAllMocks(); + }); + + describe("buildMarkerSearchQuery", () => { + it("should build query with close-older-key when provided", () => { + const { searchQuery, exactMarker } = buildMarkerSearchQuery({ + owner: "owner", + repo: "repo", + workflowId: "test-workflow", + closeOlderKey: "my-stable-key", + }); + + expect(searchQuery).toBe('repo:owner/repo is:open "gh-aw-close-key: my-stable-key" in:body'); + expect(exactMarker).toBe(""); + }); + + it("should build query with workflow-id when no close-older-key", () => { + const { searchQuery, exactMarker } = buildMarkerSearchQuery({ + owner: "owner", + repo: "repo", + workflowId: "test-workflow", + }); + + expect(searchQuery).toBe('repo:owner/repo is:open "gh-aw-workflow-id: test-workflow" in:body'); + expect(exactMarker).toBe(""); + }); + + it("should use callerWorkflowId for exact marker when provided", () => { + const { searchQuery, exactMarker } = buildMarkerSearchQuery({ + owner: "owner", + repo: "repo", + workflowId: "my-reusable-workflow", + callerWorkflowId: "owner/repo/CallerA", + }); + + expect(searchQuery).toBe('repo:owner/repo is:open "gh-aw-workflow-id: my-reusable-workflow" in:body'); + expect(exactMarker).toBe(""); + }); + + it("should append entityQualifier when provided", () => { + const { searchQuery } = buildMarkerSearchQuery({ + owner: "owner", + repo: "repo", + workflowId: "test-workflow", + entityQualifier: "is:issue", + }); + + expect(searchQuery).toBe('repo:owner/repo is:issue is:open "gh-aw-workflow-id: test-workflow" in:body'); + }); + + it("should append entityQualifier with close-older-key", () => { + const { searchQuery } = buildMarkerSearchQuery({ + owner: "owner", + repo: "repo", + workflowId: "test-workflow", + closeOlderKey: "my-key", + entityQualifier: "is:issue", + }); + + expect(searchQuery).toBe('repo:owner/repo is:issue is:open "gh-aw-close-key: my-key" in:body'); + }); + + it("should prefer close-older-key over callerWorkflowId when both are provided", () => { + const { searchQuery, exactMarker } = buildMarkerSearchQuery({ + owner: "owner", + repo: "repo", + workflowId: "my-workflow", + callerWorkflowId: "owner/repo/CallerA", + closeOlderKey: "shared-key", + }); + + expect(searchQuery).toContain("gh-aw-close-key: shared-key"); + expect(exactMarker).toBe(""); + }); + + it("should escape quotes in workflow ID to prevent query injection", () => { + const { searchQuery } = buildMarkerSearchQuery({ + owner: "owner", + repo: "repo", + workflowId: 'workflow"with"quotes', + }); + + expect(searchQuery).toContain('workflow\\"with\\"quotes'); + }); + + it("should escape quotes in close-older-key to prevent query injection", () => { + const { searchQuery } = buildMarkerSearchQuery({ + owner: "owner", + repo: "repo", + workflowId: "test-workflow", + closeOlderKey: 'key"with"quotes', + }); + + expect(searchQuery).toContain('key\\"with\\"quotes'); + }); + }); + + describe("filterByMarker", () => { + it("should exclude items matching excludeNumber", () => { + const items = [ + { number: 1, body: "", title: "Item 1" }, + { number: 2, body: "", title: "Item 2" }, + ]; + + const { filtered, counters } = filterByMarker({ + items, + excludeNumber: 1, + exactMarker: "", + entityType: "issue", + }); + + expect(filtered).toHaveLength(1); + expect(filtered[0].number).toBe(2); + expect(counters.excludedCount).toBe(1); + }); + + it("should exclude items without exact marker in body", () => { + const items = [ + { number: 1, body: "", title: "Match" }, + { number: 2, body: "", title: "No match" }, + { number: 3, body: "No marker at all", title: "No marker" }, + ]; + + const { filtered, counters } = filterByMarker({ + items, + excludeNumber: 999, + exactMarker: "", + entityType: "issue", + }); + + expect(filtered).toHaveLength(1); + expect(filtered[0].number).toBe(1); + expect(counters.markerMismatchCount).toBe(2); + }); + + it("should skip null/undefined items", () => { + const items = [null, undefined, { number: 1, body: "", title: "Valid" }]; + + const { filtered } = filterByMarker({ + items, + excludeNumber: 999, + exactMarker: "", + entityType: "discussion", + }); + + expect(filtered).toHaveLength(1); + expect(filtered[0].number).toBe(1); + }); + + it("should apply additionalFilter before standard checks", () => { + const items = [ + { number: 1, body: "", title: "Issue", pull_request: undefined }, + { number: 2, body: "", title: "PR", pull_request: {} }, + ]; + + const { filtered, counters } = filterByMarker({ + items, + excludeNumber: 999, + exactMarker: "", + entityType: "issue", + additionalFilter: (item, extra) => { + if (item.pull_request) { + extra.pullRequestCount = (extra.pullRequestCount || 0) + 1; + return false; + } + return true; + }, + }); + + expect(filtered).toHaveLength(1); + expect(filtered[0].number).toBe(1); + expect(counters.pullRequestCount).toBe(1); + }); + + it("should handle items with missing body gracefully", () => { + const items = [{ number: 1, title: "No body" }]; + + const { filtered, counters } = filterByMarker({ + items, + excludeNumber: 999, + exactMarker: "", + entityType: "issue", + }); + + expect(filtered).toHaveLength(0); + expect(counters.markerMismatchCount).toBe(1); + }); + + it("should return all matching items when no exclusions apply", () => { + const items = [ + { number: 1, body: "Content more", title: "Item 1" }, + { number: 2, body: "Other text", title: "Item 2" }, + ]; + + const { filtered, counters } = filterByMarker({ + items, + excludeNumber: 999, + exactMarker: "", + entityType: "issue", + }); + + expect(filtered).toHaveLength(2); + expect(counters.filteredCount).toBe(2); + expect(counters.excludedCount).toBe(0); + expect(counters.markerMismatchCount).toBe(0); + }); + + it("should work with discussion-specific additional filters", () => { + const items = [ + { number: 1, body: "", title: "Open", closed: false, category: { id: "CAT1" } }, + { number: 2, body: "", title: "Closed", closed: true, category: { id: "CAT1" } }, + { number: 3, body: "", title: "Wrong cat", closed: false, category: { id: "CAT2" } }, + ]; + + const categoryId = "CAT1"; + const { filtered, counters } = filterByMarker({ + items, + excludeNumber: 999, + exactMarker: "", + entityType: "discussion", + additionalFilter: (d, extra) => { + if (d.closed) { + extra.closedCount = (extra.closedCount || 0) + 1; + return false; + } + if (categoryId && (!d.category || d.category.id !== categoryId)) { + return false; + } + return true; + }, + }); + + expect(filtered).toHaveLength(1); + expect(filtered[0].number).toBe(1); + expect(counters.closedCount).toBe(1); + }); + }); + + describe("logFilterSummary", () => { + it("should log basic summary without extra labels", () => { + logFilterSummary({ + entityTypePlural: "issues", + counters: { filteredCount: 5, excludedCount: 1, markerMismatchCount: 2 }, + }); + + expect(global.core.info).toHaveBeenCalledWith("Filtering complete:"); + expect(global.core.info).toHaveBeenCalledWith(" - Matched issues: 5"); + expect(global.core.info).toHaveBeenCalledWith(" - Excluded new issue: 1"); + expect(global.core.info).toHaveBeenCalledWith(" - Excluded marker mismatch: 2"); + }); + + it("should log extra labels when provided", () => { + logFilterSummary({ + entityTypePlural: "issues", + counters: { filteredCount: 3, excludedCount: 0, markerMismatchCount: 1, pullRequestCount: 2 }, + extraLabels: [["pullRequestCount", "Excluded pull requests"]], + }); + + expect(global.core.info).toHaveBeenCalledWith(" - Excluded pull requests: 2"); + }); + + it("should log zero for missing extra counter keys", () => { + logFilterSummary({ + entityTypePlural: "discussions", + counters: { filteredCount: 3, excludedCount: 0, markerMismatchCount: 0 }, + extraLabels: [["closedCount", "Excluded closed discussions"]], + }); + + expect(global.core.info).toHaveBeenCalledWith(" - Excluded closed discussions: 0"); + }); + }); + + describe("parity: issues and discussions produce equivalent queries", () => { + it("should produce equivalent queries for close-older-key (issue vs discussion)", () => { + const issueResult = buildMarkerSearchQuery({ + owner: "owner", + repo: "repo", + workflowId: "test-workflow", + closeOlderKey: "my-key", + entityQualifier: "is:issue", + }); + + const discussionResult = buildMarkerSearchQuery({ + owner: "owner", + repo: "repo", + workflowId: "test-workflow", + closeOlderKey: "my-key", + }); + + // Both should use the same exact marker + expect(issueResult.exactMarker).toBe(discussionResult.exactMarker); + // Queries should differ only in the entity qualifier + expect(issueResult.searchQuery).toContain("is:issue"); + expect(discussionResult.searchQuery).not.toContain("is:issue"); + // Both should contain the close-key marker + expect(issueResult.searchQuery).toContain("gh-aw-close-key: my-key"); + expect(discussionResult.searchQuery).toContain("gh-aw-close-key: my-key"); + }); + + it("should produce equivalent queries for workflow-id (issue vs discussion)", () => { + const issueResult = buildMarkerSearchQuery({ + owner: "owner", + repo: "repo", + workflowId: "test-workflow", + entityQualifier: "is:issue", + }); + + const discussionResult = buildMarkerSearchQuery({ + owner: "owner", + repo: "repo", + workflowId: "test-workflow", + }); + + expect(issueResult.exactMarker).toBe(discussionResult.exactMarker); + expect(issueResult.searchQuery).toContain("is:issue"); + expect(discussionResult.searchQuery).not.toContain("is:issue"); + }); + + it("should produce equivalent exact markers for callerWorkflowId (issue vs discussion)", () => { + const issueResult = buildMarkerSearchQuery({ + owner: "owner", + repo: "repo", + workflowId: "my-reusable", + callerWorkflowId: "org/repo/Caller", + entityQualifier: "is:issue", + }); + + const discussionResult = buildMarkerSearchQuery({ + owner: "owner", + repo: "repo", + workflowId: "my-reusable", + callerWorkflowId: "org/repo/Caller", + }); + + expect(issueResult.exactMarker).toBe(discussionResult.exactMarker); + expect(issueResult.exactMarker).toBe(""); + }); + + it("should apply filterByMarker identically for issues and discussions", () => { + const sharedItems = [ + { number: 1, body: "", title: "A" }, + { number: 2, body: "", title: "B" }, + { number: 3, body: "", title: "C" }, + ]; + + const issueResult = filterByMarker({ + items: sharedItems, + excludeNumber: 3, + exactMarker: "", + entityType: "issue", + }); + + const discResult = filterByMarker({ + items: sharedItems, + excludeNumber: 3, + exactMarker: "", + entityType: "discussion", + }); + + expect(issueResult.filtered.map(i => i.number)).toEqual(discResult.filtered.map(i => i.number)); + expect(issueResult.counters.filteredCount).toBe(discResult.counters.filteredCount); + expect(issueResult.counters.excludedCount).toBe(discResult.counters.excludedCount); + expect(issueResult.counters.markerMismatchCount).toBe(discResult.counters.markerMismatchCount); + }); + }); +}); From f3aae0e8df213a3d15bd8b6b9506a993a48a130d Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Mon, 13 Apr 2026 14:08:14 +0000 Subject: [PATCH 3/3] fix: check excludeNumber before additionalFilter in filterByMarker Reorders the filter checks so the newly-created entity exclusion runs before entity-specific additional filters (e.g. closed/category). This preserves the original discussion-handler semantics and ensures counters reliably attribute the new entity to the dedicated exclusion bucket. Agent-Logs-Url: https://github.com/github/gh-aw/sessions/0ff7ecc5-4718-44ce-b2dc-2895008ca534 Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> --- actions/setup/js/close_older_search_helpers.cjs | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/actions/setup/js/close_older_search_helpers.cjs b/actions/setup/js/close_older_search_helpers.cjs index 61c43f87ce3..d37fe1c9095 100644 --- a/actions/setup/js/close_older_search_helpers.cjs +++ b/actions/setup/js/close_older_search_helpers.cjs @@ -81,18 +81,19 @@ function filterByMarker({ items, excludeNumber, exactMarker, entityType, additio return false; } - // Run entity-specific filters first (e.g. pull_request, closed, category) - if (additionalFilter && !additionalFilter(item, extraCounters)) { - return false; - } - - // Exclude the newly created entity + // Exclude the newly created entity before running any other filters so + // counters/logs consistently attribute this item to the dedicated exclusion. if (item.number === excludeNumber) { excludedCount++; core.info(` Excluding ${entityType} #${item.number} (the newly created ${entityType})`); return false; } + // Run entity-specific filters next (e.g. pull_request, closed, category) + if (additionalFilter && !additionalFilter(item, extraCounters)) { + return false; + } + // Exact-match the marker in the body to prevent GitHub search // substring tokenization from matching related workflow IDs // (e.g. "foo" would otherwise match entities from "foo-bar")