From 3da7c43604dacf907f676df3b7dec9dea357e9cc Mon Sep 17 00:00:00 2001 From: Ben Potter Date: Thu, 20 Aug 2026 19:00:28 +0000 Subject: [PATCH] feat(.github): score community-namespace modules in PR scorecard checks --- .github/scorecard/score-modules.ts | 137 ++++++++++++------ .github/workflows/module-scorecard-check.yaml | 17 ++- 2 files changed, 103 insertions(+), 51 deletions(-) diff --git a/.github/scorecard/score-modules.ts b/.github/scorecard/score-modules.ts index 97d1af43f..fadf7968f 100644 --- a/.github/scorecard/score-modules.ts +++ b/.github/scorecard/score-modules.ts @@ -10,6 +10,10 @@ * Usage: * bun run score-modules.ts [--modules a,b,c] [--dry-run] [--limit N] * + * Module specs are either a bare name (a module in registry/coder/modules) + * or namespace/name for any registry namespace. Non-coder namespaces are + * PR-report or dry-run only; discussions exist solely for coder modules. + * * --dry-run prints scorecards to stdout and skips GitHub entirely. */ @@ -36,23 +40,39 @@ const SCORECARD_PATH = path.join(import.meta.dir, "SCORECARD.md"); const ANTHROPIC_MODEL = process.env.ANTHROPIC_MODEL ?? "claude-sonnet-4-5"; const MAX_FILE_BYTES = 30_000; -async function displayName(moduleName: string): Promise { - const readme = await readFile( - path.join(MODULES_DIR, moduleName, "README.md"), - "utf8", - ); +// A module reference: bare names mean the coder namespace, and +// namespace/name addresses any registry namespace. +interface ModuleSpec { + ns: string; + name: string; + // namespace/name, used in report headings and log lines. + label: string; + dir: string; +} + +function toSpec(raw: string): ModuleSpec { + const slash = raw.indexOf("/"); + const [ns, name] = + slash === -1 ? ["coder", raw] : [raw.slice(0, slash), raw.slice(slash + 1)]; + return { + ns, + name, + label: `${ns}/${name}`, + dir: path.join(REGISTRY_ROOT, "registry", ns, "modules", name), + }; +} + +async function displayName(spec: ModuleSpec): Promise { + const readme = await readFile(path.join(spec.dir, "README.md"), "utf8"); const match = readme.match(/^display_name:\s*["']?([^"'\n]+)["']?\s*$/m); - return match ? match[1].trim() : moduleName; + return match ? match[1].trim() : spec.name; } // Internal building-block modules caution against direct use and are not // meant to be consumed by template authors, so they are excluded from // scoring entirely. -async function isInternalBuildingBlock(moduleName: string): Promise { - const readme = await readFile( - path.join(MODULES_DIR, moduleName, "README.md"), - "utf8", - ); +async function isInternalBuildingBlock(spec: ModuleSpec): Promise { + const readme = await readFile(path.join(spec.dir, "README.md"), "utf8"); return /do not recommend using this module directly|not intended to be used directly|internal building block/i.test( readme, ); @@ -104,11 +124,11 @@ async function readTruncated(filePath: string): Promise { } async function verifyReadmeImages( - moduleName: string, + spec: ModuleSpec, ): Promise { - const readmePath = path.join(MODULES_DIR, moduleName, "README.md"); + const readmePath = path.join(spec.dir, "README.md"); const readme = await readFile(readmePath, "utf8"); - const moduleDir = path.join(MODULES_DIR, moduleName); + const moduleDir = spec.dir; const imageRegex = /!\[[^\]]*\]\(([^)]+)\)/g; const results: ImageVerification[] = []; @@ -154,8 +174,8 @@ async function verifyReadmeImages( return results; } -async function gatherModuleContext(moduleName: string): Promise { - const dir = path.join(MODULES_DIR, moduleName); +async function gatherModuleContext(spec: ModuleSpec): Promise { + const dir = spec.dir; const parts: string[] = []; const files = await readdir(dir, { recursive: true }); const wanted = files.filter( @@ -173,7 +193,7 @@ async function gatherModuleContext(moduleName: string): Promise { parts.push(`=== FILE: ${f} ===\n${await readTruncated(full)}`); } - const imageVerifications = await verifyReadmeImages(moduleName); + const imageVerifications = await verifyReadmeImages(spec); if (imageVerifications.length > 0) { const verificationLines = imageVerifications.map((v) => { if (v.resolvedPath === "(external)") { @@ -247,18 +267,15 @@ function fixOverall(scorecard: string): string { return out; } -async function scoreModule( - moduleName: string, - rubric: string, -): Promise { - const context = await gatherModuleContext(moduleName); +async function scoreModule(spec: ModuleSpec, rubric: string): Promise { + const context = await gatherModuleContext(spec); const prompt = `You are scoring a Coder Registry module against a scorecard rubric. ${rubric} - + ${context} @@ -389,7 +406,7 @@ async function upsertDiscussion( // module's current discussion (the last scoring from main), so PRs can see // whether they improve, regress, or hold the score. function prReportSection( - mod: string, + spec: ModuleSpec, name: string, scorecard: string, summary: SummaryScores | null, @@ -397,10 +414,16 @@ function prReportSection( ): string { const details = `
\nFull scorecard for this PR\n\n${scorecard}\n\n
`; if (!summary) { - return `### \`coder/${mod}\`\n\nCould not parse the generated scorecard; see full output below.\n\n${details}`; + return `### \`${spec.label}\`\n\nCould not parse the generated scorecard; see full output below.\n\n${details}`; } if (!baseline) { - return `### \`coder/${mod}\`: first scorecard, **${summary.overall}**\n\nNo existing scorecard discussion found for ${name}; this is the initial score. A dedicated discussion is created after merge.\n\n${details}`; + // Discussions exist only for coder-namespace modules; community + // modules always get a standalone advisory score. + const followup = + spec.ns === "coder" + ? `No existing scorecard discussion found for ${name}; this is the initial score. A dedicated discussion is created after merge.` + : "Community modules do not get scorecard discussions; this score is advisory."; + return `### \`${spec.label}\`: first scorecard, **${summary.overall}**\n\n${followup}\n\n${details}`; } const delta = summary.overallNum - baseline.overallNum; const themes: [string, string, string][] = [ @@ -424,7 +447,7 @@ function prReportSection( } else { verdict = `\u2705 **Score unchanged** at ${summary.overall}. This PR does not affect the module's scorecard; the results are still good.`; } - return `### [\`coder/${mod}\`](${baseline.url}): ${baseline.overallNum} \u2192 ${summary.overallNum}\n\n${verdict}\n\n${table}\n\n${details}`; + return `### [\`${spec.label}\`](${baseline.url}): ${baseline.overallNum} \u2192 ${summary.overallNum}\n\n${verdict}\n\n${table}\n\n${details}`; } function discussionBody( @@ -453,46 +476,68 @@ async function main() { } const rubric = await readFile(SCORECARD_PATH, "utf8"); - let modules = + // Without --modules, enumerate only registry/coder/modules: the + // discussion-writing runs (post-merge, weekly) are for official modules + // only. Community modules are scored solely via explicit --modules specs + // in PR-report mode. + let specs = ( args.modules ?? (await readdir(MODULES_DIR, { withFileTypes: true })) .filter((d) => d.isDirectory()) .map((d) => d.name) - .sort(); - if (args.limit) modules = modules.slice(0, args.limit); + .sort() + ).map(toSpec); + if (args.limit) specs = specs.slice(0, args.limit); + + // Discussions are reserved for coder-namespace modules. Refuse to run + // non-coder specs in discussion-writing mode rather than silently + // creating community discussions. + if (!args.prReport && !args.dryRun) { + const community = specs.filter((s) => s.ns !== "coder"); + if (community.length > 0) { + throw new Error( + `non-coder modules require --pr-report or --dry-run: ${community.map((s) => s.label).join(", ")}`, + ); + } + } const prSections: string[] = []; const failed: string[] = []; - for (const mod of modules) { - if (!existsSync(path.join(MODULES_DIR, mod, "README.md"))) { - console.error(`skip ${mod}: no README.md`); + for (const spec of specs) { + if (!existsSync(path.join(spec.dir, "README.md"))) { + console.error(`skip ${spec.label}: no README.md`); continue; } - if (await isInternalBuildingBlock(mod)) { - process.stderr.write(`skip ${mod}: internal building block\n`); + if (await isInternalBuildingBlock(spec)) { + process.stderr.write(`skip ${spec.label}: internal building block\n`); continue; } - process.stderr.write(`scoring ${mod}... `); + process.stderr.write(`scoring ${spec.label}... `); try { - const scorecard = await scoreModule(mod, rubric); + const scorecard = await scoreModule(spec, rubric); if (args.dryRun) { - console.log(`\n===== coder/${mod} =====\n${scorecard}\n`); + console.log(`\n===== ${spec.label} =====\n${scorecard}\n`); process.stderr.write("done (dry-run)\n"); continue; } - const name = await displayName(mod); + const name = await displayName(spec); if (args.prReport) { // PR mode: compare against the module's current discussion and - // build a report instead of touching any discussion. + // build a report instead of touching any discussion. Community + // modules have no discussions, so they skip the baseline lookup + // and always report a standalone score. const summary = parseSummary(scorecard); - let existing = await findDiscussionByTitle(`${name} module`); + let existing = + spec.ns === "coder" + ? await findDiscussionByTitle(`${name} module`) + : null; let renameWarning = ""; - if (!existing) { + if (spec.ns === "coder" && !existing) { // A display_name rename breaks the title lookup while the old // discussion still exists. Fall back to the registry URL and // flag it so a reviewer retitles or deletes the old one; the // post-merge run creates a fresh discussion under the new name. - existing = await findDiscussionByModuleUrl(mod); + existing = await findDiscussionByModuleUrl(spec.name); if (existing) { renameWarning = `\n\n> [!WARNING]\n> This module's display name appears to have changed. Its existing scorecard discussion is titled "${existing.title}" (${existing.url}). After merge, a new discussion named "${name} module" will be created; a maintainer should retitle or delete the old one to avoid duplicates.`; } @@ -501,7 +546,7 @@ async function main() { const baseline = existing && parsed ? { ...parsed, url: existing.url } : null; prSections.push( - prReportSection(mod, name, scorecard, summary, baseline) + + prReportSection(spec, name, scorecard, summary, baseline) + renameWarning, ); process.stderr.write("compared\n"); @@ -509,11 +554,11 @@ async function main() { } const { url, created } = await upsertDiscussion( `${name} module`, - discussionBody(mod, name, scorecard), + discussionBody(spec.name, name, scorecard), ); process.stderr.write(`${created ? "created" : "updated"} ${url}\n`); } catch (err) { - failed.push(mod); + failed.push(spec.label); process.stderr.write(`FAILED: ${err}\n`); } } diff --git a/.github/workflows/module-scorecard-check.yaml b/.github/workflows/module-scorecard-check.yaml index 2e4ce1dd4..d1b223bf7 100644 --- a/.github/workflows/module-scorecard-check.yaml +++ b/.github/workflows/module-scorecard-check.yaml @@ -14,9 +14,13 @@ # OWNER/MEMBER/COLLABORATOR, so every fork-PR scoring is an explicit # maintainer decision. # +# Modules in every namespace (registry//modules/) are +# scored. Only registry/coder modules have discussion baselines; community +# modules always report a fresh advisory score. +# # Both paths execute only trusted code: the checkout is the base # repository, and PR content is fetched separately and materialized ONLY -# under registry/coder/modules/, where the scoring script reads it as +# under registry/*/modules/, where the scoring script reads it as # inert text for the LLM prompt. Nothing from the PR head is executed. # # Required repository secrets: @@ -30,7 +34,7 @@ name: Module Scorecard Check on: pull_request: paths: - - "registry/coder/modules/**" + - "registry/*/modules/**" issue_comment: types: [created] @@ -83,7 +87,10 @@ jobs: # modules (REG-74). run: | git fetch --depth=2 origin "refs/pull/${PR_NUMBER}/merge" - MODULES=$(git diff --name-only FETCH_HEAD^1 FETCH_HEAD | { grep -oP '^registry/coder/modules/\K[^/]+' || true; } | sort -u | paste -sd, -) + # Module specs are namespace/name (for example coder/git-clone or + # droopy4096/mise-install) so community-namespace modules are + # scored too, not just registry/coder. + MODULES=$(git diff --name-only FETCH_HEAD^1 FETCH_HEAD | { grep -oP '^registry/\K[^/]+/modules/[^/]+' || true; } | sed 's#/modules/#/#' | sort -u | paste -sd, -) echo "modules=${MODULES}" >> "${GITHUB_OUTPUT}" echo "pr=${PR_NUMBER}" >> "${GITHUB_OUTPUT}" echo "Changed modules: ${MODULES:-none}" @@ -95,8 +102,8 @@ jobs: # plain text for the LLM prompt; it never executes them. Everything # under .github/ stays at the trusted base. run: | - git rm -rq --ignore-unmatch registry/coder/modules - git checkout FETCH_HEAD -- registry/coder/modules + git rm -rq --ignore-unmatch 'registry/*/modules' + git checkout FETCH_HEAD -- 'registry/*/modules' - name: Score changed modules if: steps.changed.outputs.modules != ''