Skip to content

Prevent issue-intent labels from removing existing labels #53672

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 6 commits into from
Aug 18, 2026
Merged

Prevent issue-intent labels from removing existing labels #53672

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
88f31e2
Initial plan
Copilot Aug 18, 2026
8ba6bfa
Prevent intent label removal and report suggestions
Copilot Aug 18, 2026
bdb3557
Address label intent review feedback
Copilot Aug 18, 2026
6bd4121
Merge REST label recovery snapshots
Copilot Aug 18, 2026
7e18838
Clarify label recovery merge coverage
Copilot Aug 18, 2026
6a761e6
Strengthen suggested label regression
Copilot Aug 18, 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
117 changes: 93 additions & 24 deletions actions/setup/js/add_labels.cjs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,23 @@ function isPullRequestItem(issueData, nodeId) {
return Boolean(issueData?.pull_request) || nodeId.startsWith("PR_");
}

/**
* @param {...Array<string|{ name?: string }>} labelGroups
* @returns {string[]}
*/
function mergeLabelNames(...labelGroups) {
const merged = [];
const seenLower = new Set();
for (const label of labelGroups.flatMap(group => normalizeLabelNames(group))) {
const key = label.toLowerCase();
if (!seenLower.has(key)) {
seenLower.add(key);
merged.push(label);
}
}
return merged;
}

/**
* Apply labels with issue-intent metadata through the GraphQL updateIssue mutation.
* That mutation replaces the issue's label set, so the requested specs are merged with the
Expand All @@ -72,17 +89,15 @@ function isPullRequestItem(issueData, nodeId) {
* issueNodeId: string,
* labelSpecs: Array<{ name: string }>,
* }} params
* @returns {Promise<string[]>} The label names on the issue after the mutation
* @returns {Promise<string[]>} The label names on the issue after the mutation and any recovery
*/
async function applyIssueIntentLabels({ githubClient, core, repoParts, itemNumber, itemRepo, contextType, issueData, issueNodeId, labelSpecs }) {
const repoLabels = await fetchAllRepoLabels(githubClient, repoParts.owner, repoParts.repo);
const labelIdByName = new Map(repoLabels.map(label => [label.name.toLowerCase(), label.id]));

// Merge existing labels (metadata-free) with the requested specs, de-duplicating by
// lowercased name and favouring the requested specs so their intent metadata wins.
const requestedNamesLower = new Set(labelSpecs.map(spec => spec.name.toLowerCase()));
const existingLabelNames = normalizeLabelNames(issueData.labels || []);
const mergedSpecs = [...labelSpecs, ...existingLabelNames.filter(name => !requestedNamesLower.has(name.toLowerCase())).map(name => ({ name }))];
const labelSpecNamesLower = new Set(labelSpecs.map(spec => spec.name.toLowerCase()));
const mergedSpecs = [...labelSpecs, ...existingLabelNames.filter(name => !labelSpecNamesLower.has(name.toLowerCase())).map(name => ({ name }))];

const labelIntentUpdates = buildIssueIntentLabelUpdates(mergedSpecs, labelIdByName);

Expand Down Expand Up @@ -111,7 +126,29 @@ async function applyIssueIntentLabels({ githubClient, core, repoParts, itemNumbe
`add_labels to ${contextType} #${itemNumber} in ${itemRepo}`
);

return normalizeLabelNames(result?.updateIssue?.issue?.labels?.nodes || []);
let afterLabels = normalizeLabelNames(result?.updateIssue?.issue?.labels?.nodes || []);
const afterNamesLower = new Set(afterLabels.map(name => name.toLowerCase()));
const missingExistingLabels = existingLabelNames.filter(name => !afterNamesLower.has(name.toLowerCase()));

if (missingExistingLabels.length > 0) {
core.warning(
`The GraphQL intent mutation removed ${missingExistingLabels.length} pre-existing label(s) from ${contextType} #${itemNumber} in ${itemRepo}; restoring them via the REST add-labels endpoint: ${JSON.stringify(missingExistingLabels)}`
);
const { data: restoredLabels } = await withRetry(
() =>
githubClient.rest.issues.addLabels({
owner: repoParts.owner,
repo: repoParts.repo,
issue_number: itemNumber,
labels: missingExistingLabels,
}),
RATE_LIMIT_RETRY_CONFIG,
`restore labels on ${contextType} #${itemNumber} in ${itemRepo}`
);
afterLabels = mergeLabelNames(existingLabelNames, afterLabels, restoredLabels);
}

return afterLabels;
}

/**
Expand Down Expand Up @@ -345,10 +382,7 @@ const main = createCountGatedHandler({
const beforeState = await fetchIssueState(githubClient, repoParts, itemNumber);

if (useIssueIntentPath) {
// Intent metadata is only supported via the GraphQL updateIssue mutation. That
// mutation replaces the issue's label set, so merge the newly requested labels with
// the issue's existing labels to preserve add-only semantics. Existing labels are
// sent without intent metadata; newly requested labels carry their metadata.
// Intent metadata is only supported via the GraphQL updateIssue mutation.
const { data: issueData } = await withRetry(
() =>
githubClient.rest.issues.get({
Expand All @@ -372,25 +406,60 @@ const main = createCountGatedHandler({
if (isPullRequestItem(issueData, issueNodeId)) {
core.info(`Issue-intent label metadata is not supported for pull requests; falling back to the REST add-labels endpoint for ${contextType} #${itemNumber} in ${itemRepo}`);
} else {
const afterLabels = await applyIssueIntentLabels({
githubClient,
core,
repoParts,
itemNumber,
itemRepo,
contextType,
issueData,
issueNodeId,
labelSpecs: uniqueLabelSpecs,
});

core.info(`Successfully added ${uniqueLabels.length} labels to ${contextType} #${itemNumber} in ${itemRepo}`);
const existingLabels = normalizeLabelNames(issueData.labels || []);
const existingNamesLower = new Set(existingLabels.map(name => name.toLowerCase()));
const newLabelSpecs = uniqueLabelSpecs.filter(spec => !existingNamesLower.has(spec.name.toLowerCase()));
let afterLabels =
newLabelSpecs.length > 0
? await applyIssueIntentLabels({
githubClient,
core,
repoParts,
itemNumber,
itemRepo,
contextType,
issueData,
issueNodeId,
labelSpecs: newLabelSpecs,
})
: existingLabels;
let afterNamesLower = new Set(afterLabels.map(name => name.toLowerCase()));
const plainLabelsNotApplied = newLabelSpecs.filter(spec => !hasLabelIntentMetadata(spec) && !afterNamesLower.has(spec.name.toLowerCase())).map(spec => spec.name);

if (plainLabelsNotApplied.length > 0) {
core.info(`Adding ${plainLabelsNotApplied.length} metadata-free label(s) not applied by the intent mutation via the REST add-labels endpoint: ${JSON.stringify(plainLabelsNotApplied)}`);
const { data: labels } = await withRetry(
() =>
githubClient.rest.issues.addLabels({
owner: repoParts.owner,
repo: repoParts.repo,
issue_number: itemNumber,
labels: plainLabelsNotApplied,
}),
RATE_LIMIT_RETRY_CONFIG,
`add metadata-free labels to ${contextType} #${itemNumber} in ${itemRepo}`
);
afterLabels = mergeLabelNames(afterLabels, labels);
afterNamesLower = new Set(afterLabels.map(name => name.toLowerCase()));
}

const labelsAdded = newLabelSpecs.filter(spec => afterNamesLower.has(spec.name.toLowerCase())).map(spec => spec.name);
const labelsSuggested = newLabelSpecs.filter(spec => hasLabelIntentMetadata(spec) && !afterNamesLower.has(spec.name.toLowerCase())).map(spec => spec.name);

@github-actions github-actions Bot Aug 18, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

[/diagnosing-bugs] Silent loss: plain-name newLabelSpecs entries dropped by the mutation land in neither labelsAdded nor labelsSuggested.

labelsSuggested is gated on hasLabelIntentMetadata(spec), so a { name: "foo" } spec that the mutation silently drops vanishes from the result with no trace — callers can't distinguish "applied", "suggested", and "silently dropped".

💡 Suggested fix

Capture all unapplied new labels and split them afterwards:

const labelsAdded      = newLabelSpecs.filter(spec =>  afterNamesLower.has(spec.name.toLowerCase())).map(s => s.name);
const labelsNotApplied = newLabelSpecs.filter(spec => !afterNamesLower.has(spec.name.toLowerCase()));
const labelsSuggested  = labelsNotApplied.filter(spec => hasLabelIntentMetadata(spec)).map(s => s.name);
// optionally warn on labelsNotApplied that are not suggested

A regression test with a metadata-free spec that the mutation silently drops would lock this in.

@copilot please address this.


if (newLabelSpecs.length === 0) {

@github-actions github-actions Bot Aug 18, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

L412-413: delete: redundant "No new labels to add" info log. Line 415's "Successfully added 0 labels" already conveys this.

core.info(`No new labels to add to ${contextType} #${itemNumber} in ${itemRepo}`);
}
core.info(`Successfully added ${labelsAdded.length} labels to ${contextType} #${itemNumber} in ${itemRepo}`);
if (labelsSuggested.length > 0) {
core.info(`Suggested ${labelsSuggested.length} labels for ${contextType} #${itemNumber} in ${itemRepo}: ${JSON.stringify(labelsSuggested)}`);
}
return attachExecutionState(
{
success: true,
number: itemNumber,
repo: itemRepo,
labelsAdded: uniqueLabels,
labelsAdded,
labelsSuggested,
contextType,
},
beforeState,
Expand Down
191 changes: 189 additions & 2 deletions actions/setup/js/add_labels.test.cjs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ describe("add_labels", () => {
};
}
// updateIssue intent mutation: echo back the requested label names.
const labels = (variables?.labels || []).map(l => ({ name: l.name || l.labelId }));
const labels = (variables?.labels || []).map(l => ({ name: l.name || l.labelId?.replace(/^LABEL_/, "") }));
return { updateIssue: { issue: { id: variables?.issueId, labels: { nodes: labels } } } };
},
rest: {
Expand Down Expand Up @@ -246,6 +246,193 @@ describe("add_labels", () => {
expect(graphqlMutationCalls[0].labels).toEqual([{ labelId: "LABEL_bug", rationale: "Crash on upload", confidence: "HIGH" }, { labelId: "LABEL_enhancement" }]);
});

it("should skip an already-applied label instead of re-proposing its intent metadata", async () => {
const handler = await main({ max: 10 });
const graphqlMutationCalls = [];
const addLabelsCalls = [];

mockGithub.rest.issues.get = async () => ({
data: {
node_id: "ISSUE_NODE_ID",
title: "Test issue title",
labels: [{ name: "feature-openapi" }, { name: "area-minimal" }],
},
});
const originalGraphql = mockGithub.graphql;
mockGithub.graphql = async (query, variables) => {
if (typeof query === "string" && query.includes("updateIssue")) {
graphqlMutationCalls.push(variables);
}
return originalGraphql(query, variables);
};
mockGithub.rest.issues.addLabels = async params => {
addLabelsCalls.push(params);
return {};
};

const result = await handler(
{
item_number: 68619,
labels: [{ name: "area-minimal", rationale: "Minimal APIs area", confidence: "MEDIUM" }],
},
{}
);

expect(result.success).toBe(true);
expect(result.labelsAdded).toEqual([]);
expect(result.labelsSuggested).toEqual([]);
expect(result.after_state.labels).toEqual(["feature-openapi", "area-minimal"]);
expect(graphqlMutationCalls).toHaveLength(0);
expect(addLabelsCalls).toHaveLength(0);
});

it("should report a confidence-gated intent label as suggested rather than added", async () => {
const handler = await main({ max: 10 });

mockGithub.rest.issues.get = async () => ({
data: {
node_id: "ISSUE_NODE_ID",
title: "Test issue title",
labels: [{ name: "feature-openapi" }],
},
});
mockGithub.graphql = async (query, variables) => {
if (typeof query === "string" && query.includes("repository(owner")) {
return {
repository: {
labels: {
nodes: [
{ id: "LABEL_feature-openapi", name: "feature-openapi" },
{ id: "LABEL_area-minimal", name: "area-minimal" },
],
pageInfo: { hasNextPage: false, endCursor: null },
},
},
};
}
return { updateIssue: { issue: { labels: { nodes: [{ name: "feature-openapi" }] } } } };
};

const result = await handler(
{
item_number: 68619,
labels: [{ name: "area-minimal", rationale: "Minimal APIs area", confidence: "MEDIUM" }],
},
{}
);

expect(result.success).toBe(true);
expect(result.labelsAdded).toEqual([]);
expect(result.labelsSuggested).toEqual(["area-minimal"]);
expect(result.after_state.labels).toEqual(["feature-openapi"]);
expect(mockCore.infos).toContain("Successfully added 0 labels to issue #68619 in test-owner/test-repo");
});

it("should add metadata-free labels through REST when the intent mutation does not apply them", async () => {
const handler = await main({ max: 10 });
const addLabelsCalls = [];
const graphqlMutationCalls = [];

mockGithub.rest.issues.get = async () => ({
data: {
node_id: "ISSUE_NODE_ID",
title: "Test issue title",
labels: [{ name: "feature-openapi" }],
},
});
mockGithub.graphql = async (query, variables) => {
if (typeof query === "string" && query.includes("repository(owner")) {
return {
repository: {
labels: {
nodes: [
{ id: "LABEL_feature-openapi", name: "feature-openapi" },
{ id: "LABEL_area-minimal", name: "area-minimal" },
{ id: "LABEL_bug", name: "bug" },
],
pageInfo: { hasNextPage: false, endCursor: null },
},
},
};
}
graphqlMutationCalls.push(variables);
return { updateIssue: { issue: { labels: { nodes: [{ name: "feature-openapi" }] } } } };
};
mockGithub.rest.issues.addLabels = async params => {
addLabelsCalls.push(params);
return { data: [{ name: "bug" }] };
};

const result = await handler(
{
item_number: 68619,
labels: [{ name: "area-minimal", rationale: "Minimal APIs area", confidence: "MEDIUM" }, { name: "bug" }],
},
{}
);

expect(result.success).toBe(true);
expect(result.labelsAdded).toEqual(["bug"]);
expect(result.labelsSuggested).toEqual(["area-minimal"]);
expect(result.after_state.labels).toEqual(["feature-openapi", "bug"]);
expect(result.after_state.labels).not.toContain("area-minimal");
expect(graphqlMutationCalls).toHaveLength(1);
expect(graphqlMutationCalls[0].labels).toEqual([{ labelId: "LABEL_area-minimal", rationale: "Minimal APIs area", confidence: "MEDIUM" }, { labelId: "LABEL_bug" }, { labelId: "LABEL_feature-openapi" }]);
expect(addLabelsCalls).toHaveLength(1);
expect(addLabelsCalls[0].labels).toEqual(["bug"]);
});

it("should restore pre-existing labels omitted by the intent mutation", async () => {
const handler = await main({ max: 10 });
const addLabelsCalls = [];
const restoredLabels = [{ name: "area-minimal" }];

mockGithub.rest.issues.get = async () => ({
data: {
node_id: "ISSUE_NODE_ID",
title: "Test issue title",
labels: [{ name: "feature-openapi" }, { name: "area-minimal" }],
},
});
mockGithub.graphql = async (query, variables) => {
if (typeof query === "string" && query.includes("repository(owner")) {
return {
repository: {
labels: {
nodes: [
{ id: "LABEL_feature-openapi", name: "feature-openapi" },
{ id: "LABEL_area-minimal", name: "area-minimal" },
{ id: "LABEL_bug", name: "bug" },

@github-actions github-actions Bot Aug 18, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

[/tdd] The "restore pre-existing labels" test asserts result.after_state.labels but the mock addLabels REST call returns { data: [...] } — the actual after_state is captured by attachExecutionState from a separate fetchIssueState call that is NOT re-mocked in this test.

This means result.after_state.labels likely reflects the pre-mutation state fetched from the default mock (only feature-openapi), not the restored set. If so, the assertion expect(result.after_state.labels).toEqual(["feature-openapi", "area-minimal", "bug"]) is either coincidentally passing or testing the wrong thing.

💡 Suggested fix

Mock the afterState fetch (mockGithub.rest.issues.get returning the post-restore label set) or explicitly verify result.afterLabels (the value returned from applyIssueIntentLabels) rather than relying on the after_state snapshot populated by attachExecutionState.

@copilot please address this.

],
pageInfo: { hasNextPage: false, endCursor: null },
},
},
};
}
return { updateIssue: { issue: { labels: { nodes: [{ name: "feature-openapi" }, { name: "bug" }] } } } };
};
mockGithub.rest.issues.addLabels = async params => {
addLabelsCalls.push(params);
return { data: restoredLabels };
};

const result = await handler(
{
item_number: 68619,
labels: [{ name: "bug", rationale: "Confirmed defect", confidence: "HIGH" }],
},
{}
);

expect(result.success).toBe(true);
expect(result.labelsAdded).toEqual(["bug"]);
expect(result.labelsSuggested).toEqual([]);
expect(result.after_state.labels).toEqual(["feature-openapi", "area-minimal", "bug"]);
expect(addLabelsCalls).toHaveLength(1);
expect(addLabelsCalls[0].labels).toEqual(["area-minimal"]);
expect(mockCore.warnings[0]).toContain("restoring them via the REST add-labels endpoint");
});

it("should return a standardized error code when issue node_id is missing on issue-intent path", async () => {
const handler = await main({ max: 10, issue_intent: true });
mockGithub.rest.issues.get = async () => ({
Expand Down Expand Up @@ -894,7 +1081,7 @@ describe("add_labels", () => {
const result = await handler(
{
item_number: 456,
labels: [{ name: "bug", rationale: "Crash", confidence: "HIGH" }],
labels: [{ name: "enhancement", rationale: "Improves the issue", confidence: "HIGH" }],
},
{}
);
Expand Down
Loading
Loading