feat(github-app): add reviewer routing auto-request mode - #1108
Conversation
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #1108 +/- ##
==========================================
+ Coverage 95.03% 95.07% +0.04%
==========================================
Files 177 179 +2
Lines 19886 20101 +215
Branches 7166 7244 +78
==========================================
+ Hits 18898 19112 +214
+ Misses 395 393 -2
- Partials 593 596 +3
🚀 New features to boost your workflow:
|
|
Tip 🟩🟩🟩🟩🟩🟩🟩🟩🟩🟩🟩🟩 ✅ Gittensory review — safe to merge
✅ Approved — safe to merge
Review context
Contributor next steps
Signal definitions
Visual preview
Click any thumbnail to open the full-size screenshot. Before = production · After = this PR's preview deploy. 🟩 Safe / merged · 🟦 Advisory · 🟨 Held for review · 🟥 Blocked / closed 💰 Earn for open-source contributions like this. Gittensor lets GitHub contributors earn for the work they already do — register to start earning →. Checked by Gittensory, a quiet PR intelligence layer for OSS maintainers.
|
ae4cc78 to
55a243e
Compare
55a243e to
6ea0e92
Compare
6ea0e92 to
9b47190
Compare
|
Superagent didn't find any vulnerabilities or security issues in this PR. |
28de1f4 to
4332563
Compare
4332563 to
0175133
Compare
0175133 to
7327301
Compare
7327301 to
aa85d42
Compare
aa85d42 to
180b229
Compare
180b229 to
48c8918
Compare
|
@JSONbored could you please review this PR? |
48c8918 to
90ec14d
Compare
90ec14d to
6f27ad7
Compare
6f27ad7 to
0183c6a
Compare
0183c6a to
0defbe3
Compare
JSONbored
left a comment
There was a problem hiding this comment.
Nice work here — the CODEOWNERS parser, the load-de-weighted ranking, and the test coverage are
all solid. I'm closing rather than merging for two reasons: this is two features in one PR, and
the auto_request write has a real safety hole. Resubmit as the stack at the bottom and it's in
good shape.
-
Scope — two features in one PR. The advisory routing core (
src/github/codeowners.ts+
src/signals/reviewer-routing.ts, both new) stands on its own and doesn't need the write path
at all. #830 itself says theauto_requestaction "warrants its own focused review" because it
acts on live PRs. Land them separately (stack below). -
auto_request fires under dry-run — this is the blocker. Trace:
maybeAutoRequestReviewers(src/queue/processors.ts:1879) has no action-mode check — only
thereviewerRoutingMode === "auto_request"opt-in.- it's called unconditionally at
src/queue/processors.ts:2545, inside
maybePublishPrPublicSurface, which still RUNS when a repo is in dry-run (it computes the
gate verdict; only its Octokit writes get suppressed). requestPullRequestReviewers(src/github/app.ts) writes with a rawfetch, so it never
goes through the dry-run chokepointmakeInstallationOctokit(src/github/client.ts).- Net: a repo set to dry-run still requests real reviewers on real PRs.
We routed every installation write through the dry-run-aware client in #1258 specifically to
stop exactly this, and retrofitted the remaining raw-fetch writers in #1263 to honor the brake.
Follow that pattern — preferably call throughmakeInstallationOctokitso dry-run/pause suppress
it for free; or, if it stays a raw-fetch helper, early-return likefileUpstreamDriftIssues:// a paused / frozen / dry-run agent must not write (mirror #1263) if (isGlobalAgentPause(env) || (await isGlobalAgentFrozen(env))) return;
Then confirm there's no other path that reaches the write under a non-live mode. Keep the
newcomer opt-in and the already-requested idempotency you have — those are good.
Suggested stack (smallest first; split further if any exceeds ~10 files):
PR 1 — src/github/codeowners.ts (parser, pure) + test
PR 2 — src/signals/reviewer-routing.ts (ranking) + test
PR 3 — reviewerRoutingMode off|advisory: types/db/schema + migration + regenerated
openapi.json + settings-preview + focus-manifest + the PR-panel "Suggested reviewers"
render + the one-line fixture updates
PR 4 — auto_request only, with the dry-run fix above + tests
PRs 1–3 are low-risk and move fast; PR 4 then gets the focused review the live write needs.
Per PR: rebase onto latest main, 100% line + branch coverage with a regression test, no
AI-generated attribution in the PR body.




Summary
reviewerRoutingMode: off | advisory | auto_requestreviewerRoutingModeisauto_request, while skipping teams, already-requested reviewers, and first-time external contributorsScope
type(scope): short summaryConventional Commit format, for examplefix(api): restore profile access checks.CONTRIBUTING.mdand does not reintroduce GitHub Pages, VitePress,site/, orCNAME.Validation
git diff --checknpm run actionlintnpm run typechecknpm run test:coveragenpm run test:workersnpm run build:mcpnpm run test:mcp-packnpm run ui:openapi:checknpm run ui:version-auditnpm run ui:lintnpm run ui:typechecknpm run ui:testnpm run ui:buildnpm audit --audit-level=moderateIf any required check was skipped or failed, explain why:
npm run test:cilocally against the final PR tree.ui:lintpasses, but it emits existingreact-refresh/only-export-componentswarnings in route files outside this PR's scope.Safety
UI Evidencesection below with JPG/JPEG or PNG screenshots arranged as organized, captioned, clickable thumbnails. SVG screenshots are not used as review evidence. Review-only screenshots or recordings are not committed to the repository.UI Evidence
Not applicable. This PR does not introduce visible UI changes. The frontend-facing change is the generated OpenAPI artifact only.
Notes
auto_requestreviewer-routing mode correctly on currentmain.auto_requestis intentionally conservative: it only requests individual reviewers, never teams, and it does not auto-request for first-time external contributors.