Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
4 changes: 2 additions & 2 deletions .github/workflows/external-plugin-command-router.yml
Original file line number Diff line number Diff line change
Expand Up @@ -807,7 +807,7 @@ jobs:
if (process.env.QUALITY_JOB_RESULT === 'failure' || process.env.QUALITY_JOB_RESULT === 'cancelled') {
qualityResult = {
overall_status: 'infra_error',
spec_compliance_status: 'warning',
spec_compliance_status: 'infra_error',
vally_lint_status: 'infra_error',
smoke_status: 'infra_error',
version_match_status: 'infra_error',
Expand All @@ -820,7 +820,7 @@ jobs:
} else {
qualityResult = {
overall_status: 'infra_error',
spec_compliance_status: 'warning',
spec_compliance_status: 'infra_error',
vally_lint_status: 'infra_error',
smoke_status: 'infra_error',
version_match_status: 'infra_error',
Expand Down
4 changes: 2 additions & 2 deletions .github/workflows/external-plugin-intake.yml
Original file line number Diff line number Diff line change
Expand Up @@ -130,7 +130,7 @@ jobs:
if (process.env.QUALITY_JOB_RESULT === 'failure' || process.env.QUALITY_JOB_RESULT === 'cancelled') {
qualityResult = {
overall_status: 'infra_error',
spec_compliance_status: 'warning',
spec_compliance_status: 'infra_error',
vally_lint_status: 'infra_error',
smoke_status: 'infra_error',
version_match_status: 'infra_error',
Expand All @@ -143,7 +143,7 @@ jobs:
} else {
qualityResult = {
overall_status: 'infra_error',
spec_compliance_status: 'warning',
spec_compliance_status: 'infra_error',
vally_lint_status: 'infra_error',
smoke_status: 'infra_error',
version_match_status: 'infra_error',
Expand Down
17 changes: 7 additions & 10 deletions .github/workflows/external-plugin-pr-quality-gates-writer.yml
Original file line number Diff line number Diff line change
Expand Up @@ -223,22 +223,19 @@ jobs:
});

const checkedPlugins = Array.isArray(qualityResult.checked_plugins) ? qualityResult.checked_plugins.slice(0, 50) : [];
const hasSpecWarnings = checkedPlugins.some((entry) => String(entry?.quality?.spec_compliance_status || '') === 'warning');
const header = qualityResult.failure_class === 'submitter_fixes'
? '## 🛑 External plugin PR checks failed (submitter fixes required)'
: qualityResult.overall_status === 'infra_error'
? '## 🛑 External plugin PR checks failed (maintainer follow-up)'
: hasSpecWarnings
? '## ⚠️ External plugin PR checks passed with spec warnings'
: qualityResult.overall_status === 'pass' || !shouldRun
? '## ✅ External plugin PR checks passed'
: '## ⚠️ External plugin PR checks need maintainer follow-up';
const formatStatus = (rawStatus, gateName) => {
: qualityResult.overall_status === 'pass' || !shouldRun
? '## ✅ External plugin PR checks passed'
: '## ⚠️ External plugin PR checks need maintainer follow-up';
const formatStatus = (rawStatus) => {
const status = String(rawStatus || 'not_run');
if (status === 'pass') {
return '✅ pass';
}
if (status === 'warning' || (gateName === 'spec compliance' && status === 'fail')) {
if (status === 'warning') {
return '⚠️ warning';
}
if (status === 'fail' || status === 'infra_error') {
Expand Down Expand Up @@ -322,7 +319,7 @@ jobs:
const failureDetails = checkedPlugins.flatMap((entry) => {
const name = String(entry?.name || 'unknown');
const quality = entry?.quality || {};
const shouldShowSpec = quality.spec_compliance_status === 'warning' || String(quality.spec_compliance_output || '').trim().length > 0;
const shouldShowSpec = quality.spec_compliance_status === 'fail' || quality.spec_compliance_status === 'infra_error' || String(quality.spec_compliance_output || '').trim().length > 0;
const shouldShowVally = quality.vally_lint_status === 'fail' || quality.vally_lint_status === 'infra_error' || String(quality.vally_lint_output || '').trim().length > 0;
const shouldShowSmoke = quality.smoke_status === 'fail' || quality.smoke_status === 'infra_error' || String(quality.smoke_output || '').trim().length > 0;
const shouldShowVersionMatch = quality.version_match_status === 'fail' || quality.version_match_status === 'infra_error' || String(quality.version_match_output || '').trim().length > 0;
Expand Down Expand Up @@ -362,7 +359,7 @@ jobs:
'',
'### Per-plugin quality summary',
'',
'| Plugin | spec compliance (non-blocking) | vally lint | install smoke test | version match | ref/sha consistency | canvas structure | overall | source tree |',
'| Plugin | spec compliance | vally lint | install smoke test | version match | ref/sha consistency | canvas structure | overall | source tree |',
'|---|---|---|---|---|---|---|---|---|',
...rows,
'',
Expand Down
33 changes: 33 additions & 0 deletions .github/workflows/external-plugin-pr-quality-gates.yml
Original file line number Diff line number Diff line change
Expand Up @@ -183,3 +183,36 @@ jobs:
path: ${{ runner.temp }}/external-plugin-pr-quality-result/result.json
if-no-files-found: error
retention-days: 3

enforce-quality-gates:
runs-on: ubuntu-latest
needs: [detect-changed-plugins, run-quality-gates]
if: always()
Comment on lines +187 to +190
steps:
- name: Require all external plugin quality gates to pass
env:
DETECT_JOB_RESULT: ${{ needs.detect-changed-plugins.result }}
SHOULD_RUN: ${{ needs.detect-changed-plugins.outputs.should-run }}
QUALITY_JOB_RESULT: ${{ needs.run-quality-gates.result }}
QUALITY_RESULT_JSON: ${{ needs.run-quality-gates.outputs.quality-result }}
run: |
if [ "$DETECT_JOB_RESULT" != "success" ]; then
echo "::error::External plugin change detection did not succeed ($DETECT_JOB_RESULT)."
exit 1
fi
if [ "$SHOULD_RUN" != "true" ]; then
echo "No changed external plugin entries; nothing to enforce."
exit 0
fi
if [ "$QUALITY_JOB_RESULT" != "success" ]; then
echo "::error::External plugin quality gates did not complete ($QUALITY_JOB_RESULT)."
exit 1
fi
node -e '
const result = JSON.parse(process.env.QUALITY_RESULT_JSON || "{}");
console.log(result.summary || "No summary provided.");
if (result.overall_status !== "pass") {
console.log(`::error::All external plugin quality gates must pass (overall: ${result.overall_status || "unknown"}). See the PR comment for details.`);
process.exit(1);
}
'
10 changes: 8 additions & 2 deletions CONTRIBUTING.md
Original file line number Diff line number Diff line change
Expand Up @@ -255,8 +255,13 @@ The public-submission policy builds on those rules and also requires `license` p
2. **Automated intake validation** checks that the required fields are present and correctly formatted for a GitHub-hosted plugin. Invalid submissions are labeled `requires-submitter-fixes` with a comment explaining what must be fixed before maintainer review.
Intake also checks the repository owner's type using the GitHub API. Organization-owned repositories receive `first party` for the `Microsoft`, `GitHub`, `Azure`, `dotnet`, `MicrosoftDocs`, or `microsoftgraph` organizations (case-insensitive), or `third party` for any other organization. User-owned repositories receive neither label. On the triage board, `first party` skips AI review and creates an `Accept` recommendation with `/approve`; this is only a triage recommendation. Submissions must still pass automated validation and quality gates, and a human maintainer must review and approve them before listing. Ownership labels are updated on each intake pass.
3. **Automated quality gates** run after metadata validation:
- `vally lint` against the submitted plugin path/ref/sha
- Agent Plugins v1.0.0 manifest compliance, including a root `plugin.json` with the required schema and supported fields
- `vally lint` against the submitted plugin path/ref/sha, including Agent Skills spec compliance and valid file references for bundled skills
- install smoke test via Copilot CLI against an ephemeral marketplace entry generated from the submission
- version matching against the source manifest, ref/SHA consistency when both locators are provided, and the required extension structure for plugins tagged `canvas`

All applicable quality gates must pass. Spec compliance violations and lint failures are blocking errors, not warnings. Infrastructure errors keep the submission in `awaiting-review` until checks can complete or a maintainer explicitly overrides the blocker.

4. **Ready for maintainer review**: if metadata validation and quality gates pass, automation removes `awaiting-review` and adds `ready-for-review`.
5. **Submitter-fix blocker**: if metadata is valid but quality gates fail, automation applies `requires-submitter-fixes` instead of advancing to human review.
6. **Requesting another intake pass**: after updating the issue body or source plugin, the issue author or a maintainer can comment `/rerun-intake` to re-run automated intake and quality gates on demand while the issue is still open. Open issues re-trigger intake automatically on edit. When the rerun is accepted, automation reacts to the command comment with 👀 so it is visible that processing started.
Expand All @@ -270,12 +275,13 @@ The public-submission policy builds on those rules and also requires `license` p
When a pull request updates `plugins/external.json` (for example, version updates for a previously approved listing), automation runs PR quality checks and posts the result directly on the PR:

1. **Detect changed entries**: automation identifies added/updated external plugin entries in the PR.
2. **Run quality gates**: automation runs install smoke tests and `vally lint` checks against each changed plugin source ref/SHA/path.
2. **Run quality gates**: automation runs the same blocking quality gates as submission intake against each changed plugin source ref/SHA/path, including manifest compliance, Agent Skills spec compliance via `vally lint`, and install smoke tests.
3. **Post source links**: automation updates a bot comment with per-plugin results and direct GitHub tree links to each plugin source location.
4. **Sync workflow-state labels on the PR**:
- `ready-for-review` when all checks pass
- `requires-submitter-fixes` when quality checks fail due to plugin issues
- `awaiting-review` when checks cannot complete because of infrastructure/transient errors
5. **Fail the PR status check**: the `enforce-quality-gates` job fails unless every changed plugin passes every applicable gate, so the PR shows a failing check (blocking merge when configured as a required status check) while any gate is failing or incomplete.

##### Website README

Expand Down
23 changes: 9 additions & 14 deletions eng/external-plugin-intake.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -1072,13 +1072,13 @@ function normalizeQualityGateResult(rawResult) {
}

function buildQualityGatesCommentSection(qualityResult) {
const formatStatus = (rawStatus, gate) => {
const formatStatus = (rawStatus) => {
const status = String(rawStatus || "not_run");
if (status === "pass") {
return "✅ pass";
}

if (status === "warning" || (gate === "spec" && status === "fail")) {
if (status === "warning") {
return "⚠️ warning";
}
if (status === "fail" || status === "infra_error") {
Expand All @@ -1102,7 +1102,7 @@ function buildQualityGatesCommentSection(qualityResult) {
"",
"| Gate | Status |",
"|---|---|",
`| spec compliance (non-blocking) | ${formatStatus(specState, "spec")} |`,
`| spec compliance | ${formatStatus(specState, "spec")} |`,
`| vally lint | ${formatStatus(vallyState, "vally")} |`,
`| install smoke test | ${formatStatus(smokeState, "smoke")} |`,
`| version match | ${formatStatus(versionMatchState, "version match")} |`,
Expand Down Expand Up @@ -1259,7 +1259,7 @@ function getIntakeStateFromQualityResult(baseResult, qualityResult) {
return "awaiting-review";
}

return "ready-for-review";
return qualityResult.overall_status === "pass" ? "ready-for-review" : "awaiting-review";
}

function buildMergedIntakeComment(baseResult, qualityResult, runId, owner, repo, qualityLogUrl) {
Expand All @@ -1274,24 +1274,19 @@ function buildMergedIntakeComment(baseResult, qualityResult, runId, owner, repo,
? `_[Download full quality gate logs](${qualityLogUrl})_`
: "";

const hasSpecWarnings = String(qualityResult.spec_compliance_status || "") === "warning";
const intro =
qualityResult.failure_class === "submitter_fixes"
? "## 🛑 External plugin intake failed (submitter fixes required)"
: qualityResult.failure_class === "infra"
: qualityResult.overall_status !== "pass"
? "## 🛑 External plugin intake failed (quality checks could not complete)"
: hasSpecWarnings
? "## ⚠️ External plugin intake passed with spec warnings"
: "## ✅ External plugin intake passed";
: "## ✅ External plugin intake passed";

const statusLine =
qualityResult.failure_class === "submitter_fixes"
? "This submission passed metadata validation, but quality gates found issues that must be fixed before it can move to maintainer review. Update the issue details or source plugin and then comment `/rerun-intake`."
: qualityResult.failure_class === "infra"
? "This submission passed metadata validation, but the automated quality checks hit an infrastructure issue. A maintainer should rerun intake or use the explicit override command after review."
: hasSpecWarnings
? "This submission passed blocking quality checks and is ready for maintainer review, but it has non-blocking Agent Plugins spec compliance warnings."
: "This submission passed automated intake validation and quality checks and is ready for maintainer review.";
: qualityResult.overall_status !== "pass"
? "This submission passed metadata validation, but the automated quality checks did not complete successfully. A maintainer should rerun intake or use the explicit override command after review."
: "This submission passed automated intake validation and quality checks and is ready for maintainer review.";

return [
marker,
Expand Down
52 changes: 52 additions & 0 deletions eng/external-plugin-intake.test.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,58 @@ const TREE_PLUGINS = "tree-plugins";
const TREE_UPGRADE_AGENT = "tree-upgrade-agent";
const TREE_EXTENSIONS = "tree-extensions";

const validBaseEvaluation = {
valid: true,
plugin: {
name: "example-plugin",
repository: "https://github.com/example/plugin",
source: { repo: "example/plugin", ref: "v1.0.0" },
},
warnings: [],
};

test("applyQualityGateResult blocks spec failures and displays them as failures", () => {
const result = applyQualityGateResult(validBaseEvaluation, {
overall_status: "fail",
failure_class: "submitter_fixes",
spec_compliance_status: "fail",
spec_compliance_output: "Agent Plugins v1.0.0 manifest errors",
vally_lint_status: "pass",
smoke_status: "pass",
});

assert.equal(result.intakeState, "requires-submitter-fixes");
assert.match(result.commentBody, /intake failed \(submitter fixes required\)/);
assert.match(result.commentBody, /\| spec compliance \| 🛑 fail \|/);
assert.doesNotMatch(result.commentBody, /non-blocking|passed with spec warnings/);
});

test("applyQualityGateResult advances successful quality checks", () => {
const result = applyQualityGateResult(validBaseEvaluation, {
overall_status: "pass",
failure_class: "none",
spec_compliance_status: "pass",
vally_lint_status: "pass",
smoke_status: "pass",
});

assert.equal(result.intakeState, "ready-for-review");
assert.match(result.commentBody, /intake passed/);
});

for (const quality of [undefined, {}, { overall_status: "not_run" }, {
overall_status: "infra_error",
failure_class: "infra",
spec_compliance_status: "infra_error",
}]) {
test(`applyQualityGateResult keeps incomplete checks awaiting review (${JSON.stringify(quality)})`, () => {
const result = applyQualityGateResult(validBaseEvaluation, quality);
assert.equal(result.intakeState, "awaiting-review");
assert.match(result.commentBody, /quality checks could not complete/);
assert.doesNotMatch(result.commentBody, /intake passed/);
});
}

test("applyQualityGateResult links the full quality log artifact", () => {
const artifactUrl = "https://github.com/github/awesome-copilot/actions/runs/123/artifacts/456";
const result = applyQualityGateResult(
Expand Down
22 changes: 16 additions & 6 deletions eng/external-plugin-pr-quality-gates.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -41,25 +41,34 @@ export function buildSourceTreeUrl(plugin) {
return `https://github.com/${sourceRepo}/tree/${encodedLocator}/${encodedPath}`;
}

function aggregateResultStatus(pluginResults) {
if (pluginResults.some((entry) => entry.quality?.overall_status === "fail")) {
export function aggregateResultStatus(pluginResults) {
if (pluginResults.length === 0) {
return {
overallStatus: "not_run",
failureClass: "none",
};
}

const statuses = pluginResults.map((entry) => entry.quality?.overall_status);
if (statuses.includes("fail")) {
return {
overallStatus: "fail",
failureClass: "submitter_fixes",
};
}

if (pluginResults.some((entry) => entry.quality?.overall_status === "infra_error")) {
if (statuses.includes("infra_error")) {
return {
overallStatus: "infra_error",
failureClass: "infra",
};
}

if (pluginResults.length === 0) {
// Every changed plugin must fully pass; anything else (warnings, not_run, unknown) blocks.
if (statuses.some((status) => status !== "pass")) {
return {
overallStatus: "not_run",
failureClass: "none",
overallStatus: "fail",
failureClass: "submitter_fixes",
};
}

Expand All @@ -73,6 +82,7 @@ function createValidationFailureQuality(errors) {
const output = errors.map((error) => `- ${error}`).join("\n");
return {
overall_status: "fail",
spec_compliance_status: "not_run",
vally_lint_status: "fail",
smoke_status: "not_run",
version_match_status: "not_run",
Expand Down
Loading
Loading