Repository navigation
fix(plugin-dev): declare skills.sh skills in generated Codex manifests - #300
Conversation
The multi-format generator omitted `skills` from Codex manifests whenever the Claude manifest pointed at `./.agents/skills/`, so the skills of every skills.sh plugin never loaded under Codex. The Codex runtime accepts any `./`-relative path inside the plugin root and always loads root `skills/` as well; only the sample plugin-creator validator insists on `./skills/`. - Pass any valid in-root skills path through; from an array prefer the first non-root path, since Codex already auto-loads root `skills/`. - Declare `./skills/` explicitly for plugins that ship a root `skills/` dir without a manifest field. - Emit the `Skill` capability only when the Codex manifest declares `skills`, and drop the `Skill` fallback. - Update the generated Codex manifests (skills and capabilities only; the unrelated drift a full regeneration produces is left out). Closes #296
|
Warning You have reached your daily quota limit. Please wait up to 24 hours and I will start processing your requests again! |
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Up to standards ✅🟢 Issues
|
| Metric | Results |
|---|---|
| Complexity | 17 |
| Duplication | 0 |
NEW Get contextual insights on your PRs based on Codacy's metrics, along with PR and Jira context, without leaving GitHub. Enable AI reviewer
TIP This summary will be updated as you push new changes.
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
 [Medium impact] The PR appears safe to merge; no blocking issues were found. SummaryGenerated Codex manifests now declare the skills directories that plugins actually use.
Reviews (2) · Last reviewed commit: "fix(plugin-dev): reject backslash separa..." · Reviewed by Greptile |
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
cubic analysis
All reported issues were addressed across 71 files
You’re at about 95% of the monthly reviewed-line limit. You may want to disable incremental reviews to conserve quota. Reviews will continue until that limit is exceeded. If you need help avoiding interruptions, please contact contact@cubic.dev.
Linked issue analysis
Linked issue: pleaseai/claude-code-plugins#296: Review follow-up: Codex manifests omit skills for .agents/skills plugins in multi-format.ts
| Status | Acceptance criteria | Notes |
|---|---|---|
| ✅ | Declare .agents skill paths in generated Codex manifests | multi-format.ts now preserves valid in-root paths, and regenerated manifests declare ./.agents/skills/ for affected plugins. |
Architecture diagram
sequenceDiagram
participant CLI as multi-format CLI
participant Gen as generateForPlugin()
participant FS as Filesystem
participant Conv as toCodexManifest()
participant Skills as normalizeSkillsPath()
participant Caps as deriveCapabilities()
participant Out as .codex-plugin/plugin.json
participant Codex as Codex Runtime
Note over CLI,Out: Plugin manifest generation flow
CLI->>Gen: Process plugin directory
Gen->>FS: Check root skills/ directory
FS-->>Gen: hasRootSkillsDir flag
Gen->>Conv: toCodexManifest(claude, entry, { hasRootSkillsDir })
Conv->>Skills: Normalize Claude skills path
alt Claude declares skills array
Skills->>Skills: Validate each path (reject ../, /abs, ./ only)
Skills->>Skills: Prefer first non-root path (e.g. ./.agents/skills/)
Skills-->>Conv: Valid Codex skills path or null
else Claude declares single skills path
Skills->>Skills: Validate and normalize path
Skills-->>Conv: Valid Codex skills path or null
else No skills declared but root skills/ exists
Skills-->>Conv: ./skills/
else No skills and no root dir
Skills-->>Conv: null
end
Conv->>Caps: deriveCapabilities(claude, hasSkills)
alt hasSkills is true
Caps->>Caps: Include "Skill" capability
else no skills declared
Caps->>Caps: Omit "Skill" capability
end
Note over Caps: Also checks MCP servers (Tool) and hooks (Interactive)
Caps-->>Conv: capabilities array (may be empty)
opt skillsPath is not null
Conv->>Out: Write "skills": "./.agents/skills/" or "./skills/"
end
Conv->>Out: Write capabilities array
Gen->>Out: Write .codex-plugin/plugin.json
Out-->>Codex: Manifest with skills path
Note over Codex: Runtime behavior
Codex->>Codex: Load root skills/ automatically
Codex->>Codex: Load manifest skills path (any ./-relative in-root)
Codex-->>Out: Skills now load correctly
alt Invalid skills path (../, absolute, or empty)
Skills-->>Conv: null
Conv->>Out: Omit skills field
Out->>Codex: No skills declared
Codex->>Codex: Only auto-loaded root skills/ (if any)
end
Reply with feedback, questions, or to request a fix.
Turn on auto-fix | Re-trigger cubic
|
Summary
multi-format.tsomittedskillsfrom.codex-plugin/plugin.jsonwhenever the Claude manifest used./.agents/skills/, so the skills of 53 skills.sh plugins never loaded under Codex.deriveCapabilitiesalso advertisedSkillwhenever Claude declared skills, and as a fallback when nothing else applied (for example gatekeeper, which has no skills). The Codex runtime accepts any./-relative path inside the plugin root (codex-rs/core-plugins/src/manifest.rsresolve_manifest_path) and always loads rootskills/plus the manifestskillspath (loader.rsplugin_skill_roots), so the generator now declares the real path.What changed
toCodexSkillsPathrejects absolute,.., empty, and./-only paths.normalizeSkillsPathprefers the first non-root path from an array (better-auth and mastra resolve to./.agents/skills/), since Codex auto-loads rootskills/.toCodexManifest(claude, entry, { hasRootSkillsDir })declares./skills/explicitly for plugins that have a rootskills/dir but no manifest field.generateForPluginpasses the flag.Skillcapability appears if and only if the Codex manifest declaresskills. TheSkillfallback is removed (gatekeeper now gets[])..codex-plugin/plugin.jsonfiles; onlyskillsandinterface.capabilitieschanged (67 gained or changed askillsline).Scope
Out of scope, as follow-ups:
deriveCapabilitiesignoreshooks/hooks.json(auto-loaded, not in the manifest), so hook-only plugins like gatekeeper get noInteractive.multi-formatdrift. A full run on main already changes about 153 unrelated files (Antigravity$schema, stale versions, codegraph hook path, untracked portone.mcp.json); these are deliberately excluded.Tradeoffs
Only the sample plugin-creator
validate_plugin.pyrequires exactlyskills; nothing in this repo's CI runs it, so declaring non-default paths is safe. Existingskillskeys keep their position; new ones sit where the generator emits them (afterkeywords, beforemcpServers).Blast radius
Only Codex manifests change, and only
skillsandcapabilities. portone's Codex manifest is hand-curated (privacyPolicyURL,brandColor), so onlySkillwas added to its existing["Interactive","Write"]. A full regen would still rewrite it to["Skill","Tool"]; that drift predates this PR.Verification
bun test plugins/plugin-dev/scripts/multi-format.test.ts: 83 pass, 0 fail. New tests cover.agents/skillspassthrough, array preference, invalid paths omitted, the root-dir flag, andSkillonly when skills are declared. The oldcapabilities toContain("Skill")assertion on a skills-less manifest asserted the bug and now expects[].bun run lintpasses.bun scripts/cli.ts multi-formatchanges noskillsorcapabilitiesline in any Codex manifest except portone's pre-existing curated drift.fnm: not foundlocally, so lint-staged may not have run; CI is the lint gate.Related issue
Closes #296
Checklist
bun run test)bun run lint)BREAKING CHANGE:note is includedSummary by cubic
Generated Codex manifests now declare skills for the 53 skills.sh plugins that point at
./.agents/skills/, so those skills actually load under Codex instead of being silently omitted.normalizeSkillsPathnow accepts any valid in-root path so./.agents/skills/passes through, omitting absolute,.., empty,./-only, and backslash-separated paths; from an array it prefers the first non-root path, since Codex auto-loads rootskills/.skills/dir but no manifest field get an explicit./skills/declaration.Skillcapability now appears only when the Codex manifest declares skills; the fallback that wrongly advertisedSkill(e.g. gatekeeper) is removed..codex-plugin/plugin.jsonfiles; onlyskillsandinterface.capabilitieschanged.Closes #296.
Written for commit afc7803. Summary will update on new commits.