Skip to content

Fix add_labels silently applying no labels #50637

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 3 commits into from
Aug 5, 2026
+208 −44
Merged

Fix add_labels silently applying no labels #50637

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
83128b6
Initial plan
Copilot Aug 5, 2026
b635537
Fix add_labels: send plain label names to REST addLabels endpoint
Copilot Aug 5, 2026
a1c0288
Forward add_labels intent metadata via GraphQL updateIssue mutation
Copilot Aug 5, 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
93 changes: 87 additions & 6 deletions actions/setup/js/add_labels.cjs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,8 @@ const { MAX_LABELS } = require("./constants.cjs");
const { createCountGatedHandler } = require("./handler_scaffold.cjs");
const { withRetry, RATE_LIMIT_RETRY_CONFIG } = require("./error_recovery.cjs");
const { resolveInvocationContext } = require("./invocation_context_helpers.cjs");
const { normalizeIssueIntentLabelInputs } = require("./issue_intents.cjs");
const { normalizeIssueIntentLabelInputs, buildIssueIntentLabelUpdates } = require("./issue_intents.cjs");
const { fetchAllRepoLabels } = require("./github_api_helpers.cjs");

/**
* @param {{ rationale?: string, confidence?: string, suggest?: boolean } | null | undefined} spec
Expand Down Expand Up @@ -223,11 +224,18 @@ const main = createCountGatedHandler({
};
}

const labelsRequestPayload = uniqueLabels.map(name => {
const labelSpec = requestedLabelSpecByLowerName.get(name.toLowerCase()) ?? { name };
const hasIntentMetadata = hasLabelIntentMetadata(labelSpec);
return issueIntentEnabled && hasIntentMetadata ? labelSpec : labelSpec.name;
});
// Build the resolved label specs (name + optional intent metadata) for the validated
// unique labels, preserving the order returned by validation.
const uniqueLabelSpecs = uniqueLabels.map(name => requestedLabelSpecByLowerName.get(name.toLowerCase()) ?? { name });
const intentLabelSpecs = uniqueLabelSpecs.filter(spec => hasLabelIntentMetadata(spec));
const useIssueIntentPath = issueIntentEnabled && intentLabelSpecs.length > 0;

// The REST issues.addLabels endpoint only accepts label name strings; it does not
// support issue-intent metadata (rationale/confidence/suggest). Passing objects with
// those extra keys causes GitHub to return success while silently applying no labels.
// When intent metadata is present, route through the GraphQL updateIssue/LabelUpdateInput
// mutation instead (see update_issue.cjs), which does support intent metadata.
Comment on lines +233 to +237
const labelsRequestPayload = uniqueLabels;

core.info(`Adding ${uniqueLabels.length} labels to ${contextType} #${itemNumber} in ${itemRepo}: ${JSON.stringify(labelsRequestPayload)}`);

Expand All @@ -248,6 +256,79 @@ const main = createCountGatedHandler({

try {
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.
Comment on lines +261 to +264
const { data: issueData } = await withRetry(

@github-actions github-actions Bot Aug 5, 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] Redundant issues.get call: fetchIssueState (line 258) already calls github.rest.issues.get unconditionally, then the intent path calls it again here to get node_id and labels. This doubles the REST cost and creates two separate failure points for every intent-label operation.

💡 Suggested fix

Call issues.get once above the if (useIssueIntentPath) block and use the result for both beforeState extraction and the intent path:

const { data: issueData } = await withRetry(
  () => githubClient.rest.issues.get({ owner: repoParts.owner, repo: repoParts.repo, issue_number: itemNumber }),
  RATE_LIMIT_RETRY_CONFIG,
  `get ${contextType} #${itemNumber}`
);
const beforeState = attachExecutionState(extractIssueStateFromData(issueData), ...);
// reuse issueData.node_id and issueData.labels in the intent block

This removes the extra round-trip and makes the failure surface smaller.

@copilot please address this.

@github-actions github-actions Bot Aug 5, 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.

Redundant issues.get call — double-fetch on the intent path.

fetchIssueState (line 258) already calls github.rest.issues.get internally to capture the before-state. The intent path then makes a second issues.get call (here) solely to retrieve node_id and existing labels.

This doubles the GitHub API calls on every intent-metadata label add. Either expose the raw issue data from fetchIssueState, or call issues.get once before the fetchIssueState call and derive beforeState from that data:

const { data: issueData } = await withRetry(() => githubClient.rest.issues.get(...), ...);
const beforeState = extractIssueStateFromData(issueData); // reuse same data

@copilot please address this.

() =>
githubClient.rest.issues.get({
owner: repoParts.owner,
repo: repoParts.repo,
issue_number: itemNumber,
}),
RATE_LIMIT_RETRY_CONFIG,
`get ${contextType} #${itemNumber} in ${itemRepo}`
);

const issueNodeId = issueData?.node_id;
if (!issueNodeId) {
throw new Error(`Failed to resolve GraphQL node ID for ${contextType} #${itemNumber}`);
}

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(uniqueLabelSpecs.map(spec => spec.name.toLowerCase()));
const existingLabelNames = normalizeLabelNames(issueData.labels || []);
const mergedSpecs = [...uniqueLabelSpecs, ...existingLabelNames.filter(name => !requestedNamesLower.has(name.toLowerCase())).map(name => ({ name }))];

const labelIntentUpdates = buildIssueIntentLabelUpdates(mergedSpecs, labelIdByName);

core.info(`Adding ${uniqueLabels.length} labels to ${contextType} #${itemNumber} in ${itemRepo} via GraphQL intent mutation`);
const result = await withRetry(
() =>
githubClient.graphql(
`mutation($issueId: ID!, $labels: [LabelUpdateInput!]!) {
updateIssue(input: { id: $issueId, labels: $labels }) {
issue {
id
labels(first: 100) {
nodes {
name
}
}
}
}
}`,
{ issueId: issueNodeId, labels: labelIntentUpdates, headers: { "GraphQL-Features": "update_issue_suggestions" } }
),
RATE_LIMIT_RETRY_CONFIG,
`add_labels to ${contextType} #${itemNumber} in ${itemRepo}`

@github-actions github-actions Bot Aug 5, 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] Hard-coded labels(first: 100) cap in the GraphQL response could silently truncate the afterLabels list for issues with more than 100 labels, causing attachExecutionState to report an incorrect post-operation label set.

💡 Suggested fix

Use pagination or, at minimum, add a warning when the response indicates truncation:

// After the mutation response:
const labelPage = result?.updateIssue?.issue?.labels;
if (labelPage?.pageInfo?.hasNextPage) {
  core.warning(`Label list truncated; issue #${itemNumber} has more than 100 labels`);
}
const afterLabels = labelPage?.nodes || [];

Alternatively, bump the fragment to first: 250 (the GitHub API maximum) and add the guard.

@copilot please address this.

);

core.info(`Successfully added ${uniqueLabels.length} labels to ${contextType} #${itemNumber} in ${itemRepo}`);
const afterLabels = result?.updateIssue?.issue?.labels?.nodes || [];
return attachExecutionState(
{
success: true,
number: itemNumber,
repo: itemRepo,
labelsAdded: uniqueLabels,
contextType,
},
beforeState,
{
...beforeState,
labels: normalizeLabelNames(afterLabels),
}
);
}

const { data: labels } = await withRetry(
() =>
githubClient.rest.issues.addLabels({
Expand Down
159 changes: 121 additions & 38 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 @@ -35,11 +35,31 @@ describe("add_labels", () => {
};

mockGithub = {
graphql: async (query, variables) => {
// Repo labels query used by fetchAllRepoLabels: resolve label IDs by name.
if (typeof query === "string" && query.includes("repository(owner")) {
return {
repository: {
labels: {
nodes: (mockGithub._repoLabels || ["bug", "enhancement", "documentation", "security:low", "security:medium", "security:high"]).map(name => ({
id: `LABEL_${name}`,
name,
})),
pageInfo: { hasNextPage: false, endCursor: null },
},
},
};
}
// updateIssue intent mutation: echo back the requested label names.
const labels = (variables?.labels || []).map(l => ({ name: l.name || l.labelId }));
return { updateIssue: { issue: { id: variables?.issueId, labels: { nodes: labels } } } };
},
rest: {
issues: {
addLabels: async () => ({}),
get: async () => ({
data: {
node_id: "ISSUE_NODE_ID",
title: "Test issue title",
labels: [],
},
Expand Down Expand Up @@ -115,11 +135,14 @@ describe("add_labels", () => {

it("should accept structured label entries and add normalized label names", async () => {
const handler = await main({ max: 10, issue_intent: true });
const addLabelsCalls = [];

mockGithub.rest.issues.addLabels = async params => {
addLabelsCalls.push(params);
return {};
const graphqlMutationCalls = [];

const originalGraphql = mockGithub.graphql;
mockGithub.graphql = async (query, variables) => {
if (typeof query === "string" && query.includes("updateIssue")) {
graphqlMutationCalls.push(variables);
}
return originalGraphql(query, variables);
};

const result = await handler(
Expand All @@ -132,17 +155,22 @@ describe("add_labels", () => {

expect(result.success).toBe(true);
expect(result.number).toBe(456);
expect(addLabelsCalls).toHaveLength(1);
expect(addLabelsCalls[0].labels).toEqual([{ name: "bug", rationale: "Known crash path", confidence: "HIGH", suggest: true }]);
// Intent metadata routes through the GraphQL updateIssue mutation, not REST addLabels
expect(graphqlMutationCalls).toHaveLength(1);
expect(graphqlMutationCalls[0].labels).toEqual([{ labelId: "LABEL_bug", rationale: "Known crash path", confidence: "HIGH", suggest: true }]);
expect(graphqlMutationCalls[0].headers).toEqual({ "GraphQL-Features": "update_issue_suggestions" });
});

it("should send structured label metadata without requiring a runtime feature", async () => {
const handler = await main({ max: 10, issue_intent: true });
const addLabelsCalls = [];

mockGithub.rest.issues.addLabels = async params => {
addLabelsCalls.push(params);
return {};
const graphqlMutationCalls = [];

const originalGraphql = mockGithub.graphql;
mockGithub.graphql = async (query, variables) => {
if (typeof query === "string" && query.includes("updateIssue")) {
graphqlMutationCalls.push(variables);
}
return originalGraphql(query, variables);
};

const result = await handler(
Expand All @@ -154,17 +182,20 @@ describe("add_labels", () => {
);

expect(result.success).toBe(true);
expect(addLabelsCalls).toHaveLength(1);
expect(addLabelsCalls[0].labels).toEqual([{ name: "bug", rationale: "Application crashes on file uploads >5MB", confidence: "HIGH" }]);
expect(graphqlMutationCalls).toHaveLength(1);
expect(graphqlMutationCalls[0].labels).toEqual([{ labelId: "LABEL_bug", rationale: "Application crashes on file uploads >5MB", confidence: "HIGH" }]);
});

it("should normalize lowercase confidence in structured label metadata", async () => {
const handler = await main({ max: 10, issue_intent: true });
const addLabelsCalls = [];

mockGithub.rest.issues.addLabels = async params => {
addLabelsCalls.push(params);
return {};
const graphqlMutationCalls = [];

const originalGraphql = mockGithub.graphql;
mockGithub.graphql = async (query, variables) => {
if (typeof query === "string" && query.includes("updateIssue")) {
graphqlMutationCalls.push(variables);
}
return originalGraphql(query, variables);
};

const result = await handler(
Expand All @@ -176,8 +207,43 @@ describe("add_labels", () => {
);

expect(result.success).toBe(true);
expect(addLabelsCalls).toHaveLength(1);
expect(addLabelsCalls[0].labels).toEqual([{ name: "bug", rationale: "Application crashes on file uploads >5MB", confidence: "HIGH" }]);
expect(graphqlMutationCalls).toHaveLength(1);
expect(graphqlMutationCalls[0].labels).toEqual([{ labelId: "LABEL_bug", rationale: "Application crashes on file uploads >5MB", confidence: "HIGH" }]);
});

it("should preserve existing labels when adding intent labels via GraphQL", async () => {
const handler = await main({ max: 10, issue_intent: true });
const graphqlMutationCalls = [];

mockGithub.rest.issues.get = async () => ({
data: {
node_id: "ISSUE_NODE_ID",
title: "Test issue title",
labels: [{ name: "enhancement" }],
},
});

const originalGraphql = mockGithub.graphql;
mockGithub.graphql = async (query, variables) => {
if (typeof query === "string" && query.includes("updateIssue")) {
graphqlMutationCalls.push(variables);
}
return originalGraphql(query, variables);
};

const result = await handler(
{
item_number: 456,
labels: [{ name: "bug", rationale: "Crash on upload", confidence: "HIGH" }],
},
{}
);

expect(result.success).toBe(true);
expect(result.labelsAdded).toEqual(["bug"]);
expect(graphqlMutationCalls).toHaveLength(1);
// Existing labels are merged (metadata-free) so add-only semantics are preserved
expect(graphqlMutationCalls[0].labels).toEqual([{ labelId: "LABEL_bug", rationale: "Crash on upload", confidence: "HIGH" }, { labelId: "LABEL_enhancement" }]);
});

it("should accept issue_number as an alias for item_number", async () => {
Expand Down Expand Up @@ -515,11 +581,14 @@ describe("add_labels", () => {
it("should prefer the metadata-bearing entry when a duplicate label name appears", async () => {
// Default (omitted issue_intent) accepts both strings and objects; deduplication favours the metadata-bearing entry.
const handler = await main({ max: 10 });
const addLabelsCalls = [];

mockGithub.rest.issues.addLabels = async params => {
addLabelsCalls.push(params);
return {};
const graphqlMutationCalls = [];

const originalGraphql = mockGithub.graphql;
mockGithub.graphql = async (query, variables) => {
if (typeof query === "string" && query.includes("updateIssue")) {
graphqlMutationCalls.push(variables);
}
return originalGraphql(query, variables);
};

const result = await handler(
Expand All @@ -533,9 +602,9 @@ describe("add_labels", () => {

expect(result.success).toBe(true);
expect(result.labelsAdded).toEqual(["bug"]);
expect(addLabelsCalls.length).toBe(1);
// The payload sent to the API must use the spec with metadata, not the plain string
expect(addLabelsCalls[0].labels).toEqual([{ name: "bug", rationale: "Known crash path", confidence: "HIGH", suggest: true }]);
// Intent metadata routes through the GraphQL mutation; the metadata-bearing spec wins
expect(graphqlMutationCalls).toHaveLength(1);
expect(graphqlMutationCalls[0].labels).toEqual([{ labelId: "LABEL_bug", rationale: "Known crash path", confidence: "HIGH", suggest: true }]);
});

it("should strip structured intent metadata when issue_intent is disabled", async () => {
Expand All @@ -560,10 +629,18 @@ describe("add_labels", () => {
expect(addLabelsCalls[0].labels).toEqual(["bug"]);
});

it("should forward per-label intent metadata by default when issue_intent is omitted", async () => {
it("should forward per-label intent metadata via GraphQL when issue_intent is omitted", async () => {
const handler = await main({ max: 10 });
const graphqlMutationCalls = [];
const addLabelsCalls = [];

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 {};
Expand All @@ -578,8 +655,10 @@ describe("add_labels", () => {
);

expect(result.success).toBe(true);
expect(addLabelsCalls).toHaveLength(1);
expect(addLabelsCalls[0].labels).toEqual([{ name: "bug", rationale: "Known crash path", confidence: "HIGH", suggest: true }]);
// Intent metadata is forwarded through GraphQL, not the REST addLabels endpoint
expect(addLabelsCalls).toHaveLength(0);
expect(graphqlMutationCalls).toHaveLength(1);
expect(graphqlMutationCalls[0].labels).toEqual([{ labelId: "LABEL_bug", rationale: "Known crash path", confidence: "HIGH", suggest: true }]);
});

it("should accept plain string labels by default when issue_intent is omitted", async () => {
Expand Down Expand Up @@ -653,11 +732,14 @@ describe("add_labels", () => {

it("should accept label objects with both rationale and confidence in strict mode", async () => {
const handler = await main({ max: 10, issue_intent: true });
const addLabelsCalls = [];

mockGithub.rest.issues.addLabels = async params => {
addLabelsCalls.push(params);
return {};
const graphqlMutationCalls = [];

const originalGraphql = mockGithub.graphql;
mockGithub.graphql = async (query, variables) => {
if (typeof query === "string" && query.includes("updateIssue")) {
graphqlMutationCalls.push(variables);
}
return originalGraphql(query, variables);
};

const result = await handler(
Expand All @@ -669,7 +751,8 @@ describe("add_labels", () => {
);

expect(result.success).toBe(true);
expect(addLabelsCalls).toHaveLength(1);
expect(graphqlMutationCalls).toHaveLength(1);
expect(graphqlMutationCalls[0].labels).toEqual([{ labelId: "LABEL_bug", rationale: "Crash on upload", confidence: "HIGH" }]);
});

it("should sanitize and trim label names", async () => {
Expand Down
Loading