fix: update-discussion labels-only config must not mutate title or body

[Copilot]

When update-discussion is configured with only labels: or allowed-labels:, the handler was unconditionally calling the updateDiscussion GraphQL mutation, sending the current title and body back as-is — an unnecessary write that violates the intended permission boundary.

Changes

• buildDiscussionUpdateData() — Title and body are now gated behind config.allow_title === true / config.allow_body === true, consistent with how update_issue.cjs handles body permission (config.allow_body !== false)
• buildDiscussionUpdateData() — validateLabels is now called with MAX_LABELS (10) as maxCount, fixing a bug where 4–10 labels would be silently truncated to 3 due to the default maxCount = 3
• executeDiscussionUpdate() — The updateDiscussion GraphQL mutation is now skipped entirely when neither updateData.title nor updateData.body is set; only the label-replacement calls are made
• Warning message — Removed stale "title/body updated successfully" text from the label-failure warning path
• Tests — Added update_discussion.test.cjs covering title/body permission gating, label permission gating, allowed_labels filtering, MAX_LABELS enforcement, and a regression test for the maxCount truncation bug

Before / After

# Config: labels-only
safe-outputs:
  update-discussion:
    labels:
    allowed-labels: [bug, enhancement]

	Before	After
updateDiscussion mutation called	✅ always	✅ only when title/body changes
Title/body sent on labels-only update	✅ current values echoed back	❌ mutation skipped
Max labels accepted	❌ silently truncated to 3	✅ up to 10 (consistent with MAX_LABELS)

---

✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.

[mnkiefer]

@copilot How are other safe outputs such as update-issue handled (with respect to label updates only)?

[Copilot]

@copilot How are other safe outputs sich as update-issue handled (with respect to label updates only)?

update-issue naturally avoids this problem because it uses the REST API (github.rest.issues.update), which is a partial-update endpoint — it only modifies the fields you explicitly pass. So if the agent outputs only labels, only { labels: [...] } is sent to the API and title/body are left untouched.

update-discussion uses the GraphQL API, and the old updateDiscussion mutation call always included title/body (falling back to the current values when the agent hadn't provided them). That's why the fix was needed here: we now skip the updateDiscussion mutation entirely when the config doesn't permit title or body changes, and only run the label-replacement mutations.

[mnkiefer]

@copilot Investigate how labels only are updated and it is ensured title and body are not touched.

[Copilot]

@copilot Investigate how labels only are updated and it is ensured title and body are not touched.

Here's the full trace for a labels-only config (labels: set, no title: or body:):

1. Compile time (compiler_safe_outputs_config.go):
Only the keys that are explicitly set in the YAML frontmatter are forwarded to the runtime config. For a labels-only config, only allow_labels: true is added — allow_title and allow_body are never set (they remain undefined at runtime).

2. buildDiscussionUpdateData() (update_discussion.cjs):

// These conditions evaluate to false — title/body never enter updateData
if (config.allow_title === true && item.title !== undefined) { ... }  // false
if (config.allow_body === true && item.body !== undefined) { ... }     // false

// Only this runs
if (config.allow_labels === true && item.labels !== undefined) {
  updateData.labels = labelsResult.value;
}
// → updateData = { labels: [...], _includeFooter: true }

3. executeDiscussionUpdate() (update_discussion.cjs):

// Guard: skipped entirely because updateData.title and updateData.body are both undefined
if (updateData.title !== undefined || updateData.body !== undefined) {
  // ← updateDiscussion GraphQL mutation — NOT called
}

// Only this runs:
if (updateData.labels !== undefined) {
  await replaceDiscussionLabels(github, discussion.id, labelIds);
  // Uses removeLabelsFromLabelable + addLabelsToLabelable mutations only
}

So for a labels-only config, the updateDiscussion mutation (which would touch title/body) is never called. The only GraphQL mutations executed are removeLabelsFromLabelable and addLabelsToLabelable, which operate purely on the label set.

[mnkiefer]

@copilot Investigate if there is an issue on how label updates are handled. How are they passed?

[github-actions]

Hey @Copilot 👋 — great fix for the update-discussion permission-boundary bug! Skipping the updateDiscussion GraphQL mutation when neither title nor body is permitted is exactly the right approach, and the ?? operator swap is a nice correctness improvement over ||.

One thing would really strengthen this PR before it lands:

• Add tests — update_discussion.cjs is the only update handler in actions/setup/js/ without a corresponding test file. Every peer module (update_issue.cjs, update_pull_request.cjs, update_project.cjs, update_release.cjs, etc.) has a .test.cjs file using Vitest with mocked core, github, and context objects. A update_discussion.test.cjs covering at least these three scenarios would close the gap:

  1. Labels-only config — updateDiscussion mutation must not be called; only removeLabelsFromLabelable / addLabelsToLabelable should run.
  2. Title+body config — updateDiscussion mutation is called with the correct variables.
  3. allow_body === true gating — body is included in updateData only when the config flag is set.

• Consistency note (optional) — The PR body says the new allow_body === true guard is "consistent with" update_issue.cjs (allow_body !== false), but the two are semantically opposite defaults: update_issue permits body by default (opt-out), while update_discussion now requires explicit opt-in. If this is intentional (stricter default for GraphQL mutations), it's worth calling out explicitly in the description so reviewers don't flag it as an accident.

If you'd like a hand, you can assign this prompt to your coding agent:

Create actions/setup/js/update_discussion.test.cjs using Vitest (matching the patterns in update_issue.test.cjs).

Mock `@actions/core` as `mockCore` and provide a `mockGithub` object with a `graphql` mock.

Write the following test cases:

1. **Labels-only config** (`allow_labels: true`, no `allow_title`/`allow_body`):
   - Call the handler with an item that has `labels: ['bug']` but also has `title` and `body` fields.
   - Assert that `github.graphql` was NOT called with a mutation containing `updateDiscussion`.
   - Assert that `github.graphql` WAS called with `addLabelsToLabelable`.

2. **Title+body config** (`allow_title: true`, `allow_body: true`):
   - Call the handler with `title: 'New title'` and `body: 'New body'`.
   - Assert that `github.graphql` WAS called with `updateDiscussion` and the correct variables.

3. **`allow_body` gating**:
   - Call `buildDiscussionUpdateData` directly with `config = { allow_body: false }` and an item with `body: 'x'`.
   - Assert that the returned `updateData` does NOT contain a `body` key.

Follow the same `vi.mock` setup, `beforeEach` reset pattern, and describe/it block structure as `update_issue.test.cjs`.

Generated by Contribution Check · ◷

[Copilot]

@copilot Investigate if there is an issue on how label updates are handled. How are they passed?

Found a real bug in the label update path (commit e9437a7).

The issue: validateLabels was called without an explicit maxCount, so it defaulted to 3 — but tryEnforceArrayLimit was already enforcing MAX_LABELS = 10. This meant 4–10 labels would silently be truncated to 3 even though they were within the allowed limit.

Fix in buildDiscussionUpdateData:

// Before – maxCount defaults to 3, silently truncates 4–10 labels
const labelsResult = validateLabels(item.labels, allowedLabels.length > 0 ? allowedLabels : undefined);

// After – maxCount is MAX_LABELS (10), consistent with tryEnforceArrayLimit
const labelsResult = validateLabels(item.labels, allowedLabels.length > 0 ? allowedLabels : undefined, MAX_LABELS);

The full label path for a labels-only config is:

1. buildDiscussionUpdateData: validates/sanitizes labels into updateData.labels; title/body are never set (gated by allow_title/allow_body)
2. executeDiscussionUpdate: guard if (updateData.title !== undefined || updateData.body !== undefined) evaluates false → updateDiscussion mutation is skipped entirely
3. Only removeLabelsFromLabelable + addLabelsToLabelable mutations are called

Also added update_discussion.test.cjs (15 tests) covering all three gates plus the MAX_LABELS regression.

[mnkiefer]

• Raising an issue to be more thorough and also catch related bugs in similar safe outputs