Skip to content
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.

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

Labeler should never remove labels it has not configured #763

Open
2 tasks done
silverwind opened this issue Mar 17, 2024 · 3 comments
Open
2 tasks done

Labeler should never remove labels it has not configured #763

silverwind opened this issue Mar 17, 2024 · 3 comments
Labels
bug Something isn't working

Comments

@silverwind
Copy link
Contributor

silverwind commented Mar 17, 2024

Description:
The label addition is not atomic and there is a small time window after opening a PR where the action will unexpectedly remove user-added labels. Example from here:

image

I guess the API flow is like this:

  • Labeler does a GET labels request
  • User changes labels (which invalidates the labels seen in the GET)
  • Labeler does a PUT labels request with previously seen labels, reverting the user's changes

Action version:
v5

Platform:

  • All

Runner type:

  • All

Repro steps:

  1. Have a simple labeler action with this config
  2. Open a PR
  3. Immediatly set some labels on the PR
  4. If the timing is right, the labeler will unexpectedly remove the user-added labels

Expected behavior:
The action should atomically update the labels or at least never removing labels it does not have in its config.

Actual behavior:
The action will remove user-set labels when set within a certain time window.

@silverwind silverwind added bug Something isn't working needs triage labels Mar 17, 2024
@HarithaVattikuti
Copy link
Contributor

Hello @silverwind
Thank you for creating this issue. We will investigate it and get back to you as soon as we have some feedback.

@silverwind silverwind changed the title Label additions should be atomic actions Labeler should never remove labels it has not configured Mar 22, 2024
@silverwind
Copy link
Contributor Author

silverwind commented Mar 22, 2024

I reworded the PR because I think "atomic" is not the right term. Essentially it should:

  • On first run of a PR, don't remove any labels, only add.
  • On subsequent runs, never remove labels that are not in the configuration.

It will need to use a 1-shot API to achieve this, as 2-shot will always have the race condition.

@nzakas
Copy link

nzakas commented Apr 2, 2024

We are running into the same issue, where the labeler is removing a label that a different bot added.
eslint/eslint#18183
311469955-abb37dcb-fa01-4be9-84e9-b0bc5aac8583

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

3 participants