Skip to content

Extract shared helper for close-older entity flows #15933

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 2 commits into from
Feb 15, 2026
Merged

Extract shared helper for close-older entity flows #15933

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
3458c15
Initial plan
Copilot Feb 15, 2026
f2afd35
Refactor close_older_* functions to use shared helper
Copilot Feb 15, 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
150 changes: 40 additions & 110 deletions actions/setup/js/close_older_discussions.cjs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,26 +5,18 @@ const { getCloseOlderDiscussionMessage } = require("./messages_close_discussion.
const { getErrorMessage } = require("./error_helpers.cjs");
Copy link

Copilot AI Feb 15, 2026

Choose a reason for hiding this comment

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

getErrorMessage is imported but never used in this refactored file (error formatting now happens inside close_older_entities.cjs). Please remove the unused require to avoid dead code and keep lint/ts-check clean.

Suggested change
const { getErrorMessage } = require("./error_helpers.cjs");

Copilot uses AI. Check for mistakes.
const { getWorkflowIdMarkerContent } = require("./generate_footer.cjs");
const { sanitizeContent } = require("./sanitize_content.cjs");
const { closeOlderEntities, MAX_CLOSE_COUNT: SHARED_MAX_CLOSE_COUNT } = require("./close_older_entities.cjs");

/**
* Maximum number of older discussions to close
*/
const MAX_CLOSE_COUNT = 10;
const MAX_CLOSE_COUNT = SHARED_MAX_CLOSE_COUNT;

/**
* Delay between GraphQL API calls in milliseconds to avoid rate limiting
*/
const GRAPHQL_DELAY_MS = 500;

/**
* Delay execution for a specified number of milliseconds
* @param {number} ms - Milliseconds to delay
* @returns {Promise<void>}
*/
function delay(ms) {
return new Promise(resolve => setTimeout(resolve, ms));
}

/**
* Search for open discussions with a matching workflow-id marker
* @param {any} github - GitHub GraphQL instance
Expand Down Expand Up @@ -142,11 +134,13 @@ async function searchOlderDiscussions(github, owner, repo, workflowId, categoryI
/**
* Add comment to a GitHub Discussion using GraphQL
* @param {any} github - GitHub GraphQL instance
* @param {string} owner - Repository owner (unused for GraphQL, but kept for consistency)
* @param {string} repo - Repository name (unused for GraphQL, but kept for consistency)
* @param {string} discussionId - Discussion node ID
* @param {string} message - Comment body
* @returns {Promise<{id: string, url: string}>} Comment details
*/
async function addDiscussionComment(github, discussionId, message) {
async function addDiscussionComment(github, owner, repo, discussionId, message) {
const result = await github.graphql(
`
mutation($dId: ID!, $body: String!) {
Expand All @@ -166,10 +160,12 @@ async function addDiscussionComment(github, discussionId, message) {
/**
* Close a GitHub Discussion as OUTDATED using GraphQL
* @param {any} github - GitHub GraphQL instance
* @param {string} owner - Repository owner (unused for GraphQL, but kept for consistency)
* @param {string} repo - Repository name (unused for GraphQL, but kept for consistency)
* @param {string} discussionId - Discussion node ID
* @returns {Promise<{id: string, url: string}>} Discussion details
*/
async function closeDiscussionAsOutdated(github, discussionId) {
async function closeDiscussionAsOutdated(github, owner, repo, discussionId) {
const result = await github.graphql(
`
mutation($dId: ID!) {
Expand Down Expand Up @@ -199,105 +195,39 @@ async function closeDiscussionAsOutdated(github, discussionId) {
* @returns {Promise<Array<{number: number, url: string}>>} List of closed discussions
*/
async function closeOlderDiscussions(github, owner, repo, workflowId, categoryId, newDiscussion, workflowName, runUrl) {
core.info("=".repeat(70));
core.info("Starting closeOlderDiscussions operation");
core.info("=".repeat(70));

core.info(`Search criteria: workflow ID marker: "${getWorkflowIdMarkerContent(workflowId)}"`);
core.info(`New discussion reference: #${newDiscussion.number} (${newDiscussion.url})`);
core.info(`Workflow: ${workflowName}`);
core.info(`Run URL: ${runUrl}`);
core.info("");

const olderDiscussions = await searchOlderDiscussions(github, owner, repo, workflowId, categoryId, newDiscussion.number);

if (olderDiscussions.length === 0) {
core.info("✓ No older discussions found to close - operation complete");
core.info("=".repeat(70));
return [];
}

core.info("");
core.info(`Found ${olderDiscussions.length} older discussion(s) matching the criteria`);
for (const discussion of olderDiscussions) {
core.info(` - Discussion #${discussion.number}: ${discussion.title}`);
core.info(` URL: ${discussion.url}`);
}

// Limit to MAX_CLOSE_COUNT discussions
const discussionsToClose = olderDiscussions.slice(0, MAX_CLOSE_COUNT);

if (olderDiscussions.length > MAX_CLOSE_COUNT) {
core.warning("");
core.warning(`⚠️ Found ${olderDiscussions.length} older discussions, but only closing the first ${MAX_CLOSE_COUNT}`);
core.warning(` The remaining ${olderDiscussions.length - MAX_CLOSE_COUNT} discussion(s) will be processed in subsequent runs`);
}

core.info("");
core.info(`Preparing to close ${discussionsToClose.length} discussion(s)...`);
core.info("");

const closedDiscussions = [];

for (let i = 0; i < discussionsToClose.length; i++) {
const discussion = discussionsToClose[i];
core.info("-".repeat(70));
core.info(`Processing discussion ${i + 1}/${discussionsToClose.length}: #${discussion.number}`);
core.info(` Title: ${discussion.title}`);
core.info(` URL: ${discussion.url}`);

try {
// Generate closing message using the messages module
const closingMessage = getCloseOlderDiscussionMessage({
newDiscussionUrl: newDiscussion.url,
newDiscussionNumber: newDiscussion.number,
workflowName,
runUrl,
});

core.info(` Message length: ${closingMessage.length} characters`);
core.info("");

// Add comment first
await addDiscussionComment(github, discussion.id, closingMessage);

// Then close the discussion as outdated
await closeDiscussionAsOutdated(github, discussion.id);

closedDiscussions.push({
number: discussion.number,
url: discussion.url,
});

core.info("");
core.info(`✓ Successfully closed discussion #${discussion.number}`);
} catch (error) {
core.info("");
core.error(`✗ Failed to close discussion #${discussion.number}`);
core.error(` Error: ${getErrorMessage(error)}`);
if (error instanceof Error && error.stack) {
core.error(` Stack trace: ${error.stack}`);
}
// Continue with other discussions even if one fails
}

// Add delay between GraphQL operations to avoid rate limiting (except for the last item)
if (i < discussionsToClose.length - 1) {
core.info("");
core.info(`Waiting ${GRAPHQL_DELAY_MS}ms before processing next discussion to avoid rate limiting...`);
await delay(GRAPHQL_DELAY_MS);
}
}

core.info("");
core.info("=".repeat(70));
core.info(`Closed ${closedDiscussions.length} of ${discussionsToClose.length} discussion(s) successfully`);
if (closedDiscussions.length < discussionsToClose.length) {
core.warning(`Failed to close ${discussionsToClose.length - closedDiscussions.length} discussion(s) - check logs above for details`);
}
core.info("=".repeat(70));
const result = await closeOlderEntities(
github,
owner,
repo,
workflowId,
newDiscussion,
workflowName,
runUrl,
{
entityType: "discussion",
entityTypePlural: "discussions",
searchOlderEntities: searchOlderDiscussions,
getCloseMessage: params =>
getCloseOlderDiscussionMessage({
newDiscussionUrl: params.newEntityUrl,
newDiscussionNumber: params.newEntityNumber,
workflowName: params.workflowName,
runUrl: params.runUrl,
}),
addComment: addDiscussionComment,
closeEntity: closeDiscussionAsOutdated,
delayMs: GRAPHQL_DELAY_MS,
getEntityId: entity => entity.id,
getEntityUrl: entity => entity.url,
},
categoryId // Pass categoryId as extra arg
);

return closedDiscussions;
// Map to discussion-specific return type
return result.map(item => ({
number: item.number,
url: item.url || "",
Copy link

Copilot AI Feb 15, 2026

Choose a reason for hiding this comment

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

This mapping now falls back to an empty string for url, which can silently produce invalid links while the function’s return type indicates url: string. Prefer returning item.url directly (and/or validating it’s present) so unexpected missing URLs don’t get masked.

Suggested change
url: item.url || "",
url: item.url,

Copilot uses AI. Check for mistakes.
}));
}

module.exports = {
Expand Down
169 changes: 169 additions & 0 deletions actions/setup/js/close_older_entities.cjs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,169 @@
// @ts-check
/// <reference types="@actions/github-script" />

const { getErrorMessage } = require("./error_helpers.cjs");
const { getWorkflowIdMarkerContent } = require("./generate_footer.cjs");

/**
* Maximum number of older entities to close
*/
const MAX_CLOSE_COUNT = 10;

/**
* Delay execution for a specified number of milliseconds
* @param {number} ms - Milliseconds to delay
* @returns {Promise<void>}
*/
function delay(ms) {
return new Promise(resolve => setTimeout(resolve, ms));
}

/**
* Configuration for entity-specific behavior
* @typedef {Object} EntityCloseConfig
* @property {string} entityType - Entity type name for logging (e.g., "issue", "discussion")
* @property {string} entityTypePlural - Plural form (e.g., "issues", "discussions")
* @property {(github: any, owner: string, repo: string, workflowId: string, ...args: any[]) => Promise<Array<any>>} searchOlderEntities - Function to search for older entities
* @property {(params: any) => string} getCloseMessage - Function to generate closing message
* @property {(github: any, owner: string, repo: string, entityId: any, message: string) => Promise<{id: any, url?: string, html_url?: string}>} addComment - Function to add comment to entity
* @property {(github: any, owner: string, repo: string, entityId: any) => Promise<{number?: number, id?: string, url?: string, html_url?: string}>} closeEntity - Function to close entity
* @property {number} delayMs - Delay between API operations in milliseconds
* @property {(entity: any) => any} getEntityId - Function to extract entity ID (for API calls)
* @property {(entity: any) => string} getEntityUrl - Function to extract entity URL
*/

/**
* Close older entities that match the workflow-id marker
*
* This function orchestrates the complete flow:
* 1. Search for older entities with matching workflow-id marker
* 2. Log results and handle early exits (no entities found)
* 3. Apply MAX_CLOSE_COUNT limit
* 4. Process each entity: comment + close
* 5. Handle errors gracefully (continue with remaining entities)
* 6. Report summary statistics
*
* @param {any} github - GitHub API instance
* @param {string} owner - Repository owner
* @param {string} repo - Repository name
* @param {string} workflowId - Workflow ID to match in the marker
* @param {any} newEntity - The newly created entity (contains number and url/html_url)
* @param {string} workflowName - Name of the workflow
* @param {string} runUrl - URL of the workflow run
* @param {EntityCloseConfig} config - Entity-specific configuration
* @param {...any} extraArgs - Additional arguments to pass to searchOlderEntities
* @returns {Promise<Array<{number: number, url?: string, html_url?: string}>>} List of closed entities
*/
async function closeOlderEntities(github, owner, repo, workflowId, newEntity, workflowName, runUrl, config, ...extraArgs) {
core.info("=".repeat(70));
core.info(`Starting closeOlder${config.entityType.charAt(0).toUpperCase() + config.entityType.slice(1)}s operation`);
core.info("=".repeat(70));

core.info(`Search criteria: workflow-id marker: "${getWorkflowIdMarkerContent(workflowId)}"`);
core.info(`New ${config.entityType} reference: #${newEntity.number} (${newEntity.url || newEntity.html_url})`);
core.info(`Workflow: ${workflowName}`);
core.info(`Run URL: ${runUrl}`);
core.info("");

// Step 1: Search for older entities
const olderEntities = await config.searchOlderEntities(github, owner, repo, workflowId, ...extraArgs, newEntity.number);

Comment on lines +62 to +70
Copy link

Copilot AI Feb 15, 2026

Choose a reason for hiding this comment

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

The helper claims to be entity-agnostic, but it hard-requires newEntity.number (and passes it to searchOlderEntities) rather than using a config callback. Consider adding something like getNewEntityNumber (or reusing getEntityId semantics) so the helper doesn’t bake in an assumption that every entity has a numeric number field.

This issue also appears on line 78 of the same file.

Copilot uses AI. Check for mistakes.
if (olderEntities.length === 0) {
core.info(`✓ No older ${config.entityTypePlural} found to close - operation complete`);
core.info("=".repeat(70));
return [];
}

core.info("");
core.info(`Found ${olderEntities.length} older ${config.entityType}(s) matching the criteria`);
for (const entity of olderEntities) {
core.info(` - ${config.entityType.charAt(0).toUpperCase() + config.entityType.slice(1)} #${entity.number}: ${entity.title}`);
if (entity.labels) {
core.info(` Labels: ${entity.labels.map(l => l.name).join(", ") || "(none)"}`);
}
core.info(` URL: ${config.getEntityUrl(entity)}`);
}

// Step 2: Limit to MAX_CLOSE_COUNT entities
const entitiesToClose = olderEntities.slice(0, MAX_CLOSE_COUNT);

if (olderEntities.length > MAX_CLOSE_COUNT) {
core.warning("");
core.warning(`⚠️ Found ${olderEntities.length} older ${config.entityTypePlural}, but only closing the first ${MAX_CLOSE_COUNT}`);
core.warning(` The remaining ${olderEntities.length - MAX_CLOSE_COUNT} ${config.entityType}(s) will be processed in subsequent runs`);
}

core.info("");
core.info(`Preparing to close ${entitiesToClose.length} ${config.entityType}(s)...`);
core.info("");

const closedEntities = [];

// Step 3: Process each entity
for (let i = 0; i < entitiesToClose.length; i++) {
const entity = entitiesToClose[i];
core.info("-".repeat(70));
core.info(`Processing ${config.entityType} ${i + 1}/${entitiesToClose.length}: #${entity.number}`);
core.info(` Title: ${entity.title}`);
core.info(` URL: ${config.getEntityUrl(entity)}`);

try {
// Generate closing message
const closingMessage = config.getCloseMessage({
newEntityUrl: newEntity.url || newEntity.html_url,
newEntityNumber: newEntity.number,
workflowName,
runUrl,
});

core.info(` Message length: ${closingMessage.length} characters`);
core.info("");

// Add comment first
await config.addComment(github, owner, repo, config.getEntityId(entity), closingMessage);

// Then close the entity
await config.closeEntity(github, owner, repo, config.getEntityId(entity));

closedEntities.push({
number: entity.number,
...(entity.url && { url: entity.url }),
...(entity.html_url && { html_url: entity.html_url }),
});

core.info("");
core.info(`✓ Successfully closed ${config.entityType} #${entity.number}`);
} catch (error) {
core.info("");
core.error(`✗ Failed to close ${config.entityType} #${entity.number}`);
core.error(` Error: ${getErrorMessage(error)}`);
if (error instanceof Error && error.stack) {
core.error(` Stack trace: ${error.stack}`);
}
// Continue with other entities even if one fails
}

// Step 4: Add delay between API operations to avoid rate limiting (except for the last item)
if (i < entitiesToClose.length - 1) {
core.info("");
core.info(`Waiting ${config.delayMs}ms before processing next ${config.entityType} to avoid rate limiting...`);
await delay(config.delayMs);
}
}

core.info("");
core.info("=".repeat(70));
core.info(`Closed ${closedEntities.length} of ${entitiesToClose.length} ${config.entityType}(s) successfully`);
if (closedEntities.length < entitiesToClose.length) {
core.warning(`Failed to close ${entitiesToClose.length - closedEntities.length} ${config.entityType}(s) - check logs above for details`);
}
core.info("=".repeat(70));

return closedEntities;
}

module.exports = {
closeOlderEntities,
MAX_CLOSE_COUNT,
delay,
};
Loading
Loading