-
Notifications
You must be signed in to change notification settings - Fork 2
validate the root meta-plugin and reject wildcards in required/optional lists #2
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,5 +1,5 @@ | ||
| #!/usr/bin/env node | ||
| // Validates the structure of every plugin under plugins/. | ||
| // Validates the root meta-plugin and the structure of every plugin under plugins/. | ||
| // Run: node scripts/validate-template.mjs | ||
|
|
||
| import { readdirSync, readFileSync, existsSync, statSync } from "node:fs"; | ||
|
|
@@ -58,65 +58,75 @@ function checkSkillLike(dir, kind) { | |
| } | ||
| } | ||
|
|
||
| if (!existsSync(pluginsDir)) { | ||
| fail("plugins/ directory not found"); | ||
| } else { | ||
| const names = new Set(); | ||
| for (const entry of readdirSync(pluginsDir)) { | ||
| const dir = join(pluginsDir, entry); | ||
| if (!statSync(dir).isDirectory()) continue; | ||
| const names = new Set(); | ||
|
|
||
| const manifestPath = join(dir, ".devin-plugin", "plugin.json"); | ||
| const label = `plugins/${entry}`; | ||
| if (!existsSync(manifestPath)) { | ||
| fail(`${label}: missing .devin-plugin/plugin.json`); | ||
| continue; | ||
| } | ||
| const manifest = parseJson(manifestPath, `${label}/.devin-plugin/plugin.json`); | ||
| if (!manifest) continue; | ||
| function checkPlugin(dir, label, { dirName } = {}) { | ||
| const manifestPath = join(dir, ".devin-plugin", "plugin.json"); | ||
| if (!existsSync(manifestPath)) { | ||
| fail(`${label}: missing .devin-plugin/plugin.json`); | ||
| return; | ||
| } | ||
| const manifest = parseJson(manifestPath, `${label}/.devin-plugin/plugin.json`); | ||
| if (!manifest) return; | ||
|
|
||
| if (!manifest.name) fail(`${label}: manifest missing "name"`); | ||
| else { | ||
| if (!kebab.test(manifest.name)) | ||
| fail(`${label}: name "${manifest.name}" must be lowercase kebab-case`); | ||
| if (names.has(manifest.name)) fail(`${label}: duplicate plugin name "${manifest.name}"`); | ||
| names.add(manifest.name); | ||
| if (manifest.name !== entry) | ||
| fail(`${label}: manifest name "${manifest.name}" should match directory "${entry}"`); | ||
| } | ||
| if (manifest.version && !/^\d+\.\d+\.\d+([-+].*)?$/.test(manifest.version)) | ||
| fail(`${label}: version "${manifest.version}" is not semver`); | ||
| for (const list of ["requiredPlugins", "optionalPlugins", "forbiddenPlugins"]) { | ||
| if (manifest[list] !== undefined && !Array.isArray(manifest[list])) | ||
| fail(`${label}: "${list}" must be an array`); | ||
| } | ||
| // Same-repo git-subdir references must point at real sibling plugins. | ||
| for (const list of ["requiredPlugins", "optionalPlugins"]) { | ||
| for (const ref of Array.isArray(manifest[list]) ? manifest[list] : []) { | ||
| if ( | ||
| typeof ref === "object" && | ||
| ref !== null && | ||
| ref.source === "git-subdir" && | ||
| typeof ref.url === "string" && | ||
| /\/team-marketplace-template(\.git)?$/.test(ref.url) && | ||
| typeof ref.path === "string" | ||
| ) { | ||
| const target = join(root, ref.path, ".devin-plugin", "plugin.json"); | ||
| if (!existsSync(target)) | ||
| fail(`${label}: ${list} entry "${ref.path}" does not resolve to a plugin in this repo`); | ||
| } | ||
| if (!manifest.name) fail(`${label}: manifest missing "name"`); | ||
| else { | ||
| if (!kebab.test(manifest.name)) | ||
| fail(`${label}: name "${manifest.name}" must be lowercase kebab-case`); | ||
| if (names.has(manifest.name)) fail(`${label}: duplicate plugin name "${manifest.name}"`); | ||
| names.add(manifest.name); | ||
| if (dirName && manifest.name !== dirName) | ||
| fail(`${label}: manifest name "${manifest.name}" should match directory "${dirName}"`); | ||
| } | ||
| if (manifest.version && !/^\d+\.\d+\.\d+([-+].*)?$/.test(manifest.version)) | ||
| fail(`${label}: version "${manifest.version}" is not semver`); | ||
| for (const list of ["requiredPlugins", "optionalPlugins", "forbiddenPlugins"]) { | ||
| if (manifest[list] !== undefined && !Array.isArray(manifest[list])) | ||
| fail(`${label}: "${list}" must be an array`); | ||
| } | ||
| for (const list of ["requiredPlugins", "optionalPlugins"]) { | ||
| for (const ref of Array.isArray(manifest[list]) ? manifest[list] : []) { | ||
| // Required/optional entries are exact source refs; wildcards are only | ||
| // legal in forbiddenPlugins and make the whole manifest unparseable. | ||
| if (typeof ref === "string" && ref.includes("*")) | ||
| fail(`${label}: ${list} entry "${ref}" contains a wildcard — globs are only allowed in forbiddenPlugins`); | ||
| // Same-repo git-subdir references must point at real sibling plugins. | ||
| if ( | ||
| typeof ref === "object" && | ||
| ref !== null && | ||
| ref.source === "git-subdir" && | ||
| typeof ref.url === "string" && | ||
| /\/team-marketplace-template(\.git)?$/.test(ref.url) && | ||
| typeof ref.path === "string" | ||
| ) { | ||
| const target = join(root, ref.path, ".devin-plugin", "plugin.json"); | ||
| if (!existsSync(target)) | ||
| fail(`${label}: ${list} entry "${ref.path}" does not resolve to a plugin in this repo`); | ||
| } | ||
| } | ||
| } | ||
|
|
||
| checkSkillLike(join(dir, "skills"), "skill"); | ||
| checkSkillLike(join(dir, "agents"), "agent"); | ||
| checkSkillLike(join(dir, "skills"), "skill"); | ||
| checkSkillLike(join(dir, "agents"), "agent"); | ||
|
|
||
| for (const file of ["hooks.json", "mcp_config.json"]) { | ||
| const p = join(dir, file); | ||
| if (existsSync(p)) parseJson(p, `${label}/${file}`); | ||
| } | ||
| for (const file of ["hooks.json", "mcp_config.json"]) { | ||
| const p = join(dir, file); | ||
| if (existsSync(p)) parseJson(p, `${label}/${file}`); | ||
| } | ||
| } | ||
|
|
||
| // The repo root is itself a plugin — the meta-plugin admins install. | ||
| checkPlugin(root, "<root>"); | ||
|
|
||
| if (!existsSync(pluginsDir)) { | ||
| fail("plugins/ directory not found"); | ||
| } else { | ||
| for (const entry of readdirSync(pluginsDir)) { | ||
| const dir = join(pluginsDir, entry); | ||
| if (!statSync(dir).isDirectory()) continue; | ||
| checkPlugin(dir, `plugins/${entry}`, { dirName: entry }); | ||
| } | ||
| if (names.size === 0) fail("no plugins found under plugins/"); | ||
| if (names.size < 2) fail("no plugins found under plugins/"); | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🟡 Validator can wrongly claim there are no plugins, or stay silent when there really are none The count used to decide whether any plugins exist ( How the shared name set conflates the root plugin with the plugins/ entries
A robust fix is to count plugins encountered in the plugins/ loop with a dedicated counter rather than reusing the shared name set. Prompt for agentsWas this helpful? React with 👍 or 👎 to provide feedback. Debug |
||
| } | ||
|
|
||
| if (errors.length) { | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🔍 Wildcard check only inspects string entries, not object refs
The new guard only rejects wildcards for string entries in
requiredPlugins/optionalPlugins. An object ref such as{ "source": "git-subdir", "path": "plugins/*" }(or a wildcard inurl) would slip through, and the sibling-resolution check below would just report a non-resolving path (or nothing, if the URL isn't this repo). If the runtime rejects globs anywhere in required/optional refs, consider also checkingref.path/ref.urlfor*.Was this helpful? React with 👍 or 👎 to provide feedback.
Debug
Playground