Fix external-plugin quality gates for tag-name locators (#2397) (#2399)

runVersionMatchGate and runCanvasStructureGate read locator content with git show/cat-file. For a tag-name locator, `git fetch origin <tag>` only updates FETCH_HEAD and never creates refs/tags/<tag>, so `git show <tag>:...` died with 'invalid object name' and produced a false infra_error.

Read the primary locator via HEAD (already checked out during clone) and non-primary locators via FETCH_HEAD after fetching, instead of the bare locator. This handles SHAs, short tag names, and fully-qualified tag refs uniformly without classifying the locator.

Adds regression coverage for the tag-locator path in both gates.

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

eng/external-plugin-quality-gates.mjs

@@ -330,31 +330,45 @@ function isMissingPathAtLocator(output) {
   );
 }
 
-function fetchLocatorIntoRepo(repoDir, locator) {
+// Resolves the git object a locator's content should be read from.
+//
+// A locator can be a commit SHA, a short tag name (e.g. "v1.1.247"), or a
+// fully-qualified tag ref. `git fetch origin <locator>` only updates FETCH_HEAD;
+// it does NOT create a local `refs/tags/<name>`, so `git show <tag>:...` dies with
+// "invalid object name". Reading through FETCH_HEAD (or HEAD for the already
+// checked-out primary) sidesteps that without having to classify the locator.
+function resolveLocatorReadRef(repoDir, locator, primaryFetchSpec) {
+  if (locator === primaryFetchSpec) {
+    // The primary locator was fetched and checked out at HEAD when the submission
+    // was cloned, so its content is readable via HEAD without another fetch.
+    return { status: "pass", readRef: "HEAD", output: "" };
+  }
+
   const result = runCommand("git", ["fetch", "--depth=1", "origin", locator], { cwd: repoDir });
   if (result.exitCode === 0) {
-    return {
-      status: "pass",
-      output: "",
-    };
+    // FETCH_HEAD points at whatever was just fetched. At most one non-primary
+    // locator is fetched per gate and it is read immediately below, so FETCH_HEAD
+    // is not clobbered before use.
+    return { status: "pass", readRef: "FETCH_HEAD", output: "" };
   }
 
   const status = classifySmokeFailure(result.output);
   return {
     status,
+    readRef: null,
     output: `git fetch failed for "${locator}": ${result.output}`,
   };
 }
 
-function readPluginManifestAtLocator(repoDir, locator, normalizedPluginPath) {
+function readPluginManifestAtLocator(repoDir, readRef, locator, normalizedPluginPath) {
   const manifestCandidates = PLUGIN_JSON_CANDIDATES.map((segments) =>
     toPosixPath(normalizedPluginPath, ...segments)
   );
 
   for (const manifestPath of manifestCandidates) {
-    const showResult = runCommand("git", ["show", `${locator}:${manifestPath}`], { cwd: repoDir });
+    const showResult = runCommand("git", ["show", `${readRef}:${manifestPath}`], { cwd: repoDir });
     if (showResult.exitCode === 0) {
-      const rawShow = spawnSync("git", ["show", `${locator}:${manifestPath}`], { cwd: repoDir, encoding: "utf8" });
+      const rawShow = spawnSync("git", ["show", `${readRef}:${manifestPath}`], { cwd: repoDir, encoding: "utf8" });
       const rawStdout = String(rawShow.stdout ?? "");
 
       try {
@@ -388,7 +402,7 @@ function readPluginManifestAtLocator(repoDir, locator, normalizedPluginPath) {
   };
 }
 
-function runVersionMatchGate(repoDir, plugin, primaryFetchSpec) {
+export function runVersionMatchGate(repoDir, plugin, primaryFetchSpec) {
   const expectedVersion = String(plugin?.version ?? "").trim();
   const normalizedPluginPath = normalizePluginPath(plugin?.source?.path || "/");
   const locators = [plugin?.source?.ref, plugin?.source?.sha]
@@ -408,22 +422,20 @@ function runVersionMatchGate(repoDir, plugin, primaryFetchSpec) {
   let hasInfraError = false;
 
   for (const locator of locators) {
-    if (locator !== primaryFetchSpec) {
-      const fetchResult = fetchLocatorIntoRepo(repoDir, locator);
-      if (fetchResult.status === "fail") {
-        hasFailure = true;
-        messages.push(`- ${locator}: ${fetchResult.output}`);
-        continue;
-      }
-
-      if (fetchResult.status === "infra_error") {
-        hasInfraError = true;
-        messages.push(`- ${locator}: ${fetchResult.output}`);
-        continue;
-      }
+    const refResult = resolveLocatorReadRef(repoDir, locator, primaryFetchSpec);
+    if (refResult.status === "fail") {
+      hasFailure = true;
+      messages.push(`- ${locator}: ${refResult.output}`);
+      continue;
     }
 
-    const manifestResult = readPluginManifestAtLocator(repoDir, locator, normalizedPluginPath);
+    if (refResult.status === "infra_error") {
+      hasInfraError = true;
+      messages.push(`- ${locator}: ${refResult.output}`);
+      continue;
+    }
+
+    const manifestResult = readPluginManifestAtLocator(repoDir, refResult.readRef, locator, normalizedPluginPath);
     if (manifestResult.kind === "not_found" || manifestResult.kind === "invalid") {
       hasFailure = true;
       messages.push(`- ${locator}: ${manifestResult.message}`);
@@ -474,14 +486,14 @@ function runVersionMatchGate(repoDir, plugin, primaryFetchSpec) {
   };
 }
 
-function checkPathExistsAtLocator(repoDir, locator, repoPath, expectedType) {
-  const result = runCommand("git", ["cat-file", "-e", `${locator}:${repoPath}`], { cwd: repoDir });
+function checkPathExistsAtLocator(repoDir, readRef, locator, repoPath, expectedType) {
+  const result = runCommand("git", ["cat-file", "-e", `${readRef}:${repoPath}`], { cwd: repoDir });
   if (result.exitCode === 0) {
     if (!expectedType) {
       return { exists: true, output: "" };
     }
 
-    const typeResult = runCommand("git", ["cat-file", "-t", `${locator}:${repoPath}`], { cwd: repoDir });
+    const typeResult = runCommand("git", ["cat-file", "-t", `${readRef}:${repoPath}`], { cwd: repoDir });
     if (typeResult.exitCode !== 0) {
       return {
         exists: false,
@@ -546,22 +558,22 @@ export function runCanvasStructureGate(repoDir, plugin, primaryFetchSpec) {
   const messages = [];
 
   for (const locator of locators) {
-    if (locator !== primaryFetchSpec) {
-      const fetchResult = fetchLocatorIntoRepo(repoDir, locator);
-      if (fetchResult.status === "fail") {
-        hasFailure = true;
-        messages.push(`- ${locator}: ${fetchResult.output}`);
-        continue;
-      }
-
-      if (fetchResult.status === "infra_error") {
-        hasInfraError = true;
-        messages.push(`- ${locator}: ${fetchResult.output}`);
-        continue;
-      }
+    const refResult = resolveLocatorReadRef(repoDir, locator, primaryFetchSpec);
+    if (refResult.status === "fail") {
+      hasFailure = true;
+      messages.push(`- ${locator}: ${refResult.output}`);
+      continue;
     }
 
-    const extensionDirCheck = checkPathExistsAtLocator(repoDir, locator, extensionsDir, "tree");
+    if (refResult.status === "infra_error") {
+      hasInfraError = true;
+      messages.push(`- ${locator}: ${refResult.output}`);
+      continue;
+    }
+
+    const readRef = refResult.readRef;
+
+    const extensionDirCheck = checkPathExistsAtLocator(repoDir, readRef, locator, extensionsDir, "tree");
     if (extensionDirCheck.output) {
       hasInfraError = true;
       messages.push(`- ${locator}: ${extensionDirCheck.output}`);
@@ -577,7 +589,7 @@ export function runCanvasStructureGate(repoDir, plugin, primaryFetchSpec) {
       continue;
     }
 
-    const extensionEntryCheck = checkPathExistsAtLocator(repoDir, locator, extensionEntryPoint, "blob");
+    const extensionEntryCheck = checkPathExistsAtLocator(repoDir, readRef, locator, extensionEntryPoint, "blob");
     if (extensionEntryCheck.output) {
       hasInfraError = true;
       messages.push(`- ${locator}: ${extensionEntryCheck.output}`);

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

@@ -4,7 +4,7 @@ import os from "os";
 import path from "path";
 import { spawnSync } from "child_process";
 import { after, test } from "node:test";
-import { runCanvasStructureGate } from "./external-plugin-quality-gates.mjs";
+import { runCanvasStructureGate, runVersionMatchGate } from "./external-plugin-quality-gates.mjs";
 
 const tempDirs = [];
 
@@ -99,3 +99,118 @@ test("runCanvasStructureGate fails when extension entrypoint path is a directory
   assert.equal(result.status, "fail");
   assert.match(result.output, /"extensions\/extension\.mjs" must be a file/);
 });
+
+// Regression tests for issue #2397: a tag-name locator (e.g. "v1.0.0") must be
+// readable by the version-match and canvas-structure gates. `git fetch origin <tag>`
+// only updates FETCH_HEAD and never creates a local `refs/tags/<tag>`, so reading via
+// `git show <tag>:...` used to die with "fatal: invalid object name" and roll up to a
+// bogus infra_error/fail even though the referenced content was valid.
+
+function initRemoteRepo() {
+  const repoDir = fs.mkdtempSync(path.join(os.tmpdir(), "external-plugin-quality-remote-"));
+  tempDirs.push(repoDir);
+  runGit(repoDir, "init", "-q");
+  runGit(repoDir, "config", "user.name", "Copilot Test");
+  runGit(repoDir, "config", "user.email", "copilot@example.com");
+  // Mirror github.com: allow the submission repo to shallow-fetch an arbitrary SHA.
+  runGit(repoDir, "config", "uploadpack.allowAnySHA1InWant", "true");
+  return repoDir;
+}
+
+function writeValidPluginContent(repoDir) {
+  fs.mkdirSync(path.join(repoDir, ".github", "plugin"), { recursive: true });
+  fs.writeFileSync(
+    path.join(repoDir, ".github", "plugin", "plugin.json"),
+    `${JSON.stringify({ name: "tag-plugin", version: "1.0.0" }, null, 2)}\n`,
+  );
+  fs.mkdirSync(path.join(repoDir, "extensions"), { recursive: true });
+  fs.writeFileSync(path.join(repoDir, "extensions", "extension.mjs"), "export default {};\n");
+}
+
+// Mirrors cloneSubmissionRepository in external-plugin-quality-gates.mjs: fetch only the
+// primary locator and detach HEAD onto it. The tag ref is deliberately never created
+// locally, reproducing the CI environment where `git show <tag>:...` fails.
+function cloneSubmissionRepo(remoteDir, primaryFetchSpec) {
+  const repoDir = fs.mkdtempSync(path.join(os.tmpdir(), "external-plugin-quality-sub-"));
+  tempDirs.push(repoDir);
+  runGit(repoDir, "init", "-q");
+  runGit(repoDir, "remote", "add", "origin", remoteDir);
+  runGit(repoDir, "fetch", "--depth=1", "origin", primaryFetchSpec);
+  runGit(repoDir, "checkout", "--detach", "FETCH_HEAD");
+  return repoDir;
+}
+
+test("runVersionMatchGate passes for a tag ref alongside a sha", () => {
+  const remoteDir = initRemoteRepo();
+  writeValidPluginContent(remoteDir);
+  const sha = commitAll(remoteDir, "Add plugin manifest");
+  runGit(remoteDir, "tag", "-a", "v1.0.0", "-m", "release 1.0.0");
+
+  const repoDir = cloneSubmissionRepo(remoteDir, sha);
+  const plugin = {
+    name: "tag-plugin",
+    version: "1.0.0",
+    source: { source: "github", repo: "owner/repo", ref: "v1.0.0", sha },
+  };
+
+  const result = runVersionMatchGate(repoDir, plugin, sha);
+  assert.equal(result.status, "pass", result.output);
+  // Both the tag ref and the sha must be verified.
+  assert.match(result.output, /- v1\.0\.0: matched version "1\.0\.0"/);
+  assert.match(result.output, new RegExp(`- ${sha}: matched version "1\\.0\\.0"`));
+});
+
+test("runCanvasStructureGate passes for a tag ref alongside a sha", () => {
+  const remoteDir = initRemoteRepo();
+  writeValidPluginContent(remoteDir);
+  const sha = commitAll(remoteDir, "Add canvas extension container");
+  runGit(remoteDir, "tag", "-a", "v1.0.0", "-m", "release 1.0.0");
+
+  const repoDir = cloneSubmissionRepo(remoteDir, sha);
+  const plugin = {
+    name: "tag-plugin",
+    keywords: ["canvas"],
+    source: { source: "github", repo: "owner/repo", ref: "v1.0.0", sha },
+  };
+
+  const result = runCanvasStructureGate(repoDir, plugin, sha);
+  assert.equal(result.status, "pass", result.output);
+  assert.match(result.output, /- v1\.0\.0: found "extensions"/);
+  assert.match(result.output, new RegExp(`- ${sha}: found "extensions"`));
+});
+
+test("runVersionMatchGate passes when the primary locator is a tag ref", () => {
+  const remoteDir = initRemoteRepo();
+  writeValidPluginContent(remoteDir);
+  commitAll(remoteDir, "Add plugin manifest");
+  runGit(remoteDir, "tag", "-a", "v1.0.0", "-m", "release 1.0.0");
+
+  const repoDir = cloneSubmissionRepo(remoteDir, "v1.0.0");
+  const plugin = {
+    name: "tag-plugin",
+    version: "1.0.0",
+    source: { source: "github", repo: "owner/repo", ref: "v1.0.0" },
+  };
+
+  const result = runVersionMatchGate(repoDir, plugin, "v1.0.0");
+  assert.equal(result.status, "pass", result.output);
+  assert.match(result.output, /- v1\.0\.0: matched version "1\.0\.0"/);
+});
+
+test("runCanvasStructureGate passes when the primary locator is a tag ref", () => {
+  const remoteDir = initRemoteRepo();
+  writeValidPluginContent(remoteDir);
+  commitAll(remoteDir, "Add canvas extension container");
+  runGit(remoteDir, "tag", "-a", "v1.0.0", "-m", "release 1.0.0");
+
+  const repoDir = cloneSubmissionRepo(remoteDir, "v1.0.0");
+  const plugin = {
+    name: "tag-plugin",
+    keywords: ["canvas"],
+    source: { source: "github", repo: "owner/repo", ref: "v1.0.0" },
+  };
+
+  const result = runCanvasStructureGate(repoDir, plugin, "v1.0.0");
+  assert.equal(result.status, "pass", result.output);
+  assert.match(result.output, /- v1\.0\.0: found "extensions"/);
+});