diff --git a/BOTS.md b/BOTS.md index ab7ff38..999a43b 100644 --- a/BOTS.md +++ b/BOTS.md @@ -30,9 +30,9 @@ and the need to ask a maintainer for a rerun. Network, tooling, and registry failures keep their diagnostics in the workflow log and job summary and add `needs-maintainer`; they do not post an error comment on the author's issue. Inspect the failed run, resolve its cause, rerun that issue, and remove the label -when handled. Theme submissions currently need a maintainer to supply image -media in the record: the form has no media input. Manifest metadata ingestion -is a separate registry change; this workflow does not ask for a listing file. +when handled. Submission reads metadata from the pinned `paseo-plugin.json` +and preserves record overrides. Categories do not trigger content requirements; +the PR reviewer decides which screenshots the plugin needs under REVIEW.md. npm package files come from one cached tarball per version, verified against npm's SHA-512 before reading. No package code runs, and files are read to stdout diff --git a/README.md b/README.md index 599b8ae..51dea69 100644 --- a/README.md +++ b/README.md @@ -47,7 +47,8 @@ pin; `repository` is the browseable source and optional proven source commit. Humans edit categories and optional `listing` overrides (`name`, HTTPS PNG `icon`, HTTPS `media`). The bot writes artifact pins and review dates. The published -index combines records with metadata from their pinned artifacts. +index combines records with metadata from their pinned manifests. See +[plugin metadata](#plugin-metadata) for fields and override precedence. Authors must keep `OVERVIEW.md` beside `paseo-plugin.json` in the repository at the pinned source commit. Git monorepos use `artifact.pluginPath`; npm monorepos use @@ -98,21 +99,56 @@ contract is reviewed by a person. The detail document keeps its existing `readme` field. It publishes the pinned author `OVERVIEW.md`, or the registry import stopgap when author content is absent. -With neither source it fails. `README.md`, `readme.md`, and `paseo-listing.json` -readme overrides are never used for overview content. +With neither source it fails. `README.md` and `readme.md` are never used for +overview content. -A plugin can ship a separate `paseo-listing.json` next to its strict manifest: +## Plugin metadata + +Declare your display name, icon, screenshots, and demo videos in `paseo-plugin.json`: ```json { + "id": "example", "name": "Example", - "icon": "icon.png", - "media": ["https://example.com/demo.mp4", "https://example.com/screen.png"] + "description": "A short description of what the plugin does.", + "icon": "assets/icon.png", + "media": ["assets/screenshot.png", "https://example.com/demo.mp4"], + "requirements": { "paseo": ">=0.11.0" } } ``` -Relative icons resolve to the pinned artifact. Media entries are HTTPS image or video URLs; -record overrides win over this file. Cards use the first image in media order. +`name`, `icon`, and `media` are optional manifest fields. Include only assets +that exist in your release. `icon` is a package-relative PNG path. Each `media` +entry is a package-relative path or an HTTPS URL with an image extension +(`png`, `jpg`, `jpeg`, `webp`, `gif`) or video extension (`mp4`, `webm`). +SVG and URLs without a supported extension are not accepted by the registry. +Paths use forward slashes and stay inside the plugin directory. For npm, +include local asset files in the published package's `files` list. + +The registry reads the manifest from the pinned npm tarball or Git commit. +Relative assets become URLs pointing to that version, under `pluginPath` for +Git monorepos. Media keep their declared order; cards use the first image. +Manifests declaring these fields require Paseo 0.11.0 or later. + +The existing record's `listing` values override the manifest **per field**: + +| Field | First choice | Otherwise | +| --- | --- | --- | +| Name | `listing.name` | Manifest `name`, then a humanized registry id | +| Icon | `listing.icon` | Manifest `icon`, or no icon | +| Media | `listing.media` | Manifest `media`, or an empty array | + +An explicit `listing.media: []` replaces all manifest media. The submission +bot saves the issue title as `listing.name`. Maintainers can keep using the +same overrides without changing existing records. + +The registry does not read `paseo-listing.json`. Move its metadata into the +manifest when publishing a new release. + +Categories describe where a plugin is listed. Selecting **Themes** does not +identify the plugin as a theme or trigger a screenshot requirement in the +submission bot. The PR reviewer determines which screenshots are needed from +what the plugin does, following [the review policy](REVIEW.md#content). ## Maintainers diff --git a/REVIEW.md b/REVIEW.md index 830a26d..9b666a4 100644 --- a/REVIEW.md +++ b/REVIEW.md @@ -89,12 +89,16 @@ what each is judged against. - Source match: when the declared repository holds the source at the pinned commit, the artifact's code matches it. Extra files, changed logic, or a dependency the source does not declare is a mismatch, and a mismatch is rejected. -- Listing media: entries are HTTPS image (`png`, `jpg`, `jpeg`, `webp`, `gif`) or video (`mp4`, +- Listing media: review the manifest metadata after applying any registry `listing` + overrides. Entries are HTTPS image (`png`, `jpg`, `jpeg`, `webp`, `gif`) or video (`mp4`, `webm`) URLs, with case-insensitive extensions. Each URL returns HTTP 200 and an `image/*` or `video/*` content type. SVG is not accepted. The card thumbnail is the first image in media order. - Visible surfaces: every theme and any plugin that adds a panel or other UI lists at - least one image of that surface. A theme without an image is not listed. + least one image of that surface. A theme without an image is not listed. Determine + what the plugin does from the artifact; categories are organizational labels. A + plugin categorized as Themes can be a tool for creating themes. The PR reviewer + applies this requirement; submission does not infer it from a category. - Scope: a pull request changes one record and its overview. Anything touching `.github/`, `featured.json`, `scripts/`, `categories.json`, this file, or more than one record is a maintainer change and is never merged by the bot. diff --git a/scripts/lib/listing.test.ts b/scripts/lib/listing.test.ts index 8a9b17b..6b8c8fe 100644 --- a/scripts/lib/listing.test.ts +++ b/scripts/lib/listing.test.ts @@ -1,7 +1,7 @@ import assert from "node:assert/strict"; import { test } from "node:test"; -import { mergeListing, parseListingFile, resolvePlugin } from "./listing.ts"; -import type { VersionDoc } from "./npm.ts"; +import { resolvePlugin } from "./listing.ts"; +import type { NpmClient, VersionDoc } from "./npm.ts"; import type { PluginRecord } from "./record.ts"; const doc: VersionDoc = { @@ -36,58 +36,23 @@ const record: PluginRecord = { reviewedAt: "2026-10-03", }; -test("assets in the package resolve to the pinned version on the CDN", () => { - const plugin = mergeListing({ - record, - doc, - listingFile: parseListingFile( - JSON.stringify({ - name: "Dracula", - icon: "icon.png", - media: ["https://x.test/a.mp4", "https://x.test/b.png"], - }), - ), - readme: "# Dracula\n", - installs: 42, - }); +test("publication preserves author, overview, and review dates", async () => { + const client: NpmClient = { + async packument() { return { name: doc.name, "dist-tags": { latest: doc.version }, versions: { [doc.version]: doc }, time: {} }; }, + async file() { return '{"name":"Dracula"}'; }, + async provenance() { return null; }, + async tarball() { throw new Error("Not needed"); }, + }; + const plugin = await resolvePlugin(client, { ...record, repository: { url: record.repository.url } }, "Overview."); assert.equal(plugin.name, "Dracula"); - assert.equal(plugin.icon, "https://cdn.jsdelivr.net/npm/@omercnet/paseo-dracula@1.2.0/icon.png"); - assert.deepEqual(plugin.media, [ - "https://x.test/a.mp4", - "https://x.test/b.png", - ]); + assert.equal(plugin.description, doc.description); assert.deepEqual(plugin.author, { npm: "omercnet", name: "Omer Cohen", github: "omercnet" }); - assert.equal(plugin.repository?.commit, record.repository?.commit); - assert.equal(plugin.readme, "# Dracula\n"); -}); - -test("record overrides win, and a package without a listing file gets a humanized name", () => { - const plugin = mergeListing({ - record: { ...record, listing: { media: ["https://x.test/override.png"] } }, - doc: { ...doc, author: undefined, repository: undefined }, - listingFile: parseListingFile(null), - readme: null, - }); - assert.equal(plugin.name, "Dracula"); - assert.deepEqual(plugin.media, ["https://x.test/override.png"]); - assert.equal(plugin.icon, undefined); - assert.deepEqual(plugin.author, { npm: "omercnet", name: "omercnet", github: "omercnet" }); - assert.equal(plugin.readme, ""); - assert.equal(plugin.installs, undefined); -}); - -test("publication date comes from the record review date, not the npm release", () => { - const plugin = mergeListing({ - record, - doc, - listingFile: {}, - readme: "Overview.", - }); + assert.equal(plugin.readme, "Overview."); assert.equal(plugin.publishedAt, "2026-10-03T00:00:00.000Z"); assert.equal(plugin.updatedAt, "2026-10-03T00:00:00.000Z"); + assert.equal(plugin.installs, undefined); }); - test("a tagged monorepo artifact is pinned, validated and built through both submission syntaxes", async () => { const { mkdtempSync, mkdirSync, writeFileSync, readFileSync, cpSync, rmSync } = await import("node:fs"); @@ -104,8 +69,10 @@ test("a tagged monorepo artifact is pinned, validated and built through both sub mkdirSync(join(repository, pluginPath), { recursive: true }); writeFileSync( join(repository, pluginPath, "paseo-plugin.json"), - JSON.stringify({ id: "example", description: "Pinned monorepo example" }), + JSON.stringify({ id: "example", name: "Manifest name", description: "Pinned monorepo example", icon: "assets/icon.png", media: ["assets/demo.mp4", "assets/screen.png"] }), ); + mkdirSync(join(repository, pluginPath, "assets")); + for (const asset of ["icon.png", "demo.mp4", "screen.png"]) writeFileSync(join(repository, pluginPath, "assets", asset), "fixture"); writeFileSync(join(repository, pluginPath, "OVERVIEW.md"), "Monorepo example overview.\n"); writeFileSync(join(repository, pluginPath, "README.md"), "Wrong README"); writeFileSync( @@ -189,7 +156,13 @@ test("a tagged monorepo artifact is pinned, validated and built through both sub assert.equal(detail.description, "Pinned monorepo example"); assert.equal(detail.artifact.pluginPath, pluginPath); assert.equal(detail.artifact.commit, commit); - assert.deepEqual(detail.media, []); + const base = `https://github.com/acme/plugins/raw/${commit}/${pluginPath}/`; + assert.equal(detail.name, "Manifest name"); + assert.equal(detail.icon, `${base}assets/icon.png`); + assert.deepEqual(detail.media, [`${base}assets/demo.mp4`, `${base}assets/screen.png`]); + const summary = index.plugins.find((item: { id: string }) => item.id === detail.id); + const { readme, ...expectedSummary } = detail; + assert.deepEqual(summary, expectedSummary); } const overview = join(registry, "plugins/acme/example.md"); writeFileSync(overview, "# Example\n\nA curated description.\n"); diff --git a/scripts/lib/listing.ts b/scripts/lib/listing.ts index e791651..f156645 100644 --- a/scripts/lib/listing.ts +++ b/scripts/lib/listing.ts @@ -1,17 +1,9 @@ -import { parseMedia } from "./media.ts"; import { readAuthorOverview, requireOverview } from "./overview.ts"; import type { Category } from "./categories.ts"; -import { authorOf, type NpmClient, resolveVersion, type VersionDoc } from "./npm.ts"; -import { readOptional, withGitArtifact } from "./git-artifact.ts"; +import { authorOf, type NpmClient, resolveVersion } from "./npm.ts"; +import { resolveMetadata } from "./metadata.ts"; import type { PluginRecord } from "./record.ts"; -/** What paseo-listing.json in the package may declare. */ -export interface ListingFile { - name?: string; - icon?: string; - media?: string[]; -} - /** One plugin as the website reads it. */ export interface PublishedPlugin { id: string; @@ -45,123 +37,36 @@ export interface PublishedIndex { plugins: PublishedPlugin[]; } -const CDN = "https://cdn.jsdelivr.net/npm"; - -export function packageFileUrl(pkg: string, version: string, path: string): string { - return `${CDN}/${pkg}@${version}/${path.replace(/^\.?\//, "")}`; -} - -function assetUrl(pkg: string, version: string, value: string): string { - return /^https:\/\//.test(value) ? value : packageFileUrl(pkg, version, value); -} - -export function parseListingFile(text: string | null): ListingFile { - if (text === null) return {}; - const raw = JSON.parse(text) as Record; - const listing: ListingFile = {}; - if (typeof raw.name === "string") listing.name = raw.name; - if (typeof raw.icon === "string") listing.icon = raw.icon; - if (raw.media !== undefined) listing.media = parseMedia(raw.media); - return listing; -} - -export function humanizeId(id: string): string { - return id - .split("/") - .at(-1)! - .split("-") - .map((part) => part.charAt(0).toUpperCase() + part.slice(1)) - .join(" "); -} - -/** Pure merge of the record, the pinned npm version, and the package's listing file. */ -export function mergeListing(input: { - record: PluginRecord; - doc: VersionDoc; - listingFile: ListingFile; - readme: string | null; - installs?: number; -}): PublishedPluginDetail { - const { record, doc, listingFile } = input; - const author = authorOf(doc); - const media = record.listing?.media ?? listingFile.media ?? []; - const icon = record.listing?.icon ?? listingFile.icon; - return { - id: record.id, - name: record.listing?.name ?? listingFile.name ?? humanizeId(record.id), - description: doc.description?.trim() ?? "", - artifact: record.artifact, - ...(doc.license ? { license: doc.license } : {}), - repository: record.repository, - categories: record.categories, - author: { ...author, github: record.id.split("/")[0] }, - ...(icon ? { icon: assetUrl(doc.name, doc.version, icon) } : {}), - media, - submittedAt: new Date(record.submittedAt).toISOString(), - reviewedAt: new Date(record.reviewedAt).toISOString(), - publishedAt: new Date(record.reviewedAt).toISOString(), - updatedAt: new Date(record.reviewedAt).toISOString(), - ...(input.installs !== undefined ? { installs: input.installs } : {}), - readme: input.readme ?? "", - }; -} - export async function resolvePlugin( client: NpmClient, record: PluginRecord, overview: string | null = null, ): Promise { const readme = requireOverview(await readAuthorOverview(client, record), overview, record.id); - if (record.artifact.kind === "git") return resolveGitPlugin(record, readme); + const metadata = await resolveMetadata(client, record); const artifact = record.artifact; - const packument = await client.packument(artifact.package); - const doc = resolveVersion(packument, artifact.version); - if (doc.dist.integrity !== artifact.integrity || doc.dist.tarball !== artifact.resolved) { - throw new Error( - `${artifact.package}@${artifact.version} integrity on npm differs from the pinned record`, - ); - } - const listingFile = parseListingFile( - await client.file(doc.name, doc.version, "paseo-listing.json"), - ); - return mergeListing({ - record, - doc, - listingFile, + const doc = artifact.kind === "npm" + ? resolveVersion(await client.packument(artifact.package), artifact.version) + : undefined; + const date = new Date(record.reviewedAt).toISOString(); + return { + id: record.id, + ...metadata, + description: doc ? doc.description?.trim() ?? "" : metadata.description, + artifact, + ...(doc?.license ? { license: doc.license } : {}), + repository: record.repository, + categories: record.categories, + author: { ...(doc ? authorOf(doc) : {}), github: record.id.split("/")[0] }, + submittedAt: new Date(record.submittedAt).toISOString(), + reviewedAt: date, + publishedAt: date, + updatedAt: date, readme, - }); + }; } export function summarize(detail: PublishedPluginDetail): PublishedPlugin { const { readme: _readme, ...summary } = detail; return summary; } - -function resolveGitPlugin(record: PluginRecord, overview: string): PublishedPluginDetail { - const artifact = record.artifact; - if (artifact.kind !== "git") throw new Error("Expected git artifact"); - return withGitArtifact(record, (directory) => { - const manifest = JSON.parse(readOptional(directory, "paseo-plugin.json")!); - const listing = parseListingFile(readOptional(directory, "paseo-listing.json")); - const base = `${artifact.remote.replace(/\.git$/, "")}/raw/${artifact.commit}/${artifact.pluginPath ? `${artifact.pluginPath}/` : ""}`; - const asset = (value: string) => (value.startsWith("https://") ? value : `${base}${value}`); - const icon = record.listing?.icon ?? listing.icon; - const date = new Date(record.reviewedAt).toISOString(); - return { - id: record.id, - name: record.listing?.name ?? listing.name ?? humanizeId(record.id), - description: manifest.description ?? "", - artifact, - repository: record.repository, - categories: record.categories, - author: { github: record.id.split("/")[0] }, - ...(icon ? { icon: asset(icon) } : {}), - media: record.listing?.media ?? listing.media ?? [], - submittedAt: new Date(record.submittedAt).toISOString(), - reviewedAt: date, - updatedAt: date, - publishedAt: date, - readme: overview, - }; - }); -} diff --git a/scripts/lib/manifest-metadata.test.ts b/scripts/lib/manifest-metadata.test.ts new file mode 100644 index 0000000..9f3c5f9 --- /dev/null +++ b/scripts/lib/manifest-metadata.test.ts @@ -0,0 +1,144 @@ +import assert from "node:assert/strict"; +import { execFileSync } from "node:child_process"; +import { createHash } from "node:crypto"; +import { mkdirSync, mkdtempSync, rmSync, writeFileSync } from "node:fs"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; +import { test, type TestContext } from "node:test"; +import { resolvePlugin } from "./listing.ts"; +import { createNpmClient } from "./npm.ts"; +import type { PluginRecord } from "./record.ts"; +import { AuthorError } from "./problems.ts"; +import { validateArtifact } from "./validate-artifact.ts"; + +function fixture(t: TestContext, kind: "npm" | "git", manifest: unknown, listing?: PluginRecord["listing"]) { + const root = mkdtempSync(join(tmpdir(), "registry-manifest-")); + t.after(() => rmSync(root, { recursive: true, force: true })); + const pluginPath = kind === "npm" ? "package" : "plugins/example"; + const plugin = join(root, pluginPath); + mkdirSync(join(plugin, "assets"), { recursive: true }); + writeFileSync(join(plugin, "paseo-plugin.json"), JSON.stringify(manifest)); + writeFileSync(join(plugin, "OVERVIEW.md"), "A theme for focused work."); + // This obsolete file must never influence the listing. + writeFileSync(join(plugin, "paseo-listing.json"), JSON.stringify({ name: "Wrong name", media: ["https://wrong.test/wrong.png"] })); + for (const path of ["icon.png", "screen shot.PNG", "demo.mp4"]) writeFileSync(join(plugin, "assets", path), "fixture asset"); + const remote = "https://github.com/acme/manifest.git"; + const git = (...args: string[]) => execFileSync("git", args, { cwd: root, encoding: "utf8" }).trim(); + git("init", "-q"); + git("add", "."); + git("-c", "user.name=Test", "-c", "user.email=test@example.com", "commit", "-qm", "published"); + const commit = git("rev-parse", "HEAD"); + // HEAD is deliberately different from the pinned manifest and assets. + writeFileSync(join(plugin, "paseo-plugin.json"), '{"name":"Unreleased"}'); + git("add", "."); + git("-c", "user.name=Test", "-c", "user.email=test@example.com", "commit", "-qm", "unreleased"); + const keys = ["GIT_CONFIG_COUNT", "GIT_CONFIG_KEY_0", "GIT_CONFIG_VALUE_0"]; + const previous = keys.map((key) => process.env[key]); + process.env.GIT_CONFIG_COUNT = "1"; + process.env.GIT_CONFIG_KEY_0 = `url.file://${root}.insteadOf`; + process.env.GIT_CONFIG_VALUE_0 = remote; + t.after(() => keys.forEach((key, i) => { + if (previous[i] === undefined) delete process.env[key]; else process.env[key] = previous[i]; + })); + writeFileSync(join(plugin, "paseo-plugin.json"), JSON.stringify(manifest)); + const archive = execFileSync("tar", ["-czf", "-", "-C", root, pluginPath]); + const doc = { name: "@acme/example", version: "1.0.0", description: "Package description", dist: { + tarball: "https://registry.npmjs.org/example/-/example-1.0.0.tgz", + integrity: `sha512-${createHash("sha512").update(archive).digest("base64")}`, + } }; + const client = createNpmClient(async (input) => { + const url = String(input); + if (url === doc.dist.tarball) return new Response(archive); + if (url === "https://registry.npmjs.org/@acme/example") return Response.json({ + name: doc.name, "dist-tags": { latest: "9.0.0" }, versions: { "1.0.0": doc }, time: {}, + }); + throw new Error(`Unexpected request: ${url}`); + }); + const record: PluginRecord = { + id: "acme/example", categories: ["themes"], listing, + artifact: kind === "npm" + ? { kind, package: doc.name, version: doc.version, resolved: doc.dist.tarball, integrity: doc.dist.integrity } + : { kind, remote, commit, pluginPath }, + repository: { url: "https://github.com/acme/manifest" }, + submittedAt: "2026-10-07", reviewedAt: "2026-10-07", + }; + const base = kind === "npm" ? "https://cdn.jsdelivr.net/npm/@acme/example@1.0.0/" + : `https://github.com/acme/manifest/raw/${commit}/${pluginPath}/`; + const detail = () => resolvePlugin(client, record, "Imported overview"); + const validate = () => validateArtifact(client, record, { previous: record, registryOverview: "Imported overview" }); + return { record, detail, validate, base }; +} + +for (const kind of ["npm", "git"] as const) { + test(`${kind}: publication and validation read metadata from the pinned manifest`, async (t) => { + const f = fixture(t, kind, { + id: "example", name: "Author name", description: "Manifest description", icon: "assets/icon.png", + media: ["assets/demo.mp4", "assets/screen shot.PNG"], futureField: true, + }); + const detail = await f.detail(); + assert.equal(detail.name, "Author name"); + assert.equal(detail.icon, `${f.base}assets/icon.png`); + assert.deepEqual(detail.media, [`${f.base}assets/demo.mp4`, `${f.base}assets/screen%20shot.PNG`]); + assert.equal(detail.description, kind === "npm" ? "Package description" : "Manifest description"); + assert.deepEqual(await f.validate(), []); + }); + + test(`${kind}: registry overrides win per field and unused manifest files need not exist`, async (t) => { + const f = fixture(t, kind, { name: "Author name", icon: "missing.png", media: ["missing.png"] }, { + name: "Maintainer name", icon: "https://example.test/override.png", media: [], + }); + f.record.categories = ["utils"]; + const detail = await f.detail(); + assert.equal(detail.name, "Maintainer name"); + assert.equal(detail.icon, "https://example.test/override.png"); + assert.deepEqual(detail.media, []); + }); + + test(`${kind}: overriding only the name preserves manifest icon and media`, async (t) => { + const f = fixture(t, kind, { name: "Author name", icon: "assets/icon.png", media: ["assets/screen shot.PNG"] }, { name: "Issue title" }); + const detail = await f.detail(); + assert.equal(detail.name, "Issue title"); + assert.equal(detail.icon, `${f.base}assets/icon.png`); + assert.deepEqual(detail.media, [`${f.base}assets/screen%20shot.PNG`]); + }); + + test(`${kind}: an explicit empty media override is preserved regardless of category`, async (t) => { + const f = fixture(t, kind, { media: ["assets/screen shot.PNG"] }, { media: [] }); + assert.deepEqual((await f.detail()).media, []); + assert.deepEqual(await f.validate(), []); + }); + + test(`${kind}: missing metadata uses existing defaults and ignores the retired listing file`, async (t) => { + const f = fixture(t, kind, { id: "example" }); + f.record.categories = ["utils"]; + const detail = await f.detail(); + assert.equal(detail.name, "Example"); + assert.equal(detail.icon, undefined); + assert.deepEqual(detail.media, []); + }); + + test(`${kind}: a Themes category does not impose screenshot requirements`, async (t) => { + const f = fixture(t, kind, { media: ["assets/demo.mp4"] }); + assert.deepEqual((await f.detail()).media, [`${f.base}assets/demo.mp4`]); + assert.deepEqual(await f.validate(), []); + }); + + test(`${kind}: media references are published for review without requiring an image`, async (t) => { + const f = fixture(t, kind, { media: ["https://example.test/demo.mp4", "assets/review.png"] }); + assert.deepEqual((await f.detail()).media, ["https://example.test/demo.mp4", `${f.base}assets/review.png`]); + assert.deepEqual(await f.validate(), []); + }); + + for (const [label, manifest, pattern] of [ + ["path traversal", { media: ["../outside.png"] }, /inside|relative/], + ["absolute path", { icon: "/icon.png", media: [] }, /inside|relative/], + ["invalid media type", { media: "assets/icon.png" }, /array/], + ["invalid name", { name: 42 }, /name/], + ["unsupported format", { media: ["https://example.test/image.svg"] }, /png|image/], + ] as const) { + test(`${kind}: ${label} gives an author-fixable error`, async (t) => { + const f = fixture(t, kind, manifest); + await assert.rejects(f.validate(), (error: Error) => error instanceof AuthorError && pattern.test(error.message)); + }); + } +} diff --git a/scripts/lib/metadata.ts b/scripts/lib/metadata.ts new file mode 100644 index 0000000..21ac1f9 --- /dev/null +++ b/scripts/lib/metadata.ts @@ -0,0 +1,88 @@ +import { readOptional, withGitArtifact } from "./git-artifact.ts"; +import { mediaKind } from "./media.ts"; +import { type NpmClient, resolveVersion } from "./npm.ts"; +import { AuthorError } from "./problems.ts"; +import type { PluginRecord } from "./record.ts"; + +export interface PluginMetadata { + name: string; + description: string; + icon?: string; + media: string[]; +} + +/** Read metadata at the artifact pin and apply the registry's per-field overrides. + * Categories do not determine a plugin's type or its content requirements. + */ +export async function resolveMetadata(client: NpmClient, record: PluginRecord): Promise { + const artifact = record.artifact; + if (artifact.kind === "git") { + return withGitArtifact(record, (directory) => { + const base = `${artifact.remote.replace(/\.git$/, "")}/raw/${artifact.commit}/${artifact.pluginPath ? `${encodePath(artifact.pluginPath)}/` : ""}`; + return readMetadata(readOptional(directory, "paseo-plugin.json"), record, base); + }); + } + const doc = resolveVersion(await client.packument(artifact.package), artifact.version); + if (doc.dist.integrity !== artifact.integrity || doc.dist.tarball !== artifact.resolved) { + throw new Error(`${artifact.package}@${artifact.version} integrity on npm differs from the pinned record`); + } + const manifest = await client.file(artifact.package, artifact.version, "paseo-plugin.json"); + const base = `https://cdn.jsdelivr.net/npm/${artifact.package}@${artifact.version}/`; + return readMetadata(manifest, record, base); +} + +function readMetadata(text: string | null, record: PluginRecord, base: string): PluginMetadata { + if (text === null) throw new AuthorError("The package does not contain paseo-plugin.json. Include the manifest and publish a new release."); + let manifest: Record; + try { + const raw: unknown = JSON.parse(text); + if (!raw || typeof raw !== "object" || Array.isArray(raw)) throw new Error(); + manifest = raw as Record; + } catch { + throw new AuthorError("paseo-plugin.json must be a JSON object. Fix the manifest and publish a new release."); + } + const name = manifest.name; + if (name !== undefined && (typeof name !== "string" || !name.trim())) invalid("name", "a non-empty string"); + const icon = manifest.icon; + if (icon !== undefined && (typeof icon !== "string" || !isRelativePath(icon) || !/\.png$/i.test(icon))) { + invalid("icon", "a relative PNG path inside the package, such as assets/icon.png"); + } + const media = manifest.media; + if (media !== undefined) { + if (!Array.isArray(media)) invalid("media", "an array of image or video paths or HTTPS URLs"); + for (const [index, value] of (media as unknown[]).entries()) { + if (typeof value !== "string" || !(value.startsWith("https://") ? mediaKind(value) : isRelativePath(value) && mediaKind(`${base}${encodePath(value)}`))) { + invalid(`media[${index}]`, "an HTTPS image/video URL or a relative path inside the package (png, jpg, jpeg, webp, gif, mp4, webm)"); + } + } + } + + const assetUrl = (value: string): string => { + if (value.startsWith("https://")) return value; + return `${base}${encodePath(value)}`; + }; + const selectedIcon = record.listing?.icon ?? (icon as string | undefined); + const selectedMedia = record.listing?.media ?? (media as string[] | undefined) ?? []; + return { + name: record.listing?.name ?? (name as string | undefined) ?? humanizeId(record.id), + description: typeof manifest.description === "string" ? manifest.description : "", + ...(selectedIcon !== undefined ? { icon: assetUrl(selectedIcon) } : {}), + media: selectedMedia.map(assetUrl), + }; +} + +function invalid(field: string, expected: string): never { + throw new AuthorError(`paseo-plugin.json: ${field} must be ${expected}. Fix the field and publish a new release.`); +} + +function isRelativePath(value: string): boolean { + return !/[\\:?#\x00-\x1f\x7f]/.test(value) && value.split("/").every((part) => part !== "" && part !== "." && part !== ".."); +} + +function encodePath(path: string): string { + return path.split("/").map(encodeURIComponent).join("/"); +} + +function humanizeId(id: string): string { + return id.split("/").at(-1)!.split("-").map((part) => part.charAt(0).toUpperCase() + part.slice(1)).join(" "); +} diff --git a/scripts/lib/record.ts b/scripts/lib/record.ts index b1dda07..167f801 100644 --- a/scripts/lib/record.ts +++ b/scripts/lib/record.ts @@ -20,7 +20,7 @@ export interface PluginRecord { submittedBy?: string; /** Date the pinned version was approved. */ reviewedAt: string; - /** Overrides for packages that do not ship paseo-listing.json. */ + /** Per-field overrides for metadata from the pinned paseo-plugin.json. */ listing?: PluginListingOverrides; } diff --git a/scripts/lib/validate-artifact.ts b/scripts/lib/validate-artifact.ts index 75ec7ce..d6dfb66 100644 --- a/scripts/lib/validate-artifact.ts +++ b/scripts/lib/validate-artifact.ts @@ -1,6 +1,6 @@ -import { AuthorError } from "./problems.ts"; +import { resolveMetadata } from "./metadata.ts"; import { readAuthorOverview, requireOverview, validateOverview } from "./overview.ts"; -import { type NpmClient, resolveVersion } from "./npm.ts"; +import type { NpmClient } from "./npm.ts"; import { parseArtifact, type PluginRecord } from "./record.ts"; /** Check the pinned artifact without running any plugin code. */ @@ -18,19 +18,11 @@ export async function validateArtifact( const imported = unchanged || (!previous && context.allowNewImport); const registry = validateOverview(context.registryOverview ?? null, `${record.id}.md`); requireOverview(author, imported ? registry : null, record.id); + await resolveMetadata(client, record); if (record.artifact.kind === "git") return problems; const artifact = record.artifact; - const packument = await client.packument(artifact.package); - const doc = resolveVersion(packument, artifact.version); - if (doc.dist.tarball !== artifact.resolved) - problems.push(`${record.id}: tarball URL differs from pin`); - if (doc.dist.integrity !== artifact.integrity) - problems.push(`${record.id}: integrity does not match npm for ${artifact.version}`); - if ((await client.file(doc.name, doc.version, "paseo-plugin.json")) === null) { - throw new AuthorError(`${record.id}: ${artifact.version} does not ship paseo-plugin.json. Include the manifest at the package root and publish a new version.`); - } if (record.repository?.commit) { - const provenance = await client.provenance(doc.name, doc.version); + const provenance = await client.provenance(artifact.package, artifact.version); if (!provenance || provenance.commit !== record.repository.commit) { problems.push(`${record.id}: repository.commit is not backed by npm provenance`); } diff --git a/scripts/lib/validate-registry.ts b/scripts/lib/validate-registry.ts index 4a51ac8..fca5d3e 100644 --- a/scripts/lib/validate-registry.ts +++ b/scripts/lib/validate-registry.ts @@ -4,7 +4,7 @@ import { readFeatured } from "./featured.ts"; import { createNpmClient, type NpmClient } from "./npm.ts"; import { readOverview } from "./overview.ts"; import { readRecords } from "./record.ts"; -import { checkMediaUrls, mediaKind } from "./media.ts"; +import { checkMediaUrls } from "./media.ts"; import { git } from "./shell.ts"; export class RegistryValidationError extends Error { @@ -28,12 +28,6 @@ export async function validateRegistry(options: { const known = categorySlugs(readCategories()); const records = readRecords(known); readFeatured(records); - const themesWithoutImages = records.filter((record) => - record.categories.includes("themes") && !record.listing?.media?.some((url) => mediaKind(url) === "image"), - ); - if (themesWithoutImages.length) { - throw new Error(themesWithoutImages.map((record) => `${record.id}: themes require at least one image. A maintainer must supply record media; the submission form does not collect images.`).join("\n")); - } for (const record of records) readOverview(record.id); console.log(`${records.length} record(s) are well-formed`); if (!online) return; diff --git a/scripts/submit.test.ts b/scripts/submit.test.ts index ce11bc7..7f19f0b 100644 --- a/scripts/submit.test.ts +++ b/scripts/submit.test.ts @@ -194,15 +194,13 @@ test("missing author overview reaches the author with a release instruction", (t assert.doesNotMatch(comment, /Command failed|Edit the issue to fix it/); }); -test("theme metadata unsupported by the form is routed to maintainers without blaming the author", (t) => { +test("a submission categorized as Themes reaches PR review without screenshots", (t) => { const f = fixture(t, "github:acme/example:plugins/review"); writeFileSync(f.issueFile, JSON.stringify({ ...f.issue, body: f.issue.body.replace("- [ ] Themes", "- [x] Themes") })); const result = f.run(); - assert.equal(result.status, 1); - assert.equal(f.calls().some((args) => args[1] === "comment"), false); - assert.equal(f.calls().some((args) => args.includes("--add-label") && args.includes("needs-maintainer")), true); - assert.match(result.stderr, /image/i); - assert.match(readFileSync(f.summaryFile, "utf8"), /image.*maintainer/i); + assert.equal(result.status, 0, result.stderr); + assert.equal(f.calls().some((args) => args[0] === "pr" && args[1] === "create"), true); + assert.equal(f.calls().some((args) => args.includes("needs-maintainer")), false); }); test("infrastructure failure is logged and labeled, without asking the author to edit", (t) => { diff --git a/scripts/theme-media.test.ts b/scripts/theme-media.test.ts index 18e373e..9b08874 100644 --- a/scripts/theme-media.test.ts +++ b/scripts/theme-media.test.ts @@ -6,7 +6,7 @@ import { join } from "node:path"; import { fileURLToPath } from "node:url"; import { test } from "node:test"; -test("validation requires an image on every theme record", (t) => { +test("offline validation treats Themes as a category, not a plugin type", (t) => { const registry = mkdtempSync(join(tmpdir(), "theme-media-test-")); t.after(() => rmSync(registry, { recursive: true, force: true })); cpSync(fileURLToPath(new URL(".", import.meta.url)), join(registry, "scripts"), { recursive: true }); @@ -25,8 +25,7 @@ test("validation requires an image on every theme record", (t) => { }; for (const listing of [undefined, { media: [] }, { media: ["https://example.test/demo.mp4"] }]) { const result = validate({ ...record, listing }); - assert.notEqual(result.status, 0); - assert.match(result.stderr, /acme\/example: themes require at least one image/); + assert.equal(result.status, 0, result.stderr); } assert.equal(validate({ ...record, listing: { media: ["https://example.test/demo.webm", "https://example.test/screen.PNG?size=2"] } }).status, 0); assert.equal(validate({ ...record, categories: ["utils"] }).status, 0);