Fix: Preserve manually added labels during workflow run and refine label sync logic #917
Conversation
There was a problem hiding this comment.
Pull request overview
This PR enhances the labeler action to preserve labels that are manually added or added by other bots during the workflow execution, addressing issue #908. The key improvement is fetching the latest PR labels before applying changes, merging them with config-based labels, and ensuring accurate outputs.
- Fetches latest PR labels to detect manual additions during workflow execution
- Merges manually added labels with config-based labels while respecting the 100-label limit
- Updates the 'all-labels' output to reflect the final applied labels
Reviewed changes
Copilot reviewed 1 out of 2 changed files in this pull request and generated 7 comments.
| File | Description |
|---|---|
| src/labeler.ts | Implements label fetching and merging logic to preserve manually added labels, updates output to use final applied labels |
| dist/index.js | Compiled/bundled version of the TypeScript changes with identical logic updates |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
… labels are prioritized
jnewland
left a comment
jnewland
left a comment
There was a problem hiding this comment.
#908 describes a lost update where a user-added label is removed because a setLabels call is made without knowledge of the user-added label. My read of this PR is that it reduces the window of time during which updates can be lost but it does not in any way prevent those lost updates from happening.
I don't believe that ANY approach using setLabels can fix the lost-update problem introduced by #497: setLabels replaces the entire label set in a single write operation, discarding any label changes made since the last read. If concurrent changes are made between the read and write steps - by a user or another workflow - those updates will be overwritten and lost.
I believe the only path towards removing the ability of this action to clobber user writes is to rework it to use addLabels and removeLabels as it did before #497.
| const pr = await client.rest.pulls.get({ | ||
| ...github.context.repo, | ||
| pull_number: pullRequest.number | ||
| }); | ||
| latestLabels.push(...pr.data.labels.map(l => l.name).filter(Boolean)); | ||
| } | ||
|
|
||
| // Labels added manually during the run (not in first snapshot) | ||
| const manualAddedDuringRun = latestLabels.filter( | ||
| l => !preexistingLabels.includes(l) | ||
| ); | ||
|
|
||
| // Preserve manual labels first, then apply config-based labels, respecting GitHub's 100-label limit | ||
| finalLabels = [ | ||
| ...new Set([...manualAddedDuringRun, ...labelsToApply]) | ||
| ].slice(0, GITHUB_MAX_LABELS); | ||
|
|
||
| await api.setLabels(client, pullRequest.number, finalLabels); |
There was a problem hiding this comment.
What would happen if a label were added by a human while a GC pause happened at line 74? My read of this code is that it'd be lost just like the original issue describes.
2 participants
Description:
This pull request updates the
labelerfunction to improve how labels are managed and applied to pull requests, especially when labels may have been added manually or by other bots during the workflow run.Label management improvements:
GITHUB_MAX_LABELS).all-labelsnow reflects the final set of labels actually applied to the pull request, ensuring outputs are accurate and up-to-date..Related issue:
(#908).
Check list: