mirror of
https://github.com/github/awesome-copilot.git
synced 2026-10-15 16:21:51 +00:00
Merge pull request #5099 from github/aaronpowell-pr-board-review-prompt
Refine PR board review criteria and delegate reviews to background agents
This commit is contained in:
@@ -26,6 +26,9 @@ submissions before reviewing (see "Calibrate" below).
|
||||
broadly relevant, neutrally framed plugins that are clear about their limitations are welcome.
|
||||
- Marketing pitches for brand-new products, often vibe-coded dumps (repo days old, 0 stars, few commits) —
|
||||
"This is primarily a marketing push for an external service."
|
||||
- Free skills as an on-ramp to the vendor's paid/credit-based MCP or connector: vendor LICENSE or
|
||||
`metadata.author`, `mcp add` setup commands pointing at the vendor's API, UTM/referral links, or an upstream
|
||||
README that markets the free content as a funnel — "we're not a marketing channel".
|
||||
- Crypto / payments / finance niches, or otherwise niche problem spaces — "This is not a fit for the repo."
|
||||
/ "a bit too niche a problem space for this repo."
|
||||
- Duplicates of built-in Copilot features: memory ("Copilot has a built-in memory system"), tool-use guards
|
||||
|
||||
@@ -13,7 +13,9 @@ authenticated `gh` CLI.
|
||||
dragging a card overrides it. Cards in each bucket are sorted oldest to newest
|
||||
by creation date.
|
||||
- **AI reviews** — "Review pending" asks the agent to review every PR that has never
|
||||
been reviewed or whose review is stale. The agent records structured results
|
||||
been reviewed or whose review is stale. The prompt tells the session to fan the work
|
||||
out to background agents (one per PR, or small batches) so the main conversation
|
||||
stays free. The agents record structured results
|
||||
(rationale, repo fit, compliance, differentiation, risks, validation, confidence,
|
||||
suggested comment) through a canvas action.
|
||||
- **State tracking** — each PR remembers whether it has been reviewed, when, what the
|
||||
|
||||
@@ -2,7 +2,7 @@ export const RECOMMENDATION_GUIDE = [
|
||||
"`approve` — the change is correct, in scope, compliant with the contribution rules, and ready to merge.",
|
||||
"`request-changes` — the idea fits the repository but concrete fixes are required before merge.",
|
||||
"`needs-insight` — a maintainer judgement call is needed (policy, duplication, product direction) that a review cannot settle.",
|
||||
"`close` — the submission does not belong in this repository and no reasonable revision would change that.",
|
||||
"`close` — the submission does not belong in this repository and no reasonable revision would change that (including vendor funnels and non-developer domains).",
|
||||
].join("\n");
|
||||
|
||||
/** Shared schema for a single recorded review. Reused directly as the `record_review` input schema. */
|
||||
@@ -63,17 +63,29 @@ function sharedRules(repo, guidancePath) {
|
||||
" - `gh pr view <number> -R " + repo + " --comments` for the description and discussion.",
|
||||
" - `gh pr diff <number> -R " + repo + "` for the actual change (use `--name-only` first if the diff is large).",
|
||||
" - `gh pr checks <number> -R " + repo + "` when checks are failing, to see which ones and why.",
|
||||
" - `gh pr list -R " + repo + " --author <login> --state all` to find the author's sibling PRs. If siblings were closed for a reason, apply it consistently and cite them; flag any open sibling recorded more leniently in `checkNext`.",
|
||||
"Judge the submitted result, not assumptions about the tool that produced it. A title ending in 🤖🤖🤖 is a disclosure, not a defect.",
|
||||
"",
|
||||
provenanceRules(),
|
||||
"",
|
||||
knowledgeUpliftRules(),
|
||||
].join("\n");
|
||||
}
|
||||
|
||||
function provenanceRules() {
|
||||
return [
|
||||
"Provenance, repo fit, and validation — these are the checks earlier reviews under-called:",
|
||||
" - Vendor funnel: inspect the LICENSE copyright holder, frontmatter `metadata.author`, any \"Adapted from\" link, and the upstream README (`gh api -H \"Accept: application/vnd.github.raw\" repos/<owner>/<repo>/readme`). Look for MCP/connector setup commands, UTM or referral links, paid/credit upsells, and a \"free skill → paid connector\" shape, even when the vendor name was stripped from the adapted copy. A vendor funnel is `close`, not `request-changes`.",
|
||||
" - Repo fit: non-developer domains (growth marketing, dating, career/LinkedIn, ad creative) are not GitHub Copilot development workflows; weigh that heavily even when the content is polished.",
|
||||
" - Validation: AI-only synthetic evaluations (especially fixtures built to exercise the skill's own rules) and testing only in a non-Copilot harness (e.g. Claude Code) are weak evidence. Long evaluation transcripts are not evidence of value.",
|
||||
].join("\n");
|
||||
}
|
||||
|
||||
function knowledgeUpliftRules() {
|
||||
return [
|
||||
"Knowledge uplift — apply the \"Knowledge uplift\" test from the code-review skill to every agent, instruction, skill, prompt, hook, or workflow the PR adds or substantially changes. It is the criterion most often under-applied, so do not skip it for well-formatted PRs:",
|
||||
" - Always start the `differentiation` field with the `high` / `partial` / `none` classification and the quoted lines that justify it, then note any overlap with existing resources.",
|
||||
" - `none` must not be `approve`: use `needs-insight`, or `close` when it is also a near-duplicate. `partial` is usually `request-changes`; name the generic sections to cut in `suggestedComment`.",
|
||||
" - `none` must not be `approve`: use `needs-insight`, or `close` when it is also a near-duplicate. `partial` is usually `request-changes`; name the generic sections to cut in `suggestedComment`. But a thin real core wrapped in generic practice (\"cite sources\", \"label facts\", \"structure the report\") combined with weak repo fit or a vendor funnel is `close`.",
|
||||
].join("\n");
|
||||
}
|
||||
|
||||
@@ -83,12 +95,30 @@ function recordingRules(instanceId) {
|
||||
"Each review object needs `number`, `queueId`, `recommendation`, and `rationale`; fill in `summary`, `repoFit`, `compliance`, `differentiation`, `risks`, `validation`, `confidence`, `checkNext`, and `suggestedComment` whenever you have something real to say.",
|
||||
"`suggestedComment` should be the comment you would leave on the pull request — specific, actionable, and written for the author.",
|
||||
"You may record in batches as you go. If the canvas is closed, re-open it first with `open_canvas` using canvasId `open-pr-board` and that same instanceId.",
|
||||
"Check `gh pr view <number> --json state` before recording: closed or merged PRs are dropped from the board and `record_review` skips them as \"not on the board\". A \"stale review result\" skip means the queueId was already consumed; call `get_pr` to see what is stored rather than re-recording.",
|
||||
"",
|
||||
"Recommendations:",
|
||||
RECOMMENDATION_GUIDE,
|
||||
].join("\n");
|
||||
}
|
||||
|
||||
export const MAX_PRS_PER_AGENT = 3;
|
||||
|
||||
function delegationRules(count, instanceId) {
|
||||
const split =
|
||||
count === 1
|
||||
? "Launch one background agent for this pull request."
|
||||
: `Launch background agents in parallel, one per pull request (or up to ${MAX_PRS_PER_AGENT} per agent when there are many), and give each agent only its own pull request entries.`;
|
||||
return [
|
||||
"Delegate this work — do not review in the main session. Keeping the main conversation free is the point of this request.",
|
||||
` - ${split} Use the \`task\` tool with \`mode: "background"\` and a general-purpose agent type.`,
|
||||
" - Pass each agent everything below this list verbatim (rules, its pull request entries, and recording instructions); it has no other context.",
|
||||
` - Agents with \`invoke_canvas_action\` record their own results against instanceId \`${instanceId}\`. Agents without it must return the review objects as JSON; when they finish, record those yourself with \`record_review\`.`,
|
||||
" - Do not gather evidence yourself or poll the agents. After launching, reply with one short line saying the reviews are running in the background, then end your turn.",
|
||||
" - When an agent completes, read its result once, record anything it could not, and report only failures.",
|
||||
].join("\n");
|
||||
}
|
||||
|
||||
export function buildReviewPrompt({ instanceId, repo, guidancePath, items, decisions }) {
|
||||
const calibration = decisions.length
|
||||
? [
|
||||
@@ -103,6 +133,9 @@ export function buildReviewPrompt({ instanceId, repo, guidancePath, items, decis
|
||||
return [
|
||||
`Review ${items.length} open pull request${items.length === 1 ? "" : "s"} in ${repo} for the Open PR Board canvas.`,
|
||||
"",
|
||||
delegationRules(items.length, instanceId),
|
||||
"",
|
||||
"--- Agent brief ---",
|
||||
sharedRules(repo, guidancePath),
|
||||
calibration,
|
||||
"",
|
||||
@@ -129,12 +162,17 @@ export function buildRereviewPrompt({ instanceId, repo, guidancePath, item, guid
|
||||
return [
|
||||
`Re-review pull request #${item.number} in ${repo} for the Open PR Board canvas.`,
|
||||
"",
|
||||
delegationRules(1, instanceId),
|
||||
"",
|
||||
"--- Agent brief ---",
|
||||
sharedRules(repo, guidancePath),
|
||||
"",
|
||||
"Pull request:",
|
||||
describeItem(item),
|
||||
prior,
|
||||
guidance ? `\nMaintainer guidance for this re-review:\n${guidance}` : "",
|
||||
guidance
|
||||
? `\nMaintainer guidance for this re-review (a strong prior — verify it against the evidence and cite what confirms or contradicts it; it does not replace the evidence):\n${guidance}`
|
||||
: "",
|
||||
"",
|
||||
"Start fresh from the evidence; the previous review is context, not a conclusion.",
|
||||
"",
|
||||
|
||||
@@ -25,10 +25,12 @@ analysis into one of the board's four recommendations.
|
||||
| `approve` | The change is correct, in scope, meets the format rules, and a maintainer could merge it as-is. Minor nits belong in the comment, not in a lower bucket. |
|
||||
| `request-changes` | The contribution belongs in the repo but something concrete must be fixed first: missing frontmatter, bad file name, stale generated output, failing checks. |
|
||||
| `needs-insight` | The call is genuinely a maintainer judgement: unclear value, overlap with existing content, licensing or provenance questions, scope the reviewer cannot settle. |
|
||||
| `close` | The PR does not belong in this repository at all: spam, off-topic, duplicate of an existing merged PR, abandoned, or a change to generated files only. |
|
||||
| `close` | The PR does not belong in this repository at all: spam, off-topic, vendor funnel, non-developer domain, duplicate of a merged PR, abandoned, or generated files only. |
|
||||
|
||||
Default to `needs-insight` rather than guessing. A wrong `approve` or `close` costs a
|
||||
maintainer more than an honest escalation.
|
||||
maintainer more than an honest escalation. But when the evidence shows the PR cannot be
|
||||
fixed into something that belongs here (vendor funnel, off-domain), say `close`;
|
||||
`request-changes` implies a revision would make it mergeable.
|
||||
|
||||
## What to look at
|
||||
|
||||
@@ -45,12 +47,31 @@ For every pull request:
|
||||
reason for `request-changes`.
|
||||
- **Duplication** — search the repo for an existing agent/instruction/skill covering the
|
||||
same ground. Near-duplicates are `needs-insight` unless the overlap is total.
|
||||
- **Repo fit** — non-developer domains (growth marketing, dating, career/LinkedIn, ad
|
||||
creative, and similar) are not GitHub Copilot development workflows. Weigh that heavily
|
||||
even when the content is polished; on its own it usually means `close`.
|
||||
- **Knowledge uplift** — the most common way a well-formatted PR still fails. Apply the
|
||||
"Knowledge uplift" test from `.github/skills/code-review/SKILL.md` to every content
|
||||
resource. `none` is never `approve` (use `needs-insight`, or `close` if it is also a
|
||||
near-duplicate); `partial` is usually `request-changes`.
|
||||
- **Provenance** — content copied from elsewhere without attribution, or obviously
|
||||
machine-generated filler with no evidence it was tested, is `needs-insight` at best.
|
||||
near-duplicate). `partial` is usually `request-changes`, but a thin real core wrapped
|
||||
in generic practice ("cite sources", "label facts", "structure the report") combined
|
||||
with weak repo fit or a vendor funnel is `close`.
|
||||
- **Provenance and vendor funnels** — check who really owns the content: the LICENSE
|
||||
copyright holder, frontmatter `metadata.author`, any "Adapted from" link, and the
|
||||
upstream repo README (`gh api -H "Accept: application/vnd.github.raw" repos/<owner>/<repo>/readme`).
|
||||
Then look for MCP/connector setup commands, UTM or referral links, paid or credit-based
|
||||
upsells, and a "free skill → paid connector" shape — including copies where the vendor
|
||||
name was stripped but the hook remains (e.g. "if the user has a suitable research
|
||||
connector…"). A vendor funnel is `close`, not `request-changes`. Unattributed copies or
|
||||
untested machine-generated filler are `needs-insight` at best.
|
||||
- **Author batch** — run `gh pr list -R github/awesome-copilot --author <login> --state all`.
|
||||
If sibling PRs were closed for a reason (product funnel, off-domain), apply the same
|
||||
reason consistently and cite them by number. Flag in `checkNext` any open sibling that
|
||||
was previously recorded more leniently so it can be re-reviewed.
|
||||
- **Validation quality** — an AI-only synthetic evaluation (especially one whose fixture
|
||||
was built to exercise the skill's own rules) is weak evidence, as is testing only in a
|
||||
non-Copilot harness (e.g. `claude mcp add` / Claude Code). Long evaluation transcripts
|
||||
and polished PR bodies are not evidence of value.
|
||||
- **Checks and conflicts** — failing CI or a conflicting branch is `request-changes`,
|
||||
even when the content itself is fine.
|
||||
- **Labels** — respect existing triage. A PR already labelled `blocked`, `needs-author-input`,
|
||||
@@ -70,6 +91,15 @@ For every pull request:
|
||||
tone of an ordinary maintainer review.
|
||||
- `confidence` is `low`, `medium`, or `high`. Use `low` freely; it is a signal, not a
|
||||
failure.
|
||||
- On a re-review, maintainer free-text guidance is a strong prior. Verify it against the
|
||||
evidence and cite what confirms or contradicts it; it does not replace the evidence.
|
||||
|
||||
## Recording mechanics
|
||||
|
||||
- Check `gh pr view <number> --json state` before recording. Closed or merged PRs drop
|
||||
off the board, and `record_review` skips them with reason "not on the board".
|
||||
- A "stale review result" skip means that `queueId` was already consumed (or the PR
|
||||
changed). Call `get_pr` to see what is stored instead of re-recording.
|
||||
|
||||
## Hard rules
|
||||
|
||||
|
||||
@@ -107,6 +107,11 @@ Flag marketing-heavy framing only when there is concrete evidence, such as:
|
||||
- links or calls to action that dominate the resource
|
||||
- a resource whose primary purpose is acquiring users rather than helping them
|
||||
use GitHub Copilot
|
||||
- a "free resource → paid connector" structure: MCP/connector setup commands,
|
||||
UTM or referral links, or credit-based upsells pointing at the author's
|
||||
service, including adapted copies where the vendor name was removed but the
|
||||
hook remains. Check the LICENSE copyright holder, `metadata.author`, and the
|
||||
upstream repository README to confirm who owns the content.
|
||||
|
||||
Describe the specific evidence and suggest how to refocus the contribution on
|
||||
the user problem. Do not infer promotional intent solely because an author is
|
||||
@@ -141,7 +146,9 @@ appropriate evidence depends on the resource:
|
||||
describe
|
||||
|
||||
Do not require executable tests for prose-only resources when a realistic
|
||||
manual evaluation is more appropriate.
|
||||
manual evaluation is more appropriate. An AI-only synthetic evaluation whose
|
||||
fixture was built to exercise the resource's own rules, or testing only in a
|
||||
non-Copilot harness, is weak evidence on its own.
|
||||
|
||||
## Trusted and automated paths
|
||||
|
||||
|
||||
Reference in New Issue
Block a user