Enforce external plugin ref/sha consistency (#2463)

* Enforce external plugin ref/sha consistency

Extract shared ref/sha normalization and consistency checks into eng/lib and reuse them in intake plus quality gate flows.

Add a dedicated ref/sha consistency quality gate surfaced in PR/intake summaries, and add targeted tests for matching and mismatched refs.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

Copilot-Session: 6afe21ad-eafa-4c90-a1f2-053dedac7625

* Address review: tree/blob ref errors and PR workflow ref/sha column

- resolveCommitShaAtReadRef: classify rev-parse failure as 'fail'
  instead of 'infra_error' because a successfully-fetched ref that
  doesn't dereference to a commit is a submitter problem, not infra.
- validateRemoteRepository (intake): treat HTTP 422 from the commit
  endpoint as a submitter error; all other non-404 errors remain
  transient warnings requiring maintainer re-run.
- external-plugin-pr-quality-gates.yml: add ref/sha consistency
  column to the per-plugin quality table and failure details block.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 6afe21ad-eafa-4c90-a1f2-053dedac7625

---------

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 6afe21ad-eafa-4c90-a1f2-053dedac7625

Author: aaronpowell

.github/workflows/external-plugin-pr-quality-gates.yml

@@ -279,17 +279,19 @@ jobs:
                   const vallyLintStatus = escapeMarkdownTableCell(quality.vally_lint_status || 'not_run');
                   const smokeStatus = escapeMarkdownTableCell(quality.smoke_status || 'not_run');
                   const versionMatchStatus = escapeMarkdownTableCell(quality.version_match_status || 'not_run');
+                  const refShaConsistencyStatus = escapeMarkdownTableCell(quality.ref_sha_consistency_status || 'not_run');
                   const canvasStructureStatus = escapeMarkdownTableCell(quality.canvas_structure_status || 'not_run');
                   const overallStatus = escapeMarkdownTableCell(quality.overall_status || 'not_run');
-                  return `| ${name} | ${vallyLintStatus} | ${smokeStatus} | ${versionMatchStatus} | ${canvasStructureStatus} | ${overallStatus} | ${sourceCell} |`;
+                  return `| ${name} | ${vallyLintStatus} | ${smokeStatus} | ${versionMatchStatus} | ${refShaConsistencyStatus} | ${canvasStructureStatus} | ${overallStatus} | ${sourceCell} |`;
                 })
-              : ['| _none_ | not_run | not_run | not_run | not_run | not_run | _n/a_ |'];
+              : ['| _none_ | not_run | not_run | not_run | not_run | not_run | not_run | _n/a_ |'];
             const failureDetails = checkedPlugins.flatMap((entry) => {
               const name = String(entry?.name || 'unknown');
               const quality = entry?.quality || {};
               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;
+              const shouldShowRefShaConsistency = quality.ref_sha_consistency_status === 'fail' || quality.ref_sha_consistency_status === 'infra_error' || String(quality.ref_sha_consistency_output || '').trim().length > 0;
               const shouldShowCanvasStructure = quality.canvas_structure_status === 'fail' || quality.canvas_structure_status === 'infra_error' || String(quality.canvas_structure_output || '').trim().length > 0;
 
               const details = [];
@@ -302,6 +304,9 @@ jobs:
               if (shouldShowVersionMatch) {
                 details.push(formatGateOutput(name, 'version match', quality.version_match_status, quality.version_match_output));
               }
+              if (shouldShowRefShaConsistency) {
+                details.push(formatGateOutput(name, 'ref/sha consistency', quality.ref_sha_consistency_status, quality.ref_sha_consistency_output));
+              }
               if (shouldShowCanvasStructure) {
                 details.push(formatGateOutput(name, 'canvas structure', quality.canvas_structure_status, quality.canvas_structure_output));
               }
@@ -317,8 +322,8 @@ jobs:
               '',
               '### Per-plugin quality summary',
               '',
-              '| Plugin | vally lint | install smoke test | version match | canvas structure | overall | source tree |',
-              '|---|---|---|---|---|---|---|',
+              '| Plugin | vally lint | install smoke test | version match | ref/sha consistency | canvas structure | overall | source tree |',
+              '|---|---|---|---|---|---|---|---|',
               ...rows,
               '',
               ...(failureDetails.length > 0

eng/external-plugin-intake.mjs

@@ -5,6 +5,7 @@ import path from "path";
 import { fileURLToPath } from "url";
 import { ROOT_FOLDER } from "./constants.mjs";
 import { readExternalPlugins, validateExternalPlugin } from "./external-plugin-validation.mjs";
+import { evaluateRefShaConsistency, normalizeCommitSha } from "./lib/external-plugin-source-ref-sha.mjs";
 
 export const ISSUE_FORM_MARKER = "<!-- external-plugin-submission -->";
 export const EXTERNAL_PLUGIN_INTAKE_COMMENT_MARKER = "<!-- external-plugin-intake -->";
@@ -293,9 +294,23 @@ function encodeRepoPath(repo) {
   return `${encodeURIComponent(owner ?? "")}/${encodeURIComponent(name ?? "")}`;
 }
 
+async function resolveCommitSha(repo, locator, token) {
+  const encodedRepo = encodeRepoPath(repo);
+  const commitResponse = await fetchGitHubJson(`/repos/${encodedRepo}/commits/${encodeURIComponent(locator)}`, token);
+  if (commitResponse.kind !== "found") {
+    return commitResponse;
+  }
+
+  return {
+    ...commitResponse,
+    commitSha: normalizeCommitSha(commitResponse.data?.sha),
+  };
+}
+
 async function validateRemoteRepository(repo, { ref, sha }, errors, warnings, token) {
   const encodedRepo = encodeRepoPath(repo);
   const repositoryResponse = await fetchGitHubJson(`/repos/${encodedRepo}`, token);
+  const normalizedSha = normalizeCommitSha(sha);
 
   if (repositoryResponse.kind === "notFound") {
     errors.push(`submission: GitHub repository "${repo}" was not found`);
@@ -333,6 +348,19 @@ async function validateRemoteRepository(repo, { ref, sha }, errors, warnings, to
 
   }
 
+  function validateRefShaConsistency(refCommitSha) {
+    if (!normalizedSha || !refCommitSha) {
+      return;
+    }
+
+    const consistency = evaluateRefShaConsistency({ ref, sha, resolvedRefCommitSha: refCommitSha });
+    if (!consistency.matches) {
+      errors.push(
+        `submission: when both "Ref to review" and "Commit SHA to review" are provided, they must reference the same commit (ref "${ref}" resolves to "${consistency.normalizedRefCommitSha}", sha is "${sha}")`,
+      );
+    }
+  }
+
   if (!ref) {
     return;
   }
@@ -347,6 +375,8 @@ async function validateRemoteRepository(repo, { ref, sha }, errors, warnings, to
         `submission: could not verify commit "${ref}" in GitHub repository "${repo}" (${statusText}${commitResponse.reason ? ` — ${commitResponse.reason}` : ""}); a maintainer should re-run intake`,
       );
     }
+
+    validateRefShaConsistency(normalizeCommitSha(ref));
     return;
   }
 
@@ -362,6 +392,38 @@ async function validateRemoteRepository(repo, { ref, sha }, errors, warnings, to
   const tagResponse = await fetchGitHubJson(`/repos/${encodedRepo}/git/ref/tags/${encodeURIComponent(tagName)}`, token);
 
   if (tagResponse.kind === "found") {
+    if (!normalizedSha) {
+      return;
+    }
+
+    const resolvedRefResponse = await resolveCommitSha(repo, ref, token);
+    if (resolvedRefResponse.kind === "notFound") {
+      errors.push(`submission: ref "${ref}" could not be resolved to a commit in GitHub repository "${repo}"`);
+      return;
+    }
+
+    if (resolvedRefResponse.kind === "apiError") {
+      if (resolvedRefResponse.status === 422) {
+        errors.push(
+          `submission: ref "${ref}" does not resolve to a commit in GitHub repository "${repo}" (it may point to a tag object, tree, or blob); only commit-backed refs are supported`,
+        );
+        return;
+      }
+      const statusText = resolvedRefResponse.status ? `HTTP ${resolvedRefResponse.status}` : "network error";
+      warnings.push(
+        `submission: could not resolve ref "${ref}" to a commit in GitHub repository "${repo}" (${statusText}${resolvedRefResponse.reason ? ` — ${resolvedRefResponse.reason}` : ""}); a maintainer should re-run intake`,
+      );
+      return;
+    }
+
+    if (!resolvedRefResponse.commitSha) {
+      warnings.push(
+        `submission: could not determine the commit SHA for ref "${ref}" in GitHub repository "${repo}"; a maintainer should re-run intake`,
+      );
+      return;
+    }
+
+    validateRefShaConsistency(resolvedRefResponse.commitSha);
     return;
   }
 
@@ -724,12 +786,14 @@ function normalizeQualityGateResult(rawResult) {
     vally_lint_status: "not_run",
     smoke_status: "not_run",
     version_match_status: "not_run",
+    ref_sha_consistency_status: "not_run",
     canvas_structure_status: "not_run",
     failure_class: "none",
     summary: "",
     vally_lint_output: "",
     smoke_output: "",
     version_match_output: "",
+    ref_sha_consistency_output: "",
     canvas_structure_output: "",
   };
 
@@ -747,6 +811,7 @@ function buildQualityGatesCommentSection(qualityResult) {
   const vallyState = qualityResult.vally_lint_status || "not_run";
   const smokeState = qualityResult.smoke_status || "not_run";
   const versionMatchState = qualityResult.version_match_status || "not_run";
+  const refShaConsistencyState = qualityResult.ref_sha_consistency_status || "not_run";
   const canvasStructureState = qualityResult.canvas_structure_status || "not_run";
   const summaryText = String(qualityResult.summary || "").trim() || "_No quality gate details were provided._";
 
@@ -758,6 +823,7 @@ function buildQualityGatesCommentSection(qualityResult) {
     `| vally lint | ${vallyState} |`,
     `| install smoke test | ${smokeState} |`,
     `| version match | ${versionMatchState} |`,
+    `| ref/sha consistency | ${refShaConsistencyState} |`,
     `| canvas structure | ${canvasStructureState} |`,
     "",
     summaryText,
@@ -808,6 +874,21 @@ function buildQualityGatesCommentSection(qualityResult) {
     );
   }
 
+  const refShaConsistencyOutput = String(qualityResult.ref_sha_consistency_output || "").trim();
+  if (refShaConsistencyOutput) {
+    sections.push(
+      "",
+      "<details>",
+      "<summary>Ref/SHA consistency output</summary>",
+      "",
+      "```text",
+      refShaConsistencyOutput,
+      "```",
+      "",
+      "</details>",
+    );
+  }
+
   const canvasStructureOutput = String(qualityResult.canvas_structure_output || "").trim();
   if (canvasStructureOutput) {
     sections.push(

eng/external-plugin-intake.test.mjs

@@ -1,6 +1,6 @@
 import assert from "node:assert/strict";
-import { test } from "node:test";
-import { validateCanvasPluginMetadata } from "./external-plugin-intake.mjs";
+import { afterEach, test } from "node:test";
+import { evaluateExternalPluginIssue, validateCanvasPluginMetadata } from "./external-plugin-intake.mjs";
 
 const REPO = "owner/repo";
 const SHA = "0123456789abcdef0123456789abcdef01234567";
@@ -258,3 +258,146 @@ test("validateCanvasPluginMetadata accepts an entry point found within a truncat
   assert.deepEqual(errors, []);
   assert.deepEqual(warnings, []);
 });
+
+// ---------------------------------------------------------------------------
+// ref/sha consistency tests (evaluateExternalPluginIssue)
+// ---------------------------------------------------------------------------
+
+const ORIGINAL_FETCH = global.fetch;
+const INTAKE_REPO = "octo/example";
+const RESOLVED_REF_SHA = "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa";
+const PROVIDED_SHA = "bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb";
+
+afterEach(() => {
+  global.fetch = ORIGINAL_FETCH;
+});
+
+function buildIssueBody({ ref, sha }) {
+  return [
+    "<!-- external-plugin-submission -->",
+    "### Plugin name",
+    "",
+    "intake-ref-sha-consistency-test-plugin",
+    "",
+    "### Short description",
+    "",
+    "Test plugin for external intake validation.",
+    "",
+    "### GitHub repository",
+    "",
+    INTAKE_REPO,
+    "",
+    "### Plugin path inside the repository",
+    "",
+    "_No response_",
+    "",
+    "### Ref to review",
+    "",
+    ref,
+    "",
+    "### Commit SHA to review",
+    "",
+    sha,
+    "",
+    "### Version",
+    "",
+    "1.2.3",
+    "",
+    "### License identifier",
+    "",
+    "MIT",
+    "",
+    "### Author name",
+    "",
+    "Copilot Test",
+    "",
+    "### Author URL",
+    "",
+    "_No response_",
+    "",
+    "### Homepage URL",
+    "",
+    "_No response_",
+    "",
+    "### Keywords",
+    "",
+    "testing",
+    "",
+    "### Additional notes for reviewers",
+    "",
+    "_No response_",
+    "",
+    "### Submission checklist",
+    "",
+    "- [x] The plugin lives in a public GitHub repository.",
+    "- [x] The ref and/or sha I provided is immutable (release tag and/or full 40-character commit SHA), not a branch.",
+    "- [x] This submission follows this repository's contribution, security, and responsible AI policies.",
+    "- [x] This plugin is not already listed in the Awesome Copilot marketplace.",
+    "",
+  ].join("\n");
+}
+
+function jsonResponse(payload, { status = 200 } = {}) {
+  return {
+    ok: status >= 200 && status < 300,
+    status,
+    statusText: status === 404 ? "Not Found" : "OK",
+    headers: new Map(),
+    async json() {
+      return payload;
+    },
+  };
+}
+
+function installMockFetch() {
+  global.fetch = async (url) => {
+    const requestUrl = String(url);
+    if (requestUrl === `https://api.github.com/repos/${INTAKE_REPO}`) {
+      return jsonResponse({ private: false, archived: false });
+    }
+
+    if (
+      requestUrl === `https://api.github.com/repos/${INTAKE_REPO}/git/commits/${PROVIDED_SHA}` ||
+      requestUrl === `https://api.github.com/repos/${INTAKE_REPO}/git/commits/${RESOLVED_REF_SHA}`
+    ) {
+      const sha = requestUrl.endsWith(RESOLVED_REF_SHA) ? RESOLVED_REF_SHA : PROVIDED_SHA;
+      return jsonResponse({ sha });
+    }
+
+    if (requestUrl === `https://api.github.com/repos/${INTAKE_REPO}/git/ref/tags/v1.2.3`) {
+      return jsonResponse({ object: { type: "tag", sha: "cccccccccccccccccccccccccccccccccccccccc" } });
+    }
+
+    if (requestUrl === `https://api.github.com/repos/${INTAKE_REPO}/commits/v1.2.3`) {
+      return jsonResponse({ sha: RESOLVED_REF_SHA });
+    }
+
+    return jsonResponse({}, { status: 404 });
+  };
+}
+
+test("evaluateExternalPluginIssue fails when ref and sha resolve to different commits", async () => {
+  installMockFetch();
+  const issue = { body: buildIssueBody({ ref: "v1.2.3", sha: PROVIDED_SHA }) };
+
+  const result = await evaluateExternalPluginIssue({ issue });
+
+  assert.equal(result.valid, false);
+  assert.match(
+    result.commentBody,
+    /must reference the same commit \(ref "v1\.2\.3" resolves to "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa", sha is "bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb"\)/,
+  );
+});
+
+test("evaluateExternalPluginIssue passes when ref and sha resolve to the same commit", async () => {
+  installMockFetch();
+  const issue = { body: buildIssueBody({ ref: "v1.2.3", sha: RESOLVED_REF_SHA }) };
+
+  const result = await evaluateExternalPluginIssue({ issue });
+
+  assert.equal(result.valid, true);
+  assert.equal(
+    result.errors.some((error) => error.includes("must reference the same commit")),
+    false,
+  );
+});

eng/external-plugin-pr-quality-gates.mjs

@@ -74,12 +74,14 @@ function createValidationFailureQuality(errors) {
     vally_lint_status: "fail",
     smoke_status: "not_run",
     version_match_status: "not_run",
+    ref_sha_consistency_status: "not_run",
     canvas_structure_status: "not_run",
     failure_class: "submitter_fixes",
     summary: "Plugin entry failed external.json validation. Fix the listed errors and re-run quality checks.",
     vally_lint_output: output,
     smoke_output: "Install smoke test skipped due to external.json validation errors.",
     version_match_output: "Version match skipped due to external.json validation errors.",
+    ref_sha_consistency_output: "Ref/SHA consistency check skipped due to external.json validation errors.",
     canvas_structure_output: "Canvas structure check skipped due to external.json validation errors.",
   };
 }
@@ -107,7 +109,7 @@ export async function runExternalPluginPrQualityGates(plugins) {
     ? "No changed external plugin entries were detected in plugins/external.json."
     : checkedPlugins
       .map((entry) =>
-        `- ${entry.name}: vally-lint=${entry.quality.vally_lint_status}, install-smoke=${entry.quality.smoke_status}, version-match=${entry.quality.version_match_status}, canvas-structure=${entry.quality.canvas_structure_status}, overall=${entry.quality.overall_status}`
+        `- ${entry.name}: vally-lint=${entry.quality.vally_lint_status}, install-smoke=${entry.quality.smoke_status}, version-match=${entry.quality.version_match_status}, ref-sha-consistency=${entry.quality.ref_sha_consistency_status}, canvas-structure=${entry.quality.canvas_structure_status}, overall=${entry.quality.overall_status}`
       )
       .join("\n");