feat: Add a skill that shows PRs that need to be reviewed#59
feat: Add a skill that shows PRs that need to be reviewed#59
Conversation
The skill will run a script that outputs all the PRs that the user should review for their team. It can then mark those notifications as read.
evanh
changed the title
![sentry[bot]](https://avatars.githubusercontent.com/in/12637?s=60&v=4){kind=small}
| requested_team_names = [t["name"] for t in reviewers_data.get("teams", [])] | ||
| matching_teams = [ | ||
| t for t in requested_team_names | ||
| if any(slug.lower() == t.lower() for slug in team_slugs) | ||
| ] |
There was a problem hiding this comment.
Bug: The script incorrectly compares team slugs against team display names from the GitHub API, causing the matching logic to fail and miss relevant pull requests.
Severity: CRITICAL
Suggested Fix
Modify the team matching logic to compare the input team slugs against the slug field from the team objects returned by the GitHub API. The API response for requested reviewers includes a teams array, where each object has both a name and a slug. The comparison should be [t["slug"] for t in reviewers_data.get("teams", [])] against the input team_slugs.
Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent.
Verify if this is a real issue. If it is, propose a fix; if not, explain why it's not
valid.
Location:
plugins/sentry-skills/skills/gh-review-requests/scripts/fetch_review_requests.py#L80-L84
Potential issue: The script compares team slugs provided via the `--teams` argument
(e.g., `"streaming-platform"`) against team display names from the GitHub API response
(e.g., `"Streaming Platform"`). Even after converting both to lowercase, the strings
`"streaming-platform"` and `"streaming platform"` will not match. This causes the team
matching logic to fail, preventing the script from correctly identifying pull requests
that have a pending review request from the specified team. An unused
`team_display_names` dictionary, which maps slugs to display names, suggests an
incomplete implementation.
Did we get this right? 👍 / 👎 to inform future reviews.
| result = subprocess.run(cmd, capture_output=True, text=True) | ||
| if result.returncode != 0: | ||
| return [] if paginate else {} | ||
| return json.loads(result.stdout) |
There was a problem hiding this comment.
Bug: The gh function does not handle empty API responses, which will cause a json.JSONDecodeError and crash the script if the gh api command returns an empty string.
Severity: MEDIUM
Suggested Fix
Before calling json.loads, add a check to ensure result.stdout is not empty or contains only whitespace. Follow the pattern used in other scripts in the repository, such as return json.loads(result.stdout) if result.stdout.strip() else {}.
Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent.
Verify if this is a real issue. If it is, propose a fix; if not, explain why it's not
valid.
Location:
plugins/sentry-skills/skills/gh-review-requests/scripts/fetch_review_requests.py#L32
Potential issue: The `gh` helper function calls `json.loads(result.stdout)` without
first verifying that the `result.stdout` from a `subprocess.run` call is not empty. If
the `gh api` command returns a success exit code (0) but an empty string for its
standard output, `json.loads('')` will raise a `json.JSONDecodeError`, causing the
script to crash. Other similar scripts within the same repository include a defensive
check for this condition, suggesting it is a known risk.
Did we get this right? 👍 / 👎 to inform future reviews.
1 participant
The skill will run a script that outputs all the PRs that the user should review for their team. It
can then mark those notifications as read.