Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
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
7 changes: 6 additions & 1 deletion actions/setup/js/safe_outputs_tools.json
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,7 @@
},
{
"name": "update_discussion",
"description": "Update an existing GitHub discussion's title or body. Use this to modify discussion properties after creation. Only the fields you specify will be updated; other fields remain unchanged.",
"description": "Update an existing GitHub discussion's title, body, or labels. Use this to modify discussion properties after creation. Only the fields you specify will be updated; other fields remain unchanged.",
"inputSchema": {
"type": "object",
"properties": {
Expand All @@ -110,6 +110,11 @@
"type": "string",
"description": "New discussion body to replace the existing content. Use Markdown formatting."
},
"labels": {
"type": "array",
"items": { "type": "string" },
"description": "Labels to set on the discussion. Replaces all existing labels. Only labels in the configured allowed-labels list will be applied."
},
"discussion_number": {
"type": ["number", "string"],
"description": "Discussion number to update. This is the numeric ID from the GitHub URL (e.g., 345 in github.com/owner/repo/discussions/345). Required when the workflow target is '*' (any discussion)."
Expand Down
223 changes: 200 additions & 23 deletions actions/setup/js/update_discussion.cjs
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,122 @@
const { isDiscussionContext, getDiscussionNumber } = require("./update_context_helpers.cjs");
const { createUpdateHandlerFactory, createStandardFormatResult } = require("./update_handler_factory.cjs");
const { sanitizeTitle } = require("./sanitize_title.cjs");
const { validateLabels } = require("./safe_output_validator.cjs");
const { tryEnforceArrayLimit } = require("./limit_enforcement_helpers.cjs");
const { getErrorMessage } = require("./error_helpers.cjs");
const { ERR_NOT_FOUND } = require("./error_codes.cjs");
const { parseBoolTemplatable } = require("./templatable.cjs");
const { MAX_LABELS } = require("./constants.cjs");

/**
* Fetches label node IDs for the given label names from a repository
* @param {any} githubClient - GitHub API client
* @param {string} owner - Repository owner
* @param {string} repo - Repository name
* @param {string[]} labelNames - Array of label names to resolve
* @returns {Promise<Array<{name: string, id: string}>>} Array of matched label objects with name and ID
*/
async function fetchLabelIds(githubClient, owner, repo, labelNames) {
if (!labelNames || labelNames.length === 0) {
return [];
}

try {
const labelsQuery = `
query($owner: String!, $repo: String!) {
repository(owner: $owner, name: $repo) {
labels(first: 100) {
nodes {
id
name
}
}
}
}
`;

const queryResult = await githubClient.graphql(labelsQuery, { owner, repo });
const repoLabels = queryResult?.repository?.labels?.nodes || [];
const labelMap = new Map(repoLabels.map(/** @param {any} l */ l => [l.name.toLowerCase(), l]));

const matchedLabels = [];
const unmatchedLabels = [];

for (const requestedLabel of labelNames) {
const matched = labelMap.get(requestedLabel.toLowerCase());
if (matched) {
matchedLabels.push({ name: matched.name, id: matched.id });
} else {
unmatchedLabels.push(requestedLabel);
}
}

if (unmatchedLabels.length > 0) {
core.warning(`Could not find label IDs for: ${unmatchedLabels.join(", ")}`);
core.info(`These labels may not exist in the repository. Available: ${repoLabels.map(/** @param {any} l */ l => l.name).join(", ")}`);
}

return matchedLabels;
} catch (error) {
core.warning(`Failed to fetch label IDs: ${getErrorMessage(error)}`);
return [];
}
}

/**
* Removes labels from a discussion using GraphQL
* @param {any} githubClient - GitHub API client
* @param {string} discussionId - Discussion node ID
* @param {string[]} labelIds - Label node IDs to remove
* @returns {Promise<void>}
*/
async function removeLabelsFromDiscussion(githubClient, discussionId, labelIds) {
if (!labelIds || labelIds.length === 0) {
return;
}

const mutation = `
mutation($labelableId: ID!, $labelIds: [ID!]!) {
removeLabelsFromLabelable(input: {
labelableId: $labelableId,
labelIds: $labelIds
}) {
labelable {
... on Discussion { id }
}
}
}
`;

await githubClient.graphql(mutation, { labelableId: discussionId, labelIds });
}

/**
* Adds labels to a discussion using GraphQL
* @param {any} githubClient - GitHub API client
* @param {string} discussionId - Discussion node ID
* @param {string[]} labelIds - Label node IDs to add
* @returns {Promise<void>}
*/
async function addLabelsToDiscussion(githubClient, discussionId, labelIds) {
if (!labelIds || labelIds.length === 0) {
return;
}

const mutation = `
mutation($labelableId: ID!, $labelIds: [ID!]!) {
addLabelsToLabelable(input: {
labelableId: $labelableId,
labelIds: $labelIds
}) {
labelable {
... on Discussion { id }
}
}
}
`;

await githubClient.graphql(mutation, { labelableId: discussionId, labelIds });
}

/**
* Execute the discussion update API call using GraphQL
Expand All @@ -20,7 +134,7 @@ const { parseBoolTemplatable } = require("./templatable.cjs");
* @returns {Promise<any>} Updated discussion
*/
async function executeDiscussionUpdate(github, context, discussionNumber, updateData) {
// First, fetch the discussion node ID
// Fetch the discussion node ID and current labels in one query
const getDiscussionQuery = `
query($owner: String!, $repo: String!, $number: Int!) {
repository(owner: $owner, name: $repo) {
Expand All @@ -29,6 +143,12 @@ async function executeDiscussionUpdate(github, context, discussionNumber, update
title
body
url
labels(first: 100) {
nodes {
id
name
}
}
}
}
}
Expand All @@ -45,28 +165,62 @@ async function executeDiscussionUpdate(github, context, discussionNumber, update
throw new Error(`${ERR_NOT_FOUND}: Discussion #${discussionNumber} not found`);
}

// Build mutation for updating discussion
let mutation = `
mutation($discussionId: ID!, $title: String, $body: String) {
updateDiscussion(input: { discussionId: $discussionId, title: $title, body: $body }) {
discussion {
id
title
body
url
// Update title and/or body if provided
if (updateData.title !== undefined || updateData.body !== undefined) {
const mutation = `
mutation($discussionId: ID!, $title: String, $body: String) {
updateDiscussion(input: { discussionId: $discussionId, title: $title, body: $body }) {
discussion {
id
title
body
url
}
}
}
}
`;
`;

const variables = {
discussionId: discussion.id,
title: updateData.title || discussion.title,
body: updateData.body || discussion.body,
};
const variables = {
discussionId: discussion.id,
title: updateData.title || discussion.title,
body: updateData.body || discussion.body,
};

const mutationResult = await github.graphql(mutation, variables);
// Merge updated title/body back into discussion for return value
const updated = mutationResult.updateDiscussion.discussion;
discussion.title = updated.title;
discussion.body = updated.body;
discussion.url = updated.url;
}

const mutationResult = await github.graphql(mutation, variables);
return mutationResult.updateDiscussion.discussion;
// Handle label replacement if labels were provided
if (updateData.labels !== undefined) {
const currentLabels = discussion.labels?.nodes || [];
const currentLabelIds = new Set(currentLabels.map(/** @param {any} l */ l => l.id));

// Look up node IDs for the requested labels
const requestedLabelData = await fetchLabelIds(github, context.repo.owner, context.repo.repo, updateData.labels);
const requestedLabelIdSet = new Set(requestedLabelData.map(/** @param {any} l */ l => l.id));

// Compute add/remove sets
const labelsToAdd = requestedLabelData.filter(l => !currentLabelIds.has(l.id)).map(/** @param {any} l */ l => l.id);
const labelsToRemove = currentLabels.filter(/** @param {any} l */ l => !requestedLabelIdSet.has(l.id)).map(/** @param {any} l */ l => l.id);

Comment on lines +206 to +209
Copy link

Copilot AI Mar 23, 2026

Choose a reason for hiding this comment

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

Label replacement currently removes any existing labels whose IDs aren’t in requestedLabelIdSet, but requestedLabelIdSet is derived only from labels successfully resolved by fetchLabelIds. If the agent provides a label that doesn’t exist (or isn’t in the first 100 repo labels), fetchLabelIds will drop it and you can end up removing all existing labels unintentionally. Consider failing the update when any requested labels can’t be resolved, or (safer) only perform removals when all requested labels were successfully resolved.

Suggested change
// Compute add/remove sets
const labelsToAdd = requestedLabelData.filter(l => !currentLabelIds.has(l.id)).map(/** @param {any} l */ l => l.id);
const labelsToRemove = currentLabels.filter(/** @param {any} l */ l => !requestedLabelIdSet.has(l.id)).map(/** @param {any} l */ l => l.id);
// Detect any requested labels that could not be resolved
const resolvedLabelNames = new Set(requestedLabelData.map(/** @param {any} l */ l => l.name));
const unresolvedLabels = Array.isArray(updateData.labels)
? updateData.labels.filter((/** @type {string} */ name) => !resolvedLabelNames.has(name))
: [];
// Compute add/remove sets
const labelsToAdd = requestedLabelData
.filter((/** @param {any} l */ l) => !currentLabelIds.has(l.id))
.map((/** @param {any} l */ l) => l.id);
/** @type {string[]} */
let labelsToRemove = [];
if (unresolvedLabels.length === 0) {
// Only remove labels when all requested labels were successfully resolved
labelsToRemove = currentLabels
.filter((/** @param {any} l */ l) => !requestedLabelIdSet.has(l.id))
.map((/** @param {any} l */ l) => l.id);
} else {
core.warning(
`Skipping label removals for discussion #${discussionNumber} because some requested labels could not be resolved: ${unresolvedLabels.join(
", "
)}`
);
}

Copilot uses AI. Check for mistakes.
if (labelsToAdd.length > 0) {
core.info(`Adding ${labelsToAdd.length} label(s) to discussion #${discussionNumber}`);
await addLabelsToDiscussion(github, discussion.id, labelsToAdd);
}
if (labelsToRemove.length > 0) {
core.info(`Removing ${labelsToRemove.length} label(s) from discussion #${discussionNumber}`);
await removeLabelsFromDiscussion(github, discussion.id, labelsToRemove);
}
if (labelsToAdd.length === 0 && labelsToRemove.length === 0) {
core.info(`Labels unchanged for discussion #${discussionNumber}`);
}
}

return discussion;
}

/**
Expand Down Expand Up @@ -136,8 +290,31 @@ function buildDiscussionUpdateData(item, config) {
updateData.body = item.body;
}

// Pass footer config to executeUpdate (default to true)
updateData._includeFooter = parseBoolTemplatable(config.footer, true);
// Handle labels - consistent with update_issue: labels are always processed when provided.
// Optional allowed_labels config restricts which labels may be set.
if (item.labels !== undefined) {
const allowedLabels = config.allowed_labels || [];

// Enforce max label count
const labelsLimitResult = tryEnforceArrayLimit(item.labels, MAX_LABELS, "labels");
if (!labelsLimitResult.success) {
core.warning(`Discussion label update limit exceeded: ${labelsLimitResult.error}`);
return { success: false, error: labelsLimitResult.error };
}

if (allowedLabels.length > 0) {
// Filter to allowed labels only; if none remain treat as an empty set
const labelsResult = validateLabels(item.labels, allowedLabels);
if (!labelsResult.valid) {
// All labels were filtered out (e.g. none in allowed list) - treat as empty set
updateData.labels = [];
} else {
updateData.labels = labelsResult.value ?? [];
}
Comment on lines +305 to +313
Copy link

Copilot AI Mar 23, 2026

Choose a reason for hiding this comment

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

In the allowed-labels branch, validateLabels(item.labels, allowedLabels) uses validateLabels' default maxCount=3, which will silently truncate discussions to 3 labels even though MAX_LABELS is 10 and you already enforce tryEnforceArrayLimit with MAX_LABELS. Pass MAX_LABELS (and any other intended params) into validateLabels, or avoid calling validateLabels for limiting and do only allowed-list filtering + sanitization here.

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

Copilot uses AI. Check for mistakes.
} else {
updateData.labels = item.labels;
}
}

return { success: true, data: updateData };
}
Expand Down Expand Up @@ -167,4 +344,4 @@ const main = createUpdateHandlerFactory({
formatSuccessResult: formatDiscussionSuccessResult,
});

module.exports = { main };
module.exports = { main, buildDiscussionUpdateData };
Loading
Loading