Home

boring / v2

publicBoring-Software-Inc/v2
Overview Code History Branches Pull requestsIssuesInsights Campfire Docs
main
HomeOverview Code PRsIssues

#166 [Detail Bug] Workflows: reviewer sync deletes valid reviewers for repos with >1000 collaborators

open

Opened by detail-app[bot] · 6 days ago

detail-app[bot]opened this issueAuthor· 6 days ago

Sign in to comment.

Linked pull requests

No linked pull requests yet.

Detail Bug Report

app.detail.dev/org_0f605249-55f3-46eb-880c-20ae3e45ae99/…/bug_a7e5a900-95ba-48f2-9328-dc5253ed7cf7

Introduced in #40 by @ripgrim on Oct 22, 2026

Summary

  • Context: The reviewer sync job keeps the repo_reviewers table (a DB cache of who can review each GitHub repo) in sync with GitHub, and that table drives the workflow editor's reviewer picker and the enable-time stale-reviewer lint.
  • Bug: listRepoReviewers paginates GitHub collaborators with a hard cap of 10 pages × 100 rows and, when page 10 comes back full, silently stops at 1000 collaborators — then replaceRepoReviewers deletes every stored reviewer row for the repo and re-inserts only the truncated list.
  • Actual vs. expected: Every collaborator beyond the first 1000 is deleted from repo_reviewers on every sync (the wrong state refreshes every 24h and never self-corrects), even though the GitHub API has the full list; expected behavior is that all collaborators are synced, or a truncated read is flagged/refused rather than silently replacing the previous complete set.
  • Impact: For repos with more than 1000 push-access collaborators, valid reviewers are persistently wiped from the DB, making the UI falsely warn "can no longer review this repo" about people who can in fact review, and tempting users to remove them from workflows via the lint's "Remove" fix.

Code with Bug

packages/forge-github/src/client/reviewers.ts:

const PAGE_SIZE = 100;
const MAX_PAGES = 10;
...
	for (let page = 1; page <= MAX_PAGES; page++) {
		const rows = pageSchema.parse(
			await http.get(
				repoFullName,
				`/repos/${repoFullName}/collaborators?affiliation=all&permission=push&per_page=${PAGE_SIZE}&page=${page}`,
			),
		);
		...
		if (rows.length < PAGE_SIZE) {
			break;
		}
	}
	return found;
const reviewers = await listRepoReviewers(http, repo.fullName);
await reviewerServices.replaceRepoReviewers(db, repo.id, reviewers);
// <-- BUG 🔴 truncated reviewer list can overwrite the full DB set

Explanation

  • listRepoReviewers stops after 10 pages regardless of whether more pages exist. If page 10 returns a full 100 rows, the loop ends due to MAX_PAGES without any truncation signal.
  • The sync job then calls replaceRepoReviewers, which deletes all existing rows for the repo and reinserts only what listRepoReviewers returned. When the fetch was silently capped at 1000, every reviewer beyond the first 1000 is dropped from repo_reviewers.
  • This incorrect state is user-visible:
    • Workflow editor reviewer picker marks missing reviewers as warn-styled chips with sr-only text “can no longer review this repo” once the repo has been synced.
    • setWorkflowEnabled emits stale-reviewer warnings (“Remove X. They can no longer review this repo.”) with a one-click Remove fix.
  • The issue persists because the 24h re-sync repeats the same truncation and replace behavior.

Codebase Inconsistency

Other read paths explicitly surface truncation, but this one does not (e.g. packages/forge-github/src/client/reads.ts and packages/contracts/src/review.ts document/mark truncation). Here, truncation is silent yet the downstream operation is destructive (delete-and-replace).

Recommended Fix

Remove the cap with proper pagination, or keep a cap but treat a full final page at MAX_PAGES as “possibly truncated” and return a truncated flag. The sync job should then skip replaceRepoReviewers and log loudly (keeping the last known set), instead of replacing a potentially complete set with a truncated one.

History

This bug was introduced in commit 274de6d1 ("feat(workflows): pick github reviewers from a synced list").