diff --git a/.github/workflows/external-plugin-command-router.yml b/.github/workflows/external-plugin-command-router.yml index 7b79d22b..8bce714f 100644 --- a/.github/workflows/external-plugin-command-router.yml +++ b/.github/workflows/external-plugin-command-router.yml @@ -767,6 +767,7 @@ jobs: BASE_RESULT_JSON: ${{ needs.rerun-intake-parse.outputs.base-result }} BASE_VALID: ${{ needs.rerun-intake-parse.outputs.valid }} QUALITY_RESULT_JSON: ${{ needs.rerun-intake-quality-gates.outputs.quality-result }} + QUALITY_LOG_URL: ${{ needs.rerun-intake-quality-gates.outputs.quality-log-url }} QUALITY_JOB_RESULT: ${{ needs.rerun-intake-quality-gates.result }} ISSUE_STATE: ${{ needs.rerun-intake-parse.outputs.issue-state }} ISSUE_LABELS: ${{ needs.rerun-intake-parse.outputs.issue-labels }} @@ -809,7 +810,14 @@ jobs: }; } - finalResult = intake.applyQualityGateResult(baseResult, qualityResult, context.runId, context.repo.owner, context.repo.repo); + finalResult = intake.applyQualityGateResult( + baseResult, + qualityResult, + context.runId, + context.repo.owner, + context.repo.repo, + process.env.QUALITY_LOG_URL + ); } await intakeState.applyExternalPluginIntakeEvaluation({ diff --git a/.github/workflows/external-plugin-intake.yml b/.github/workflows/external-plugin-intake.yml index de664028..a8066153 100644 --- a/.github/workflows/external-plugin-intake.yml +++ b/.github/workflows/external-plugin-intake.yml @@ -94,6 +94,7 @@ jobs: BASE_RESULT_JSON: ${{ needs.evaluate-submission.outputs.evaluation }} BASE_VALID: ${{ needs.evaluate-submission.outputs.valid }} QUALITY_RESULT_JSON: ${{ needs.quality-gates.outputs.quality-result }} + QUALITY_LOG_URL: ${{ needs.quality-gates.outputs.quality-log-url }} QUALITY_JOB_RESULT: ${{ needs.quality-gates.result }} ISSUE_STATE: ${{ needs.evaluate-submission.outputs.issue-state }} ISSUE_LABELS: ${{ needs.evaluate-submission.outputs.issue-labels }} @@ -136,7 +137,14 @@ jobs: }; } - finalResult = intake.applyQualityGateResult(baseResult, qualityResult, context.runId, context.repo.owner, context.repo.repo); + finalResult = intake.applyQualityGateResult( + baseResult, + qualityResult, + context.runId, + context.repo.owner, + context.repo.repo, + process.env.QUALITY_LOG_URL + ); } await intakeState.applyExternalPluginIntakeEvaluation({ diff --git a/.github/workflows/external-plugin-pr-quality-gates-writer.yml b/.github/workflows/external-plugin-pr-quality-gates-writer.yml index 8e5017d1..a46d541d 100644 --- a/.github/workflows/external-plugin-pr-quality-gates-writer.yml +++ b/.github/workflows/external-plugin-pr-quality-gates-writer.yml @@ -143,6 +143,18 @@ jobs: const intakeState = await import(pathToFileURL(path.join(process.env.GITHUB_WORKSPACE, 'eng', 'external-plugin-intake-state.mjs')).href); const marker = ''; + const artifacts = await github.paginate(github.rest.actions.listWorkflowRunArtifacts, { + owner: context.repo.owner, + repo: context.repo.repo, + run_id: workflowRun.id, + per_page: 100, + }); + const qualityLogArtifact = artifacts.find((artifact) => + artifact.name === 'external-plugin-pr-quality-logs' && !artifact.expired + ); + const qualityLogUrl = qualityLogArtifact + ? `https://github.com/${context.repo.owner}/${context.repo.repo}/actions/runs/${workflowRun.id}/artifacts/${qualityLogArtifact.id}` + : ''; const detectJobResult = payload.detect_job_result; const shouldRun = payload.should_run; const changedCount = payload.changed_count; @@ -345,6 +357,7 @@ jobs: '', `- **Changed entries detected:** ${changedCount}`, `- **Workflow state label:** \`${stateLabel}\``, + qualityLogUrl ? `- **Full logs:** [Download quality gate artifact](${qualityLogUrl})` : '', '- **Status legend:** ✅ pass · ⚠️ warning · 🛑 fail', '', '### Per-plugin quality summary', diff --git a/.github/workflows/external-plugin-pr-quality-gates.yml b/.github/workflows/external-plugin-pr-quality-gates.yml index f92f4358..b99c8c0b 100644 --- a/.github/workflows/external-plugin-pr-quality-gates.yml +++ b/.github/workflows/external-plugin-pr-quality-gates.yml @@ -117,13 +117,24 @@ jobs: env: CHANGED_PLUGINS_JSON: ${{ needs.detect-changed-plugins.outputs.changed-plugins }} run: | - result=$(node ./eng/external-plugin-pr-quality-gates.mjs --plugins-json "$CHANGED_PLUGINS_JSON") + result=$(node ./eng/external-plugin-pr-quality-gates.mjs \ + --plugins-json "$CHANGED_PLUGINS_JSON" \ + --logs-directory "${RUNNER_TEMP}/external-plugin-pr-quality-logs") { echo 'quality-result<> "$GITHUB_OUTPUT" + - name: Upload full quality gate logs + if: always() + uses: actions/upload-artifact@043fb46d1a93c77aae656e7c1c64a875d1fc6a0a # v7.0.1 + with: + name: external-plugin-pr-quality-logs + path: ${{ runner.temp }}/external-plugin-pr-quality-logs + if-no-files-found: warn + retention-days: 14 + publish-quality-result: runs-on: ubuntu-latest needs: [detect-changed-plugins, run-quality-gates] diff --git a/.github/workflows/external-plugin-quality-gates.yml b/.github/workflows/external-plugin-quality-gates.yml index 559b0c5f..77c0ccdc 100644 --- a/.github/workflows/external-plugin-quality-gates.yml +++ b/.github/workflows/external-plugin-quality-gates.yml @@ -11,6 +11,9 @@ on: quality-result: description: JSON result for quality checks value: ${{ jobs.quality.outputs.quality-result }} + quality-log-url: + description: URL for the full quality gate log artifact + value: ${{ jobs.quality.outputs.quality-log-url }} permissions: contents: read @@ -20,6 +23,7 @@ jobs: runs-on: ubuntu-latest outputs: quality-result: ${{ steps.quality.outputs.quality-result }} + quality-log-url: ${{ steps.upload-logs.outputs.artifact-url }} steps: - name: Checkout main branch uses: actions/checkout@34e114876b0b11c390a56381ad16ebd13914f8d5 # v4.3.1 @@ -44,9 +48,21 @@ jobs: env: PLUGIN_JSON: ${{ inputs.plugin-json }} run: | - result=$(node ./eng/external-plugin-quality-gates.mjs --plugin-json "$PLUGIN_JSON") + result=$(node ./eng/external-plugin-quality-gates.mjs \ + --plugin-json "$PLUGIN_JSON" \ + --log-file "${RUNNER_TEMP}/external-plugin-quality-logs/quality-gates.log") { echo 'quality-result<> "$GITHUB_OUTPUT" + + - name: Upload full quality gate logs + id: upload-logs + if: always() + uses: actions/upload-artifact@043fb46d1a93c77aae656e7c1c64a875d1fc6a0a # v7.0.1 + with: + name: external-plugin-quality-logs + path: ${{ runner.temp }}/external-plugin-quality-logs + if-no-files-found: warn + retention-days: 14 diff --git a/eng/external-plugin-intake.mjs b/eng/external-plugin-intake.mjs index 3e04f428..f6bf5743 100644 --- a/eng/external-plugin-intake.mjs +++ b/eng/external-plugin-intake.mjs @@ -1242,7 +1242,7 @@ function getIntakeStateFromQualityResult(baseResult, qualityResult) { return "ready-for-review"; } -function buildMergedIntakeComment(baseResult, qualityResult, runId, owner, repo) { +function buildMergedIntakeComment(baseResult, qualityResult, runId, owner, repo, qualityLogUrl) { if (!baseResult.valid) { return baseResult.commentBody; } @@ -1250,6 +1250,9 @@ function buildMergedIntakeComment(baseResult, qualityResult, runId, owner, repo) const marker = baseResult.commentMarker ?? EXTERNAL_PLUGIN_INTAKE_COMMENT_MARKER; const qualitySection = buildQualityGatesCommentSection(qualityResult); const runLink = runId && owner && repo ? `_[View workflow run](https://github.com/${owner}/${repo}/actions/runs/${runId})_` : ""; + const logLink = /^https:\/\/github\.com\/[^/\s]+\/[^/\s]+\/actions\/runs\/\d+\/artifacts\/\d+$/.test(String(qualityLogUrl || "")) + ? `_[Download full quality gate logs](${qualityLogUrl})_` + : ""; const hasSpecWarnings = String(qualityResult.spec_compliance_status || "") === "warning"; const intro = @@ -1276,6 +1279,8 @@ function buildMergedIntakeComment(baseResult, qualityResult, runId, owner, repo) "", statusLine, "", + [runLink, logLink].filter(Boolean).join(" · "), + "", `- **Plugin:** ${baseResult.plugin?.name ?? "unknown"}`, `- **Repository:** ${baseResult.plugin?.repository ?? "unknown"}`, baseResult.plugin?.source?.ref ? `- **Ref:** [\`${baseResult.plugin.source.ref.replaceAll('\`', '\\\`')}\`](https://github.com/${encodeRepoPath(baseResult.plugin.source.repo)}/tree/${encodeURIComponent(baseResult.plugin.source.ref).replaceAll("%2F", "/")})` : undefined, @@ -1295,11 +1300,10 @@ function buildMergedIntakeComment(baseResult, qualityResult, runId, owner, repo) baseResult.warnings?.length ? ["", "### Warnings", "", ...baseResult.warnings.map((warning) => `- ${warning}`)].join("\n") : "", - runLink ? `\n${runLink}` : "", ].join("\n"); } -export function applyQualityGateResult(baseEvaluation, qualityGateResult, runId, owner, repo) { +export function applyQualityGateResult(baseEvaluation, qualityGateResult, runId, owner, repo, qualityLogUrl) { const baseResult = typeof baseEvaluation === "string" ? JSON.parse(baseEvaluation) : baseEvaluation; const qualityResult = normalizeQualityGateResult( typeof qualityGateResult === "string" ? JSON.parse(qualityGateResult) : qualityGateResult, @@ -1310,7 +1314,7 @@ export function applyQualityGateResult(baseEvaluation, qualityGateResult, runId, ...baseResult, qualityGates: qualityResult, intakeState, - commentBody: buildMergedIntakeComment(baseResult, qualityResult, runId, owner, repo), + commentBody: buildMergedIntakeComment(baseResult, qualityResult, runId, owner, repo, qualityLogUrl), }; } diff --git a/eng/external-plugin-intake.test.mjs b/eng/external-plugin-intake.test.mjs index b55b6956..66f3ee4a 100644 --- a/eng/external-plugin-intake.test.mjs +++ b/eng/external-plugin-intake.test.mjs @@ -2,6 +2,7 @@ import assert from "node:assert/strict"; import http from "node:http"; import { afterEach, test } from "node:test"; import { + applyQualityGateResult, evaluateExternalPluginIssue, PinnedAddressDispatcher, validateCanvasPluginMetadata, @@ -15,6 +16,34 @@ const TREE_PLUGINS = "tree-plugins"; const TREE_UPGRADE_AGENT = "tree-upgrade-agent"; const TREE_EXTENSIONS = "tree-extensions"; +test("applyQualityGateResult links the full quality log artifact", () => { + const artifactUrl = "https://github.com/github/awesome-copilot/actions/runs/123/artifacts/456"; + const result = applyQualityGateResult( + { + valid: true, + plugin: { + name: "example-plugin", + repository: "https://github.com/example/plugin", + source: { repo: "example/plugin", ref: "v1.0.0" }, + }, + warnings: [], + }, + { + overall_status: "fail", + failure_class: "submitter_fixes", + vally_lint_status: "fail", + vally_lint_output: "...output truncated...", + }, + 123, + "github", + "awesome-copilot", + artifactUrl, + ); + + assert.match(result.commentBody, /Download full quality gate logs/); + assert.match(result.commentBody, new RegExp(artifactUrl.replace(/[.*+?^${}()|[\]\\]/g, "\\$&"))); +}); + function fileNode(content) { return { type: "file", content: Buffer.from(content, "utf8").toString("base64") }; } diff --git a/eng/external-plugin-pr-quality-gates.mjs b/eng/external-plugin-pr-quality-gates.mjs index 1c26b7a2..50d19347 100644 --- a/eng/external-plugin-pr-quality-gates.mjs +++ b/eng/external-plugin-pr-quality-gates.mjs @@ -1,5 +1,7 @@ #!/usr/bin/env node +import fs from "fs"; +import path from "path"; import { runExternalPluginQualityGates } from "./external-plugin-quality-gates.mjs"; import { validateExternalPlugin } from "./external-plugin-validation.mjs"; @@ -86,16 +88,52 @@ function createValidationFailureQuality(errors) { }; } -export async function runExternalPluginPrQualityGates(plugins) { +function createLogFilePath(logsDirectory, pluginName, index) { + if (!logsDirectory) { + return undefined; + } + + const safeName = String(pluginName || "unknown") + .toLowerCase() + .replace(/[^a-z0-9.-]+/g, "-") + .replace(/^-+|-+$/g, "") + .slice(0, 64) || "unknown"; + return path.join(logsDirectory, `${String(index + 1).padStart(2, "0")}-${safeName}.log`); +} + +function writeValidationFailureLog(logFile, plugin, quality) { + if (!logFile) { + return; + } + + fs.mkdirSync(path.dirname(logFile), { recursive: true }); + fs.writeFileSync( + logFile, + [ + `External plugin quality gate log: ${plugin?.name || "unknown"}`, + "", + "External plugin entry validation", + quality.vally_lint_output || "No output captured.", + "", + ].join("\n"), + ); +} + +export async function runExternalPluginPrQualityGates(plugins, { logsDirectory } = {}) { if (!Array.isArray(plugins)) { throw new Error("plugins must be an array"); } - const checkedPlugins = await Promise.all(plugins.map(async (plugin) => { + const checkedPlugins = await Promise.all(plugins.map(async (plugin, index) => { const validation = validateExternalPlugin(plugin, "changed-plugin", { policy: "marketplace" }); - const quality = validation.errors.length > 0 - ? createValidationFailureQuality(validation.errors) - : await runExternalPluginQualityGates(plugin); + const logFile = createLogFilePath(logsDirectory, plugin?.name, index); + let quality; + if (validation.errors.length > 0) { + quality = createValidationFailureQuality(validation.errors); + writeValidationFailureLog(logFile, plugin, quality); + } else { + quality = await runExternalPluginQualityGates(plugin, { logFile }); + } return { name: plugin?.name ?? "unknown", source: plugin?.source ?? {}, @@ -143,6 +181,8 @@ if (import.meta.url === `file://${process.argv[1]}`) { } const plugins = JSON.parse(args["plugins-json"]); - const result = await runExternalPluginPrQualityGates(plugins); + const result = await runExternalPluginPrQualityGates(plugins, { + logsDirectory: args["logs-directory"], + }); process.stdout.write(`${JSON.stringify(result)}\n`); } diff --git a/eng/external-plugin-quality-gates.mjs b/eng/external-plugin-quality-gates.mjs index f0d641a5..3d56b892 100644 --- a/eng/external-plugin-quality-gates.mjs +++ b/eng/external-plugin-quality-gates.mjs @@ -47,8 +47,12 @@ const INFRA_ERROR_PATTERNS = [ /etimedout/, ]; +function normalizeOutput(value) { + return String(value ?? "").replace(/\x1b\[[0-9;]*m/g, "").trim(); +} + function truncateOutput(value) { - const normalized = String(value ?? "").replace(/\x1b\[[0-9;]*m/g, "").trim(); + const normalized = normalizeOutput(value); if (normalized.length <= MAX_OUTPUT_LENGTH) { return normalized; } @@ -366,18 +370,46 @@ async function runVallyLintGate(pluginRoot) { } } + const logOutput = normalizeOutput(combinedOutput); return { status: anyFailure ? "fail" : "pass", - output: truncateOutput(combinedOutput), + output: truncateOutput(logOutput), + logOutput, }; } catch (error) { + const logOutput = normalizeOutput(error.stack || error.message); return { status: "infra_error", - output: truncateOutput(error.message), + output: truncateOutput(logOutput), + logOutput, }; } } +export function formatQualityGateLog(plugin, result, { vallyLintOutput } = {}) { + const pluginName = String(plugin?.name || "unknown"); + const sections = [ + `External plugin quality gate log: ${pluginName}`, + "", + "Summary", + String(result.summary || "No summary provided."), + ]; + const gateOutputs = [ + ["Spec compliance", result.spec_compliance_output], + ["Vally lint", vallyLintOutput ?? result.vally_lint_output], + ["Install smoke test", result.smoke_output], + ["Version match", result.version_match_output], + ["Ref/SHA consistency", result.ref_sha_consistency_output], + ["Canvas structure", result.canvas_structure_output], + ]; + + for (const [heading, output] of gateOutputs) { + sections.push("", heading, normalizeOutput(output) || "No output captured."); + } + + return `${sections.join("\n")}\n`; +} + function buildEphemeralMarketplace(workDir, plugin) { const marketplaceDir = path.join(workDir, "marketplace"); ensureDirectory(marketplaceDir); @@ -952,8 +984,9 @@ function toFailureClass(overallStatus) { return "none"; } -export async function runExternalPluginQualityGates(plugin) { +export async function runExternalPluginQualityGates(plugin, { logFile } = {}) { const workDir = fs.mkdtempSync(path.join(os.tmpdir(), "external-plugin-quality-")); + let vallyLintLogOutput = ""; const result = { overall_status: "not_run", vally_lint_status: "not_run", @@ -1014,6 +1047,7 @@ export async function runExternalPluginQualityGates(plugin) { const vallyResult = await runVallyLintGate(pluginRoot); result.vally_lint_status = vallyResult.status; result.vally_lint_output = vallyResult.output; + vallyLintLogOutput = vallyResult.logOutput; const smokeResult = runInstallSmokeGate(workDir, plugin); result.smoke_status = smokeResult.status; @@ -1043,8 +1077,16 @@ export async function runExternalPluginQualityGates(plugin) { result.failure_class = "infra"; result.summary = truncateOutput(error.message); result.vally_lint_output = truncateOutput(error.stack || error.message); + vallyLintLogOutput = normalizeOutput(error.stack || error.message); return result; } finally { + if (logFile) { + fs.mkdirSync(path.dirname(logFile), { recursive: true }); + fs.writeFileSync( + logFile, + formatQualityGateLog(plugin, result, { vallyLintOutput: vallyLintLogOutput }), + ); + } fs.rmSync(workDir, { recursive: true, force: true }); } } @@ -1071,6 +1113,6 @@ if (import.meta.url === `file://${process.argv[1]}`) { } const plugin = JSON.parse(args["plugin-json"]); - const result = await runExternalPluginQualityGates(plugin); + const result = await runExternalPluginQualityGates(plugin, { logFile: args["log-file"] }); process.stdout.write(`${JSON.stringify(result)}\n`); } diff --git a/eng/external-plugin-quality-gates.test.mjs b/eng/external-plugin-quality-gates.test.mjs index cad43a88..cc6e706b 100644 --- a/eng/external-plugin-quality-gates.test.mjs +++ b/eng/external-plugin-quality-gates.test.mjs @@ -4,7 +4,13 @@ import os from "os"; import path from "path"; import { spawnSync } from "child_process"; import { after, test } from "node:test"; -import { runCanvasStructureGate, runRefShaConsistencyGate, runVersionMatchGate } from "./external-plugin-quality-gates.mjs"; +import { + formatQualityGateLog, + runCanvasStructureGate, + runRefShaConsistencyGate, + runVersionMatchGate, +} from "./external-plugin-quality-gates.mjs"; +import { runExternalPluginPrQualityGates } from "./external-plugin-pr-quality-gates.mjs"; const tempDirs = []; @@ -38,6 +44,46 @@ function commitAll(repoDir, message) { return runGit(repoDir, "rev-parse", "HEAD"); } +test("formatQualityGateLog preserves full vally output for artifacts", () => { + const fullVallyOutput = `lint failure\n${"x".repeat(13000)}`; + const log = formatQualityGateLog( + { name: "example-plugin" }, + { + summary: "- vally lint: fail", + spec_compliance_output: "spec output", + vally_lint_output: "truncated output", + smoke_output: "smoke output", + version_match_output: "version output", + ref_sha_consistency_output: "ref output", + canvas_structure_output: "canvas output", + }, + { vallyLintOutput: fullVallyOutput }, + ); + + assert.match(log, /External plugin quality gate log: example-plugin/); + assert.match(log, /lint failure/); + assert.equal(log.includes("x".repeat(13000)), true); + assert.equal(log.includes("truncated output"), false); +}); + +test("runExternalPluginPrQualityGates writes logs for validation failures", async () => { + const logsDirectory = fs.mkdtempSync(path.join(os.tmpdir(), "external-plugin-pr-logs-")); + tempDirs.push(logsDirectory); + + const result = await runExternalPluginPrQualityGates( + [{ name: "Invalid Plugin", source: {} }], + { logsDirectory }, + ); + + assert.equal(result.overall_status, "fail"); + const logFiles = fs.readdirSync(logsDirectory); + assert.deepEqual(logFiles, ["01-invalid-plugin.log"]); + assert.match( + fs.readFileSync(path.join(logsDirectory, logFiles[0]), "utf8"), + /External plugin entry validation/, + ); +}); + test("runCanvasStructureGate passes when a named extension exists", () => { const repoDir = createTempRepo(); fs.mkdirSync(path.join(repoDir, "com.github.copilot", "extensions", "canvas-plugin"), { recursive: true });