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

Stale label not being removed after new activity #441

Closed
Rylan12 opened this issue May 8, 2021 · 20 comments · Fixed by #519
Closed

Stale label not being removed after new activity #441

Rylan12 opened this issue May 8, 2021 · 20 comments · Fixed by #519
Labels
bug Something isn't working

Comments

@Rylan12
Copy link
Contributor

Rylan12 commented May 8, 2021

Describe your issue

The stale label isn't being removed from PRs when there's new activity even though remove-stale-when-updated isn't set.

Your stale action configuration

on:
  push:
    paths:
      - .github/workflows/triage-issues.yml
  schedule:
    # Once every day at midnight UTC
    - cron: "0 0 * * *"
  issue_comment:

jobs:
  stale:
    if: >
      startsWith(github.repository, 'Homebrew/') && (
        github.event_name != 'issue_comment' || (
          contains(github.event.issue.labels.*.name, 'stale') ||
          contains(github.event.pull_request.labels.*.name, 'stale')
        )
      )
    runs-on: ubuntu-latest
    steps:
      - name: Mark/Close Stale Issues and Pull Requests
        uses: actions/stale@v3
        with:
          repo-token: ${{ secrets.GITHUB_TOKEN }}
          days-before-stale: 21
          days-before-close: 7
          stale-issue-message: >
            This issue has been automatically marked as stale because it has not had
            recent activity. It will be closed if no further activity occurs.
          stale-pr-message: >
            This pull request has been automatically marked as stale because it has not had
            recent activity. It will be closed if no further activity occurs.
          exempt-issue-labels: "gsoc-outreachy,help wanted,in progress"
          exempt-pr-labels: "gsoc-outreachy,help wanted,in progress"

Further context

I made a comment on Homebrew/brew#11140 (comment) that should have caused the action to remove the stale label. The action did run successfully (the logs are available here). Here are the relevant lines from the logs:

[#11140] Found this pull request last updated 2021-05-08T15:44:35Z
[#11140] The option "onlyLabels" was not specified. Continuing the process for this pull request
[#11140] Days before pull request stale: 21
[#11140] The pull request is not closed nor locked. Trying to remove the close label...
[#11140] There is no close label on this pull request. Skip
[#11140] This pull request has a stale label
[#11140] This pull request has no assignee
[#11140] Skip the assignees checks
[#11140] Found a stale pull request
[#11140] Checking for label on pull request
[#11140] Pull request marked stale on: 2021-05-08T15:44:35Z
[#11140] Checking for comments on pull request since 2021-05-08T15:44:35Z
[#11140] Comments not made by actor or another bot: 0
[#11140] Pull request has been commented on: false
[#11140] Days before pull request close: 7
[#11140] Pull request has been updated: true
[#11140] Stale pull request is not old enough to close yet (hasComments? false, hasUpdate? true)

The line that says Pull request marked stale on: 2021-05-08T15:44:35Z seems to be incorrect, as the stale label was added 16 hours ago before that time. I wonder if the action is using the time of the latest comment as the time that the stale label was added.

@Rylan12 Rylan12 added the bug Something isn't working label May 8, 2021
@Rylan12
Copy link
Contributor Author

Rylan12 commented May 8, 2021

This line looks like it explains the output that I'm seeing:

const markedStaleOn: string =
(await this.getLabelCreationDate(issue, staleLabel)) || issue.updated_at;

To me, this means that getLabelCreationDate isn't working properly because it seems to be returning a falsey value. I'm not sure what's going on here exactly. I simulated running the getLabelCreationDate locally and it seemed to return the correct value which makes me think that this query might not be working as expected:

const options = this.client.issues.listEvents.endpoint.merge({
owner: context.repo.owner,
repo: context.repo.repo,
per_page: 100,
issue_number: issue.number
});

@Rylan12
Copy link
Contributor Author

Rylan12 commented May 9, 2021

One more thing to add, a colleague pointed out to me that the label is removed as expected during a scheduled run (we have it run every 24 hours). It seems that the times where it doesn't work are on issue_comment events. The reason we run on issue_comment events is so the label can be immediately removed without needing to wait up to 24 hours. I guess we could run the job more frequently but even that wouldn't be as immediate as we'd like, I think.

@MAGICCC
Copy link

MAGICCC commented Jun 2, 2021

@Rylan12 I am thinking about using issue_comment to trigger the workflow after a comment has been made in an issue/pr to remove the label, but I suppose this doesnt work yet right?

@Rylan12
Copy link
Contributor Author

Rylan12 commented Jun 3, 2021

I haven't seen a change since I've opened this issue, so I'd say don't bother with it at the moment. There are several downsides to having this (seemingly) useless issue_comment run, the biggest being that I get a notification every time that I comment on a non-stale issue or PR telling me that the run was skipped.

But if you try and it works for you I'd love to see the configuration to try to figure out if I'm just doing something wrong 😅

I'm probably going to remove the issue_comment event from Homebrew's stale action configuration because it seems like it's totally useless. I don't have the time or the will to work on a PR to fix this, but I'm going to leave the issue open until it's auto-closed unless in case someone wants to work on it (I will be very grateful to that person)

@T6751
Copy link

T6751 commented Jun 3, 2021

I think, this PR can fix this problem with issue_comment #390

@MAGICCC
Copy link

MAGICCC commented Jun 5, 2021

I think, this PR can fix this problem with issue_comment #390

This would be great to get this merged!

C0ZEN added a commit to C0ZEN/stale that referenced this issue Jun 7, 2021
C0ZEN added a commit to C0ZEN/stale that referenced this issue Jun 7, 2021
Helping to close actions#441, actions#470, actions#435?
Closes actions#390 due to no activity

BREAKING CHANGES: the options related to remove-stale-when-updated will only check the updates, not the comment. It is only impactint the configurations using the value at false
C0ZEN added a commit to C0ZEN/stale that referenced this issue Jun 8, 2021
Helping to close actions#441, actions#470, actions#435?
Closes actions#390 due to no activity

BREAKING CHANGES: the options related to remove-stale-when-updated will only check the updates, not the comment. It is only impactint the configurations using the value at false
C0ZEN added a commit to C0ZEN/stale that referenced this issue Jun 10, 2021
Helping to close actions#441, actions#470, actions#435?
Closes actions#390 due to no activity

BREAKING CHANGES: the options related to remove-stale-when-updated will only check the updates, not the comment. It is only impactint the configurations using the value at false
@Rylan12
Copy link
Contributor Author

Rylan12 commented Jun 14, 2021

Thanks for working on this, @C0ZEN! 🎉

Did a quick test with this and it looks like it will fix our issue. Once this is released and we start using it I'll follow up if things still aren't working.

@C0ZEN
Copy link
Contributor

C0ZEN commented Jun 14, 2021

@Rylan12 sure, thanks for the feedbacks and I hope it's working as expected.

@MAGICCC
Copy link

MAGICCC commented Jun 14, 2021

Hi @C0ZEN
I tested it on a testrepo for me but somehow it doesnt work as expected, I guess.

The issue was already old enough for being stale but the workflow didnt start yet. After a comment on issue https://github.com/MAGICCC/teststale/issues/3 the workflow triggered and added a comment on https://github.com/MAGICCC/teststale/issues/4 + https://github.com/MAGICCC/teststale/issues/5 and added stale label + removed it again.

[#5] Issue #5
  [#5] Found this issue last updated at: 2021-06-13T13:51:22Z
  [#5] The option only-labels (​https://github.com/actions/stale#only-labels​) was not specified
  [#5] └── Continuing the process for this issue
  [#5] Days before issue stale: 1
  [#5] The issue is not closed nor locked. Trying to remove the close label...
  [#5] ├── The close-issue-label (​https://github.com/actions/stale#close-issue-label​) option was not set
  [#5] └── Skipping the removal of the close label
  [#5] This issue hasn't a stale label
  [#5] The option any-of-labels (​https://github.com/actions/stale#any-of-labels​) was not specified
  [#5] └── Continuing the process for this issue
  [#5] This issue has no milestone
  [#5] └── Skip the milestones checks
  [#5] This issue has no assignee
  [#5] └── Skip the assignees checks
  [#5] This issue is not stale
  [#5] This issue should be stale based on the last update date the 13-06-2021 (2021-06-13T13:51:22Z)
  [#5] This issue should be marked as stale based on the option days-before-stale (​https://github.com/actions/stale#days-before-stale​) (1)
  [#5] Marking this issue as stale
  [#5] This issue is now stale
  [#5] This issue is already stale
  [#5] Checking for label on this issue
  [#5] Issue marked stale on: 2021-06-14T16:21:58Z
  [#5] Checking for comments on issue since: 2021-06-14T16:21:58Z
  [#5] Comments not made by actor or another bot: 0
  [#5] Issue has been commented on: false
  [#5] Days before issue close: 2
  [#5] Issue has been updated: true
  [#5] The option remove-stale-when-updated (​https://github.com/actions/stale#remove-stale-when-updated​) is: true
  [#5] The stale label should not be removed due to an update
  [#5] The option remove-stale-when-commented (​https://github.com/actions/stale#remove-stale-when-commented​) is: true
  [#5] The stale label should not be removed due to a comment
  [#5] Remove the stale label since the issue has an update and the workflow should remove the stale label when updated
  [#5] The issue is no longer stale. Removing the stale label...
  [#5] Removing the label "stale" from this issue...
  [#5] The label "stale" was removed
  [#5] Skipping the process since the issue is now un-stale
  [#5] 4 operations consumed for this issue

Maybe I did something wrong...

@C0ZEN
Copy link
Contributor

C0ZEN commented Jun 14, 2021

@MAGICCC there is bad condition on the logger on the PR I provided sadly 🐛

if (shouldRemoveStaleWhenUpdated) {
issueLogger.info(
`The stale label should not be removed due to an update`
);
} else {
issueLogger.info(
`The stale label should be removed if all conditions met`
);
}

FYI, I do not test the logs 😞

So, the correct logs should be:

  [#5] Issue has been updated: true
  [#5] The option remove-stale-when-updated (​https://github.com/actions/stale#remove-stale-when-updated​) is: true
  [#5] The stale label should be removed if all conditions met
  [#5] The option remove-stale-when-commented (​https://github.com/actions/stale#remove-stale-when-commented​) is: true
  [#5] The stale label should be removed if all conditions met
  [#5] Remove the stale label since the issue has an update and the workflow should remove the stale label when updated
  [#5] The issue is no longer stale. Removing the stale label...
  [#5] Removing the label "stale" from this issue...
  [#5] The label "stale" was removed
  [#5] Skipping the process since the issue is now un-stale
  [#5] 4 operations consumed for this issue

Nice catch!

Regarding the strange behaviour, I think I can sum up like this:

  • The issue should be stale based on the config and age
  • The action mark as stale by adding the stale label and add a comment as well
  • The action continue to process the issue as a stale one
  • Since the label was added, the field updated_at is considered has updated from GitHub POV (this is done manually by updating the date as today to avoid fetching once again the API)
  • The configuration says that if you have an update, you should unstale
  • My logic would say that it should not be considered as updated but based on the logs the updated since calc is saying that yes it was updated...
  • The action unstale by removing the label...
  • And so, the workflow is useless

@C0ZEN
Copy link
Contributor

C0ZEN commented Jun 14, 2021

const updated_at = new Date().toString();
const daysBeforeClose = 2;

function updatedSince(timestamp, num_days) {
	  const daysInMillis = 1000 * 60 * 60 * 24 * num_days;
	  const millisSinceLastUpdated =
	    new Date().getTime() - new Date(timestamp).getTime();
	  
	  return millisSinceLastUpdated <= daysInMillis;
}

updatedSince(updated_at, daysBeforeClose) // true;

This is currently the behaviour.

@C0ZEN
Copy link
Contributor

C0ZEN commented Jun 15, 2021

@Rylan12 "I hope it's working as expected."... The only time I say this and bingo a bug is introduced...

@C0ZEN
Copy link
Contributor

C0ZEN commented Jun 15, 2021

This one should be reopened then, due to the revert.

C0ZEN added a commit to C0ZEN/stale that referenced this issue Jun 18, 2021
Helping to close actions#441, actions#470, actions#435?
Closes actions#390 due to no activity

BREAKING CHANGES: the options related to remove-stale-when-updated will only check the updates, not the comment. It is only impactint the configurations using the value at false
@C0ZEN
Copy link
Contributor

C0ZEN commented Jun 24, 2021

@luketomlinson could you reopen please?

@C0ZEN
Copy link
Contributor

C0ZEN commented Jun 24, 2021

Seems same as #509

@luketomlinson luketomlinson reopened this Jun 24, 2021
C0ZEN added a commit to C0ZEN/stale that referenced this issue Jun 24, 2021
Helping to close actions#441, actions#470, actions#435?
Closes actions#390 due to no activity

BREAKING CHANGES: the options related to remove-stale-when-updated will only check the updates, not the comment. It is only impactint the configurations using the value at false
@Chriscbr
Copy link

Chriscbr commented Jul 8, 2021

Are there any suggested workarounds for this? We were running into this as well (here is an issue where the comment is ignored: projen/projen#723 and here is how the GitHub action was set up: https://github.com/projen/projen/blob/139d9e3fb66dffdcc976c5a6e873c98099258fcf/.github/workflows/stale.yml )

@Rylan12
Copy link
Contributor Author

Rylan12 commented Jul 13, 2021

Thanks, @luketomlinson and @C0ZEN!

Did a quick test using the main branch and it seems to fix our problem! 🎉 (hopefully for real this time)

@MAGICCC
Copy link

MAGICCC commented Jul 13, 2021

Indeed, tested it with my testrepo and its look fine! Thanks you. I am looking forward for v4 now :)

@C0ZEN
Copy link
Contributor

C0ZEN commented Jul 13, 2021

@luketomlinson well done

C0ZEN added a commit to C0ZEN/stale that referenced this issue Jul 18, 2021
Helping to close actions#441, actions#470, actions#435?
Closes actions#390 due to no activity

BREAKING CHANGES: the options related to remove-stale-when-updated will only check the updates, not the comment. It is only impactint the configurations using the value at false
@marcomarsala
Copy link

marcomarsala commented Oct 28, 2022

not fixed. Just commented selectize/selectize.js#1824 and stale label not removed

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

Successfully merging a pull request may close this issue.

7 participants