Feature: add stalled PR detection and reviewer ping banner#631
Conversation
|
please fix the conflict |
|
@Rhea-15 is attempting to deploy a commit to the codersogs-3057's projects Team on Vercel. A member of the Team first needs to authorize it. |
|
CI is failing, can you please fix it |
e13928a to
1ada540
Compare
jakharmonika364
left a comment
There was a problem hiding this comment.
Great work on this feature! The banner UI and the SQL query for identifying stale PRs both look excellent.
Before we can merge this, there are a couple of things we need to address, including an authorization issue in the new server action.
1. Accidental files committed
It looks like two local Supabase Studio scratchpad files were accidentally included in this PR. Could you please delete these files?
supabase/snippets/Untitled query 835.sqlsupabase/snippets/Untitled query 938.sql
2. Missing Authorization & Rate Limiting in pingReviewers
Currently, pingReviewers only checks if the user has a global maintainer role. It doesn't verify if the user actually has permission to manage the specific repository they are trying to ping. This would allow a maintainer to ping reviewers on repositories they don't own by guessing the prId. It's also missing a rate limit.
Could you update the pingReviewers function in src/app/actions/maintainer/index.ts to the following? This adds the required listMaintainerRepos check and applies standard rate limiting:
import { listMaintainerRepos } from '@/lib/maintainer/detect';
import { requireMaintainer } from '@/lib/action-auth';
import { RATE_LIMIT_TIERS } from '@/lib/rate-limit';
export async function pingReviewers(prId: number): Promise<Result<{ commented: boolean }>> {
// Use requireMaintainer to enforce rate limiting on this action
const authRes = await requireMaintainer({
rateLimit: { namespace: 'maint:ping-reviewers', ...RATE_LIMIT_TIERS.STANDARD },
requireService: true,
});
if (!authRes.ok) return authRes;
const { user, service } = authRes.data;
// Fetch the PR row to get installation and metadata
const { data: pr, error: prErr } = await service
.from('pull_requests')
.select('number, repo_full_name, installation_id, url')
.eq('id', prId)
.maybeSingle();
if (prErr || !pr) return err('not_found', 'PR not found');
if (!pr.installation_id) return err('invalid_data', 'PR missing installation ID');
// AUTHORIZATION CHECK: Ensure the user actually has access to this repo
const repos = await listMaintainerRepos(user.id, pr.installation_id);
if (!repos.includes(pr.repo_full_name)) {
return err('unauthorized', 'You are not a maintainer for this repository');
}
const [owner, repo] = pr.repo_full_name.split('/');
if (!owner || !repo) return err('invalid_data', 'Could not parse repo name');
try {
const octokit = await getInstallOctokit(pr.installation_id);
await octokit.rest.issues.createComment({
owner,
repo,
issue_number: pr.number,
body: '👋 **Ping from MergeShip**: This PR has been waiting for reviewer feedback for over 14 days. Could reviewers please take a look when they get a chance?',
});
return ok({ commented: true });
} catch (e) {
const msg = e instanceof Error ? e.message : 'GitHub API error';
return err('github_error', msg);
}
}
Summary
Adds a dismissible banner on the maintainer dashboard that surfaces PRs stalled for 14+ days without reviewer activity, with a one-click action to post a GitHub comment pinging reviewers.
Type of Change
New feature
UI / UX improvement
Related Issue
Closes #445
What was changed?
New files:
src/app/(app)/maintainer/stale-pr-banner.tsx
Modified files:
src/app/actions/maintainer/analytics.ts
src/app/actions/maintainer/index.ts
src/app/(app)/maintainer/page.tsx
Checklist