Add stale unassign GitHub Action #31
Conversation
- Automatically unassigns stale issues after grace period - Checks for open PRs authored by assignee - Supports minutes (testing) and days (production) - Tested with 5-minute grace period in talawa-admin - Includes retry logic and dry-run capability Resolves PalisadoesFoundation#24
WalkthroughAdds a new composite GitHub Action and a scheduled workflow that automatically unassigns issues labeled as stale after a configurable grace period, checks linked open PRs and assignees, handles API retries with backoff, and posts templated comments; files added at Changes
Sequence Diagram(s)sequenceDiagram
participant Scheduler as GitHub Scheduler
participant Workflow as stale-unassign.yml
participant Action as stale-unassign Action
participant API as GitHub API
participant Repo as Repository
Scheduler->>Workflow: Trigger (daily or manual)
Workflow->>Action: Start with inputs (grace period, dry-run, etc.)
Action->>API: Check repository archived status
API-->>Action: Repo status
Action->>API: Verify stale label & exclusion labels
API-->>Action: Label existence
Action->>API: List open issues with stale label (capped)
API-->>Action: Issues list
loop For each issue
Action->>API: Get issue details & timeline events
API-->>Action: Details & events
Action->>Action: Detect open linked PRs and authors
alt Blocking open linked PR by assignee exists
Action->>Action: Skip issue
else No blocking PR
Action->>Action: Compute time since stale label added
alt Grace period exceeded
Action->>API: Remove assignees (or log dry-run)
API-->>Action: Confirmation
Action->>API: Post comment using template (or log dry-run)
API-->>Action: Comment created
else Grace period not exceeded
Action->>Action: Skip issue
end
end
end
Action-->>Workflow: Emit summary (processed/skipped/errors)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Our Pull Request Approval ProcessThis PR will be reviewed according to our: Your PR may be automatically closed if:
Thanks for contributing! |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In @.github/actions/stale-unassign/action.yml:
- Around line 69-75: The Retry-After handling block currently assumes a numeric
value and uses parseInt which yields NaN for HTTP-date strings; update the logic
inside the same conditional (the block that reads retryAfter from
error.response.headers['retry-after']) to detect whether retryAfter is numeric
and, if not, parse it as an HTTP-date (using Date.parse) to compute
milliseconds; ensure you only call Math.max with valid numbers (fallback to
existing waitTime if parsing fails) so setTimeout never receives NaN.
In @.github/workflows/stale-unassign.yml:
- Around line 49-52: Update the example comment-template to remove the extra '@'
in the placeholder so it doesn't produce a duplicate at-sign; replace the
current placeholder syntax @{assignee} with {assignee} in the comment-message
example (referencing the comment-message key in the workflow) so the action,
which adds '@' itself, will render a single `@username`.
🧹 Nitpick comments (2)
.github/actions/stale-unassign/action.yml (2)
121-125: Consider throwing an error for invalid stale-label instead of silent return.When
stale-labelis empty, the script returns silently and the workflow succeeds. This could mask configuration issues. Throwing an error would make misconfiguration more visible.Suggested fix
// Validate stale-label is non-empty if (!staleLabel || staleLabel.trim() === '') { - console.error('Error: stale-label input is empty. Cannot proceed.'); - return; + throw new Error('stale-label input is empty. Cannot proceed.'); }
323-326: Consider tracking comment failures in error metrics.Comment errors are logged but not reflected in
errorCount, while assignee removal errors (lines 301-306) do increment it. If comment failures become frequent, they'd be invisible in the summary report.Optional: Track comment errors
+ let commentErrorCount = 0; + // ... (at top of script) + } catch (error) { console.error(`Issue #${issue.number}: Error posting comment: ${error.message}`); - // Don't fail the whole workflow for a comment error + commentErrorCount++; }Then include in the summary table.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
.github/actions/stale-unassign/action.yml.github/workflows/stale-unassign.yml
🔇 Additional comments (6)
.github/actions/stale-unassign/action.yml (4)
1-34: LGTM - Well-designed action inputs.Good security practice reading inputs via environment variables to prevent script injection. The
grace-period-minutesfor testing with override behavior is a thoughtful addition for development workflows.
222-249: LGTM - Solid linked PR detection logic.Correctly checks all required criteria: same repository, PR is open (ignoring closed/merged), and authored by one of the assignees. The logging for non-matching PRs aids debugging.
260-283: LGTM - Grace period calculation handles re-labeling correctly.Using the most recent label event handles edge cases where the stale label was removed and re-added, resetting the grace period appropriately.
338-349: LGTM - Good use of job summary for visibility.The summary table provides clear metrics for each run, making it easy to monitor the action's effectiveness over time.
.github/workflows/stale-unassign.yml (2)
30-33: LGTM - Appropriately scoped permissions.Minimal permissions following principle of least privilege:
issues: writefor unassignment and commenting,contents: readfor checkout,pull-requests: readfor linked PR detection.
40-48: LGTM - Clean action invocation.The checkout step correctly precedes the local action reference, and input fallbacks (
|| '0',|| '7') properly handle scheduled runs wheregithub.event.inputsis null.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
- Enhanced Retry-After header parsing to support both numeric (seconds) and HTTP-date formats
- Added validation to prevent NaN from being passed to setTimeout
- Fixed duplicate '@' in comment-message example by removing '@' from {assignee} placeholder
|
@palisadoes pls review i have tested it fully |
palisadoes
left a comment
palisadoes
left a comment
There was a problem hiding this comment.
Please explain how this works.
- There are lots of inputs
- It's not clear whether the grace period is after the label is found, though I assume it is.
|
@palisadoes |
|
@palisadoes pls review and tell me if any more change is required |
|
Please update the script so that the Action isn't activated for any one of a list of labels not just the |
- Renamed 'stale-label' input to 'exclusion-labels' to accept comma-separated list - Updated logic to process issues with ANY of the specified exclusion labels - Added validation to check each label exists in repository - Maintained backward compatibility with default 'no-issue-activity' label - Added example in workflow file showing multi-label configuration
|
@palisadoes done |
|
@coderabbitai full review based on your configuration. Provide your output in a readable format. This GitHub action aims to unassign people from issues if a specific label is assigned to the issue and there are no PRs referencing the issue after a timeout period. There is the option of the action exiting without implementing the feature if one or more specified labels are applied. |
🧩 Analysis chain🏁 Script executed: Length of output: 17005 🏁 Script executed: Length of output: 1873
📋 Overall AssessmentThe action is well-structured with robust error handling and retry logic. However, there's a critical naming and logic inconsistency that makes the configuration confusing and potentially contradicts the stated objectives. 🔴 Critical Issues1. Naming Confusion: "exclusion-labels" vs Actual BehaviorProblem: The input is named Current behavior: // This filters issues that HAVE the exclusion labels
const issues = allIssues.filter(issue => {
const issueLabels = issue.labels.map(l => l.name);
return validExclusionLabels.some(label => issueLabels.includes(label));
});What's happening: Issues WITH Expected interpretation based on naming:
Your comment mentions: "There is the option of the action exiting without implementing the feature if one or more specified labels are applied." This suggests there should be BOTH:
But the current implementation only has one set of labels doing both jobs, which is confusing. Recommendation: inputs:
stale-label:
description: 'Label that triggers unassignment countdown (e.g., "no-issue-activity")'
required: false
default: 'no-issue-activity'
exclusion-labels:
description: 'Comma-separated labels that prevent unassignment (e.g., "waiting-for-maintainer,blocked")'
required: false
default: ''🟡 Design Questions2. PR Link Logic ClarificationThe code checks for open PRs authored by the assignee: const isAuthoredByAssignee = assigneeLogins.includes(prAuthor);
return isSameRepo && isOpen && isAuthoredByAssignee;Current behavior: Skips unassignment if assignee has an open linked PR (they're working). Question: Should this also check for ANY open PR linked to the issue (not just by assignee)? If someone else has an active PR for the issue, should we still unassign the original assignee? From PR objectives: "Authored by the issue assignee" suggests current logic is correct, but please confirm this is the desired behavior. 3. Multiple Assignees HandlingWhen an issue has multiple assignees: // Removes ALL assignees at once
await github.rest.issues.removeAssignees({
assignees: assigneeLogins
});Behavior: All assignees are removed together, even if only one is inactive. Question: Is this intended? Should the action evaluate each assignee individually (checking if each has their own PR)? 🟢 Strengths4. Excellent Error Handling
5. Security Best Practices
6. Testing & Safety Features
7. Robust Validation
🔵 Minor Improvements8. Workflow Configuration
Suggestion: Add a README or documentation showing common configurations. 9. Grace Period Calculationconst daysSinceLabel = (now - labelAddedDate) / (1000 * 60 * 60 * 24);
10. Timeline Event Lookupconst labelEvents = timelineEvents.filter(event =>
event.event === 'labeled' &&
validExclusionLabels.includes(event.label?.name)
);
📝 Documentation Gaps11. Missing DocumentationThe action would benefit from:
🎯 Summary & Recommendations
Key Action Items:
The action is functionally solid with excellent error handling, but the naming confusion around "exclusion-labels" could lead to misconfiguration. Once this is clarified or fixed, it will be production-ready. Would you like me to help create a refactored version that separates trigger and exclusion labels, or is the current single-label behavior actually what you intended? ✅ Actions performedFull review triggered. |
|
In reference to this analysis: These are the responses:
|
|
Tip For best results, initiate chat on the files or code changes.
That leaves the critical naming inconsistency as the remaining issue to resolve. The current implementation has a logical contradiction: 🔴 Remaining Critical Issue: "exclusion-labels" LogicCurrent code behavior: // Line ~155 - Filters issues that HAVE the exclusion labels
const issues = allIssues.filter(issue => {
const issueLabels = issue.labels.map(l => l.name);
return validExclusionLabels.some(label => issueLabels.includes(label));
});What this does: Issues WITH Problem: The input is named ❓ Clarification NeededIn your earlier comment you mentioned: "There is the option of the action exiting without implementing the feature if one or more specified labels are applied." Question: Should there be TWO separate label concepts? Option A - Current Implementation (Single Label System):
Option B - Dual Label System:
Which behavior do you want?
Please clarify which approach you'd like, and I can help implement or document it accordingly. |
- stale-label: triggers unassignment countdown (default: no-issue-activity) - exclusion-labels: PREVENTS unassignment when present (e.g., waiting-for-maintainer,blocked) - Fixed naming confusion: exclusion labels now correctly exclude/protect issues - Updated workflow comments to clarify the dual-label system
2 participants
Resolves #24
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.