diff --git a/eng/external-plugin-quality-gates.mjs b/eng/external-plugin-quality-gates.mjs index 507cc28f2..1b9fb3c46 100644 --- a/eng/external-plugin-quality-gates.mjs +++ b/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 ` only updates FETCH_HEAD; +// it does NOT create a local `refs/tags/`, so `git show :...` 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; - } + const refResult = resolveLocatorReadRef(repoDir, locator, primaryFetchSpec); + if (refResult.status === "fail") { + hasFailure = true; + messages.push(`- ${locator}: ${refResult.output}`); + continue; + } - if (fetchResult.status === "infra_error") { - hasInfraError = true; - messages.push(`- ${locator}: ${fetchResult.output}`); - continue; - } + if (refResult.status === "infra_error") { + hasInfraError = true; + messages.push(`- ${locator}: ${refResult.output}`); + continue; } - const manifestResult = readPluginManifestAtLocator(repoDir, locator, normalizedPluginPath); + 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; - } + const refResult = resolveLocatorReadRef(repoDir, locator, primaryFetchSpec); + if (refResult.status === "fail") { + hasFailure = true; + messages.push(`- ${locator}: ${refResult.output}`); + continue; + } - if (fetchResult.status === "infra_error") { - hasInfraError = true; - messages.push(`- ${locator}: ${fetchResult.output}`); - continue; - } + if (refResult.status === "infra_error") { + hasInfraError = true; + messages.push(`- ${locator}: ${refResult.output}`); + continue; } - const extensionDirCheck = checkPathExistsAtLocator(repoDir, locator, extensionsDir, "tree"); + 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}`); diff --git a/eng/external-plugin-quality-gates.test.mjs b/eng/external-plugin-quality-gates.test.mjs index 5f619faec..04eb27e70 100644 --- a/eng/external-plugin-quality-gates.test.mjs +++ b/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 ` +// only updates FETCH_HEAD and never creates a local `refs/tags/`, so reading via +// `git show :...` 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 :...` 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"/); +});