diff --git a/actions/setup/js/safe_outputs_tools.json b/actions/setup/js/safe_outputs_tools.json index b907d4e37b9..4312fde0b52 100644 --- a/actions/setup/js/safe_outputs_tools.json +++ b/actions/setup/js/safe_outputs_tools.json @@ -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": { @@ -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)." diff --git a/actions/setup/js/update_discussion.cjs b/actions/setup/js/update_discussion.cjs index 036bed7371a..07ed18cb0c4 100644 --- a/actions/setup/js/update_discussion.cjs +++ b/actions/setup/js/update_discussion.cjs @@ -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 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} + */ +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} + */ +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 @@ -20,7 +134,7 @@ const { parseBoolTemplatable } = require("./templatable.cjs"); * @returns {Promise} 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) { @@ -29,6 +143,12 @@ async function executeDiscussionUpdate(github, context, discussionNumber, update title body url + labels(first: 100) { + nodes { + id + name + } + } } } } @@ -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); + + 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; } /** @@ -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 ?? []; + } + } else { + updateData.labels = item.labels; + } + } return { success: true, data: updateData }; } @@ -167,4 +344,4 @@ const main = createUpdateHandlerFactory({ formatSuccessResult: formatDiscussionSuccessResult, }); -module.exports = { main }; +module.exports = { main, buildDiscussionUpdateData }; diff --git a/actions/setup/js/update_discussion.test.cjs b/actions/setup/js/update_discussion.test.cjs new file mode 100644 index 00000000000..17122375101 --- /dev/null +++ b/actions/setup/js/update_discussion.test.cjs @@ -0,0 +1,289 @@ +// @ts-check +import { describe, it, expect, beforeEach, vi } from "vitest"; +import { createRequire } from "module"; + +const require = createRequire(import.meta.url); + +describe("update_discussion.cjs - label support", () => { + let mockGithub; + let mockCore; + let mockContext; + + beforeEach(() => { + vi.clearAllMocks(); + vi.resetModules(); + + mockCore = { + debug: vi.fn(), + info: vi.fn(), + notice: vi.fn(), + warning: vi.fn(), + error: vi.fn(), + setFailed: vi.fn(), + setOutput: vi.fn(), + summary: { + addRaw: vi.fn().mockReturnThis(), + write: vi.fn().mockResolvedValue(undefined), + }, + }; + + mockGithub = { + graphql: vi.fn(), + }; + + mockContext = { + eventName: "discussion", + repo: { owner: "testowner", repo: "testrepo" }, + serverUrl: "https://github.com", + runId: 12345, + payload: { + discussion: { number: 42 }, + }, + }; + + global.core = mockCore; + global.github = mockGithub; + global.context = mockContext; + }); + + /** + * Builds the default graphql mock responses for a discussion query + mutation sequence. + * @param {object} opts + * @param {string[]} [opts.currentLabelIds] - IDs of labels currently on the discussion + * @param {string[]} [opts.currentLabelNames] - Names of labels currently on the discussion + * @param {Array<{id: string, name: string}>} [opts.repoLabels] - Labels available in the repo + */ + function buildGraphqlMock({ currentLabelIds = [], currentLabelNames = [], repoLabels = [] } = {}) { + const currentLabels = currentLabelIds.map((id, i) => ({ id, name: currentLabelNames[i] ?? id })); + + return vi.fn().mockImplementation(async query => { + // Fetch discussion node ID + current labels + if (query.includes("discussion(number:")) { + return { + repository: { + discussion: { + id: "D_disc123", + title: "Test Discussion", + body: "Test body", + url: "https://github.com/testowner/testrepo/discussions/42", + labels: { nodes: currentLabels }, + }, + }, + }; + } + // Update discussion title/body mutation + if (query.includes("updateDiscussion")) { + return { + updateDiscussion: { + discussion: { + id: "D_disc123", + title: "Test Discussion", + body: "Test body", + url: "https://github.com/testowner/testrepo/discussions/42", + }, + }, + }; + } + // Fetch repository labels (for label ID lookup) + if (query.includes("labels(first:")) { + return { + repository: { + labels: { nodes: repoLabels }, + }, + }; + } + // addLabelsToLabelable / removeLabelsFromLabelable mutations + if (query.includes("addLabelsToLabelable") || query.includes("removeLabelsFromLabelable")) { + return {}; + } + return {}; + }); + } + + describe("label updates", () => { + it("should add labels when discussion has no current labels", async () => { + mockGithub.graphql = buildGraphqlMock({ + currentLabelIds: [], + currentLabelNames: [], + repoLabels: [ + { id: "LA_bug", name: "bug" }, + { id: "LA_idea", name: "idea" }, + ], + }); + + const { main } = require("./update_discussion.cjs"); + const handler = await main({ target: "triggering", max: 1 }); + const result = await handler({ labels: ["bug"] }, {}); + + expect(result.success).toBe(true); + + const addCall = mockGithub.graphql.mock.calls.find(c => c[0].includes("addLabelsToLabelable")); + expect(addCall).toBeDefined(); + expect(addCall[1].labelIds).toEqual(["LA_bug"]); + + const removeCall = mockGithub.graphql.mock.calls.find(c => c[0].includes("removeLabelsFromLabelable")); + expect(removeCall).toBeUndefined(); + }); + + it("should remove labels no longer in the requested set", async () => { + mockGithub.graphql = buildGraphqlMock({ + currentLabelIds: ["LA_bug", "LA_old"], + currentLabelNames: ["bug", "old-label"], + repoLabels: [ + { id: "LA_bug", name: "bug" }, + { id: "LA_idea", name: "idea" }, + ], + }); + + const { main } = require("./update_discussion.cjs"); + const handler = await main({ target: "triggering", max: 1 }); + const result = await handler({ labels: ["bug"] }, {}); + + expect(result.success).toBe(true); + + const removeCall = mockGithub.graphql.mock.calls.find(c => c[0].includes("removeLabelsFromLabelable")); + expect(removeCall).toBeDefined(); + expect(removeCall[1].labelIds).toEqual(["LA_old"]); + + const addCall = mockGithub.graphql.mock.calls.find(c => c[0].includes("addLabelsToLabelable")); + expect(addCall).toBeUndefined(); + }); + + it("should replace all labels (add new, remove old)", async () => { + mockGithub.graphql = buildGraphqlMock({ + currentLabelIds: ["LA_old"], + currentLabelNames: ["old-label"], + repoLabels: [ + { id: "LA_bug", name: "bug" }, + { id: "LA_idea", name: "idea" }, + ], + }); + + const { main } = require("./update_discussion.cjs"); + const handler = await main({ target: "triggering", max: 1 }); + const result = await handler({ labels: ["bug", "idea"] }, {}); + + expect(result.success).toBe(true); + + const addCall = mockGithub.graphql.mock.calls.find(c => c[0].includes("addLabelsToLabelable")); + expect(addCall).toBeDefined(); + expect(addCall[1].labelIds).toEqual(expect.arrayContaining(["LA_bug", "LA_idea"])); + + const removeCall = mockGithub.graphql.mock.calls.find(c => c[0].includes("removeLabelsFromLabelable")); + expect(removeCall).toBeDefined(); + expect(removeCall[1].labelIds).toEqual(["LA_old"]); + }); + + it("should do nothing when requested labels are unchanged", async () => { + mockGithub.graphql = buildGraphqlMock({ + currentLabelIds: ["LA_bug"], + currentLabelNames: ["bug"], + repoLabels: [{ id: "LA_bug", name: "bug" }], + }); + + const { main } = require("./update_discussion.cjs"); + const handler = await main({ target: "triggering", max: 1 }); + const result = await handler({ labels: ["bug"] }, {}); + + expect(result.success).toBe(true); + + const addCall = mockGithub.graphql.mock.calls.find(c => c[0].includes("addLabelsToLabelable")); + expect(addCall).toBeUndefined(); + + const removeCall = mockGithub.graphql.mock.calls.find(c => c[0].includes("removeLabelsFromLabelable")); + expect(removeCall).toBeUndefined(); + }); + + it("should filter labels against allowed_labels list", async () => { + mockGithub.graphql = buildGraphqlMock({ + currentLabelIds: [], + currentLabelNames: [], + repoLabels: [ + { id: "LA_bug", name: "bug" }, + { id: "LA_idea", name: "idea" }, + { id: "LA_other", name: "other" }, + ], + }); + + const { main } = require("./update_discussion.cjs"); + const handler = await main({ + target: "triggering", + max: 1, + allowed_labels: ["bug", "idea"], + }); + // "other" is not in allowed_labels and should be filtered out + const result = await handler({ labels: ["bug", "other"] }, {}); + + expect(result.success).toBe(true); + + const addCall = mockGithub.graphql.mock.calls.find(c => c[0].includes("addLabelsToLabelable")); + expect(addCall).toBeDefined(); + // Only "bug" should be added since "other" is not in allowed_labels + expect(addCall[1].labelIds).toEqual(["LA_bug"]); + }); + + it("should update labels regardless of allow_labels config (consistent with update_issue)", async () => { + mockGithub.graphql = buildGraphqlMock({ + currentLabelIds: [], + currentLabelNames: [], + repoLabels: [{ id: "LA_bug", name: "bug" }], + }); + + const { main } = require("./update_discussion.cjs"); + // No allow_labels in config - labels are still processed (consistent with update_issue behavior) + const handler = await main({ target: "triggering", max: 1 }); + const result = await handler({ labels: ["bug"] }, {}); + + expect(result.success).toBe(true); + // Labels should be updated even without explicit allow_labels flag + const addCall = mockGithub.graphql.mock.calls.find(c => c[0].includes("addLabelsToLabelable")); + expect(addCall).toBeDefined(); + expect(addCall[1].labelIds).toEqual(["LA_bug"]); + }); + + it("should update labels in combination with title", async () => { + mockGithub.graphql = buildGraphqlMock({ + currentLabelIds: [], + currentLabelNames: [], + repoLabels: [{ id: "LA_bug", name: "bug" }], + }); + + const { main } = require("./update_discussion.cjs"); + const handler = await main({ target: "triggering", max: 1 }); + const result = await handler({ title: "New Title", labels: ["bug"] }, {}); + + expect(result.success).toBe(true); + + const updateCall = mockGithub.graphql.mock.calls.find(c => c[0].includes("updateDiscussion")); + expect(updateCall).toBeDefined(); + + const addCall = mockGithub.graphql.mock.calls.find(c => c[0].includes("addLabelsToLabelable")); + expect(addCall).toBeDefined(); + expect(addCall[1].labelIds).toEqual(["LA_bug"]); + }); + }); + + describe("label-only updates (no title/body)", () => { + it("should update only labels without touching title or body", async () => { + mockGithub.graphql = buildGraphqlMock({ + currentLabelIds: [], + currentLabelNames: [], + repoLabels: [{ id: "LA_idea", name: "idea" }], + }); + + const { main } = require("./update_discussion.cjs"); + const handler = await main({ target: "triggering", max: 1 }); + const result = await handler({ labels: ["idea"] }, {}); + + expect(result.success).toBe(true); + + // updateDiscussion mutation should NOT be called (no title/body update) + const updateCall = mockGithub.graphql.mock.calls.find(c => c[0].includes("updateDiscussion")); + expect(updateCall).toBeUndefined(); + + // addLabelsToLabelable should be called + const addCall = mockGithub.graphql.mock.calls.find(c => c[0].includes("addLabelsToLabelable")); + expect(addCall).toBeDefined(); + }); + }); +}); diff --git a/pkg/workflow/safe_outputs_validation_config.go b/pkg/workflow/safe_outputs_validation_config.go index 30342a63a44..f7fd81e4094 100644 --- a/pkg/workflow/safe_outputs_validation_config.go +++ b/pkg/workflow/safe_outputs_validation_config.go @@ -318,10 +318,11 @@ var ValidationConfig = map[string]TypeValidationConfig{ }, "update_discussion": { DefaultMax: 1, - CustomValidation: "requiresOneOf:title,body", + CustomValidation: "requiresOneOf:title,body,labels", Fields: map[string]FieldValidation{ "title": {Type: "string", Sanitize: true, MaxLength: 128}, "body": {Type: "string", Sanitize: true, MaxLength: MaxBodyLength}, + "labels": {Type: "array", ItemType: "string", ItemSanitize: true, ItemMaxLength: 128}, "discussion_number": {IssueOrPRNumber: true}, "repo": {Type: "string", MaxLength: 256}, // Optional: target repository in format "owner/repo" },