Fix domain plugin manifests: all 10 packs fail install validation - #469
Merged
Conversation
…lidation (#468) commands/skills/agents path entries were missing the required "./" prefix (Claude Code plugin manifest spec: "all paths must be relative to the plugin root and start with ./") and agents pointed at bare directories where the schema only accepts individual .md file paths. Fixed the generator and regenerated all 10 domain plugin.json manifests + marketplace.json. Live-verified: /plugin install backend (and the other 9 domain packs) now install cleanly with no manifest validation errors.
Strix Security ReviewNo security issues found. Updated for Reviewed by Strix |
Contributor
📝 WalkthroughWalkthroughPlugin manifests now enumerate individual agent Markdown files, while command and skill directories use ChangesPlugin manifest path expansion
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant DomainGenerator
participant PluginFolders
participant DomainManifest
participant Inventory
DomainGenerator->>PluginFolders: inspect agent files and resource directories
PluginFolders-->>DomainGenerator: return discovered paths
DomainGenerator->>DomainManifest: write explicit agents and ./ resource paths
Inventory->>DomainManifest: read manifest entries
Inventory->>PluginFolders: resolve files or directories
PluginFolders-->>Inventory: return Markdown files
Possibly related issues
Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
PR Summary by QodoFix consolidated domain plugin manifests to pass Claude Code install validation
AI Description
Diagram
High-Level Assessment
Files changed (11)
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@scripts/generate-domain-plugins.mjs`:
- Around line 94-102: Update the manifest.agents consumer in the inventory
backend, including its collectAgentItems flow, to detect explicit file entries
and collect them directly while continuing to enumerate directory entries for
legacy manifests. Preserve the file-level agent paths emitted by the generator
and enforce this distinction at runtime before directory traversal.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 091d6e31-e491-4238-88b0-624919842a06
📒 Files selected for processing (11)
plugins/backend/plugin.jsonplugins/data-ml/plugin.jsonplugins/devops-cloud/plugin.jsonplugins/docs-content/plugin.jsonplugins/frontend-mobile/plugin.jsonplugins/languages/plugin.jsonplugins/product-team/plugin.jsonplugins/quality-testing/plugin.jsonplugins/security/plugin.jsonplugins/specialized/plugin.jsonscripts/generate-domain-plugins.mjs
Code Review by Qodo
Context used✅ Compliance rules (platform):
300 rules✅ Skills:
|
![qodo-code-review[bot]](https://avatars.githubusercontent.com/in/484649?s=60&v=4){kind=small}
Comment on lines
+89
to
+102
| for (const kind of ['commands', 'skills']) { | ||
| const paths = subDirs.filter((sub) => existsSync(join(PLUGINS_DIR, domain, sub, kind))) | ||
| .map((sub) => `${sub}/${kind}`); | ||
| .map((sub) => `./${sub}/${kind}`); | ||
| if (paths.length) entry[kind] = paths; | ||
| } | ||
| const agentPaths = subDirs.flatMap((sub) => { | ||
| const dir = join(PLUGINS_DIR, domain, sub, 'agents'); | ||
| if (!existsSync(dir)) return []; | ||
| return readdirSync(dir) | ||
| .filter((f) => f.endsWith('.md') && statSync(join(dir, f)).isFile()) | ||
| .sort() | ||
| .map((f) => `./${sub}/agents/${f}`); | ||
| }); | ||
| if (agentPaths.length) entry.agents = agentPaths; |
There was a problem hiding this comment.
1. No tests for manifest fix 📘 Rule violation ▣ Testability
This PR changes install-critical manifest generation logic (path prefixing and agent file expansion) but does not add/modify any automated tests to exercise the new behavior. Without test coverage, future refactors may reintroduce the same install validation failure undetected.
Agent Prompt
## Issue description
The PR changes `scripts/generate-domain-plugins.mjs` to generate different manifest paths and to expand `agents/` directories into individual `.md` entries, but no automated test was added/updated to assert this behavior.
## Issue Context
Compliance requires that non-trivial behavior modifications ship with tests in the same changeset to prevent regressions (e.g., paths must start with `./`, and `agents` must list `.md` files rather than directories).
## Fix Focus Areas
- scripts/generate-domain-plugins.mjs[89-102]
- tests/domain-plugin-manifest-generation.test.js[1-200]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
Owner
Author
There was a problem hiding this comment.
✅ Fixed — added tests/generate-domain-plugins.test.js: fixture-based unit tests pinning the ./-prefix rule and the agents-as-individual-files rule, plus a static assertion that all 10 committed domain manifests conform. Exported buildDomainManifest with an injectable pluginsDir param (same pattern as generate-marketplace.mjs's buildMarketplace(root)) so it's testable against a fixture tree instead of only the real plugins/ dir. CLI behavior unchanged.
Comment on lines
10
to
+18
| "agents": [ | ||
| "api-scaffolding/agents", | ||
| "api-testing-observability/agents", | ||
| "backend-api-security/agents", | ||
| "backend-development/agents", | ||
| "c4-architecture/agents", | ||
| "database-cloud-optimization/agents", | ||
| "database-design/agents", | ||
| "database-migrations/agents", | ||
| "full-stack-orchestration/agents" | ||
| "./api-scaffolding/agents/api-scaffolding-backend-architect.md", | ||
| "./api-scaffolding/agents/api-scaffolding-graphql-architect.md", | ||
| "./api-scaffolding/agents/django-pro.md", | ||
| "./api-scaffolding/agents/fastapi-pro.md", | ||
| "./api-testing-observability/agents/api-documenter.md", | ||
| "./backend-api-security/agents/backend-api-security-backend-architect.md", | ||
| "./backend-api-security/agents/backend-security-coder.md", | ||
| "./backend-development/agents/backend-development-backend-architect.md", |
There was a problem hiding this comment.
2. Inventory enotdir on agents 🐞 Bug ≡ Correctness
Domain plugin manifests now list individual agent .md files in manifest.agents, but backend inventory treats each entry as a directory and calls readdir() on it. This will deterministically throw ENOTDIR and break rstack-agents inventory (and any other inventory builds) for these domain plugins.
Agent Prompt
## Issue description
Domain plugin manifests now provide `agents` as a list of individual `./.../*.md` files, but `src/core/inventory/backend-inventory.js` assumes `manifest.agents` contains directories and passes each entry into `listFilesRecursive(...)`, which calls `readdir(...)`. When the entry is a file, this throws `ENOTDIR` and breaks inventory generation.
## Issue Context
This PR intentionally changed domain `plugin.json` manifests to satisfy Claude Code manifest validation rules (agents must be files, path must start with `./`). Repo tooling must accept both shapes (directory-based and file-based) to remain compatible.
## Fix Focus Areas
- src/core/inventory/backend-inventory.js[82-95]
- src/core/inventory/backend-inventory.js[223-255]
## Implementation notes
- Update the inventory plugin scanning to support `manifest.agents` entries that are either:
- directories (existing behavior), OR
- individual `.md` files (new domain-manifest behavior).
- One robust approach:
- Replace `listFilesRecursive(dir, ...)` with logic that `stat`s the path:
- if directory: recurse as today
- if file: treat it as a singleton candidate (include it if it matches predicate)
- Ensure the same logic is safe for `commands` and `skills` if they ever become file-based (optional), but at minimum fix `agents` so current manifests don’t crash inventory.
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
Owner
Author
There was a problem hiding this comment.
✅ Fixed — already fixed in this same PR by an earlier commit (5b1aa49), pushed before this review ran: added resolveMarkdownFiles() in src/core/inventory/backend-inventory.js, which stats each manifest path and handles both directories and individual .md files instead of assuming scandir() always works. Verified via tests/backend-inventory.test.js (5/5 pass) and this PR's CI (Node 18.x/20.x/22.x test jobs, now green).
…t change buildBackendInventory's collectAgentItems assumed every plugin.json "agents" entry was a directory and called readdir() on it directly. That broke once domain manifests started listing individual .md agent files (per this PR's earlier commit), tripping ENOTDIR in CI (backend-inventory.test.js, caught by the Node 18/20/22 test jobs). resolveMarkdownFiles() now stats each entry and handles either shape (directory -> recursive walk, single .md file -> itself) — matching Claude Code's own plugin manifest schema, which documents `agents` as accepting both directories and individual files.
…#469) Qodo flagged that the ./-prefix + agents-as-files generator change shipped with no test coverage. Export buildDomainManifest with an injectable pluginsDir (same pattern as generate-marketplace.mjs's buildMarketplace(root)) so it's testable against a fixture tree instead of only the real plugins/ dir (whose per-sub-plugin manifests are deleted after a real generation run). Adds: - fixture-based unit tests for the ./-prefix and agents-as-files rules - a static assertion that all 10 committed domain manifests conform CLI behavior (buildDomainManifest(domain) defaulting to the real plugins/ dir) is unchanged.
Owner
Author
Qodo Fix Summary — Round 1Reviewed and addressed Qodo review issues: ✅ Fixed Issues
|
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@tests/generate-domain-plugins.test.js`:
- Around line 104-117: Update the manifest validation loop in the
generate-domain-plugins test to resolve each agents, commands, and skills entry
against its domain root and verify the target exists. Require agents entries to
resolve to regular .md files, and require commands and skills entries to resolve
to directories, while retaining the existing prefix and agent-extension checks.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro Plus
Run ID: fe1cda62-e162-486e-975a-86f391c85b2f
📒 Files selected for processing (3)
scripts/generate-domain-plugins.mjssrc/core/inventory/backend-inventory.jstests/generate-domain-plugins.test.js
Comment on lines
+104
to
+117
| for (const domain of domains) { | ||
| const manifestPath = join(pluginsDir, domain, 'plugin.json'); | ||
| const parsed = JSON.parse(readFileSync(manifestPath, 'utf8')); | ||
| for (const kind of ['agents', 'commands', 'skills']) { | ||
| if (!Array.isArray(parsed[kind])) continue; | ||
| for (const entry of parsed[kind]) { | ||
| assert.ok(entry.startsWith('./'), `${domain}: ${kind} entry "${entry}" must start with ./`); | ||
| } | ||
| } | ||
| if (Array.isArray(parsed.agents)) { | ||
| for (const entry of parsed.agents) { | ||
| assert.ok(entry.endsWith('.md'), `${domain}: agents entry "${entry}" must be an individual .md file`); | ||
| } | ||
| } |
Contributor
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win
Validate committed manifest targets, not just their strings.
A deleted target—or a directory named *.md—still passes these assertions. Resolve each entry under its domain root; require agent targets to be regular files and generated command/skill targets to be directories.
As per coding guidelines, “Add regression and mutation-oriented tests for … data-integrity behavior; tests should exercise the real path where mocks could hide integration bugs.”
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@tests/generate-domain-plugins.test.js` around lines 104 - 117, Update the
manifest validation loop in the generate-domain-plugins test to resolve each
agents, commands, and skills entry against its domain root and verify the target
exists. Require agents entries to resolve to regular .md files, and require
commands and skills entries to resolve to directories, while retaining the
existing prefix and agent-extension checks.
Source: Coding guidelines
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
backend,data-ml,devops-cloud,docs-content,frontend-mobile,languages,product-team,quality-testing,security,specialized) failed/plugin installwithValidation errors: commands: Invalid input, agents: Invalid input, skills: Invalid input../;generate-domain-plugins.mjsbuilt bare paths ("api-scaffolding/agents") with no prefix.commands/skillsaccept directory paths, butagentsonly accepts individual.mdfile paths per the documented schema — a bareagentsdirectory still failed. The generator now expands each sub-plugin'sagents/folder into its individual file entries instead.plugins/<domain>/plugin.jsonmanifests +.claude-plugin/marketplace.jsonfrom the fixed generator (the original per-sub-plugin manifests it reads from were already removed by the earlier domain-consolidation cleanup, so entries were patched to match what the fixed generator now produces byte-for-byte).Test plan
node scripts/generate-marketplace.mjs— marketplace.json regenerates identically (no diff)npx tsx --test tests/marketplace-conformance.test.js— 4/4 pass, including "marketplace.json matches a fresh generation from plugins/*/plugin.json"/plugin install backend(and the other domain packs) now install cleanly with zero manifest validation errors, vs. failing 100% of the time before this fixSummary by CodeRabbit
Improvements
Bug Fixes
Tests