Skip to content

some bugs #126

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
nourshoreibah merged 1 commit into from
Feb 9, 2026
Merged

some bugs #126

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
53 changes: 46 additions & 7 deletions .github/workflows/pr-review-status.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -66,8 +66,15 @@ jobs:

const prData = JSON.parse(fs.readFileSync(prFilePath, 'utf8'));

// Only care about the assigned main reviewer's reviews
// Check if reviewer is one of the assigned reviewers
const isMainReviewer = reviewer.toLowerCase() === prData.main_reviewer.toLowerCase();
const isSecondReviewer = prData.second_reviewer &&
reviewer.toLowerCase() === prData.second_reviewer.toLowerCase();
const isAssignedReviewer = isMainReviewer || isSecondReviewer;

if (!isAssignedReviewer) {
console.log(`${reviewer} is not an assigned reviewer for PR #${prNumber}. Skipping state update.`);
}

// ── Determine emoji and label ────────────────────────────────────
let emoji, label;
Expand Down Expand Up @@ -99,8 +106,40 @@ jobs:
}
}

// ── Update the original Slack message with a status banner ───────
if (prData.slack_thread_ts && isMainReviewer) {
// ── Check if all assigned reviewers are now done ─────────────────
// We need to query all reviews to know full status
let allReviews = [];
try {
const resp = await github.rest.pulls.listReviews({
owner: context.repo.owner,
repo: context.repo.repo,
pull_number: prNumber,
});
allReviews = resp.data;
} catch (err) {
console.log(`Could not fetch reviews: ${err.message}`);
}

const REVIEW_STATES = ['APPROVED', 'CHANGES_REQUESTED', 'COMMENTED'];
const hasReviewed = (login) => allReviews.some(
r => r.user.login.toLowerCase() === login.toLowerCase() && REVIEW_STATES.includes(r.state)
);

const mainDone = hasReviewed(prData.main_reviewer);
const secondDone = !prData.second_reviewer || hasReviewed(prData.second_reviewer);
const allDone = mainDone && secondDone;

// ── Update the original Slack message if all reviewers are done ──
if (prData.slack_thread_ts && allDone && isAssignedReviewer) {
// Build reviewer status lines
const reviewerLines = [];
const mainStatus = mainDone ? ':white_check_mark:' : ':hourglass_flowing_sand:';
reviewerLines.push(`${mainStatus} *${prData.main_reviewer}* <@${prData.main_reviewer_slack}>`);
if (prData.second_reviewer) {
const secondStatus = secondDone ? ':white_check_mark:' : ':hourglass_flowing_sand:';
reviewerLines.push(`${secondStatus} *${prData.second_reviewer}* <@${prData.second_reviewer_slack}>`);
}

await slackApi('chat.update', {
channel: config.slack_channel_id,
ts: prData.slack_thread_ts,
Expand All @@ -117,7 +156,7 @@ jobs:
type: 'section',
text: {
type: 'mrkdwn',
text: `:mag: *Main reviewer:* <@${prData.main_reviewer_slack}>\n:eyes: *Also reviewing:* <@${config.always_reviewer_slack}>`,
text: reviewerLines.join('\n'),
},
},
],
Expand All @@ -137,12 +176,12 @@ jobs:
});
}

// ── Update bot state if this is the main reviewer ────────────────
if (isMainReviewer && prData.status === 'open') {
// ── Update bot state when all assigned reviewers are done ─────────
if (allDone && prData.status === 'open') {
prData.status = 'reviewed';
prData.resolved_at = new Date().toISOString();
fs.writeFileSync(prFilePath, JSON.stringify(prData, null, 2) + '\n');
console.log(`Marked PR #${prNumber} as reviewed`);
console.log(`All reviewers done — marked PR #${prNumber} as reviewed`);
}

- name: Commit and push bot-state
Expand Down
50 changes: 36 additions & 14 deletions .github/workflows/pr-reviewer-assign.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -137,59 +137,79 @@ jobs:
return;
}

// ── Round-robin: pick next reviewer ──────────────────────
// ── Detect if author is the always-reviewer ─────────────
const authorSlackId = config.github_to_slack[prAuthor];
const authorIsAlwaysReviewer = authorSlackId === config.always_reviewer_slack;
const reviewersNeeded = authorIsAlwaysReviewer ? 2 : 1;

// ── Round-robin: pick reviewer(s) ─────────────────────────
const roster = config.roster;
let cursor = state.cursor || 0;
let mainReviewer = null;
const pickedReviewers = [];
let newCursor = cursor;

for (let i = 0; i < roster.length; i++) {
for (let i = 0; i < roster.length && pickedReviewers.length < reviewersNeeded; i++) {
const idx = (cursor + i) % roster.length;
const candidate = roster[idx];
if (candidate.toLowerCase() !== prAuthor.toLowerCase()) {
mainReviewer = candidate;
pickedReviewers.push(candidate);
newCursor = (idx + 1) % roster.length;
break;
}
}

// Fallback: if the only person in the roster is the author,
// assign the always-reviewer instead.
if (!mainReviewer) {
// Fallback: if we couldn't find enough reviewers
if (pickedReviewers.length === 0) {
const alwaysEntry = Object.entries(config.github_to_slack).find(
([, slackId]) => slackId === config.always_reviewer_slack
);
if (alwaysEntry) {
mainReviewer = alwaysEntry[0];
pickedReviewers.push(alwaysEntry[0]);
} else {
core.setFailed('No eligible reviewer found and could not determine always-reviewer.');
return;
}
newCursor = (cursor + 1) % roster.length;
}

const mainReviewer = pickedReviewers[0];
const secondReviewer = pickedReviewers.length > 1 ? pickedReviewers[1] : null;

const mainReviewerSlack = config.github_to_slack[mainReviewer];
if (!mainReviewerSlack) {
core.setFailed(`No Slack ID mapping found for reviewer "${mainReviewer}".`);
return;
}
const secondReviewerSlack = secondReviewer ? config.github_to_slack[secondReviewer] : null;

console.log(`Assigning ${mainReviewer} to PR #${prNumber} (author: ${prAuthor}, cursor: ${cursor} → ${newCursor})`);
if (authorIsAlwaysReviewer) {
console.log(`Author is always-reviewer — assigning ${pickedReviewers.join(' + ')} to PR #${prNumber} (cursor: ${cursor} → ${newCursor})`);
} else {
console.log(`Assigning ${mainReviewer} to PR #${prNumber} (author: ${prAuthor}, cursor: ${cursor} → ${newCursor})`);
}

// ── Request review on GitHub ─────────────────────────────
try {
await github.rest.pulls.requestReviewers({
owner: context.repo.owner,
repo: context.repo.repo,
pull_number: prNumber,
reviewers: [mainReviewer],
reviewers: pickedReviewers,
});
console.log(`Review requested from ${mainReviewer}`);
console.log(`Review requested from ${pickedReviewers.join(', ')}`);
} catch (err) {
console.log(`Warning: could not request review from ${mainReviewer}: ${err.message}`);
console.log(`Warning: could not request reviewers: ${err.message}`);
}

// ── Post Slack notification ──────────────────────────────
let reviewerLines;
if (authorIsAlwaysReviewer && secondReviewer) {
// Author is the always-reviewer, so show two round-robin picks
reviewerLines = `:mag: *Reviewer 1:* <@${mainReviewerSlack}>\n:mag: *Reviewer 2:* <@${secondReviewerSlack}>`;
} else {
// Normal case: one main reviewer + always-reviewer
reviewerLines = `:mag: *Main reviewer:* <@${mainReviewerSlack}>\n:eyes: *Also reviewing:* <@${config.always_reviewer_slack}>`;
}

const slackTs = await postSlack({
channel: config.slack_channel_id,
text: `New PR for review: <${prUrl}|#${prNumber} ${prTitle}>`,
Expand All @@ -205,7 +225,7 @@ jobs:
type: 'section',
text: {
type: 'mrkdwn',
text: `:mag: *Main reviewer:* <@${mainReviewerSlack}>\n:eyes: *Also reviewing:* <@${config.always_reviewer_slack}>`,
text: reviewerLines,
},
},
],
Expand All @@ -222,6 +242,8 @@ jobs:
author: prAuthor,
main_reviewer: mainReviewer,
main_reviewer_slack: mainReviewerSlack,
second_reviewer: secondReviewer,
second_reviewer_slack: secondReviewerSlack,
created_at: new Date().toISOString(),
last_reminded_at: null,
slack_thread_ts: slackTs,
Expand Down
63 changes: 42 additions & 21 deletions .github/workflows/pr-reviewer-remind.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ jobs:
const BOT_STATE_DIR = 'bot-state';
const configPath = path.join(BOT_STATE_DIR, 'config.json');
const prsDir = path.join(BOT_STATE_DIR, 'prs');
const REMINDER_INTERVAL_MS = 48 * 60 * 60 * 1000; // 48 hours
const REMINDER_INTERVAL_MS = 24 * 60 * 60 * 1000; // 24 hours
const CLEANUP_AGE_MS = 7 * 24 * 60 * 60 * 1000; // 7 days

// ── Load config (Terraform-managed) ──────────────────────
Expand Down Expand Up @@ -122,38 +122,49 @@ jobs:
continue;
}

// ── Check if main reviewer has submitted a review ──────
let hasReviewed = false;
// ── Check if reviewers have submitted reviews ──────────
let reviews = [];
try {
const reviews = await github.rest.pulls.listReviews({
const resp = await github.rest.pulls.listReviews({
owner: prOwner,
repo: prRepo,
pull_number: prData.pr_number,
});
hasReviewed = reviews.data.some(
r =>
r.user.login.toLowerCase() === prData.main_reviewer.toLowerCase() &&
['APPROVED', 'CHANGES_REQUESTED', 'COMMENTED'].includes(r.state)
);
reviews = resp.data;
} catch (err) {
console.log(`Could not fetch reviews for PR #${prData.pr_number}: ${err.message}`);
}

if (hasReviewed) {
console.log(`PR #${prData.pr_number}: ${prData.main_reviewer} has reviewed. Done.`);
const REVIEW_STATES = ['APPROVED', 'CHANGES_REQUESTED', 'COMMENTED'];
const hasReviewed = (login) => reviews.some(
r => r.user.login.toLowerCase() === login.toLowerCase() && REVIEW_STATES.includes(r.state)
);

const mainDone = hasReviewed(prData.main_reviewer);
const secondDone = !prData.second_reviewer || hasReviewed(prData.second_reviewer);

if (mainDone && secondDone) {
console.log(`PR #${prData.pr_number}: all assigned reviewers have reviewed. Done.`);
prData.status = 'reviewed';
prData.resolved_at = new Date().toISOString();
fs.writeFileSync(filePath, JSON.stringify(prData, null, 2) + '\n');
changed = true;
continue;
}

// ── Check if reviewer is still requested ───────────────
const isStillRequested = ghPr.requested_reviewers.some(
r => r.login.toLowerCase() === prData.main_reviewer.toLowerCase()
);
if (!isStillRequested) {
console.log(`PR #${prData.pr_number}: ${prData.main_reviewer} is no longer requested. Skipping reminder.`);
// ── Check if reviewers are still requested ─────────────
const requestedLogins = ghPr.requested_reviewers.map(r => r.login.toLowerCase());
const pendingReviewers = [];

if (!mainDone && requestedLogins.includes(prData.main_reviewer.toLowerCase())) {
pendingReviewers.push({ login: prData.main_reviewer, slack: prData.main_reviewer_slack });
}
if (prData.second_reviewer && !secondDone && requestedLogins.includes(prData.second_reviewer.toLowerCase())) {
pendingReviewers.push({ login: prData.second_reviewer, slack: prData.second_reviewer_slack });
}

if (pendingReviewers.length === 0) {
console.log(`PR #${prData.pr_number}: no pending requested reviewers. Skipping reminder.`);
continue;
}

Expand All @@ -169,11 +180,22 @@ jobs:
}

// ── Send reminder ──────────────────────────────────────
console.log(`PR #${prData.pr_number}: sending reminder for ${prData.main_reviewer}`);
const pendingNames = pendingReviewers.map(r => r.login).join(', ');
console.log(`PR #${prData.pr_number}: sending reminder for ${pendingNames}`);

const mentionLines = pendingReviewers.map(
r => `:mag: <@${r.slack}> — please review when you get a chance.`
);
// Only cc the always-reviewer if they aren't the PR author
// (if they are the author, they're not reviewing this one)
const authorSlackId = config.github_to_slack[prData.author];
if (authorSlackId !== config.always_reviewer_slack) {
mentionLines.push(`:eyes: cc <@${config.always_reviewer_slack}>`);
}

const reminderMsg = {
channel: config.slack_channel_id,
text: `Reminder: PR #${prData.pr_number} awaits review from ${prData.main_reviewer}`,
text: `Reminder: PR #${prData.pr_number} awaits review from ${pendingNames}`,
// Thread under the original assignment message when possible
...(prData.slack_thread_ts ? { thread_ts: prData.slack_thread_ts } : {}),
blocks: [
Expand All @@ -184,8 +206,7 @@ jobs:
text: [
`:bell: *Reminder:* <${prData.pr_url}|PR #${prData.pr_number}> is still awaiting review.`,
``,
`:mag: <@${prData.main_reviewer_slack}> — please review when you get a chance.`,
`:eyes: cc <@${config.always_reviewer_slack}>`,
...mentionLines,
].join('\n'),
},
},
Expand Down
Loading