Splitting ref and sha into two fields correctly for the intake form (#1788)

* Splitting ref and sha into two fields correctly for the intake form * Enforce 40-character commit SHA in validateImmutableRef Co-authored-by: aaronpowell <434140+aaronpowell@users.noreply.github.com> * Add backward compatibility for legacy checklist text and field title Co-authored-by: aaronpowell <434140+aaronpowell@users.noreply.github.com> * Avoid unnecessary array spread when iterating checklist equivalents Co-authored-by: aaronpowell <434140+aaronpowell@users.noreply.github.com> --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: aaronpowell <434140+aaronpowell@users.noreply.github.com>
2026-05-28 01:21:46 +00:00 · 2026-05-22 10:54:34 +10:00
parent 2ca49df9d4
commit 6fc05f480e
5 changed files with 111 additions and 32 deletions
@@ -15,11 +15,17 @@ const RERUN_INTAKE_COMMAND_PATTERN = new RegExp(
 );
 const PLUGINS_DIR = path.join(ROOT_FOLDER, "plugins");

+// Each entry is a Set of equivalent checklist item texts (new + legacy aliases).
+// A submission passes if the checked items contain at least one text from each Set.
 const REQUIRED_CHECKLIST_ITEMS = [
-  "The plugin lives in a public GitHub repository.",
-  "The ref I provided is an immutable release tag or full 40-character commit SHA, not a branch.",
-  "This submission follows this repository's contribution, security, and responsible AI policies.",
-  "This plugin is not already listed in the Awesome Copilot marketplace.",
+  new Set(["The plugin lives in a public GitHub repository."]),
+  new Set([
+    "The ref and/or sha I provided is immutable (release tag and/or full 40-character commit SHA), not a branch.",
+    // Legacy text used in the original issue template
+    "The ref I provided is an immutable release tag or full 40-character commit SHA, not a branch.",
+  ]),
+  new Set(["This submission follows this repository's contribution, security, and responsible AI policies."]),
+  new Set(["This plugin is not already listed in the Awesome Copilot marketplace."]),
 ];

 const FIELD_TITLES = Object.freeze({
@@ -27,7 +33,8 @@ const FIELD_TITLES = Object.freeze({
  shortDescription: "Short description",
  githubRepository: "GitHub repository",
  pluginPath: "Plugin path inside the repository",
-  immutableRef: "Immutable ref to review",
+  immutableRef: "Ref to review",
+  immutableSha: "Commit SHA to review",
  version: "Version",
  license: "License identifier",
  authorName: "Author name",
@@ -38,6 +45,11 @@ const FIELD_TITLES = Object.freeze({
  submissionChecklist: "Submission checklist",
 });

+// Legacy field title used in the original issue template (before the ref/sha split)
+const LEGACY_FIELD_TITLES = Object.freeze({
+  immutableRef: "Immutable ref to review",
+});
+
 function normalizeMultilineText(value) {
  return String(value ?? "").replace(/\r\n/g, "\n");
 }
@@ -156,7 +168,7 @@ function encodeRepoPath(repo) {
  return `${encodeURIComponent(owner ?? "")}/${encodeURIComponent(name ?? "")}`;
 }

-async function validateRemoteRepository(repo, ref, errors, warnings, token) {
+async function validateRemoteRepository(repo, { ref, sha }, errors, warnings, token) {
  const encodedRepo = encodeRepoPath(repo);
  const repositoryResponse = await fetchGitHubJson(`/repos/${encodedRepo}`, token);

@@ -177,6 +189,15 @@ async function validateRemoteRepository(repo, ref, errors, warnings, token) {
    warnings.push(`submission: GitHub repository "${repo}" is archived`);
  }

+  if (sha) {
+    if (/^[0-9a-f]{40}$/i.test(sha)) {
+      const commitResponse = await fetchGitHubJson(`/repos/${encodedRepo}/commits/${encodeURIComponent(sha)}`, token);
+      if (!commitResponse.ok) {
+        errors.push(`submission: commit "${sha}" was not found in GitHub repository "${repo}"`);
+      }
+    }
+  }
+
  if (!ref) {
    return;
  }
@@ -189,6 +210,14 @@ async function validateRemoteRepository(repo, ref, errors, warnings, token) {
    return;
  }

+  if (ref.startsWith("refs/heads/") || ["main", "master", "develop", "development", "dev", "trunk"].includes(ref)) {
+    return;
+  }
+
+  if (ref.startsWith("refs/") && !ref.startsWith("refs/tags/")) {
+    return;
+  }
+
  const tagName = ref.startsWith("refs/tags/") ? ref.slice("refs/tags/".length) : ref;
  const tagResponse = await fetchGitHubJson(`/repos/${encodedRepo}/git/ref/tags/${encodeURIComponent(tagName)}`, token);

@@ -197,7 +226,7 @@ async function validateRemoteRepository(repo, ref, errors, warnings, token) {
  }

  if (/^[0-9a-f]+$/i.test(ref) && ref.length !== 40) {
-    errors.push('submission: commit SHAs in "Immutable ref to review" must use the full 40-character SHA');
+    errors.push('submission: commit SHAs in "Ref to review" must use the full 40-character SHA or be submitted in "Commit SHA to review"');
    return;
  }

@@ -221,7 +250,11 @@ export function parseExternalPluginIssueBody(body) {
  const pluginName = requiredField(FIELD_TITLES.pluginName);
  const shortDescription = requiredField(FIELD_TITLES.shortDescription);
  const repoInput = normalizeGitHubRepo(requiredField(FIELD_TITLES.githubRepository));
-  const immutableRef = requiredField(FIELD_TITLES.immutableRef);
+  // Support both the current field title and the legacy title used before the ref/sha split
+  const immutableRef = stripNoResponse(
+    sections.get(FIELD_TITLES.immutableRef) ?? sections.get(LEGACY_FIELD_TITLES.immutableRef),
+  );
+  const immutableSha = stripNoResponse(sections.get(FIELD_TITLES.immutableSha));
  const version = requiredField(FIELD_TITLES.version);
  const license = requiredField(FIELD_TITLES.license);
  const authorName = requiredField(FIELD_TITLES.authorName);
@@ -233,9 +266,22 @@ export function parseExternalPluginIssueBody(body) {
  const additionalNotes = stripNoResponse(sections.get(FIELD_TITLES.additionalNotes));
  const checkedItems = parseChecklist(sections.get(FIELD_TITLES.submissionChecklist));

-  for (const item of REQUIRED_CHECKLIST_ITEMS) {
-    if (!checkedItems.has(item)) {
-      errors.push(`submission: checklist item must be checked: "${item}"`);
+  if (!immutableRef && !immutableSha) {
+    errors.push(`submission: one of "${FIELD_TITLES.immutableRef}" or "${FIELD_TITLES.immutableSha}" is required`);
+  }
+
+  for (const equivalents of REQUIRED_CHECKLIST_ITEMS) {
+    let isChecked = false;
+    for (const text of equivalents) {
+      if (checkedItems.has(text)) {
+        isChecked = true;
+        break;
+      }
+    }
+    if (!isChecked) {
+      // Report using the canonical (first) text in each equivalents Set
+      const [canonical] = equivalents;
+      errors.push(`submission: checklist item must be checked: "${canonical}"`);
    }
  }

@@ -256,6 +302,7 @@ export function parseExternalPluginIssueBody(body) {
      repo: repoInput,
      ...(pluginPath ? { path: pluginPath } : {}),
      ...(immutableRef ? { ref: immutableRef } : {}),
+      ...(immutableSha ? { sha: immutableSha } : {}),
    },
  };

@@ -297,8 +344,8 @@ export async function evaluateExternalPluginIssue({ issue, token } = {}) {
    }
  }

-  if (parsed.plugin?.source?.repo && parsed.plugin?.source?.ref) {
-    await validateRemoteRepository(parsed.plugin.source.repo, parsed.plugin.source.ref, errors, warnings, token);
+  if (parsed.plugin?.source?.repo && (parsed.plugin?.source?.ref || parsed.plugin?.source?.sha)) {
+    await validateRemoteRepository(parsed.plugin.source.repo, parsed.plugin.source, errors, warnings, token);
  }

  const dedupedErrors = [...new Set(errors)];
@@ -324,7 +371,8 @@ export async function evaluateExternalPluginIssue({ issue, token } = {}) {
        "",
        `- **Plugin:** ${parsed.plugin.name}`,
        `- **Repository:** ${parsed.plugin.repository}`,
-        `- **Ref:** ${parsed.plugin.source.ref}`,
+        parsed.plugin.source.ref ? `- **Ref:** ${parsed.plugin.source.ref}` : undefined,
+        parsed.plugin.source.sha ? `- **SHA:** ${parsed.plugin.source.sha}` : undefined,
        `- **Keywords:** ${normalizedKeywords}`,
        "",
        "### Canonical external.json payload",