Skip to content

validate the root meta-plugin and reject wildcards in required/optional lists - #2

Closed
charliemeyer2000 wants to merge 1 commit into
mainfrom
devin/1785125147-validate-root
Closed

validate the root meta-plugin and reject wildcards in required/optional lists#2
charliemeyer2000 wants to merge 1 commit into
mainfrom
devin/1785125147-validate-root

Conversation

@charliemeyer2000

@charliemeyer2000 charliemeyer2000 commented Jul 27, 2026

Copy link
Copy Markdown
Contributor

Summary

CI passed on the commit that broke the meta-plugin (c54cabf) because the validator only looked at plugins/* — it never checked the root manifest after the meta-plugin moved to the repo root. Two fixes to scripts/validate-template.mjs:

  • Extract the per-plugin checks into checkPlugin(dir, label) and run it on the repo root too (skipping the name-matches-directory check there).
  • Add the exact check that would have caught the bug: string entries in requiredPlugins/optionalPlugins containing * fail validation ("globs are only allowed in forbiddenPlugins"), matching the runtime's wildcard rejection.

Verified: passes on current main; fails on the c54cabf manifest with
<root>: optionalPlugins entry "CognitionAI/**" contains a wildcard — globs are only allowed in forbiddenPlugins.

Link to Devin session: https://staging.itsdev.in/sessions/4dedf92ab76a44c78c984fb138682bb0
Requested by: @charliemeyer2000


Devin Review

Status Commit
🟢 Reviewed 9953ac0
Open in Devin Review (Staging)

…al lists

Co-Authored-By: Staging-Devin AI <166158716+staging-devin-ai-integration[bot]@users.noreply.github.com>
@staging-devin-ai-integration

Copy link
Copy Markdown
Original prompt from charlie.meyer

SYSTEM:
=== BEGIN THREAD HISTORY (in #cmeyer-internal) ===
<most_recent_message>
Charlie Meyer (U0AS1R4CM5M) [ts=1785099102.637089]: @Devin help me build enablement materials for plugins. look at notion and C3: Git Plugin marketplace to undersatand what's going on, then help me underastand the state of how plugins work on cloud and local, what morgante is asking for, and then a plan to build this --action-model claude-fable-5-max

Let me search for those threads now.

Those results don't match what you're looking for. Let me search more specifically for mortgante (likely Marco Morgante or similar) talking about enablement materials for plugins with a GitHub repo example.

Got it — that's Morgante Pell. Let me search for his messages about enablement materials and plugins with a GitHub repo example.

Found one — now let me look for the second thread. The first one is from
<#C0B15442C4E>
(eng-plugins-rollout). Let me search for another message from Morgante about enablement/plugins with a usacognition repo example.

I'm clearly finding the same one thread. The second one may involve a
usacognition example repo specifically — let me check if there's another thread where Morgante shared one of those repos.

Let me check the thread on that first message and also look for the second thread — it may have been a reply in a thread rather than a top-level message, or it could be in a different channel.

I found one thread clearly. Now let me look more broadly — the second thread may involve a
usacognition example repo that Morgante shared, possibly in a different channel or as a reply.

I can see a reference in the search results to a second thread — Morgante's "Quick QA on plugins" thread from July 23 against &lt;http://github.com/COG-GTM/plugins-marketplace|github.com/COG-GTM/plugins-marketplace&gt;. Let me find that one.

Found both threads. Here they are:

Thread 1 — Tagging you for enablement materials ownership <https://cognition.enterprise.slack.com/arch... (2957 chars truncated...)

@staging-devin-ai-integration

Copy link
Copy Markdown

🤖 Devin AI Engineer

I'll be helping with this pull request! Here's what you should know:

✅ I will automatically:

  • Address comments on this PR. Add '(aside)' to your comment to have me ignore it.
  • Look at CI failures and help fix them

⚙️ Control Options:

  • Disable automatic comment, CI, and merge conflict monitoring

@staging-devin-ai-integration staging-devin-ai-integration Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Devin Review found 2 potential issues.

Open in Devin Review (Staging)
Debug

Playground

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/");

Copy link
Copy Markdown

Choose a reason for hiding this comment

The 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 (names.size < 2 at scripts/validate-template.mjs:129) now also includes the repo-root entry, so the check no longer reflects how many plugins actually live under the plugins folder.
Impact: A repository with exactly one valid plugin is reported as having none whenever the root manifest is missing/invalid, and the message shown is misleading.

How the shared name set conflates the root plugin with the plugins/ entries

names is now module-level (scripts/validate-template.mjs:61) and checkPlugin adds the root manifest's name to it (scripts/validate-template.mjs:77) before the plugins/ loop runs. Two failure modes:

  1. Root manifest missing/invalid JSON/missing namecheckPlugin returns early without adding a name, so a repo with one legitimate plugin yields names.size === 1 and emits the spurious no plugins found under plugins/ error.
  2. Conversely, if two plugins under plugins/ share the same name, the set dedupes them, again skewing the count (duplicate is separately reported, but the count is not a plugin count).

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 agents
In scripts/validate-template.mjs, the 'no plugins found under plugins/' check was changed from names.size === 0 to names.size < 2 because the shared `names` set now also receives the root meta-plugin's name from checkPlugin(). This makes the count depend on whether the root manifest parsed successfully and on name uniqueness, so a repo with one valid plugin plus a broken root manifest reports a misleading 'no plugins found' error. Track the number of plugin directories actually validated under plugins/ with a separate counter (incremented in the loop) and use that for the emptiness check, keeping `names` solely for duplicate detection.
Open in Devin Review (Staging)

Was this helpful? React with 👍 or 👎 to provide feedback.

Debug

Playground

Comment on lines +91 to +92
if (typeof ref === "string" && ref.includes("*"))
fail(`${label}: ${list} entry "${ref}" contains a wildcard — globs are only allowed in forbiddenPlugins`);

Copy link
Copy Markdown

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 in url) 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 checking ref.path/ref.url for *.

Open in Devin Review (Staging)

Was this helpful? React with 👍 or 👎 to provide feedback.

Debug

Playground

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant