diff --git a/.github/extensions/external-plugin-review-board/lib/review-prompt.mjs b/.github/extensions/external-plugin-review-board/lib/review-prompt.mjs index efff58cc19..70a4324779 100644 --- a/.github/extensions/external-plugin-review-board/lib/review-prompt.mjs +++ b/.github/extensions/external-plugin-review-board/lib/review-prompt.mjs @@ -66,6 +66,8 @@ ${previous} Recent maintainer decisions (calibrate against these as well as the guidance): ${formatHistory(history)} +Evaluate concrete Copilot relevance and specific standalone user value across supported workflows, not only coding or text-based developer tasks. Image, video, audio, other media-generation, and creative work may be in scope where applicable; modality or non-coding use alone is not a rejection reason. Assess knowledge uplift, provenance, product/vendor-funnel evidence, responsible use, and validation appropriate to the workflow. Preserve rejection of marketing-only funnels, paid or credit-based acquisition paths without standalone user value, duplicates of native Copilot features, and generic content with no meaningful uplift. + When you're done, send your result back to the session that created you (your creator) as a single message containing ONLY a JSON object with these fields: ${REVIEW_FIELD_LIST}. Include queueId: "${item.queueId}". In the rationale, explicitly address the maintainer's guidance.`; return `The maintainer requested a guided re-review of #${item.number} from the External Plugin Review Board canvas. Run it as a separate sub-session; do NOT perform the review yourself in this session. @@ -97,6 +99,8 @@ It contains the maintainer's decision pattern, the per-issue review procedure, a Recent decisions made from the board (calibrate against these as well as the guidance): ${recent} +Evaluate concrete Copilot relevance and specific standalone user value across supported workflows, not only coding or text-based developer tasks. Image, video, audio, other media-generation, and creative work may be in scope where applicable; modality or non-coding use alone is not a rejection reason. Assess knowledge uplift, provenance, product/vendor-funnel evidence, responsible use, and validation appropriate to the workflow. Preserve rejection of marketing-only funnels, paid or credit-based acquisition paths without standalone user value, duplicates of native Copilot features, and generic content with no meaningful uplift. + Submissions to review: ${list} diff --git a/.github/extensions/external-plugin-review-board/review-guidance.md b/.github/extensions/external-plugin-review-board/review-guidance.md index 5bd0644aa7..610a565269 100644 --- a/.github/extensions/external-plugin-review-board/review-guidance.md +++ b/.github/extensions/external-plugin-review-board/review-guidance.md @@ -11,7 +11,7 @@ submissions before reviewing (see "Calibrate" below). **Typically approved** -- Established vendors or well-known OSS projects with genuine developer-workflow value +- Established vendors or well-known OSS projects with genuine Copilot user value across supported workflows (e.g. Sumo Logic, Atlassian Teamwork Graph CLI, bitdrift, Remotion, Radius, incident.io-style SRE tooling). - Genuinely useful Copilot tooling and canvases that don't duplicate built-in features (e.g. excel-cli/excel-mcp, chat-fork-map, cache-stats, usage-insights). @@ -29,8 +29,8 @@ submissions before reviewing (see "Calibrate" below). - 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." +- Narrow submissions with no demonstrated Copilot relevance, standalone user value, or meaningful uplift — + "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 ("Copilot harnesses already have built-in provisions for controlling tool use"), markdown viewing, browser automation ("This can be done using Playwright, which is available in the Copilot harnesses"). @@ -40,6 +40,23 @@ submissions before reviewing (see "Calibrate" below). - Bulk submissions of many tiny related repos from one submitter. - Resubmissions: check whether the reasons for the previous rejection were actually addressed. +## Assess Copilot value across workflows + +Do not assume submissions must address coding or other text-based developer tasks. +Consider Copilot-supported workflows across modalities, including image, video, +audio, other media generation, and creative work where applicable. Modality or +non-coding use alone is not a reason to reject. Assess the concrete Copilot +relevance, the specific user problem and standalone value, and what the plugin +adds beyond a capable model acting without it (knowledge uplift). + +Keep that scope assessment separate from the quality gates: reject marketing-only +funnels and paid or credit-based acquisition paths that provide no standalone +user value; identify duplicates of native Copilot features and generic content +with no meaningful uplift. Check provenance and ownership, responsible use, and +validation evidence appropriate to the workflow (for example, meaningful output +evaluation for a media-generation workflow rather than code-only tests). A +non-coding workflow still needs concrete value and credible evidence. + ## Procedure per issue 1. `gh issue view -R github/awesome-copilot --json title,body,author,comments` — read the submission diff --git a/.github/extensions/open-pr-board/lib/decisions.mjs b/.github/extensions/open-pr-board/lib/decisions.mjs index 3434340f90..d08802263a 100644 --- a/.github/extensions/open-pr-board/lib/decisions.mjs +++ b/.github/extensions/open-pr-board/lib/decisions.mjs @@ -5,6 +5,15 @@ const DECISION_KINDS = ["approve", "request-changes", "comment", "close"]; const MAX_COMMENT_LENGTH = 60000; const decisionsInFlight = new Set(); +function decisionError(error) { + if (/\bHTTP 422\b/.test(error?.message ?? "")) { + return expectedNotice( + "GitHub rejected this action (HTTP 422). Refresh the board and check that your account can perform this action on the pull request.", + ); + } + return error; +} + export async function applyDecisionOnGitHub({ store, number, kind, comment, headRefOid, refresh, github = gh }) { if (!DECISION_KINDS.includes(kind)) throw new Error(`Unknown decision "${kind}".`); const key = `${store.repo}#${number}`; @@ -26,15 +35,19 @@ export async function applyDecisionOnGitHub({ store, number, kind, comment, head } let result; - if (kind === "approve") { - if (!headRefOid || headRefOid !== item.headRefOid) { - throw expectedNotice("PR changed since it was displayed; refresh and review it again before approving."); + try { + if (kind === "approve") { + if (!headRefOid || headRefOid !== item.headRefOid) { + throw expectedNotice("PR changed since it was displayed; refresh and review it again before approving."); + } + result = await github.approveAndEnableAutoMerge(store.repo, number, { body, headRefOid }); + } else if (kind === "close") { + result = await github.closePullRequest(store.repo, number, { body }); + } else { + result = await github.submitPullRequestReview(store.repo, number, { kind, body }); } - result = await github.approveAndEnableAutoMerge(store.repo, number, { body, headRefOid }); - } else if (kind === "close") { - result = await github.closePullRequest(store.repo, number, { body }); - } else { - result = await github.submitPullRequestReview(store.repo, number, { kind, body }); + } catch (error) { + throw decisionError(error); } const warnings = result.warning ? [result.warning] : []; diff --git a/.github/extensions/open-pr-board/lib/review-prompt.mjs b/.github/extensions/open-pr-board/lib/review-prompt.mjs index 075846b7a1..26a56574df 100644 --- a/.github/extensions/open-pr-board/lib/review-prompt.mjs +++ b/.github/extensions/open-pr-board/lib/review-prompt.mjs @@ -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 (including vendor funnels and non-developer domains).", + "`close` — the submission does not belong in this repository and no reasonable revision would change that (including marketing-only vendor funnels and paid or credit-based acquisition paths without standalone user value).", ].join("\n"); /** Shared schema for a single recorded review. Reused directly as the `record_review` input schema. */ @@ -23,7 +23,10 @@ export const REVIEW_FIELDS_SCHEMA = { }, rationale: { type: "string", description: "One sentence explaining the deciding factor, shown on the card." }, summary: { type: "string", description: "What the pull request actually changes." }, - repoFit: { type: "string", description: "Repository fit and meaningful value for Copilot users." }, + repoFit: { + type: "string", + description: "Concrete Copilot relevance and specific user value across supported workflows and modalities.", + }, compliance: { type: "string", description: "Deterministic checks: frontmatter, naming, README regeneration, manifests." }, differentiation: { type: "string", @@ -67,6 +70,7 @@ function sharedRules(repo, guidancePath) { " - `gh pr checks -R " + repo + "` when checks are failing, to see which ones and why.", " - `gh pr list -R " + repo + " --author --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.", + "Do not limit repository fit to coding or text-based developer tasks. Evaluate Copilot-supported workflows across modalities, including image, video, audio, other media generation, and creative work where applicable. Modality or non-coding use alone is not a rejection reason; assess concrete Copilot relevance, specific standalone user value, knowledge uplift, provenance, product/vendor-funnel evidence, responsible use, and validation appropriate to the workflow.", "", provenanceRules(), "", @@ -78,7 +82,8 @@ 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///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.", + " - Repo fit: judge concrete Copilot relevance and user value, not whether a task is coding- or text-based. Image, video, audio, media-generation, and creative workflows may be in scope where Copilot supports them; modality or non-coding use alone is not grounds to close a PR.", + " - Keep repo fit separate from the other quality gates: reject marketing-only funnels, paid or credit-based acquisition paths without standalone user value, duplicates of native Copilot features, and generic content with no meaningful knowledge uplift. Check provenance, responsible use, and validation appropriate to the workflow.", " - 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"); } diff --git a/.github/extensions/open-pr-board/review-guidance.md b/.github/extensions/open-pr-board/review-guidance.md index 5b2529754b..279b02f36e 100644 --- a/.github/extensions/open-pr-board/review-guidance.md +++ b/.github/extensions/open-pr-board/review-guidance.md @@ -25,7 +25,7 @@ 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, vendor funnel, non-developer domain, duplicate of a merged PR, abandoned, or generated files only. | +| `close` | The PR does not belong in this repository at all: spam, off-topic, vendor funnel, 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. But when the evidence shows the PR cannot be @@ -57,9 +57,14 @@ For every pull request: require a particular scalar quote style. - **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`. +- **Repo fit** — assess concrete relevance to GitHub Copilot and a specific user + workflow, not whether the task is coding- or text-based. Copilot-supported + workflows may include image, video, audio, other media generation, and creative + work where applicable. Modality or non-coding use alone is not a rejection + reason; evaluate the submission's standalone value and knowledge uplift, and + verify provenance, product/vendor-funnel evidence, responsible use, and + workflow-appropriate validation. Keep those quality checks separate from + assumptions about the task's modality. - **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 diff --git a/.github/skills/code-review/SKILL.md b/.github/skills/code-review/SKILL.md index 4fe5c85163..fa5a3cd651 100644 --- a/.github/skills/code-review/SKILL.md +++ b/.github/skills/code-review/SKILL.md @@ -40,8 +40,15 @@ be evaluated according to their source change. ## Repository fit Confirm that a submission addresses a specific GitHub Copilot workflow, -technology, domain constraint, or user problem. Flag contributions that: +technology, domain constraint, or user problem. +Repository fit is not limited to coding or other text-based developer tasks. +Consider the workflows Copilot supports, including image, video, audio, other +media generation, and creative work where applicable. Modality or non-coding +use alone is not a reason to reject; require concrete Copilot relevance and +specific standalone user value. + +Flag contributions that: - provide generic advice that current models already handle well without meaningful uplift (see [Knowledge uplift](#knowledge-uplift)) - restate an existing resource without a clear differentiator @@ -53,6 +60,12 @@ Paid or commercial services are not automatically unsuitable. Evaluate whether the contribution provides standalone user value and follows the repository's guidance for paid-service submissions. +Assess knowledge uplift, provenance, product/vendor-funnel evidence, +responsible use, and validation appropriate to the workflow independently of +its modality. Reject marketing-only funnels, paid or credit-based acquisition +paths without standalone user value, duplicates of native Copilot features, +and generic content with no meaningful uplift. + ## Knowledge uplift A resource is only valuable if it changes what the model does. Well-formatted, diff --git a/eng/external-plugin-review-board.test.mjs b/eng/external-plugin-review-board.test.mjs index 1872e44f9b..b877532211 100644 --- a/eng/external-plugin-review-board.test.mjs +++ b/eng/external-plugin-review-board.test.mjs @@ -3,6 +3,43 @@ import { readFile } from "node:fs/promises"; import vm from "node:vm"; import { test } from "node:test"; import { BoardStore } from "../.github/extensions/external-plugin-review-board/lib/state.mjs"; +import { + buildReviewPrompt, + buildRereviewPrompt, +} from "../.github/extensions/external-plugin-review-board/lib/review-prompt.mjs"; + +test("external-plugin review prompts and guidance assess Copilot workflows across modalities", async () => { + const prompts = [ + buildReviewPrompt({ items: [], instanceId: "board-1", canvasId: "external-plugin-review-board", guidancePath: "guidance.md", history: [] }), + buildRereviewPrompt({ + item: { number: 1, title: "Example", author: "someone", queueId: "q-1" }, + instanceId: "board-1", + canvasId: "external-plugin-review-board", + guidancePath: "guidance.md", + history: [], + repo: "github/awesome-copilot", + }), + ]; + for (const prompt of prompts) { + assert.match(prompt, /not only coding or text-based developer tasks/); + assert.match(prompt, /Image, video, audio, other media-generation, and creative work/); + assert.match(prompt, /modality or non-coding use alone is not a rejection reason/); + assert.match(prompt, /knowledge uplift, provenance, product\/vendor-funnel evidence, responsible use/); + assert.match(prompt, /validation appropriate to the workflow/); + assert.match(prompt, /paid or credit-based acquisition paths without standalone user value/); + assert.match(prompt, /duplicates of native Copilot features/); + assert.match(prompt, /generic content with no meaningful uplift/); + } + + const guidance = await readFile( + new URL("../.github/extensions/external-plugin-review-board/review-guidance.md", import.meta.url), + "utf8", + ); + assert.match(guidance, /Assess Copilot value across workflows/); + assert.match(guidance, /non-coding use alone is not a reason to reject/); + assert.match(guidance, /validation evidence appropriate to the workflow/); + assert.match(guidance, /duplicates of native Copilot features/); +}); test("external-plugin-review-board excludes items with pending decisions from AI review", () => { const store = new BoardStore("unused", "github/awesome-copilot"); diff --git a/eng/open-pr-board.test.mjs b/eng/open-pr-board.test.mjs index 8dc283e597..327864bfe6 100644 --- a/eng/open-pr-board.test.mjs +++ b/eng/open-pr-board.test.mjs @@ -423,6 +423,27 @@ test("decision lock is released on failure and independent repositories can proc await first; }); +test("GitHub 422 decision failures are shown as actionable notices and not recorded", async (t) => { + const store = await createStore(t); + let refreshes = 0; + await assert.rejects(applyDecisionOnGitHub({ + store, + number: 1, + kind: "comment", + comment: "Feedback", + headRefOid: "first", + github: { submitPullRequestReview: async () => { throw new Error("gh: Unprocessable Entity (HTTP 422)"); } }, + refresh: async () => { refreshes++; }, + }), (error) => { + assert.equal(error.handoff, false); + assert.match(error.message, /GitHub rejected this action \(HTTP 422\)/); + assert.match(error.message, /Refresh the board/); + return true; + }); + assert.equal(refreshes, 0); + assert.equal(store.getItem(1).decision, null); +}); + class Element { constructor() { this.listeners = {}; @@ -748,7 +769,7 @@ test("initial and re-review agent briefs accept valid YAML scalar styles while p } }); -test("review prompts include vendor-funnel, author-batch, and recording checks", () => { +test("review prompts assess Copilot workflows across modalities and retain independent quality gates", () => { const prompts = [ buildReviewPrompt({ ...promptArgs, items: [promptItem], decisions: [] }), buildRereviewPrompt({ ...promptArgs, item: promptItem }), @@ -759,7 +780,13 @@ test("review prompts include vendor-funnel, author-batch, and recording checks", assert.match(prompt, /repos\/\/\/readme/); assert.match(prompt, /UTM or referral links/); assert.match(prompt, /vendor funnel is `close`, not `request-changes`/); - assert.match(prompt, /non-developer domains/); + assert.match(prompt, /Do not limit repository fit to coding or text-based developer tasks/); + assert.match(prompt, /image, video, audio, other media generation, and creative work/); + assert.match(prompt, /Modality or non-coding use alone is not a rejection reason/); + assert.match(prompt, /validation appropriate to the workflow/); + assert.match(prompt, /paid or credit-based acquisition paths without standalone user value/); + assert.match(prompt, /duplicates of native Copilot features/); + assert.match(prompt, /generic content with no meaningful knowledge uplift/); assert.match(prompt, /AI-only synthetic evaluations/); assert.match(prompt, /weak repo fit or a vendor funnel is `close`/); assert.match(prompt, /gh pr view --json state/); @@ -767,6 +794,24 @@ test("review prompts include vendor-funnel, author-batch, and recording checks", } }); +test("review guidance and canonical skill don't treat non-coding workflows as out of scope", async () => { + const guidance = await readFile( + new URL("../.github/extensions/open-pr-board/review-guidance.md", import.meta.url), + "utf8", + ); + const skill = await readFile( + new URL("../.github/skills/code-review/SKILL.md", import.meta.url), + "utf8", + ); + for (const content of [guidance, skill]) { + assert.match(content, /alone is not a rejection\s+reason|alone is not a reason to reject/); + assert.match(content, /image, video/); + assert.match(content, /knowledge uplift/); + assert.match(content, /responsible use/); + assert.match(content, /validation appropriate to the workflow|workflow-appropriate validation/); + } +}); + test("re-review prompt frames maintainer guidance as a prior to verify", () => { const prompt = buildRereviewPrompt({ ...promptArgs, item: promptItem, guidance: "thinly veiled product integration" }); assert.match(prompt, /strong prior — verify it against the evidence[^\n]*\nthinly veiled product integration/);