From d9d7929d3ad3ea7193dd9ec54c9a84c41d763f5d Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Mon, 23 Mar 2026 06:22:49 +0000 Subject: [PATCH 1/4] Initial plan From c4bc8b079ebdbe31d939f90f4c1f871637419ed9 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Mon, 23 Mar 2026 06:41:49 +0000 Subject: [PATCH 2/4] feat: add label update support to update-discussion safe-output Co-authored-by: mnkiefer <8320933+mnkiefer@users.noreply.github.com> Agent-Logs-Url: https://github.com/github/gh-aw/sessions/16915229-625b-491a-ae2b-fbc8e13b3d49 --- actions/setup/js/safe_outputs_tools.json | 7 +- actions/setup/js/update_discussion.cjs | 225 ++++++++++++-- actions/setup/js/update_discussion.test.cjs | 291 ++++++++++++++++++ .../safe_outputs_validation_config.go | 3 +- 4 files changed, 505 insertions(+), 21 deletions(-) create mode 100644 actions/setup/js/update_discussion.test.cjs 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..2fb523641c1 100644 --- a/actions/setup/js/update_discussion.cjs +++ b/actions/setup/js/update_discussion.cjs @@ -8,8 +8,123 @@ 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 +135,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 +144,12 @@ async function executeDiscussionUpdate(github, context, discussionNumber, update title body url + labels(first: 100) { + nodes { + id + name + } + } } } } @@ -45,28 +166,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 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 variables = { - discussionId: discussion.id, - title: updateData.title || discussion.title, - body: updateData.body || discussion.body, - }; + // 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)); - const mutationResult = await github.graphql(mutation, variables); - return mutationResult.updateDiscussion.discussion; + // 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,6 +291,38 @@ function buildDiscussionUpdateData(item, config) { updateData.body = item.body; } + // Handle labels if enabled by config + const canUpdateLabels = config.allow_labels === true; + if (item.labels !== undefined && canUpdateLabels) { + 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 }; + } + + // Validate and sanitize labels against allowed list. + // An empty allowed_labels array means no restriction (pass undefined to validateLabels). + // A non-empty allowed_labels array restricts which labels may be set. + const effectiveAllowedLabels = allowedLabels.length > 0 ? allowedLabels : undefined; + const labelsResult = validateLabels(item.labels, effectiveAllowedLabels); + if (!labelsResult.valid) { + if (labelsResult.error?.includes("No valid labels")) { + // All labels were filtered out (e.g. none in allowed list) - treat as empty set + updateData.labels = []; + } else { + core.warning(`Label validation failed: ${labelsResult.error}`); + return { success: false, error: labelsResult.error ?? "Invalid labels" }; + } + } else { + updateData.labels = labelsResult.value ?? []; + } + } else if (item.labels !== undefined && !canUpdateLabels) { + core.warning("Label update not allowed by safe-outputs configuration"); + } + // Pass footer config to executeUpdate (default to true) updateData._includeFooter = parseBoolTemplatable(config.footer, true); diff --git a/actions/setup/js/update_discussion.test.cjs b/actions/setup/js/update_discussion.test.cjs new file mode 100644 index 00000000000..a9385ff1ba5 --- /dev/null +++ b/actions/setup/js/update_discussion.test.cjs @@ -0,0 +1,291 @@ +// @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, allow_labels: true }); + 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, allow_labels: true }); + 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, allow_labels: true }); + 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, allow_labels: true }); + 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, + allow_labels: true, + 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 not update labels when allow_labels is false", async () => { + mockGithub.graphql = buildGraphqlMock({ + currentLabelIds: [], + currentLabelNames: [], + repoLabels: [{ id: "LA_bug", name: "bug" }], + }); + + const { main } = require("./update_discussion.cjs"); + // allow_labels not set (undefined) - labels update disabled; title update still works + const handler = await main({ target: "triggering", max: 1, allow_title: true }); + const result = await handler({ title: "Updated Title", labels: ["bug"] }, {}); + + expect(result.success).toBe(true); + // Labels should not be updated (allow_labels not set) + const addCall = mockGithub.graphql.mock.calls.find(c => c[0].includes("addLabelsToLabelable")); + expect(addCall).toBeUndefined(); + // A warning should have been logged + expect(mockCore.warning).toHaveBeenCalledWith(expect.stringContaining("Label update not allowed")); + }); + + 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, allow_title: true, allow_labels: true }); + 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, allow_labels: true }); + 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" }, From 53f187d31cd20329f3e48b79a66b7481bf3c7f8f Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Mon, 23 Mar 2026 08:18:23 +0000 Subject: [PATCH 3/4] fix: make update_discussion label handling consistent with update_issue Remove the allow_labels === true gate from buildDiscussionUpdateData so labels are always processed when provided, consistent with update_issue behavior. Optional allowed_labels config still restricts which labels may be set. Co-authored-by: mnkiefer <8320933+mnkiefer@users.noreply.github.com> Agent-Logs-Url: https://github.com/github/gh-aw/sessions/1ed3573c-43c2-487f-b854-fbc8e7b139f8 --- actions/setup/js/update_discussion.cjs | 24 +++++++----------- actions/setup/js/update_discussion.test.cjs | 28 ++++++++++----------- 2 files changed, 22 insertions(+), 30 deletions(-) diff --git a/actions/setup/js/update_discussion.cjs b/actions/setup/js/update_discussion.cjs index 2fb523641c1..d1fc4d79560 100644 --- a/actions/setup/js/update_discussion.cjs +++ b/actions/setup/js/update_discussion.cjs @@ -291,9 +291,9 @@ function buildDiscussionUpdateData(item, config) { updateData.body = item.body; } - // Handle labels if enabled by config - const canUpdateLabels = config.allow_labels === true; - if (item.labels !== undefined && canUpdateLabels) { + // 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 @@ -303,24 +303,18 @@ function buildDiscussionUpdateData(item, config) { return { success: false, error: labelsLimitResult.error }; } - // Validate and sanitize labels against allowed list. - // An empty allowed_labels array means no restriction (pass undefined to validateLabels). - // A non-empty allowed_labels array restricts which labels may be set. - const effectiveAllowedLabels = allowedLabels.length > 0 ? allowedLabels : undefined; - const labelsResult = validateLabels(item.labels, effectiveAllowedLabels); - if (!labelsResult.valid) { - if (labelsResult.error?.includes("No valid labels")) { + 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 { - core.warning(`Label validation failed: ${labelsResult.error}`); - return { success: false, error: labelsResult.error ?? "Invalid labels" }; + updateData.labels = labelsResult.value ?? []; } } else { - updateData.labels = labelsResult.value ?? []; + updateData.labels = item.labels; } - } else if (item.labels !== undefined && !canUpdateLabels) { - core.warning("Label update not allowed by safe-outputs configuration"); } // Pass footer config to executeUpdate (default to true) diff --git a/actions/setup/js/update_discussion.test.cjs b/actions/setup/js/update_discussion.test.cjs index a9385ff1ba5..17122375101 100644 --- a/actions/setup/js/update_discussion.test.cjs +++ b/actions/setup/js/update_discussion.test.cjs @@ -112,7 +112,7 @@ describe("update_discussion.cjs - label support", () => { }); const { main } = require("./update_discussion.cjs"); - const handler = await main({ target: "triggering", max: 1, allow_labels: true }); + const handler = await main({ target: "triggering", max: 1 }); const result = await handler({ labels: ["bug"] }, {}); expect(result.success).toBe(true); @@ -136,7 +136,7 @@ describe("update_discussion.cjs - label support", () => { }); const { main } = require("./update_discussion.cjs"); - const handler = await main({ target: "triggering", max: 1, allow_labels: true }); + const handler = await main({ target: "triggering", max: 1 }); const result = await handler({ labels: ["bug"] }, {}); expect(result.success).toBe(true); @@ -160,7 +160,7 @@ describe("update_discussion.cjs - label support", () => { }); const { main } = require("./update_discussion.cjs"); - const handler = await main({ target: "triggering", max: 1, allow_labels: true }); + const handler = await main({ target: "triggering", max: 1 }); const result = await handler({ labels: ["bug", "idea"] }, {}); expect(result.success).toBe(true); @@ -182,7 +182,7 @@ describe("update_discussion.cjs - label support", () => { }); const { main } = require("./update_discussion.cjs"); - const handler = await main({ target: "triggering", max: 1, allow_labels: true }); + const handler = await main({ target: "triggering", max: 1 }); const result = await handler({ labels: ["bug"] }, {}); expect(result.success).toBe(true); @@ -209,7 +209,6 @@ describe("update_discussion.cjs - label support", () => { const handler = await main({ target: "triggering", max: 1, - allow_labels: true, allowed_labels: ["bug", "idea"], }); // "other" is not in allowed_labels and should be filtered out @@ -223,7 +222,7 @@ describe("update_discussion.cjs - label support", () => { expect(addCall[1].labelIds).toEqual(["LA_bug"]); }); - it("should not update labels when allow_labels is false", async () => { + it("should update labels regardless of allow_labels config (consistent with update_issue)", async () => { mockGithub.graphql = buildGraphqlMock({ currentLabelIds: [], currentLabelNames: [], @@ -231,16 +230,15 @@ describe("update_discussion.cjs - label support", () => { }); const { main } = require("./update_discussion.cjs"); - // allow_labels not set (undefined) - labels update disabled; title update still works - const handler = await main({ target: "triggering", max: 1, allow_title: true }); - const result = await handler({ title: "Updated Title", labels: ["bug"] }, {}); + // 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 not be updated (allow_labels not set) + // Labels should be updated even without explicit allow_labels flag const addCall = mockGithub.graphql.mock.calls.find(c => c[0].includes("addLabelsToLabelable")); - expect(addCall).toBeUndefined(); - // A warning should have been logged - expect(mockCore.warning).toHaveBeenCalledWith(expect.stringContaining("Label update not allowed")); + expect(addCall).toBeDefined(); + expect(addCall[1].labelIds).toEqual(["LA_bug"]); }); it("should update labels in combination with title", async () => { @@ -251,7 +249,7 @@ describe("update_discussion.cjs - label support", () => { }); const { main } = require("./update_discussion.cjs"); - const handler = await main({ target: "triggering", max: 1, allow_title: true, allow_labels: true }); + const handler = await main({ target: "triggering", max: 1 }); const result = await handler({ title: "New Title", labels: ["bug"] }, {}); expect(result.success).toBe(true); @@ -274,7 +272,7 @@ describe("update_discussion.cjs - label support", () => { }); const { main } = require("./update_discussion.cjs"); - const handler = await main({ target: "triggering", max: 1, allow_labels: true }); + const handler = await main({ target: "triggering", max: 1 }); const result = await handler({ labels: ["idea"] }, {}); expect(result.success).toBe(true); From 66d3e526b1ee5db2c0f2fe2e94c80741758d24b9 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Mon, 23 Mar 2026 12:36:00 +0000 Subject: [PATCH 4/4] fix: remove dead _includeFooter code and export buildDiscussionUpdateData - Remove unused parseBoolTemplatable import and _includeFooter assignment from buildDiscussionUpdateData; executeDiscussionUpdate never reads it - Export buildDiscussionUpdateData matching update_issue's export of buildIssueUpdateData for unit-test parity Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> Agent-Logs-Url: https://github.com/github/gh-aw/sessions/bb00d1b8-9f7d-4585-9d0d-eb12204d2716 --- actions/setup/js/update_discussion.cjs | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/actions/setup/js/update_discussion.cjs b/actions/setup/js/update_discussion.cjs index d1fc4d79560..07ed18cb0c4 100644 --- a/actions/setup/js/update_discussion.cjs +++ b/actions/setup/js/update_discussion.cjs @@ -12,7 +12,6 @@ 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"); /** @@ -317,9 +316,6 @@ function buildDiscussionUpdateData(item, config) { } } - // Pass footer config to executeUpdate (default to true) - updateData._includeFooter = parseBoolTemplatable(config.footer, true); - return { success: true, data: updateData }; } @@ -348,4 +344,4 @@ const main = createUpdateHandlerFactory({ formatSuccessResult: formatDiscussionSuccessResult, }); -module.exports = { main }; +module.exports = { main, buildDiscussionUpdateData };