fix(ci): refresh stale issues on human comments#20914
fix(ci): refresh stale issues on human comments#20914hesreallyhim wants to merge 2 commits intoanthropics:mainfrom
Conversation
Post stale issue warning after 30 days of inactivity. Refresh timeout when human comments are made. Close issue after 60 days of continuous inactivity.
|
I believe this is the wrong approach. The correct logic IMHO should be:
Of course an even better logic:
|
|
Thanks @ANogin that's good feedback and I appreciate your comments. I guess I'm not sure I understand why one approach is "wrong" (unless you mean technically incorrect) and the other is "correct". Since you don't identify any logical issues, I have to assume it's more of a stylistic preference. Because this is the Anthropic Claude Code repository, my approach was guided by "minimal change set". I think your label-based approach may be sound, but currently the removal of the label is handled by a separate workflow. This creates a coupling between the two workflows that could be avoided for simplicity and to avoid brittleness. Your suggested flow sounds like you want to incorporate the label-removal action into this workflow. That's reasonable, but it's a larger change. The second proposal you make is a completely new flow. It might be a good one, but you should consider how likely is it that a larger refactor is going to be reviewed or accepted, and do you want this problem to be fixed or not. So, my reply would be that unless you think that my change is logically incorrect or deficient (or more complex than the alternative you suggest), the strategic thing to do is to try to push through the smallest change possible in order to maximize the likelihood that a maintainer will review it. I hope that clarifies the proposed changes, which were not made in the spirit of "What's the best possible workflow I could imagine" but rather "what has the highest chance of being reviewed and accepted without corrections." Thanks. 👍🏽 |
|
As noted in #16497 the first question to ask is, does t make sense to close bugs as "stale"? If so, is 30+30 days really enough time to make sure active bugs are not closed needlessly? Both of these are, at the very least, reasonable to challenge. |
|
@fsc-eriker that's a very good point and I recommend you continue to press the team on this issue if it is important to you. That said, to be fair, that's outside the scope of this PR, which is to implement a bug fix that at least ensures that the bot is doing what it says it's going to do, and hopefully represents an improvement from the current state of affairs, given your concerns. I can certainly sympathize if you feel that the whole stale-issue workflow is unjust, but I hope that people recognize that raising those challenges in the context of this PR may be counter-productive. The scope of this PR is to fix a bug, not to raise a policy debate. |
You are right - I misunderstood your suggestion, sorry. Your approach is effectively equivalent and is indeed a lot simpler. |
3 participants
Description
The
manage-stale-issues.ymlworkflow suggests that the issue -> "stale" warning -> closed flow is such that 30 days of inactivity leads to a warning, and 60 days of (consecutive) inactivity leads to a closure. For example, the bot says:It also suggests that if an issue receives a warning, the user can effectively "dismiss" it by adding a keep-alive comment:
Unfortunately, the logic of the workflow does not follow these statements, and a few users have opened issues reporting that their issue was closed prematurely. @marcindulak has been compiling a list of these issues as though preparing for a senate committee hearing. (#16497 )
The Issue
The workflow does this:
The problem is: if the last comment was by a human, all this does is affect the "last_updated is before 30-days-ago" check (if it is, it just bypasses the rest). It doesn't actually take into account "Oh, someone has commented since the first warning, so I guess this is not a stale issue." What it does is, effectively, restarts a 30-day "countdown timer", which opens up a second window of potential inactivity - and if there are no new comments, then it sees two 30-day periods of inactivity, and it closes the issue.
Another way to frame it is: an issue will be closed if it has two 30-day periods of no new comments - but those periods do not have to be consecutive. So the bot's statement, "inactive for 60 days", which implies 60 days continuously, is inaccurate, or highly misleading at best.
This explains why if a user gets a warning on their thread, and then immediately comes to post "Yes this is still happening," it basically has no effect.
TL/DR: what, that was long?
The Fix
In this PR, I propose a minimal change which basically sets the simple condition:
Because the intentions/design of the maintainers can only be inferred by the bot's messages and the code/comments, there is no uniquely correct fix. So, I just went with the natural interpretation - that 60 days of inactivity means 60 continuous days, and it checks by comparing the date against the last human comment, and not computing something on the basis of (a) a 30-day period of inactivity, and (b) a previous comment made by the bot.
EDIT: Fixes: #16497