feat(marketplace): add unified marketplace machinery (2/3)#25
Open
sidneyswift wants to merge 1 commit into
Open
feat(marketplace): add unified marketplace machinery (2/3)#25sidneyswift wants to merge 1 commit into
sidneyswift wants to merge 1 commit into
Conversation
📝 WalkthroughWalkthroughThe PR consolidates marketplace configuration into a single source of truth ( ChangesMarketplace Consolidation & Generation
Marketplace Validation & CI
Documentation & Contribution Workflow
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
2 tasks
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
3 issues found across 13 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="README.md">
<violation number="1" location="README.md:12">
P1: Custom agent: **Flag AI Slop and Fabricated Changes**
README references `music-catalog-diligence` plugin files, install commands, and directory tree, but `plugins/music-catalog-diligence/` does not exist and the plugin is not registered in `marketplace.source.json`.</violation>
</file>
<file name="scripts/validate-manifests.py">
<violation number="1" location="scripts/validate-manifests.py:135">
P2: `source` paths are only checked for existence, not that they are directories, so invalid plugin manifests can pass validation.</violation>
<violation number="2" location="scripts/validate-manifests.py:140">
P1: Virtual plugin `skills` entries are not validated as skill directories with valid `SKILL.md` frontmatter, so malformed marketplace entries can pass CI.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
sidneyswift
force-pushed
the
feat/marketplace-machinery
branch
from
cc7923a to
afe7858
Compare
sidneyswift
changed the title
sidneyswift
force-pushed
the
feat/marketplace-machinery
branch
3 times, most recently
from
ad0dffb to
972245c
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 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-marketplaces.py`:
- Around line 149-154: The code currently calls path.parent.mkdir(...) before
checking the check flag, causing side effects in --check mode; change the flow
in the generate script so you only create directories and write files when not
in check mode by moving or guarding the mkdir and any file-write operations
behind a condition that ensures if check is True you do not call
path.parent.mkdir or modify the filesystem—use the existing/content/check
variables (path, existing, content, check) to first read path.exists()/content,
return True/False as appropriate, and only perform path.parent.mkdir(...) and
subsequent writes when check is False.
- Around line 81-83: The "interface" dict currently hardcodes "displayName":
"Recoup"; change this to pull the display name from the canonical source file
(marketplace.source.json) instead of a literal string: locate the code that
builds the "interface" object (the block assigning the "interface" dictionary
and key "displayName") and replace the hardcoded "Recoup" with the value read
from the loaded marketplace source data (e.g., use the parsed
marketplace.source.json entry for this marketplace, such as
source_data["displayName"] or marketplace_source["interface"]["displayName"]) so
the generator derives the displayName from the source-of-truth.
In `@scripts/validate-manifests.py`:
- Around line 134-149: The current validation allows paths like "../..." that
resolve outside the repository; update both the source check (variable target)
and the skill checks (variable resolved) to reject any resolved path that is not
inside REPO_ROOT. Concretely, after computing target = (REPO_ROOT /
source).resolve() and after computing resolved = (REPO_ROOT /
skill_path).resolve(), add a containment test against REPO_ROOT.resolve() (e.g.,
ensure REPO_ROOT.resolve() is equal to or is a parent of the resolved path) and
call report(errors, ...) with the same messages if the path is outside the repo;
keep the existing existence/is_dir checks unchanged. Ensure you reference the
same identifiers: REPO_ROOT, target, resolved, source, skill_path,
plugin.get("skills"), report, and name.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: 02d10401-2243-4cb7-99ce-2b2e78ec0fe3
📒 Files selected for processing (12)
.agents/plugins/marketplace.json.claude-plugin/marketplace.json.claude-plugin/plugin.json.codex-plugin/plugin.json.cursor-plugin/marketplace.json.github/workflows/validate.ymlAGENTS.mdREADME.mdcontributing.mdmarketplace.source.jsonscripts/generate-marketplaces.pyscripts/validate-manifests.py
💤 Files with no reviewable changes (2)
- .codex-plugin/plugin.json
- .claude-plugin/plugin.json
Comment on lines
+81
to
+83
| "interface": { | ||
| "displayName": "Recoup", | ||
| }, |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick win
Derive Codex displayName from source-of-truth instead of hardcoding.
"Recoup" is hardcoded here, so this file can drift from marketplace.source.json during future branding edits.
Proposed patch
- "interface": {
- "displayName": "Recoup",
- },
+ "interface": {
+ "displayName": mp["name"],
+ },🤖 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 `@scripts/generate-marketplaces.py` around lines 81 - 83, The "interface" dict
currently hardcodes "displayName": "Recoup"; change this to pull the display
name from the canonical source file (marketplace.source.json) instead of a
literal string: locate the code that builds the "interface" object (the block
assigning the "interface" dictionary and key "displayName") and replace the
hardcoded "Recoup" with the value read from the loaded marketplace source data
(e.g., use the parsed marketplace.source.json entry for this marketplace, such
as source_data["displayName"] or marketplace_source["interface"]["displayName"])
so the generator derives the displayName from the source-of-truth.
Comment on lines
+149
to
+154
| path.parent.mkdir(parents=True, exist_ok=True) | ||
| existing = path.read_text() if path.exists() else None | ||
| if existing == content: | ||
| return True | ||
| if check: | ||
| return False |
There was a problem hiding this comment.
Keep --check mode read-only.
This path creates directories even in check mode, which introduces side effects in validation/CI runs.
Proposed patch
def write_if_changed(path: Path, content: str, *, check: bool) -> bool:
"""Return True when path matches content; False when it differs."""
- path.parent.mkdir(parents=True, exist_ok=True)
+ if not check:
+ path.parent.mkdir(parents=True, exist_ok=True)
existing = path.read_text() if path.exists() else None📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| path.parent.mkdir(parents=True, exist_ok=True) | |
| existing = path.read_text() if path.exists() else None | |
| if existing == content: | |
| return True | |
| if check: | |
| return False | |
| if not check: | |
| path.parent.mkdir(parents=True, exist_ok=True) | |
| existing = path.read_text() if path.exists() else None | |
| if existing == content: | |
| return True | |
| if check: | |
| return False |
🤖 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 `@scripts/generate-marketplaces.py` around lines 149 - 154, The code currently
calls path.parent.mkdir(...) before checking the check flag, causing side
effects in --check mode; change the flow in the generate script so you only
create directories and write files when not in check mode by moving or guarding
the mkdir and any file-write operations behind a condition that ensures if check
is True you do not call path.parent.mkdir or modify the filesystem—use the
existing/content/check variables (path, existing, content, check) to first read
path.exists()/content, return True/False as appropriate, and only perform
path.parent.mkdir(...) and subsequent writes when check is False.
Comment on lines
+134
to
+149
| target = (REPO_ROOT / source).resolve() | ||
| if not target.exists(): | ||
| report(errors, f"plugin `{name}`: source path `{source}` does not exist") | ||
| continue | ||
| if not target.is_dir(): | ||
| report(errors, f"plugin `{name}`: source path `{source}` is not a directory") | ||
| continue | ||
| if plugin.get("type") == "virtual": | ||
| for skill_path in plugin.get("skills", []): | ||
| resolved = (REPO_ROOT / skill_path).resolve() | ||
| if not resolved.exists(): | ||
| report(errors, f"plugin `{name}`: skill path `{skill_path}` does not exist") | ||
| continue | ||
| if not resolved.is_dir(): | ||
| report(errors, f"plugin `{name}`: skill path `{skill_path}` is not a directory") | ||
| continue |
There was a problem hiding this comment.
Reject plugin/skill paths that resolve outside the repository.
Current checks allow ../... paths as long as they exist, so external directories can pass validation and leak out of repo scope.
Proposed patch
def check_source_paths(errors: list[str]) -> None:
print("→ checking marketplace.source.json plugin paths")
with SOURCE_FILE.open() as fh:
data = json.load(fh)
+ repo_root_resolved = REPO_ROOT.resolve()
+
+ def _is_within_repo(path: Path) -> bool:
+ try:
+ path.relative_to(repo_root_resolved)
+ return True
+ except ValueError:
+ return False
+
for plugin in data.get("plugins", []):
name = plugin["name"]
source = plugin["source"]
target = (REPO_ROOT / source).resolve()
+ if not _is_within_repo(target):
+ report(errors, f"plugin `{name}`: source path `{source}` resolves outside repository")
+ continue
if not target.exists():
report(errors, f"plugin `{name}`: source path `{source}` does not exist")
continue
if not target.is_dir():
report(errors, f"plugin `{name}`: source path `{source}` is not a directory")
continue
if plugin.get("type") == "virtual":
for skill_path in plugin.get("skills", []):
resolved = (REPO_ROOT / skill_path).resolve()
+ if not _is_within_repo(resolved):
+ report(errors, f"plugin `{name}`: skill path `{skill_path}` resolves outside repository")
+ continue
if not resolved.exists():
report(errors, f"plugin `{name}`: skill path `{skill_path}` does not exist")
continue🤖 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 `@scripts/validate-manifests.py` around lines 134 - 149, The current validation
allows paths like "../..." that resolve outside the repository; update both the
source check (variable target) and the skill checks (variable resolved) to
reject any resolved path that is not inside REPO_ROOT. Concretely, after
computing target = (REPO_ROOT / source).resolve() and after computing resolved =
(REPO_ROOT / skill_path).resolve(), add a containment test against
REPO_ROOT.resolve() (e.g., ensure REPO_ROOT.resolve() is equal to or is a parent
of the resolved path) and call report(errors, ...) with the same messages if the
path is outside the repo; keep the existing existence/is_dir checks unchanged.
Ensure you reference the same identifiers: REPO_ROOT, target, resolved, source,
skill_path, plugin.get("skills"), report, and name.
…nding) Sets up the single-source-of-truth marketplace pattern. After this lands, adding a new plugin is one entry in marketplace.source.json + a regen. No end-user behavior change yet — this PR ships the infrastructure only. What this PR adds - marketplace.source.json — single source of truth for the marketplace + the recoup-skills virtual plugin (the existing 12 broad music skills). - scripts/generate-marketplaces.py — emits the three platform manifests: .claude-plugin/marketplace.json, .agents/plugins/marketplace.json, .cursor-plugin/marketplace.json. --check mode detects drift. - scripts/validate-manifests.py — checks generator parity, plugin source paths, .claude/.codex/.cursor plugin manifests on each plugins/ folder, and SKILL.md frontmatter (name + description) on every skill. - .github/workflows/validate.yml — runs both scripts on every PR. - AGENTS.md, README.md, contributing.md, CHANGELOG.md — describe the source-of-truth + generated-files pattern (with recoup branding). - Removes the now-superseded root .claude-plugin/plugin.json and .codex-plugin/plugin.json (the recoup-skills virtual plugin entry in the marketplace replaces them). Branding - Marketplace name: recoup - Plugin name: recoup-skills - Display names: Recoup Skills, developerName: Recoup - Owner/author: Recoup - Install path: @recoup - recoupable.com URLs, agent@recoupable.com, recoupable/skills repo path, @recoupable/cli npm package, and LICENSE entity stay unchanged. After this lands /plugin marketplace add recoupable/skills /plugin install recoup-skills@recoup Why this is small enough to review - 13 files, +961 / -137. All-new infrastructure, zero behavior change for end users (no skill paths under skills/ moved). - Generated files are mechanically derivable from marketplace.source.json. Spot-check the source file; trust the generator. What's intentionally NOT in this PR - No catalog plugin (separate PR — adds plugins/music-catalog-diligence/ directory and registers it in marketplace.source.json). - No getting-started orchestrator rewrite (separate PR). Test plan - python3 scripts/generate-marketplaces.py --check → exit 0 - python3 scripts/validate-manifests.py → exit 0 Replaces #23 (split into 3 PRs; rename baked into each). Co-authored-by: Cursor <cursoragent@cursor.com>
sidneyswift
force-pushed
the
feat/marketplace-machinery
branch
from
972245c to
6794f24
Compare
sidneyswift
changed the title
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
1 participant
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.
The problem
Recoup ships AI-agent skills and plugins, but they live in 3 separate GitHub repos. Customers have to install 3 different marketplaces. Each marketplace has 3 platform-specific manifest files (Claude Code, Codex, Cursor) — 9 files total, all hand-maintained, all drifting out of sync.
The fix
One source of truth:
marketplace.source.json. A generator script produces the 3 platform marketplace files. CI catches drift on every PR.After this lands, customers add ONE marketplace and install whichever plugins they want:
This PR ships only the machinery + the existing 12 broad music skills as a virtual plugin. Vertical plugins (catalog) ship in #27.
Files (12)
marketplace.source.json— single source of truth (edit this, regenerate)scripts/generate-marketplaces.py— produces the 3 platform manifests from sourcescripts/validate-manifests.py— checks parity, paths, plugin manifests, SKILL.md frontmatter.github/workflows/validate.yml— runs validator on every PR.claude-plugin/marketplace.json,.agents/plugins/marketplace.json,.cursor-plugin/marketplace.json— generated, don't editAGENTS.md,README.md,contributing.md— describe the new patternWhat you should focus on
marketplace.source.json— does the schema look right? Are the fields what we want a plugin to declare?scripts/generate-marketplaces.py— is the generation logic obvious enough that the next person can extend it?marketplace.jsonfiles — don't read line-by-line. Trust--check.Test plan
python3 scripts/generate-marketplaces.py --checkexits 0python3 scripts/validate-manifests.pyexits 0Merge order
PR 2 of 3. Independent of #29 (which can land before or after this). #27 is stacked on this — must land before #27.
Merge preference
Please merge yourself when satisfied — I'd rather you push the button than approve.
🤖 Made with Cursor