Add first-class OpenAI Codex plugin and marketplace linting - #451
Add first-class OpenAI Codex plugin and marketplace linting#451stbenjam wants to merge 50 commits into
Conversation
Codex plugin (`.codex-plugin/plugin.json`) and marketplace (`.agents/plugins/marketplace.json`) manifests were invisible to skillsaw — not detected, not in the lint tree, not linted. This adds four rules, two repo types, and two lint-tree nodes for them. Codex gets its own rules rather than overloading the Claude ones. The schemas are not compatible: Claude requires `owner` (0 of the Codex catalogs surveyed have it), Codex's source types include `local` (209/209 entries in openai/plugins) which the Claude rule would call unknown, and Codex publishes no semver constraint on `version`. `.claude-plugin/ marketplace.json` stays owned by `marketplace-json-valid`. New rules, all `enabled: auto` gated on the new repo types: - codex-plugin-json-valid — manifest fields and every path it declares - codex-plugin-structure — "Only plugin.json belongs in .codex-plugin/" - codex-marketplace-json-valid — catalog schema, source types, policy - codex-marketplace-registration — catalog and directories agree, with a SUGGEST autofix Two pre-existing false positives on Codex repos are fixed: a bare `plugins/` directory made `marketplace-json-valid` demand a Claude manifest (fired on 13 of 35 real Codex marketplaces, openai/plugins among them), and `plugin-json-required` demanded a Claude manifest from Codex plugins that ship a `commands/` directory. A plugin the Claude marketplace itself lists is still required to have one. Codex plugin `hooks/hooks.json` is routed to the existing HooksBlock, so Codex plugins get hooks-json-valid and hooks-dangerous rather than duplicate rules. Validated against 35 real Codex repositories (~460 manifests): zero findings on openai/plugins, and every finding elsewhere traced to a real divergence — `..` escaping the plugin root, a stray `hooks.json` inside `.codex-plugin/`, an unreplaced `[TODO: ./assets/icon.png]`, duplicate catalog names, a dangling local source. ai-helpers is byte-identical on all 481 violations. Closes #446 Co-Authored-By: Claude <noreply@anthropic.com>
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds OpenAI Codex repository discovery and linting for plugin and marketplace manifests, including validation, registration autofix, Claude-rule compatibility, documentation, configuration, security hardening, and comprehensive tests. ChangesOpenAI Codex linting
Estimated code review effort: 5 (Critical) | ~120 minutes Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 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 |
github-actions
Bot
removed
the
panel-review
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #451 +/- ##
==========================================
+ Coverage 93.54% 93.88% +0.33%
==========================================
Files 145 155 +10
Lines 11418 13163 +1745
==========================================
+ Hits 10681 12358 +1677
- Misses 737 805 +68 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 63330d9830
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
github-actions
Bot
removed
the
panel-review
Review Panel VerdictDisposition: REQUEST_CHANGES
Specialist FindingsArchitecture Reviewer Re-confirmed the prior pass's read of the design: separate
Python Expert Clean, py3.9-compatible (
Security & Supply Chain Reviewer No
QA Engineer Coverage is strong and behavioral. Independently reproduced the PR's headline validation: against a fresh clone of
Technical Writer Documentation is the strongest part of this PR, and it is fully regenerated —
Ecosystem Reviewer In scope for core — no redirect. OpenAI Codex is a first-party product from one of the two vendors whose formats skillsaw most centrally targets, with a published specification at The standardization question favors core more than the PR argues. Maintenance cost is the one real concern and the reason this reviewer would have pushed back had the PR not addressed it: OpenAI publishes no JSON Schema, and On the hooks question the PR raises: keep it. See the Security finding. No ecosystem finding. Panel SynthesisThe engineering standard here remains high, and the panel re-verified the PR's claims rather than taking them on trust: 3047 tests pass, Three blocking defects, all narrow, all in new code. Two are carried forward unfixed from the prior review; one is new. The two carried-forward findings — the The Worth noting what the three findings have in common: each is a place where discovery scope and enforcement scope were assumed to coincide. The stand-down assumes "repo has a Codex catalog" implies "repo has no Claude plugins"; On the two questions the PR asks the maintainer directly, the panel's answers are unchanged: keep the separate Arbiter biases applied. Security over ergonomics: the Backward compatibility otherwise holds: no rule ID renamed, no config-format change, every new rule Required Actions Before Merge
Optional Follow-ups
Generated by skillsaw-review-panel |
Three BLOCKING findings from the panel, plus the suggestions and notes: - marketplace-json-valid stood down from "Marketplace file not found" for any repo with a Codex catalog, even one shipping real Claude plugins. It now stands down only when no discovered plugin carries .claude-plugin/plugin.json, matching plugin-json-required's rule. - codex-marketplace-registration demanded that plugins under .codex/plugins/ — where Codex installs plugins into a developer's checkout — be listed in the repository's own catalog. That failed the lint of anyone who installed a Codex plugin, and `fix --suggest` wrote the third-party install into the published catalog. They stay discovered (their hooks and skills are still linted) but are skipped by the registration check. - urlparse() raises "Invalid IPv6 URL" on an unbalanced '['; the exception escaped check() and aborted the whole rule, so the credential and scheme checks failed open on exactly the input they exist to catch. It now reports a violation. Also: empty bare-string marketplace sources are an error rather than an INFO about './'; a non-string policy value-list from config no longer crashes the rule's own error message; a duplicate entry name falls through to the kebab-case check; dead codex_marketplace_data removed; codex_plugin_name and the marketplace-path cache go through the shared readers instead of hand-rolled I/O and __dict__. Regenerates .skillsaw-card.svg, which was stale on this branch. Co-Authored-By: Claude <noreply@anthropic.com>
# Conflicts: # .skillsaw-card.svg
Co-Authored-By: Claude <noreply@anthropic.com>
Review panel findings addressedAll three BLOCKING findings are fixed, along with every SUGGESTION and NOTE. Each was reproduced first with the panel's own repro, then re-run after the fix. BLOCKING
Pure-Codex repos still stand down —
SUGGESTION / NOTE
Tests added for all of the above (5 new in Validation
Merged |
![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
🧹 Nitpick comments (3)
src/skillsaw/rules/builtin/codex/marketplace_registration.py (1)
266-334: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueAutofix re-serializes the whole catalog, so it reformats files that don't use 2-space indent.
json.dumps(data, indent=2)rewrites key ordering-preserved but re-indents the entire document, producing large diffs on catalogs formatted with 4 spaces or tabs. Consider inferring the existing indent fromoriginal(e.g. leading whitespace of the first nested line) and passing it through, so the fix stays scoped to the appended entry.🤖 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 `@src/skillsaw/rules/builtin/codex/marketplace_registration.py` around lines 266 - 334, Update the fixed_content serialization in fix to infer the marketplace file’s existing indentation style from original, including spaces versus tabs, and pass that indent to json.dumps instead of hardcoding indent=2. Preserve the existing JSON content, Unicode handling, and trailing newline while matching the catalog’s established formatting.src/skillsaw/rules/builtin/codex/__init__.py (1)
5-15: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueRemove the hand-maintained Codex re-export list.
src/skillsaw/rules.builtinauto-discovers concreteRulesubclasses, so this explicit import list plus matching__all__can drift from the actual Codex rule modules. Let the package discovery reach the four Codex rule classes directly.🤖 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 `@src/skillsaw/rules/builtin/codex/__init__.py` around lines 5 - 15, Remove the explicit rule imports and __all__ list from the Codex package initializer so discovery through rules.builtin reaches the concrete classes directly. Leave the individual Codex rule modules and their class definitions unchanged.Source: Coding guidelines
.apm/skills/skillsaw-maintenance/references/codex.md (1)
16-18: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAvoid hard-coding the catalog size in this maintenance reference.
The upstream catalog is a live collection, so “180 plugins” will age and make this reference misleading. Describe it as the official conformance corpus without a fixed count, or add a dated snapshot. (github.com)
Suggested wording
- Reference corpus: https://github.com/openai/plugins — the official catalog (180 - plugins across `marketplace.json` and `api_marketplace.json`). It is the de-facto + Reference corpus: https://github.com/openai/plugins — the official catalog and + de-facto conformance suite: skillsaw must stay silent on it.🤖 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 @.apm/skills/skillsaw-maintenance/references/codex.md around lines 16 - 18, Update the “Reference corpus” description in the maintenance reference to remove the hard-coded “180 plugins” count, while retaining the official openai/plugins catalog and its role as the conformance corpus.
🤖 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/repo-types.md`:
- Around line 85-97: Add the text language identifier to the opening fence of
each tree diagram: docs/repo-types.md lines 85-97 and 107-117, and
docs/rules/codex-plugin-structure.md lines 28-34 and 38-45. Leave the diagram
contents unchanged.
In `@src/skillsaw/rules/builtin/codex/marketplace_json_valid.py`:
- Around line 48-59: Resolve RUF012 by annotating the mutable class-level
defaults with ClassVar: update config_schema in
src/skillsaw/rules/builtin/codex/marketplace_json_valid.py (lines 48-59), and
update DEFAULT_RECOMMENDED_FIELDS plus config_schema in
src/skillsaw/rules/builtin/codex/plugin_json_valid.py (lines 32-47). Add or
reuse the appropriate typing import while preserving the existing values and
behavior.
In `@src/skillsaw/rules/docs/codex-plugin-structure.md`:
- Around line 13-30: Update the two directory-tree code fences in the plugin
structure documentation to specify the text language, changing each opening
fence to ```text while preserving the examples’ contents and closing fences.
---
Nitpick comments:
In @.apm/skills/skillsaw-maintenance/references/codex.md:
- Around line 16-18: Update the “Reference corpus” description in the
maintenance reference to remove the hard-coded “180 plugins” count, while
retaining the official openai/plugins catalog and its role as the conformance
corpus.
In `@src/skillsaw/rules/builtin/codex/__init__.py`:
- Around line 5-15: Remove the explicit rule imports and __all__ list from the
Codex package initializer so discovery through rules.builtin reaches the
concrete classes directly. Leave the individual Codex rule modules and their
class definitions unchanged.
In `@src/skillsaw/rules/builtin/codex/marketplace_registration.py`:
- Around line 266-334: Update the fixed_content serialization in fix to infer
the marketplace file’s existing indentation style from original, including
spaces versus tabs, and pass that indent to json.dumps instead of hardcoding
indent=2. Preserve the existing JSON content, Unicode handling, and trailing
newline while matching the catalog’s established formatting.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 5838c365-e62f-4230-ae6b-f329e2dd8600
⛔ Files ignored due to path filters (17)
.agents/skills/skillsaw-maintenance/SKILL.mdis excluded by!.agents/**.agents/skills/skillsaw-maintenance/references/codex.mdis excluded by!.agents/**.claude/skills/skillsaw-maintenance/SKILL.mdis excluded by!.claude/**.claude/skills/skillsaw-maintenance/references/codex.mdis excluded by!.claude/**.skillsaw-card.svgis excluded by!**/*.svgtests/fixtures/codex/broken/.agents/plugins/marketplace.jsonis excluded by!tests/fixtures/**tests/fixtures/codex/broken/plugins/escaping_paths/.codex-plugin/plugin.jsonis excluded by!tests/fixtures/**tests/fixtures/codex/broken/plugins/stray-manifest-dir/.codex-plugin/hooks.jsonis excluded by!tests/fixtures/**tests/fixtures/codex/broken/plugins/stray-manifest-dir/.codex-plugin/plugin.jsonis excluded by!tests/fixtures/**tests/fixtures/codex/broken/plugins/unregistered/.codex-plugin/plugin.jsonis excluded by!tests/fixtures/**tests/fixtures/codex/clean/.agents/plugins/marketplace.jsonis excluded by!tests/fixtures/**tests/fixtures/codex/clean/.codex/plugins/installed-helper/.codex-plugin/plugin.jsonis excluded by!tests/fixtures/**tests/fixtures/codex/clean/plugins/note-taker/.codex-plugin/plugin.jsonis excluded by!tests/fixtures/**tests/fixtures/codex/clean/plugins/note-taker/assets/logo.pngis excluded by!**/*.png,!tests/fixtures/**tests/fixtures/codex/clean/plugins/note-taker/skills/capture-notes/SKILL.mdis excluded by!tests/fixtures/**tests/fixtures/codex/clean/plugins/repo-policy/.codex-plugin/plugin.jsonis excluded by!tests/fixtures/**tests/fixtures/codex/clean/plugins/repo-policy/hooks/hooks.jsonis excluded by!tests/fixtures/**
📒 Files selected for processing (32)
.apm/skills/skillsaw-maintenance/SKILL.md.apm/skills/skillsaw-maintenance/references/codex.md.skillsaw.yaml.skillsaw.yaml.exampledocs/cli.mddocs/repo-types.mddocs/rules/codex-marketplace-json-valid.mddocs/rules/codex-marketplace-registration.mddocs/rules/codex-plugin-json-valid.mddocs/rules/codex-plugin-structure.mddocs/rules/codex.mddocs/rules/index.mdmkdocs.ymlscripts/generate-docs.pyscripts/generate-site-content.pysrc/skillsaw/context.pysrc/skillsaw/lint_target.pysrc/skillsaw/lint_tree.pysrc/skillsaw/rules/builtin/codex/__init__.pysrc/skillsaw/rules/builtin/codex/_helpers.pysrc/skillsaw/rules/builtin/codex/marketplace_json_valid.pysrc/skillsaw/rules/builtin/codex/marketplace_registration.pysrc/skillsaw/rules/builtin/codex/plugin_json_valid.pysrc/skillsaw/rules/builtin/codex/plugin_structure.pysrc/skillsaw/rules/builtin/marketplace/json_valid.pysrc/skillsaw/rules/builtin/plugins/json_required.pysrc/skillsaw/rules/docs/codex-marketplace-json-valid.mdsrc/skillsaw/rules/docs/codex-marketplace-registration.mdsrc/skillsaw/rules/docs/codex-plugin-json-valid.mdsrc/skillsaw/rules/docs/codex-plugin-structure.mdtests/test_codex_rules.pytests/test_integration.py
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 300824c5db
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: aec70d2efb
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
# Conflicts: # .skillsaw-card.svg
Four findings from the chatgpt-codex-connector review: - Route declared Codex hooks through the hook checks. A manifest may point ``hooks`` at a file other than the conventional ``hooks/hooks.json``; that file carries the same executable commands, so it now becomes a HooksBlock and gets hooks-json-valid, hooks-dangerous and hooks-prohibited. Inline hooks objects and paths that escape the plugin root are not followed. - Discover skills inside installed Codex plugins. ``.codex/plugins/`` is hidden, so the repository-wide skill scan never walks it; Codex plugin roots now join Claude plugin roots as embedded-skill roots. - Reject an empty ``source`` discriminator in a Codex marketplace entry as missing rather than as an unknown future source type, so an entry Codex cannot resolve no longer passes a default ``fail-on: error`` run. - Add Codex plugins and marketplaces to the README's supported-format summary, and document the hooks discovery contract in docs/repo-types.md. Co-Authored-By: Claude <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 253a9d923d
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
Security and correctness:
- _add_parser_block records only its (path, parser) role, never the
path-only dedupe set: a manifest declaring hooks/mcpServers at an
in-plugin markdown file silently dropped that file from every content
and prose rule that attaches later.
- Prose for every discovered Codex plugin: a plugin at the repository
root, or reached through a catalog source outside plugins/*, gets its
commands/, agents/, rules/ and README attached under its container
with the same containment check the config files use; codex-only
directories route hooks/MCP exclusively through the contained path so
a symlinked hooks.json cannot ride in on the Claude attach.
- is_codex_claimed() is filesystem-first (a contained .codex-plugin
manifest claims its directory whatever --type said), the divergent
copies in plugin-json-required and the docs extractor delegate to it,
and the Claude-format gates are directory-based so container-attached
prose is covered; an invariance test pins the --type behavior.
- codex_catalog_exists() shares discovery's positive-evidence duck-type,
so a sourceless version-pin sibling neither lints as a catalog nor
stands the Claude missing-manifest diagnostic down.
- safe_display() covers every manifest-value echo (path_problem, both
validity rules, structure, registration names, brand_color), redacts
userinfo of any length, and strips C1 controls and bidi overrides.
- _table_cell escapes Markdown link delimiters and the per-plugin page
heading routes through it — a display name carrying "](", a newline,
or a pipe arrives inert.
- The manifest-input $HOME bound compares the resolved home directory,
so a symlinked $HOME cannot slip past it.
- codex_plugin_owning() and the docs extractor's skill attribution walk
ancestors against a set instead of scanning every root per path.
Docs and hygiene: pre-commit.md matches the published files-filter hook
and states the deletion-only gap honestly; the three flagged comments
are re-tensed or restored; reserved-stem handling and remaining
optional items are tracked in review replies.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Second panel verdict addressed — all eight required actions in dd19a08
The optional follow-ups are noted; the pre-existing Claude-path Also in this push: the three verified items from the latest connector round (root-level plugin prose — the same class as panel RA1's remainder; the shared catalog duck-type in Validation: 3,445 passed / 1 xfailed / 94%; lint and |
github-actions
Bot
removed
the
panel-review
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: dd19a081cf
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
Review Panel VerdictDisposition: REQUEST_CHANGES Advisory only — this panel does not gate merge. Seventeenth round, reviewing Specialist FindingsArchitecture ReviewerPrior required actions verified: #1 ( BLOCKING — This is the only gate that ANDs Reproduced. Root One run now says both "you must add a Claude Silent coverage loss on content whose author declared Claude intent, against this repo's own "never break existing rules for users of skillsaw." Action: move the carve-out into the shared predicate rather than replicating it seven times — e.g. SUGGESTION — A2
SUGGESTION — A3 SUGGESTION — A4 SUGGESTION — A5 (prior, unfixed) — SUGGESTION — A6 (prior, unfixed) — Codex block ownership is still not expressible through the lint tree; three shims work around it ( SUGGESTION — A7 (prior, unfixed) — SUGGESTION — A8 (prior, unfixed) — provenance-keyed strictness in NOTES: Checked clean: Python ExpertBaseline health: full suite Prior required actions: #1, #3, #4, #5, #7, #8 confirmed complete. Live check on a Codex-only repo: BLOCKING — Reproduced with Raw ESC reaches the text formatter and the credential reaches the JSON/SARIF artifact — the exact T14 path the PR claims closed. The second Action: wrap SUGGESTION — for skill_resolved, skill_node in resolved_skills:
if not skill_resolved.is_relative_to(plugin_resolved): # ← 300 × 1500 calls
continueMeasured on 300 plugins × 5 skills (
All three produce byte-identical output ( NOTE — NOTE — NOTE — NOTE — NOTE — carried over, unaddressed: Checked clean: Security & Supply Chain ReviewerVerified the eight required actions against working proof-of-concept repos and a BLOCKING 1 — if not context.is_codex_only_plugin(plugin_path):
_add_block(plugin_node, plugin_path / "hooks" / "hooks.json", HooksBlock)
_add_block(plugin_node, plugin_path / "settings.json", SettingsBlock)
_add_block(plugin_node, plugin_path / "settings.local.json", SettingsBlock)
_add_block(plugin_node, plugin_path / ".mcp.json", McpBlock)
# Codex-only: the manifest loop below attaches hooks and MCP with a
# containment check ...The comment's premise is false when Confirmed 0.17 → 0.18 regression. Repo: Claude marketplace listing
A second, narrower half persists even for a well-formed Codex plugin: the Codex loop re-attaches only Action: gate the stand-down on the Codex loop actually being able to take over — skip these four BLOCKING 2 — required action 2 is incomplete; three echo sites still leak and (a) Verbatim (b) (c) safe_display('//user:ghp_SCHEMERELATIVE@host/x') -> '//user:ghp_SCHEMERELATIVE@host/x'Live: The guard test the prior verdict asked for was not added.
Action: wrap SUGGESTION 3 — Same pattern exists pre-PR in SUGGESTION 4 — SUGGESTION 5 — SUGGESTION 6 — NOTES: Verified clean, empirically with fixtures: path containment fails closed ( QA EngineerSuite verified, not assumed: I verified each of the eight required actions by building probe repos. All eight code fixes are real and work. Four shipped without the regression test the action named — and one of those missing tests is exactly what would have caught a leak that is still live. BLOCKING 1 —
@pytest.mark.parametrize("value", [
"https://u:{s}@h/x", "./https://u:{s}@h/x", "ssh://u:{s}@h/x",
"//u:{s}@h/x", "u:{s}@h/x", "{s}@github.com:o/r.git",
])
def test_userinfo_is_redacted_whatever_the_url_shape(value):
secret = "ghp_SHAPE0000000" # notsecret
assert secret not in safe_display(value.format(s=secret))BLOCKING 2 — But SUGGESTION 3 — SUGGESTION 4 — no assertion anywhere that a rule message is free of raw control characters. The required action asked for it; it would have caught SUGGESTION 5 — required-action fixtures for the dedupe poisoning and the Markdown table were not added. Both code fixes are correct and verified: SUGGESTION 6 — SUGGESTION 7 — SUGGESTION 8 — SUGGESTION 9 — SUGGESTION 10 — SUGGESTION 11 — NOTES: Checked and genuinely well covered: registration autofix idempotency, re-lint-clean, BOM/CRLF preservation, no-op without Technical WriterRe-ran the full generated-file check on a clean clone of BLOCKING — The doc's "Why" states without qualification: "A plugin must have a Same tree with This exemption is the PR's headline behavior — The PR already set the correct precedent one directory over: Concrete harm: a Codex-plugin author who reads Action: add a short scope paragraph to BLOCKING — SUGGESTION — Commit SUGGESTION — NOTES: Checked clean: generated-file sync (zero diff from both generators, Ecosystem ReviewerBottom line: CORE-WORTHY. No scope objection. No BLOCKING findings. Re-measured everything from scratch rather than trusting the prior panel or the PR body. The numbers came back higher than the PR claims.
The PR's "180 plugins, 581 skills" is accurate and now conservative. The third-party code-search numbers are new information no prior panel had: adoption exists well outside OpenAI's own catalog, roughly an order of magnitude beyond it. Supporting Claude Code plugins while rejecting Codex plugins is the position that would need defending. Standardization & churn risk: Is Core vs. plugin: SUGGESTION — NOTE — NOTE — RESOLVED from my prior round — Checked clean: rule scoping ( The maintenance-cost objection — the one that usually sinks a new core format — is the best-answered part of this PR. Slopinator ReviewerThe source comments the last panel flagged were genuinely fixed ( BLOCKING 1 —
A percentage captured while linting a corpus during one review round, presented as a permanent fact, against a sample no future reader can identify or reproduce. Stale the moment the rule changes, and it says nothing about what the test asserts. Action: BLOCKING 2 —
Identical in shape to BLOCKING 3 —
The second sentence narrates the pre-fix state of this branch but reads as a live claim — and it is now the opposite of what ships. BLOCKING 4 —
The Codex manifest file-path input form is new in this PR, so this describes a defect that existed only between two commits on this branch. Action: state the invariant in the present tense. SUGGESTION 5 — SUGGESTION 6 — SUGGESTION 7 — SUGGESTION 8 — past-tense defect narration across four more test docstrings/comments, one edit each: NOTE 9 — two rejected-alternative asides in past tense: NOTE 10 — 39 non-merge commits organized by review round, HEAD included ( Checked clean: AI-vocabulary sweep over every added line in Panel SynthesisScope is settled — merge it, once the blockers close. The Ecosystem Reviewer re-measured from scratch and came back with numbers above the PR's own: 101.8k stars on Six of the eight prior required actions are genuinely, independently verified complete by at least two specialists each: the The two incomplete actions are what block. Required action #5 (unify the Codex-only predicate) made Required action #2 (the On disagreements. Architecture and QA reached opposite conclusions on Architecture's A1 stands on its own repro and no other lane contradicts it: a plugin the author declared to Claude by listing it in The Slopinator's four items are blocking by this panel's own calibration, and by the maintainer's explicit request in this PR's thread that comments not backreference previous commits. Three are past-tense narration of defects that existed only between two commits on this branch; the fourth ( Filtered as noise or already-resolved: the Ecosystem lane's prior Trade-offs ratified. The Required Actions Before Merge
Optional Follow-upsWorth doing in this PR if convenient
Post-merge issues
Generated by skillsaw-review-panel |
Ownership of a plugin directory is now decided in exactly one place. RepositoryContext.provenance() returns a cached PluginProvenance record — which ecosystems claim the directory (claude, codex, both), on what evidence, and whether it is vendor-installed — built filesystem-first from markers, contained manifests, Claude marketplace listings, and Codex catalog files enumerated independently of discovery gating, so the answer is invariant under --type overrides. Every ownership predicate is a view over this record. The lint tree builds plugins in ONE pass over the union of claimed directories: one container per directory, prose (commands/, agents/, rules/, README) attached once with containment for every ecosystem, Claude configs and the Codex manifest cluster attached from provenance through the contained helpers. The per-ecosystem loop pair — whose gaps produced the prose-drop, the hook-drop, and this round's catalog-claimed regression — no longer exists. Hook/MCP scanning is restored for catalog-claimed plugins without a manifest (the Claude attach stands down only when the Codex cluster re-attaches), and settings.json always attaches. Adding the next ecosystem is now a recipe, documented under "Ecosystem provenance" in the development rules: an evidence probe, a catalog enumerator, a config cluster in the single pass, and a format gate — prose comes free. Also from the third panel round: the redaction sweep is complete (source_type, plugin_structure names, path_problem returns) with the userinfo pattern covering scheme-relative, bare, and scp-style credentials and the redaction test inverted to assert over every message every Codex rule produces; the Claude-marketplace carve-out lives in the shared predicate so all seven format gates honor it; the seven Claude-format rule docs state their Codex scope; four review-history docstrings are re-tensed; the --type invariance test covers the manifest-less claim; module layering covers formats/*; and rule-impact pins openai/plugins to the same SHA as test.yml. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Third panel verdict addressed in 74ec317 — with the structural fix, not another patch roundPer the maintainer's direction, this round went past the finding list to the primitive underneath it. The architecture change. Ecosystem ownership is now a first-class record: The five required actions, in that frame:
From the optional list: the Validation: 3,447 passed / 1 xfailed / 94%; lint and |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 74ec317995
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
github-actions
Bot
removed
the
panel-review
All from one connector round against the new pass — every finding verified real and fixed: - Codex-claimed directories get hooks and MCP exclusively through the contained Codex cluster: the Claude-side attach ran first for dual-manifest and manifest-less claims, without containment, so a symlinked hooks.json parsed an external file's commands (P1). - The plugin pass unions in catalog claims with no manifest and no legacy marker — such a directory had no container at all, so its hooks, prose, and skills were silently skipped while the registration rule reasoned about the same source. - settings.json attaches only for Claude-style directories: it is Claude-side configuration, and probing it in hostile Codex-only checkouts reached _add_block's bare resolve(). - The generated docs read a Codex-only plugin's commands, agents, and rules from its container (scoped by containment) instead of hard-coding them empty; the prose helpers gained block-list variants. - A Codex catalog listing ./.claude cannot flip the repository's own command content to Codex-only provenance. - Declared JSON configs recorded under parser roles are excluded from the extra-content prose pass, so a broad content glob cannot re-lint structured config as instruction text. - commented_key_line() tolerates YAML merge-key inheritance instead of crashing the rule; openai.yaml parser diagnostics pass through safe_display(); agentskill-evals-required uses the shared contained_eval_file() so an escaping symlink cannot satisfy it. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Review Panel VerdictDisposition: REQUEST_CHANGES Specialist FindingsArchitecture ReviewerNo BLOCKING issues. The SUGGESTION — SUGGESTION — SUGGESTION — SUGGESTION — seven files ( SUGGESTION — SUGGESTION — SUGGESTION — SUGGESTION — five sites ( SUGGESTION — SUGGESTION — NOTEs. Verified clean: no import cycles; Python ExpertSUGGESTION — SUGGESTION — SUGGESTION — SUGGESTION — SUGGESTION — NOTEs.
Verified clean: Python 3.9 compat (no Security & Supply Chain ReviewerAll 3447 tests pass. BLOCKING — _URL_USERINFO = re.compile(r"(?:[^/@\s:]+:[^@\s]*@)|(?:[^/@\s]+@(?=[^@\s]*[.:]))")The inline comment claims "The single character class cannot backtrack." That is wrong — each alternative has a backtrackable BLOCKING — The PR adds SUGGESTION (arbiter escalated to BLOCKING — see synthesis) — SUGGESTION — SUGGESTION — SUGGESTION — SUGGESTION — NOTEs. Violation messages are unbounded in length — the finding-1 repro produced a single 60 072-character message carried into JSON and SARIF, a payload-size hazard for code-scanning ingestion (capping in Verified clean: path containment (claim 1) — QA EngineerSuite status: clean. SUGGESTION — SUGGESTION — SUGGESTION — SUGGESTION — SUGGESTION — SUGGESTION — NOTEs. Also checked and adequate: Technical WriterSUGGESTION — SUGGESTION — SUGGESTION — SUGGESTION — SUGGESTION — SUGGESTION — NOTEs.
Verified correct: cross-read all five Ecosystem ReviewerVerdict on core-vs-plugin: correctly scoped for core. No redirect-to-plugin. Adoption clears the bar comfortably. It is not purely a single-vendor private format. Two of three surfaces are in shared namespaces: the catalog lives under Decisive point: the essential half of this work is not expressible from a plugin. The false positive exists on today's Calibration is good. On the real BLOCKING — SUGGESTION — SUGGESTION — NOTEs. Positives recorded: the Slopinator ReviewerPart B (AI prose tells) is essentially clean. Zero hits across all 60+ changed BLOCKING — BLOCKING — BLOCKING — BLOCKING — SUGGESTION — SUGGESTION — NOTE — three one-liners. (a) Checked and clean: all six new rule doc pages (both copies); Panel SynthesisThe panel ran seven specialists over 170 files and ~13.9k insertions. The feature is well-built and correctly scoped for skillsaw core — the Ecosystem Reviewer's independent adoption research (OpenAI-owned Evidence of care is unusually strong and worth naming: zero Four blocking defects survived verification, and I reproduced or confirmed each one directly rather than taking the specialist's word. Three cluster on the same theme — a security hardening layer that is thorough where it was aimed and absent one step to either side:
On the Slopinator's four BLOCKING findings: I verified each mechanically rather than by taste. Corroboration strengthened three findings. The unguarded Findings I resolved against the specialist. Architecture, the Python Expert, and QA all independently traced the One decision I am asking for rather than deciding. The Ecosystem Reviewer's case for renaming Filtered as noise or out of scope: the The shape of this review is worth stating plainly: none of the four blockers is architectural, none requires rethinking the design, and none is in the Codex feature's core logic. Three are one-to-ten-line fixes at the edges of an otherwise careful security layer; one is a one-line missing Required Actions Before Merge
Fold in while touching these files (small, corroborated):
Optional Follow-ups
Generated by skillsaw-review-panel |
- safe_display() redaction is a single-pass linear scan: the previous alternation regex backtracked quadratically on adversarial input, and an intermediate rebuild-per-@ variant was quadratic on @-storms. The scan covers scheme-full, scheme-relative, bare, and scp-style credential spellings, caps host inspection (treating an over-long host as host-like — the redacting direction), and bounds output length. An adversarial-runtime test pins linearity. - The registration autofix declines a symlinked catalog: the lexical path is what the repository ships, and resolution moving it means the write lands on a file the entry does not describe. - The Claude docs-extraction path routes homepage/repository through _safe_url(), matching the Codex path; the marketplace index heading and back-link fold through _table_cell(); a test covers javascript: and paren-bearing URLs on both paths in both renderers. - agentskill-evals' wrong-shape violation carries the document's root line (JSON parsed as YAML for position), so baselines fingerprint on line content and survive message rewording — test included. - The manifest-input $HOME bound computes inside the widening branch, guarded against an unresolvable home, with a regression test. - safe_display() covers the last three bypassed diagnostics (source_type, plugin_structure entry names, openai.yaml parser errors); module layering scans class bodies again (they execute at import); repo-types documents the sourced-entry requirement for sibling catalogs; the hooks/MCP rule docs count a Claude marketplace listing as a Claude declaration; the remaining past-tense residue in source and test docstrings is re-tensed. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Fourth panel verdict addressed in c25cbbc + c924978The headline this round was the panel's own: Architecture found no blocking issues and confirmed the provenance primitive is the right shape. All six required actions and the four fold-ins are done:
Also in this push ( Validation: 3,456 passed / 1 xfailed / 94%; lint and |
github-actions
Bot
removed
the
panel-review
The 1.0s wall-clock bound failed on the CI 3.11 job at 3.16s: coverage.py has no sys.monitoring backend before 3.12, so every line of the linear scan pays a Python-level trace call. The quadratic implementation this test guards against took >14s bare on the same inputs, so a 10s bound keeps an order of magnitude between pass and failure mode while giving slow traced runners 3x headroom. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c92497803e
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
Review Panel VerdictDisposition: REQUEST_CHANGES Advisory only — this panel does not gate merge. Fifth panel round, reviewing Specialist FindingsArchitecture ReviewerNo blocking issues in the design. The
Verified fixed, not re-raised: Python Expert
Verified sound: the Security & Supply Chain Reviewer
Verified sound: the registration autofix's symlink refusal genuinely fails closed ( Attacked, no finding: path traversal and symlink escape ( QA Engineer
Checked and adequate: the non-finite-number refusal (parametrized at both parse and serialize boundaries), duplicate-JSON-key refusal (byte-identity + Technical Writer
Verified clean: generated-file freshness — rebuilt Ecosystem ReviewerScope verdict: IN CORE. OpenAI-vendored, and adoption is not marginal — GitHub code search returns 1,228 repos with
Verified clean, no action: Slopinator Reviewer
Swept and clean: all five new rule docs and their Panel SynthesisThe headline is that round four's security fixes are not in effect, and one of them broke a shipped feature. Three specialists independently re-derived the ReDoS the last panel blocked on, and I reproduced all four items below myself before writing this. That pattern — a fix that reads correct, ships, and is confirmed by a test that cannot fail — is the thing to fix about the process, not just the code. 1. return escAttr(String(str).replace(/\/g, '\\').replace(/'/g, "\'"));
2. The ReDoS is still there — the rewrite traded exponential for quadratic. Architecture, Python and Security all found this independently, each with measurements. In Security reproduced it end-to-end: a 2 MB 3. Confirmed on this checkout. The PR reports 3,456 passed, which is the uninstrumented number — so 4.
5. The backward-compatibility claim is unpinned, and one security guard's test is vacuous. QA mutation-proved both. There is no dual-manifest fixture anywhere in 6. Generated docs are an unmodelled trust boundary. Findings 1 and 4 both live on a surface What is genuinely good, and worth saying. Architecture found no blocking issues in the Codex feature's design and confirmed the Required Actions Before Merge
Optional Follow-upsReal-repo false positive, worth doing (Ecosystem #2). Architecture cleanups, in rough value order — all carried from last round:
Performance, all measured on a 300-plugin / 3000-skill synthetic catalog:
Test coverage beyond the required items: three declared refusal conditions in Documentation: fix PR description: move the Noted, no action: the Generated by skillsaw-review-panel |
|
A 6,051 line individual test file is unacceptable - please fix |
…entity bypass - escJsAttr was emitted from a non-raw string, halving its backslashes into an unterminated JS regex — every docs HTML page was a blank shell. Fixed and pinned by parsing the emitted script blocks. - safe_display now truncates before scanning, treats the cut edge as a severed credential, and floors the backward search window — work is bounded by the display cap (2 MB manifest value: 2m17s -> ~10ms). - _safe_url decodes HTML character references to a fixed point before scheme validation, and the Markdown sink re-encodes '&' so entity text cannot reassemble a scheme at render time. - _scoped_prose matches prose to its nearest owning plugin root, so a repo-root Codex plugin no longer republishes nested plugins' files. - Skill discovery derives its containment boundary from provenance at the walk chokepoint, closing the symlink escape on every route (the strict-xfail documenting it now passes and lost its marker). - Non-string hook types route through the diagnostic redactor. - The vacuous symlinked-catalog fix test now reaches the guard it names; a dual-manifest fixture pins the 0.17 compat claim; both are mutation-verified. TestPanelFourRegressions redistributed into behavior-named classes; timing test asserts a scaling ratio. - THREAT_MODEL models the generated-docs boundary (T15); the policy-enum provenance note names the right upstream document. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Response to the fifth panel roundAll ten required actions are addressed in 3cdee89. The panel's headline was right and worth conceding plainly: two of round four's fixes did not work, and the canary test that caught one of them was misread as a timing flake. This round's fixes were therefore verified by mechanism, not by assertion:
Also folded in: the connector's round-seventeen P1 (skill discovery now derives containment from provenance at the walk chokepoint — the strict xfail documenting the uncontained walk now passes and lost its marker), its docs-TypeError, hook-type redaction, and entity-URL findings; the raw Validation: The remaining optional items (architecture cleanups, perf polish, fixture ports) are follow-up material, deliberately not folded into a PR this size. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 6a1d5c70d4
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| ): | ||
| violations.append( | ||
| self.violation( | ||
| f"MCP server '{server_name}' '{required_field}' must be a non-empty string", |
There was a problem hiding this comment.
For a Codex-only plugin with inline configuration such as "mcpServers":{"https://user:secret@host.example/server":{"command":""}}, this new branch interpolates the manifest-controlled server name directly into the diagnostic. Credentials and terminal control characters can therefore reach text, JSON, and SARIF reports; pass server_name through safe_display() as the adjacent Codex diagnostics do.
AGENTS.md reference: AGENTS.md:L164-L168
Useful? React with 👍 / 👎.
| A path can also exist and still be reported for its *kind*: fields that | ||
| name a file (`hooks`, the `interface` asset paths) are warned about | ||
| when they resolve to a directory, and fields that name a directory | ||
| (`skills`, `apps`) when they resolve to a file. The path is fine — |
There was a problem hiding this comment.
The rule does not perform the promised filesystem-kind checks for apps or any interface asset: _EXPECTED_KIND contains only hooks, mcpServers, and skills, and _check_paths() skips fields absent from that mapping. Thus an apps path pointing to a file or an interface.logo path pointing to a directory receives no documented warning; either narrow this paragraph to the implemented fields or add the promised validation.
AGENTS.md reference: AGENTS.md:L27-L29
Useful? React with 👍 / 👎.
| wall-clock assertion here was once misread as a timing flake | ||
| while it was correctly reporting quadratic behavior. |
There was a problem hiding this comment.
This docstring records how an earlier wall-clock assertion was interpreted during review rather than explaining the durable test contract. That history will become misleading as the implementation and instrumentation change; retain only the stable rationale that a scaling ratio avoids instrumentation-dependent absolute timings and detects nonlinear growth.
Useful? React with 👍 / 👎.
| loose = [ | ||
| b | ||
| for b in context.lint_tree.find(McpBlock) + context.lint_tree.find(HooksBlock) | ||
| if safe_resolve(b.path) in wanted |
There was a problem hiding this comment.
For a repository-root Codex plugin whose .mcp.json is a symlink to an existing file outside the checkout, the generic root attachment follows the link and this resolved-path comparison accepts the resulting block because both entries in wanted and b.path resolve to the same external file. _extract_mcp_servers() then publishes that external server configuration in generated Markdown and HTML, potentially exposing developer or CI commands, URLs, and secrets; require recovered blocks to remain under the resolved plugin root.
AGENTS.md reference: AGENTS.md:L159-L162
Useful? React with 👍 / 👎.
| if widened not in bounds: | ||
| resolved = widened |
There was a problem hiding this comment.
When skillsaw lint ~/.agents/plugins/marketplace.json is invoked for the documented user-level catalog, widened equals Path.home() and this branch deliberately leaves the context rooted at ~/.agents/plugins. Codex discovery then probes ~/.agents/plugins/.agents/plugins/marketplace.json, so the explicitly supplied real catalog receives no marketplace rules; retain a direct-catalog seed when widening is declined rather than silently dropping the target.
Useful? React with 👍 / 👎.
| it — ``..`` is not allowed in a source; | ||
| * the catalog cannot be rewritten safely at all. | ||
| """ | ||
| if _mutable_marketplace_data(_read_text(marketplace_file)) is None: |
There was a problem hiding this comment.
For an in-repository marketplace.json symlink to a contained sibling, this check sees parseable data and _registerable() marks an unregistered-plugin violation as fixable. However, fix() unconditionally refuses every symlinked catalog at lines 410–418, so text, JSON, and SARIF advertise a SUGGEST fix that always produces no result; apply the same lexical-versus-resolved guard while computing registerability.
Useful? React with 👍 / 👎.
|
|
||
| for node in context.lint_tree.find(CodexMarketplaceConfigNode): | ||
| marketplace_file = node.path | ||
| data, error = read_json(marketplace_file) |
There was a problem hiding this comment.
A catalog such as {"name":"cat","plugins":[{"name":"remote","source":{"source":"url","url":"https://example/x"}}],"plugins":[]} is accepted as clean because the shared reader silently retains only the final plugins value. Entries hidden by a duplicate key therefore escape every schema and registration check, even though the registration autofix already treats this document as malformed; parse validity checks with duplicate-key rejection as well.
Useful? React with 👍 / 👎.
| "array, got a " | ||
| f"{'JSON array' if isinstance(data, list) else 'JSON scalar'}", | ||
| file_path=evals_json, | ||
| line=commented_root_line(shaped), |
There was a problem hiding this comment.
A baseline generated by 0.17 for a top-level array or scalar evals.json fingerprinted the old line-less message, but this change both rewrites that message and adds line 1, switching the fingerprint to the root-line content. The unchanged violation therefore becomes newly visible and its previous baseline entry is reported stale after upgrade; retain the required line attribution while supporting or migrating the legacy fingerprint.
AGENTS.md reference: AGENTS.md:L88-L90
Useful? React with 👍 / 👎.
| except RecursionError: | ||
| return None, _TOO_DEEP, None |
There was a problem hiding this comment.
On Python 3.11+, an unquoted YAML integer longer than the interpreter's digit limit makes ruamel's integer constructor raise a bare ValueError, not _RuamelYAMLError. Because this reader catches only YAML errors and RecursionError, accessing such an agents/openai.yaml through OpenAIMetadataBlock.raw_data produces rule-execution-error and discards the metadata rule's remaining checks; catch ValueError here and return it as a parser diagnostic, as read_json() already does.
Useful? React with 👍 / 👎.
CodeQL flags a case-sensitive <script> regex as a bad HTML tag filter (py/bad-tag-filter). The test slices blocks out of our own renderer's output for a node --check parse, not sanitizing — plain string splitting does the same job without reading as a sanitizer. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Closing in favor of a feature-branch workflowThis PR is being closed without merging — not because the work is rejected, but because the delivery vehicle is being replaced. Where things stand: the feature is functionally complete and validated — 3,472 tests green under coverage, zero The plan:
When the branch reaches a respectable state, a fresh PR from a new branch to The full review record here remains the archaeology: what a multi-model adversarial review loop does to a 2,500-line PR, and why the next one this size gets split on arrival. |
2 participants
Summary
Adds first-class OpenAI Codex plugin and marketplace support without treating
Codex repositories as malformed Claude plugins.
.codex-plugin/plugin.json,.agents/plugins/*marketplace.jsoncatalogs (siblings need at least one sourced entry, so version-pin files are
not claimed), declared skills, hooks, MCP configs, and installed plugins.
codex-plugin-json-valid,codex-plugin-structure,codex-marketplace-json-valid,codex-marketplace-registration, andcodex-openai-metadata.agents/openai.yaml—interface strings,
#RRGGBBbrand color, owner-relative icon paths, policyand tool dependencies — matching the field set of OpenAI's bundled
validate_plugin.py. The plugin-root form is supported as an observedOpenAI catalog compatibility form, and the file is a reachability root for
agentskill-unreferenced-files.commands/,agents/,rules/and README files are read by all content and security rules(secret scanning included), while the Claude-format rules — command/agent
frontmatter, naming, sections,
plugin-json-required, Claude marketplaceregistration — exempt Codex-only directories via a provenance predicate.
Dual-manifest plugins keep every Claude check and their existing results.
Behavior changes to existing rules
agentskill-structureno longer governsagents/: host metadata isskipped unconditionally via
_HOST_METADATA_DIRSand removed from theallowed_dirsdefault.hooks-json-validmatcher-type check andmcp-valid-jsonnon-empty-string checks added here are gated to Codex-only plugins, so
Claude-only and dual-manifest repositories report exactly what 0.17 did.
content-broken-internal-referencetreats any RFC 3986 URI scheme(
app://,vscode://, …) as external, not just http(s)/mailto./**matches strictly inside the directory,never the directory entry itself.
skillsaw lint <manifest file>roots the lint at the owning plugin orrepository root (bounded — never
$HOMEor/).formatters and the badge card.
content-*rule implementation is changed.Correctness and safety
within the repository and their owning plugin/skill; symlink loops and
oversized JSON numbers degrade to diagnostics rather than aborting the lint.
.codex/plugins/diagnostic-only:executable hooks/MCP and skill checks run; manifest-quality rules
(
codex-openai-metadataincluded) and every autofix stand down.echoes a manifest value, so JSON/SARIF artifacts cannot carry a pasted
token.
non-finite numbers, duplicate names, excluded plugins, unsafe paths, and
names the catalog already lists are never written; a listed name whose
source path does not resolve gets an accurate WARNING instead of a false
"not registered" ERROR.
through URLs and metadata fields, filename collisions, control characters,
and unsafe URL schemes.
Validation
make update,make lint,make test— 3,442 passed, 1 xfailed, 94%coverage.
codex-marketplacescale is part of the benchmark harness.pinned CI gate also asserts plugin prose stays visible to content rules and
the gated Claude-format rules stay silent. Against the pre-Codex baseline,
698 error/warning findings are removed at the intended false-positive
sites and 0 error/warning findings are added (info-level findings differ).
openshift-eng/ai-helperswith built-in rules — A+, exit 0.References