resolving bug from added feature to not remove unassigned contributor who's made corresponding PR #56

tsu-ki · 2025-01-02T12:30:21Z

I'll ready the PR once successfully tested the proposed changes in a test repository. I'll add screenshots for the same.

Summary by CodeRabbit

Bug Fixes
- Automation now checks for an open pull request by the assignee and skips unassigning if one exists.
- Issues are only unassigned when they have no labels; labeled issues are left unchanged.
- Removed the previous unconditional removal of the "assigned" label and its automatic comment; logging and messaging for skipped/unassigned cases were clarified.

DonnieBLT

Does this work? Can you please update it check the new label logic?

coderabbitai · 2025-11-01T20:22:42Z

Walkthrough

Replaced the unconditional unassign path with a pre-check that searches for an open PR authored by the assignee mentioning the issue; if found, unassign is skipped. If no PR is found, the code now only unassigns when the issue has no labels. Removed the unconditional removal of the assigned label and its comment.

Changes

Cohort / File(s)	Summary
Unassign/inactivity pre-check & message change `src/index.js`	Added a pre-check that searches for open PRs by the assignee mentioning the issue; if a PR exists, skip unassign. If no PR, only unassign when the issue has no labels. Removed the previous unconditional removal of the `assigned` label and its comment.

Sequence Diagram(s)

sequenceDiagram
    participant Workflow
    participant UnassignLogic
    participant PRSearch
    participant LabelCheck

    Workflow->>UnassignLogic: Trigger stale-assignment unassign flow
    rect rgb(220,230,241)
        Note over UnassignLogic: NEW — search for open PR by assignee mentioning issue
        UnassignLogic->>PRSearch: Search open PRs authored by assignee mentioning issue
    end

    alt PR Found
        PRSearch-->>UnassignLogic: Open PR exists
        Note over UnassignLogic: Log and skip unassign
    else No PR Found
        PRSearch-->>UnassignLogic: No open PR
        UnassignLogic->>LabelCheck: Check issue labels
        alt No Labels
            LabelCheck-->>UnassignLogic: No labels present
            Note over UnassignLogic: Remove assignee and post unassignment comment
        else Labels Present
            LabelCheck-->>UnassignLogic: Labels exist
            Note over UnassignLogic: Skip unassign due to labels
        end
    end

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Focus review on PR search query/filter correctness and performance.
Verify the new conditional correctly replaces the previous unconditional assigned label removal and comment posting.
Confirm logging/messages reflect the changed skip/unassign conditions.

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly describes the main change: preventing the removal of a contributor who has created a corresponding PR when handling stale assignments.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✨ Finishing touches

📝 Generate docstrings

🧪 Generate unit tests (beta)

Create PR with unit tests
Post copyable unit tests in a comment

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

Knowledge base: Disabled due to Reviews -> Disable Knowledge Base setting

📥 Commits

Reviewing files that changed from the base of the PR and between ca6b13d and 50c1b32.

📒 Files selected for processing (1)

src/index.js (1 hunks)

🚧 Files skipped from review as they are similar to previous changes (1)

src/index.js

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)

GitHub Check: Agent

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.

❤️ Share

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

DonnieBLT · 2025-11-01T20:22:51Z

please fix the conflicts if you'd like to continue working on this

coderabbitai

Actionable comments posted: 1

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

Knowledge base: Disabled due to Reviews -> Disable Knowledge Base setting

📥 Commits

Reviewing files that changed from the base of the PR and between b6c7a62 and ca6b13d.

📒 Files selected for processing (1)

src/index.js (1 hunks)

🔇 Additional comments (1)

src/index.js (1)

152-154: Logic is correct but search query has limitations; runtime verification not possible in sandbox.

The conditional structure properly implements the intended behavior: skip unassignment if an open PR by the assignee mentions the issue, otherwise proceed with label-based logic. The else if on line 154 is essential for correct flow.

However, the search query (${event.issue.number} in:body) has a limitation: it only detects issue references in PR body text, not in title, and won't catch PRs linked via GitHub's UI linking feature without explicit text reference. This may miss some linked PRs, though the standard convention ("Fixes #123") typically places references in the body.

Runtime verification of the search query effectiveness cannot be completed in the sandbox environment (git repository context unavailable for gh CLI).

src/index.js

Copilot

Pull Request Overview

This PR aims to fix a bug in the inactivity automation to prevent unassigning contributors who have created a corresponding pull request for the issue they're working on.

Key Changes:

Added search query logic to check for open PRs by the assignee that reference the issue
Modified the unassignment flow to skip unassigning if an open PR exists
Changed the label-based condition logic for when to unassign

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2025-11-16T18:58:30Z

src/index.js

-                            console.log(`Issue #${event.issue.number} does not have the "assigned" label, skipping unassign.`);
+                            if (searchResult.data.total_count > 0) {
+                                console.log(`Issue #${event.issue.number} has an open PR by ${event.issue.assignee.login}, skipping unassign.`);
+                            } else if (issueDetails.data.labels.length === 0) {  


Bug: The logic is incorrect - this condition checks if labels.length === 0 (no labels), but the else branch at line 331 says "has labels, skipping unassign". This means the code will only unassign when there are NO labels, which contradicts the original logic that checked for the presence of the "assigned" label specifically (line 295). The logic should check whether to proceed after label removal, not based on total label count.

Suggested change

} else if (issueDetails.data.labels.length === 0) {

} else if (!issueDetails.data.labels.some(label => label.name === "assigned")) {

Copilot · 2025-11-16T18:58:31Z

src/index.js

+                                await octokit.issues.removeAssignees({
+                                    owner,
+                                    repo,
+                                    issue_number: event.issue.number,
+                                    assignees: [event.issue.assignee.login],
+                                });


Bug: Duplicate unassignment operation. The assignee is already removed at lines 298-303, so this second call to removeAssignees is redundant and will fail or do nothing since the assignee has already been removed.

Suggested change

await octokit.issues.removeAssignees({

owner,

repo,

issue_number: event.issue.number,

assignees: [event.issue.assignee.login],

});

Copilot · 2025-11-16T18:58:31Z

src/index.js

+
+                                await octokit.issues.removeAssignees({
+                                    owner,
+                                    repo,


Bug: Variable inconsistency - repo should be repoName to match the rest of the codebase. This variable is used consistently as repoName in lines 300, 291, 267, 253.

Copilot · 2025-11-16T18:58:31Z

src/index.js

+                                // Add a comment about unassignment
+                                await octokit.issues.createComment({
+                                    owner,
+                                    repo,


Bug: Variable inconsistency - repo should be repoName to match the rest of the codebase.

Copilot · 2025-11-16T18:58:31Z

src/index.js

+                                await octokit.issues.createComment({
+                                    owner,
+                                    repo,
+                                    issue_number: event.issue.number,
+                                    body: `⏰ This issue has been automatically unassigned due to 24 hours of inactivity. 
+                                    The issue is now available for anyone to work on again.`
+                                });


The removeLabel call for the "assigned" label has been removed from the original code. If the label-based tracking system is still being used, the label should be removed when unassigning the contributor to maintain consistency.

Copilot · 2025-11-16T18:58:32Z

src/index.js

+                            const query = `type:pr state:open repo:${owner}/${repo} author:${event.issue.assignee.login} ${event.issue.number} in:body`;
+                            const searchResult = await octokit.search.issuesAndPullRequests({
+                                q: query
                            });


This code duplicates logic that already exists above (lines 272-282). The timeline events check for cross-referenced PRs should be sufficient. Adding a separate search query here creates redundant checks and makes the code harder to maintain. Consider removing this duplicate check or consolidating the PR detection logic.

Copilot · 2025-11-16T18:58:32Z

src/index.js

-                                repo: repoName,
-                                issue_number: event.issue.number,
-                                name: "assigned"
+                            const query = `type:pr state:open repo:${owner}/${repo} author:${event.issue.assignee.login} ${event.issue.number} in:body`;


Bug: Variable inconsistency - repo is used here but should be repoName to match the variable used throughout the rest of this function (see lines 300, 291, 267, 253, etc.). This will cause a reference error.

Suggested change

const query = `type:pr state:open repo:${owner}/${repo} author:${event.issue.assignee.login} ${event.issue.number} in:body`;

const query = `type:pr state:open repo:${owner}/${repoName} author:${event.issue.assignee.login} ${event.issue.number} in:body`;

Bug fix

ca6b13d

DonnieBLT requested changes Mar 2, 2025

View reviewed changes

DonnieBLT marked this pull request as ready for review November 1, 2025 20:22

github-project-automation bot added this to 📌 OWASP BLT Project Board Nov 1, 2025

github-project-automation bot moved this to Backlog in 📌 OWASP BLT Project Board Nov 1, 2025

DonnieBLT added the 2025-11-01 label Nov 1, 2025

coderabbitai bot requested changes Nov 1, 2025

View reviewed changes

src/index.js Show resolved Hide resolved

github-project-automation bot moved this from Backlog to Ready in 📌 OWASP BLT Project Board Nov 1, 2025

Merge branch 'main' into patch-5

50c1b32

DonnieBLT requested a review from Copilot November 16, 2025 18:55

Copilot started reviewing on behalf of DonnieBLT November 16, 2025 18:55 View session

coderabbitai bot approved these changes Nov 16, 2025

View reviewed changes

Copilot finished reviewing on behalf of DonnieBLT November 16, 2025 18:57

Copilot AI reviewed Nov 16, 2025

View reviewed changes

	} else if (issueDetails.data.labels.length === 0) {
	} else if (!issueDetails.data.labels.some(label => label.name === "assigned")) {

	const query = `type:pr state:open repo:${owner}/${repo} author:${event.issue.assignee.login} ${event.issue.number} in:body`;
	const query = `type:pr state:open repo:${owner}/${repoName} author:${event.issue.assignee.login} ${event.issue.number} in:body`;

Uh oh!

resolving bug from added feature to not remove unassigned contributor who's made corresponding PR #56

Are you sure you want to change the base?

resolving bug from added feature to not remove unassigned contributor who's made corresponding PR #56

Uh oh!

Conversation

tsu-ki commented Jan 2, 2025 • edited by coderabbitai bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary by CodeRabbit

Uh oh!

DonnieBLT left a comment

Choose a reason for hiding this comment

Uh oh!

coderabbitai bot commented Nov 1, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Changes

Sequence Diagram(s)

Estimated code review effort

Pre-merge checks and finishing touches

Uh oh!

DonnieBLT commented Nov 1, 2025

Uh oh!

coderabbitai bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Uh oh!

Copilot AI Nov 16, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 16, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 16, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 16, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 16, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 16, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 16, 2025

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

tsu-ki commented Jan 2, 2025 •

edited by coderabbitai bot

Loading

coderabbitai bot commented Nov 1, 2025 •

edited

Loading