Add Codex plugin marketplace support - #47
Conversation
|
🤖 Finished Review · ✅ Success · Started 5:49 PM UTC · Completed 6:01 PM UTC |
WalkthroughAdds Codex plugin support to the ai-helpers repository by introducing ChangesCodex Plugin Integration
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
Makefile (1)
51-63:⚠️ Potential issue | 🟠 Major | 🏗️ Heavy liftUpdate
new-plugintarget to align with the new Codex plugin structure.The
new-plugintarget creates only.claude-plugin/directories and registers plugins in.claude-plugin/marketplace.json(line 57), but this PR introduces.codex-plugin/plugin.jsonmanifests and.agents/plugins/marketplace.jsonas the new marketplace registry. Developers usingmake new-pluginwill create plugins that don't follow the new structure being established by this PR.Consider updating the target to:
- Create both
.codex-plugin/and.claude-plugin/directories for dual-agent support, or- Update to create only
.codex-plugin/and register in.agents/plugins/marketplace.json, or- Document in the Makefile which plugin structure is current and update the target accordingly.
🤖 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 `@Makefile` around lines 51 - 63, The new-plugin target creates plugins using the old .claude-plugin/ structure and registers them in .claude-plugin/marketplace.json, but the PR introduces a new .codex-plugin/ structure with .agents/plugins/marketplace.json as the marketplace registry. Update the new-plugin target to align with the new plugin structure by either creating both .codex-plugin/ and .claude-plugin/ directories for backwards compatibility, or migrate entirely to create only .codex-plugin/ and register in .agents/plugins/marketplace.json. Ensure the mkdir command creates the appropriate directories, the plugin.json is written to the correct location, and the Python script updates the correct marketplace.json file.
🧹 Nitpick comments (1)
.agents/plugins/marketplace.json (1)
1-44: 💤 Low valueSchema deviation from marketplace template.
The marketplace config deviates from the template schema in
.claude-plugin/marketplace.json.templatein three ways:
Missing
owner.namefield: The template definesowner.name(template lines 3-4), but this file usesinterface.displayNameinstead (lines 3-5). The build script has a fallback chain that handles this gracefully (build-website.py:170-174), so this works but differs from the documented contract.Plugin
sourceformat: Each plugin uses an object format{"source": "local", "path": "..."}(lines 9-12, 21-24, 33-36), whereas the template shows a simple string path"./plugins/example-plugin". The build script correctly handles both (build-website.py:182-185), but document whether the object format is the new Codex standard.Missing per-plugin
description: The template includes adescriptionfield for each plugin entry (template line 11), but this file omits it. The build script falls back to reading descriptions from each plugin'splugin.json(build-website.py:210), so metadata is not lost, but the schema is inconsistent.Consider either updating the template to match the new Codex schema or adding the missing fields for consistency. The current implementation is functional due to robust fallback logic.
🤖 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 @.agents/plugins/marketplace.json around lines 1 - 44, Update the marketplace.json file to align with the template schema by making three changes: First, replace the interface.displayName field with an owner object containing a name field matching the template structure. Second, simplify the source field in each plugin entry (dev, jira, and openshift-testing) from the object format with source and path properties to a simple string path value. Third, add a description field to each of the three plugin entries to match the template definition. These changes ensure consistency with the documented schema while maintaining the metadata currently provided through fallback mechanisms.
🤖 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 `@docs/data.json`:
- Line 7: The description property in the data.json file enumerates various
skills in a comma-separated list but omits "debug-playwright" even though this
skill is defined as a separate entry elsewhere in the file. Add
"debug-playwright" to the skill enumeration in the description string to
maintain consistency between the documentation and the actual skill definitions
in the JSON structure.
---
Outside diff comments:
In `@Makefile`:
- Around line 51-63: The new-plugin target creates plugins using the old
.claude-plugin/ structure and registers them in .claude-plugin/marketplace.json,
but the PR introduces a new .codex-plugin/ structure with
.agents/plugins/marketplace.json as the marketplace registry. Update the
new-plugin target to align with the new plugin structure by either creating both
.codex-plugin/ and .claude-plugin/ directories for backwards compatibility, or
migrate entirely to create only .codex-plugin/ and register in
.agents/plugins/marketplace.json. Ensure the mkdir command creates the
appropriate directories, the plugin.json is written to the correct location, and
the Python script updates the correct marketplace.json file.
---
Nitpick comments:
In @.agents/plugins/marketplace.json:
- Around line 1-44: Update the marketplace.json file to align with the template
schema by making three changes: First, replace the interface.displayName field
with an owner object containing a name field matching the template structure.
Second, simplify the source field in each plugin entry (dev, jira, and
openshift-testing) from the object format with source and path properties to a
simple string path value. Third, add a description field to each of the three
plugin entries to match the template definition. These changes ensure
consistency with the documented schema while maintaining the metadata currently
provided through fallback mechanisms.
🪄 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: Organization UI
Review profile: CHILL
Plan: Enterprise
Run ID: 781fc797-8e96-4633-a481-12ed2b6bd478
📒 Files selected for processing (10)
.agents/plugins/marketplace.jsonMakefilePLUGINS.mdREADME.mddocs/data.jsonplugins/dev/.codex-plugin/plugin.jsonplugins/jira/.codex-plugin/plugin.jsonplugins/openshift-testing/.codex-plugin/plugin.jsonscripts/build-website.pyscripts/generate_plugin_docs.py
📜 Review details
🔇 Additional comments (18)
plugins/dev/.codex-plugin/plugin.json (1)
1-27: LGTM!plugins/jira/.codex-plugin/plugin.json (1)
1-26: LGTM!plugins/openshift-testing/.codex-plugin/plugin.json (1)
1-26: LGTM!scripts/build-website.py (5)
163-165: LGTM!
170-174: LGTM!
182-185: LGTM!
188-190: LGTM!
210-210: LGTM!scripts/generate_plugin_docs.py (2)
79-81: LGTM!
149-149: LGTM!README.md (3)
139-165: LGTM!
169-178: LGTM!
28-44: Codex CLI command syntax is correct.The command
codex plugin marketplace add .referenced in line 37 is the valid syntax for adding a local plugin marketplace to Codex using the current directory shorthand.PLUGINS.md (1)
3-3: LGTM!docs/data.json (3)
3-3: LGTM!
86-86: LGTM!
146-146: LGTM!Makefile (1)
1-1: LGTM!
| { | ||
| "name": "dev", | ||
| "description": "Ralph Loop development lifecycle: start, code, pr, poll, ci, backport, work", | ||
| "description": "Ralph Loop development lifecycle: ticket assignment through merge-ready PR. Includes start, code, pr, poll, ci, backport, work, grill-with-docs, to-prd, to-issues, and handoff.", |
There was a problem hiding this comment.
Add "debug-playwright" to the skill list in the description.
The description lists skills but omits "debug-playwright", which is defined as a skill at lines 34-37. For consistency and discoverability, include it in the enumeration.
📝 Suggested fix
- "description": "Ralph Loop development lifecycle: ticket assignment through merge-ready PR. Includes start, code, pr, poll, ci, backport, work, grill-with-docs, to-prd, to-issues, and handoff.",
+ "description": "Ralph Loop development lifecycle: ticket assignment through merge-ready PR. Includes start, code, pr, poll, ci, backport, work, grill-with-docs, to-prd, to-issues, handoff, and debug-playwright.",🤖 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 `@docs/data.json` at line 7, The description property in the data.json file
enumerates various skills in a comma-separated list but omits "debug-playwright"
even though this skill is defined as a separate entry elsewhere in the file. Add
"debug-playwright" to the skill enumeration in the description string to
maintain consistency between the documentation and the actual skill definitions
in the JSON structure.
| ## Installation | ||
|
|
||
| ### Via Lola (recommended) | ||
| ### Via Codex Plugin |
There was a problem hiding this comment.
do we want to keep the instructions for Lola?
ReviewFindingsHigh
Medium
Low
Info
|
![fullsend-ai-review[bot]](https://avatars.githubusercontent.com/in/3478501?s=60&v=4){kind=small}
| # Get repository root (parent of scripts directory) | ||
| base_path = Path(__file__).parent.parent | ||
| marketplace_file = base_path / ".claude-plugin" / "marketplace.json" | ||
| marketplace_file = base_path / ".agents" / "plugins" / "marketplace.json" |
There was a problem hiding this comment.
[medium] logic-error
The new .agents/plugins/marketplace.json has no 'owner' key. The fallback logic resolves owner to 'Quay AI Helpers' (from interface.displayName), silently changing the website data owner from 'quay' to 'Quay AI Helpers'.
Suggested fix: Verify the owner value change is intentional. If it should remain 'quay', add an explicit owner field to .agents/plugins/marketplace.json.
3 participants
No description provided.