Add OpenAI Codex plugin and marketplace linting

[not-stbenjam]

Adds first-class OpenAI Codex plugin and marketplace linting: five new rules, Codex-aware discovery, and docs generation for Codex catalogs — no new Codex rules fire on Claude-only repositories (three deliberate fixes to neutral machinery, plus a change to generated-docs output, are listed below).

What this adds

• Five rules under rules/builtin/codex/: codex-plugin-json-valid, codex-plugin-structure, codex-marketplace-json-valid, codex-marketplace-registration (with a SUGGEST-confidence autofix that appends missing catalog entries), and codex-openai-metadata (validates agents/openai.yaml, the first structured-YAML lint target). All default to enabled: auto, gated on marker-detected repo types — Claude-only repos run none of them.
• Discovery for .codex-plugin/plugin.json manifests, .agents/plugins/marketplace.json catalogs (and sibling catalogs), catalog-claimed manifest-less directories, and vendor installs under .codex/plugins/ (linted, but diagnostic-only: autofix stands down for content the repo doesn't author).
• skillsaw docs renders Codex plugins and marketplaces (Markdown + HTML), with the docs boundary threat-modeled (T15) and hardened: URL scheme allow-listing with entity decoding, attribute/JS-string escaping, filename collision handling, credential redaction on every diagnostic sink.

Architecture

• PluginProvenance — ecosystem ownership is decided once, in RepositoryContext.provenance(), filesystem-first and --type-invariant. Every consumer reads the record; no rule or tree code probes the filesystem for ownership.
• Single-pass lint tree — one plugin loop over the union of claimed directories; prose attaches once for every claimed directory, so content and security rules read Codex plugins' files with no per-ecosystem duplication.
• Declarative rule gating — Claude-format rules declare provenance_scope = "claude"; filtering happens in one place. The ecosystem-tightened hooks/MCP checks apply only to Codex-exclusive directories, so dual-manifest repos keep their exact 0.17 results (mutation-pinned by TestDualManifestBackwardCompat).
• discovery/codex.py — state-free discovery module; the context stays the stateful orchestrator holding caches and rendering provenance verdicts. Adding the next ecosystem = an evidence probe, a discovery leg, and a rules package (recipe in the development rules).
• New core modules paths.py additions (safe_*, contained_resolve) and diagnostics.py (safe_display), with a ratchet test banning new bare .resolve() calls (Retire the symlink-crash bug class: sweep bare Path.resolve() behind safe_resolve() and enforce with a banned-API test #463).

Behavior changes affecting existing (non-Codex) repositories

Deliberate changes to neutral machinery — flagged here because they can change results on upgrade:

1. content-broken-internal-reference: URI-scheme targets (e.g. app:// connector URIs) are treated as external, not broken repo paths.
2. hooks-json-valid: a non-string hook type is now reported instead of aborting the entire rule with a TypeError (base behavior silently skipped all hook validation for the repo).
3. Exclusion-pattern matching guards against symlink loops instead of raising. Pattern semantics are unchanged: _pattern_variants produces the same expansion as base for every pattern.
4. skillsaw docs output changed for every repository, Claude-only ones included: the generated HTML escapes JavaScript-string contexts with a dedicated escJsAttr and attribute contexts with escAttr, and plugin pages carry the manifest's author field. A repository that commits generated docs and gates CI on them must regenerate on upgrade — disclosed in docs/ci.md under "Committed generated docs" for the 0.18.0 notes.

Validation

• 3,505 tests, 94% coverage; make lint/make update clean; self-lint A+.
• Real-corpus gate (CI-pinned): openai/plugins @ 11c74d6 — zero codex-* findings across 180 plugins / 582 skills, and a net false-positive reduction vs. base (~700 fewer, mostly agentskill-unreferenced-files and provenance-gated Claude-format FPs), with zero new violations from any pre-existing rule.
• openshift-eng/ai-helpers (Claude-only): zero codex-* findings and no new lint violations. Its committed docs/index.html does change (84-line diff — the escJsAttr/escAttr hardening plus the new author field), so its plugins-doc-up-to-date custom rule fails until the repo regenerates. Verified by bisect: base regenerates byte-identically, this branch does not.
• Docs byte-stability, --type invariance, and dual-manifest compat are all test-pinned; a serial deep review of the final branch returned zero blockers.

Provenance

Supersedes #451 (closed; its review history stays there). Developed on the codex-plugin-rules feature branch through reviewed refactor PRs #464–#469; further context.py decomposition tracked in #470.

🤖 Generated with Claude Code

[github-actions]

Review Panel Verdict

Disposition: REQUEST_CHANGES

---

Specialist Findings

Architecture Reviewer

Verified against the code, not the description. Full suite run: 3482 passed (make test, 690s).

1. BLOCKING — silent lint/fix scope expansion for existing Claude repos — src/skillsaw/cli/_helpers.py:66

_resolve_lint_paths now widens a file argument's parent when that parent is named .codex-plugin or .claude-plugin. The justifying comment only argues the Codex case, but .claude-plugin is a long-standing Claude path.

# base: roots at <repo>/.claude-plugin, detects UNKNOWN, reports ~nothing
# HEAD:
$ skillsaw lint /tmp/wtest/.claude-plugin/plugin.json
Linting: /tmp/wtest
  ✗ ERROR (command-frontmatter) [commands/deploy.md]: Missing frontmatter
  ⚠ WARNING (plugin-readme) [README.md]: Missing README.md

_resolve_lint_paths is shared by _lint.py:80 and _fix.py:35, with no post-filter narrowing violations or autofixes back to the named path. So skillsaw fix some/plugin/.claude-plugin/plugin.json now rewrites commands/*.md, SKILL.md, README.md across the whole plugin directory — files the caller did not name. CI setups passing changed files (git diff --name-only | xargs skillsaw) flip from near-noop to full-tree lint, and from no writes to repo-wide writes.

Action: restrict the widening to the genuinely-new surfaces (.codex-plugin, .agents/plugins), leaving .claude-plugin on its historical root. If widening .claude-plugin is intended, call it out as a fourth deliberate behavior change and gate fix so writes stay within the named path.

2. SUGGESTION — PluginProvenance.installed is written but never read — src/skillsaw/context.py:805

provenance() computes installed=self.is_codex_installed_plugin(plugin_dir) on every uncached record. Grep across src/ and tests/ finds exactly one occurrence of .installed — the assignment. All eleven consumers go around the record, via context.is_codex_installed_plugin(...) (7 sites), agentskills/_helpers.py:56 is_installed_plugin_skill() (a pass-through alias), and Linter._is_vendor_managed() (a third alias with its own cache). The vendor-managed half of provenance has four names and a dead field while the ecosystem half has one.

Action: either delete the installed field and keep is_codex_installed_plugin as the single spelling, or route all consumers through context.provenance(x).installed and drop the aliases.

3. SUGGESTION — a rule answers a Claude-ownership question with a fresh filesystem probe — src/skillsaw/rules/builtin/marketplace/json_valid.py:55

_has_claude_plugin() re-derives "does this directory declare Claude intent" by stat-ing .claude-plugin, inside a rule body. RepositoryContext.provenance(node.path).claude is the recorded answer. They agree today only because the branch is reached when no marketplace.json exists; if either definition moves they drift.

Action: any(context.provenance(n.path).claude for n in context.lint_tree.find(PluginNode)).

4. SUGGESTION — the lint tree reaches into a private context method — src/skillsaw/lint_tree.py:316

*sorted(p for p in context._codex_claim_set() if not context.is_path_excluded(p)),

The single-pass plugin loop depends on a private. distinct_plugin_dirs() already exists but excludes catalog-only claims, so two definitions of "every plugin directory" now differ silently — the badge/stats path uses one, the tree the other.

Action: expose the union as a public method (e.g. claimed_plugin_dirs()), and have build_lint_tree and distinct_plugin_dirs agree.

5. SUGGESTION — provenance() is construction-order-coupled and defends with getattr — src/skillsaw/context.py:790

_detect_types() → ... → provenance() runs at __init__ line 262, before self.marketplace_entries = {} at line 269. The getattr hides an AttributeError; correctness of records cached in that window depends on the unconditional _provenance_cache.clear() at the tail of apply_excludes(). Latent hazard, not a live bug.

Action: initialize marketplace_entries/plugin_metadata before _detect_types() and drop the getattr.

6. SUGGESTION — provenance_scope = "claude" is inert on the three PluginNode rules — src/skillsaw/rule.py:198; plugins/json_required.py:27, plugins/naming.py:23, marketplace/registration.py:78

build_lint_tree constructs a PluginNode iff prov.claude or not prov.codex (lint_tree.py:342). in_format_scope(node, "claude") returns not ecosystems or "claude" in ecosystems. Both branches satisfy the predicate, so no PluginNode can ever be filtered out — the gate is provably a no-op; the exemption is delivered by container typing (CodexPluginNode). tests/codex/test_provenance.py:789 concedes this in a comment. Meanwhile plugin-readme ships a "Codex plugins — exempt" doc section with no provenance_scope, exempt purely by tree shape. Two mechanisms encode one policy.

Action: pick one. Either make provenance_scope meaningful for PluginNode, or drop it from those three rules and document container typing as the mechanism for directory-level rules.

7. SUGGESTION — the new discovery/ layering invariant is unenforced — tests/test_module_layering.py:124

discovery/__init__.py states "These modules must never import from skillsaw.context." _core_module_files() globs *.py, blocks/*.py, formats/*.py — not discovery/*.py, and discovery is absent from CYCLE_SENSITIVE_MODULES. The one architectural rule the new package exists to hold is the one the layering test doesn't check.

Action: add discovery/*.py to _core_module_files(), add skillsaw.discovery.codex to CYCLE_SENSITIVE_MODULES, extend the AST check.

8. SUGGESTION — _is_vendor_managed misuses a plugin-directory API on file paths — src/skillsaw/linter.py:484

context.is_codex_installed_plugin(file_path) is documented and named for a plugin directory. It works on files because both checks are prefix tests, but the contract doesn't say so. Separately, codex_install_root() returns non-None even when .codex/plugins/ does not exist, so every repository pays a resolve() per distinct violation file path on the _filter_violations hot path.

Action: add an explicit is_vendor_managed_path(path) documenting file-or-directory semantics; short-circuit when .codex/plugins/ is absent.

9. NOTE — new bare .resolve() calls in the single-pass loop — lint_tree.py:318, :335, :363. lint_tree.py is grandfathered so the ratchet doesn't fire, but resolved_candidate = candidate.resolve() runs over catalog-supplied local sources — exactly the manifest-derived input safe_resolve exists for. Also hoist marketplace_dir.resolve() out of the loop.

10. NOTE — URI-scheme handling inconsistent between two link scanners — agentskills/unreferenced_files.py:132. content-broken-internal-reference moved to a generic scheme test; _EXTERNAL_LINK_PREFIXES still hard-codes ("http://", "https://", "#", "mailto:"). Benign today; consider sharing the predicate.

11. NOTE — provenance is not quite --type-invariant — formats/codex.py:223 vs discovery/codex.py:215. codex_manifest_is_contained() requires safe_is_file(manifest); discovery's _add() accepts the marker directory without a readable plugin.json. A directory with .codex-plugin/ but a missing manifest is Codex-claimed by default but unclaimed under --type marketplace, contradicting the invariance docstring at context.py:146.

12. NOTE — one of the three claimed behavior changes isn't in this diff. _pattern_variants at HEAD is semantically identical to base. The dir/** change described in the PR body landed before 43654b2. Worth correcting the PR description.

Checked and clean: import cycles/module boundaries (discovery/codex.py and formats/codex.py have no edge back to context); the single-pass plugin loop (exactly one loop; _add_plugin_prose._contained correctly defers nested plugins; seen/seen_roles split sound); container-type dispatch (dual-manifest → one PluginNode; Codex-only → CodexPluginNode; no directory falls between paths); provenance_dir() overrides present on all three scope-declaring node types; conditional strictness in hooks-json-valid/mcp-valid-json; error propagation in utils.read_json/read_yaml/FileCache; the hooks-json-valid isinstance guard is a strict improvement; the scheme regex is anchored and /-terminated; autofix blast radius for codex-marketplace-registration.

Python Expert

Full test suite passes (3482), make lint clean. All perf claims measured against the merge base on the repo's own benchmarks/ harness.

1. SUGGESTION — skill discovery is ~2.5x slower on repos with no Codex content — src/skillsaw/context.py:1343-1367

_discover_skills_in_dir now does per-directory-entry work that did not exist before: safe_resolve(item) for every entry; safe_resolve(item / "SKILL.md") unconditionally, before the .exists() test (and since Path.resolve() is non-strict this is a wasted realpath() per directory, unused when contain_within is None); and self.provenance(item) for every non-skill directory descended into, ~5 filesystem round-trips each.

Measured on benchmarks/genrepo.py --scale large (a pure Claude marketplace, zero Codex content), min of 3:

phase	base 43654b2	branch
context	94.0 ms	238.2 ms

cProfile: 0.320s of a 0.509s cold context build in _discover_skills, of which provenance() (203 calls) is 0.193s. Base spends 0.084s in the same function. Cost scales with directory count.

The only thing provenance() is consulted for here is codex_only, which requires a .codex-plugin/ marker or claim-set membership. Gating on one exists() stat first recovers most of it — measured 0.194s → 0.124s (36% reduction in context construction).

Action:

if child_bound is None and (
    safe_exists(item / CODEX_PLUGIN_MANIFEST[0]) or resolved in self._codex_claim_set()
) and self.provenance(item).codex_only:
    child_bound = resolved

and move safe_resolve(item / "SKILL.md") inside the .exists() branch.

2. SUGGESTION — codex_install_root() never returns None, so the vendor-managed check never short-circuits — src/skillsaw/discovery/codex.py:295-297

Path.resolve() is non-strict, so on a repo with no .codex/ it still returns /repo/.codex/plugins. The install_root is None early-out (line 305) is dead in the common case, so every call pays a lexical is_relative_to plus a safe_resolve(plugin_dir) syscall. That predicate runs per SkillNode, per CodexPluginConfigNode, and via Linter._is_vendor_managed once per distinct violation file path on every lint, Codex or not.

Measured: gating on safe_is_dir cuts context construction on the Codex-free large repo from 178.4ms to 161.4ms (~10%), independent of #1.

Action: return safe_resolve(candidate) if safe_is_dir(candidate) else None. The _UNSET caching already distinguishes "not computed" from a computed None.

3. SUGGESTION — _owned_blocks / _extract_codex_skills are O(plugins × blocks) — src/skillsaw/docs/extractor.py:301-322, :437-450

_owned_blocks calls context.lint_tree.find(block_cls) and filters by b.plugin_owner == plugin_resolved, once per plugin per block class (5 classes). find() is memoized so the list isn't rebuilt, but the filter scan is not — quadratic in catalog size, exactly the pattern build_lint_tree replaced with dict lookups at lint_tree.py:474-484.

Profiled at 120 plugins: extract_docs takes 611ms; top skillsaw-level entry is the generator at extractor.py:317 — 0.138s cumulative, 110,363 Path.__eq__ calls.

Action: build the ownership index once — by_owner[b.plugin_owner].append(b) — and pass it down. plugin_owner is already a resolved Path.

4. SUGGESTION — a file created by this PR is on the bare-resolve() grandfather list — tests/test_banned_apis.py:45 / codex/marketplace_registration.py:249

The ratchet's docstring says "existing debt is grandfathered per issue #463, but any new file reaching for bare .resolve() fails here". rules/builtin/codex/marketplace_registration.py was added by this PR (commit 63330d9) and is on the list. Its single offender is if plugin_dir.resolve() in registered_dirs: — everything around it already uses safe_resolve, and registered_dirs is a set of safe_resolve results, so it is both inconsistent and the only call that can raise. A ratchet that grandfathers its own new code on day one loses its guarantee.

Action: use safe_resolve (skip on None, matching _check_entries) and drop the file from GRANDFATHERED_BARE_RESOLVE.

5. NOTE — _safe_stat dispatches on a method-name string — paths.py:77-93. getattr(path, predicate)() is correct but opaque to type checkers and adds a getattr to a helper called thousands of times. Pass the unbound method or a Callable[[Path], bool].

6. NOTE — tree builder reaches into context._codex_claim_set() — lint_tree.py:316. Every other provenance consult in build_lint_tree uses a public accessor. (Corroborates Architecture #4.)

7. NOTE — self._codex_install_root: Any = _UNSET — context.py:235. The sentinel forces Any, erasing downstream types. Cosmetic.

Checked and clean: Python 3.9 compatibility (no match, zip(strict=), pairwise, functools.cache, tomllib; from __future__ import annotations present everywhere needed); paths.is_absolute_path tightening exercised against POSIX/Windows/drive-relative/UNC/colon-in-name inputs with no false positives on ordinary relative sources; YAML/JSON conventions (read_yaml_commented, commented_key_line/commented_item_line, no fabricated lines, new KeyError guard for YAML merge keys correct); find() memoization validity (no new post-set_parents() mutation; _owned_blocks copies before extending); no per-line × per-pattern regex loops introduced, _redact_userinfo is a bounded linear scan; FileCache key fallback and RecursionError/ValueError handling correctly scoped; _InlineJsonPayload MRO (@dataclass(eq=False) gives identity semantics, required); pyproject.toml is a version bump only and the new discovery/ package is picked up; provenance/init ordering traced and cannot currently produce a stale record.

Security & Supply Chain Reviewer

1. BLOCKING — skillsaw docs --format markdown emits unescaped raw HTML and active-scheme links from untrusted third-party metadata; T15 is marked mitigated but is not — src/skillsaw/docs/markdown_renderer.py:118 (_table_cell), :149, :220, :242, :249-253, :260, :268, :293, :300

The PR adds _table_cell() specifically to neutralize third-party catalog metadata, and adds THREAT_MODEL.md T15 (asset published_docs) with status mitigated. Both are inaccurate for the Markdown renderer:

• _table_cell escapes \ | [ ] ( ) but not < / >, so raw HTML passes through every sink it guards.
• Several metadata sinks get no folding at all: ### {skill.name} (:242), ### {agent.name} (:260), ### {cmd.name} (:220), ### {rule.name} (:293), ### {hook.event_type} (:268), the skill **License:**/**Compatibility:**/**Allowed tools:** lines (:249-253), rule.globs (:300).
• plugin.description (:149) is emitted raw, bypassing _safe_url's scheme allow-list entirely.

Demonstrated against the code as checked out. A plugin shipping skills/evil/SKILL.md with frontmatter name: "evil\n\n# INJECTED HEADING\n\n<script>alert(1)</script>" produces in p0.md:

## Skills

### evil

# INJECTED HEADING

<script>alert(1)</script>

And a manifest with "description": "desc [click](javascript:alert(1)) <img src=x onerror=alert(2)>" plus interface.displayName: "<svg onload=alert(1)>":

# <svg onload=alert\(1\)>

> desc [click](javascript:alert(1)) <img src=x onerror=alert(2)>

**Version:** 1.0.0 | **Author:** <script>alert\(1\)</script> | **License:** MIT" onload="alert\(1\) | **Category:** "><img src=x onerror=alert\(1\)>

The parentheses are escaped (alert\(1\)) — the new sanitizer ran and still let the payload through. The HTML renderer handles all of these correctly, so the two renderers disagree about whether this data is hostile. GitHub's renderer sanitizes; MkDocs / Docusaurus / python-markdown (verified) do not — and "project sites, catalog indexes" is exactly the published_docs asset T15 defines.

Action: add < and > to the escape tuple in _table_cell (CommonMark treats \< as a literal <); apply _table_cell to the un-guarded heading/meta sinks; route plugin.description / skill.description / agent.description through the same scheme allow-list _md_link applies — or restate T15 as partially_mitigated with the Markdown path called out as accepted risk. Add a test asserting the Markdown output of a hostile fixture contains no raw <script/onerror/javascript: — tests/codex/test_docs_output_safety.py currently tests only the HTML renderer and _safe_url.

2. SUGGESTION — the generated page filename is interpolated into the index link destination with no escaping — src/skillsaw/docs/markdown_renderer.py:91

lines.append(f"| [{label}]({fname}) | {desc} | {ver} |")

label, desc, ver go through _table_cell; fname goes through neither _table_cell nor _link_dest. _plugin_filename maps / \ : < > " | ? * and controls to -, but leaves ( ) [ ] ' ! #.

Plugin name a) [CLICKME](javascript:alert(document.domain) yields an index row that python-markdown renders as two anchors — <a href="a"> and <a href="javascript-alert(document.domain).md"> — instead of one link to the plugin page. The : mapping stops this reaching a live javascript: URL, so impact is limited to injecting arbitrary relative links/images and orphaning the real page, but the sink is unguarded by construction. Any plugin name with an unbalanced ) silently breaks its index link.

Action: use the CommonMark pointy-bracket form f"| [{label}](<{fname}>) | ...", or add ( ) [ ] ! to _UNSAFE_FILENAME_CHARS.

3. SUGGESTION — _md() is quadratic; skillsaw docs --format html hangs on a crafted plugin — src/skillsaw/docs/html_renderer.py:1261

re.sub(r"\[([^\]]+)\]\(([^)]+)\)", _md_link, safe) restarts the character-class scan at every start offset — O(n²) in value length. _md is applied to cmd.body (html_renderer.py:477), the entire ## Description section of a command file, with no length bound.

input	_md alone	skillsaw docs --format html
"[a]("*8000 (32 KB)	1.3 s	—
"[a]("*32000 (128 KB)	20.8 s	—
160 KB command body	—	26.8 s

Extrapolating, a 1.6 MB command body is ~45 minutes for one plugin. The regex is unchanged from base, but this PR newly routes Codex plugin commands/ bodies into it and re-attests T11.

Action: cap the input to _md, or precheck with a cheap "](" in safe guard.

4. NOTE — escJsAttr does not escape newlines — html_renderer.py:1210. Fuzzed escJsAttr + attribute entity decode + JS parse in Node against 14 breakout payloads (');alert(1);//, \');…, ', ', ', ', </script>…) — all correctly contained; the escaping is sound. The one failure is a literal newline in name/category/tag: the emitted onclick="navigateTo('a\nb')" is a JS SyntaxError, so the card/tag becomes unclickable. Not injection, just silent breakage. Add .replace(/[\r\n

]/g, '').

5. NOTE — the .resolve() ratchet grandfathers exactly the files where this PR adds new bare .resolve() calls — tests/test_banned_apis.py:22, lint_tree.py:318. The ratchet works as advertised in both directions (verified green). But it matches on AST shape, so getattr(p, "resolve")() slips past; and context.py, lint_tree.py, linter.py, docs/extractor.py — the discovery files whose raises abort the whole lint, the stated rationale for the wrappers — are all grandfathered, so the new .resolve() at lint_tree.py:318 is invisible to it. That call runs before the safe_is_dir(candidate) guard. I could not construct a reaching input, so this is defensive only. Consider a per-function rather than per-file allowlist.

6. NOTE — _safe_url allows protocol-relative URLs; T14's redaction is narrower than "credential disclosure" — docs/extractor.py:427, diagnostics.py:21. _safe_url("//evil.example/x") returns unchanged (if not sep: return candidate # relative or bare host); impact is nil since https://evil.example is allowed anyway, but the comment reads as if scheme-less means in-origin. safe_display redacts URL userinfo well (verified: https://u:tok@h/x, scp-form, truncation-severed credentials; 1 MB adversarial input stays ~3 ms, output capped at 501 chars) but does not redact ?access_token=…, Authorization: Bearer …, or scp-form to a dotless host. T14's evidence column is scoped to URL userinfo, so this is a scope note.

Checked and clean: paths.py containment — verified with a real symlink-escape repo where a plugins/escapee → ../outside symlink, an absolute "/etc" source, and a "../outside" source were all dropped; _plugin_filename/_unique_filenames (no traversal from ../../etc/passwd, /etc/passwd, ....//....//evil, NUL, newline, 300-byte names, con/com1; case-folded collision reservation works); HTML renderer XSS (end-to-end against a hostile catalog: zero unescaped <img/<svg/onmouseover=; _safe_url rejects javascript: in every spelling tried — case, whitespace, tab/ZWSP/NBSP, :, j, j, data:, vbscript:; the &→& double-encode neutralizes [x](javascript:…); </> escaping blocks </script> breakout); supply chain (no new dependencies, pyproject.toml is a version bump only; workflows use pull_request not pull_request_target, permissions: contents: read, persist-credentials: false, actions pinned to SHAs; the openai/plugins clone pinned to SHA 11c74d6b and linted with --no-custom-rules); injection (no new subprocess, shell=True, eval, exec, pickle, importlib, or file-write calls in the src/ diff); autofix write path (declines on symlinked catalog paths, re-derives its registerable set, allow_nan=False, vendor content excluded at the Linter level).

QA Engineer

Test suite: PASSES. make test → 3482 passed, 0 failed, exit 0 in 696s. Coverage TOTAL 94% (13151 stmts, 792 miss). Both PR claims verified exactly.

Module	Cover
rules/builtin/codex/plugin_json_valid.py	100%
rules/builtin/codex/_helpers.py	100%
rules/builtin/codex/openai_metadata.py	99%
rules/builtin/codex/marketplace_registration.py	95%
rules/builtin/codex/plugin_structure.py	94%
rules/builtin/codex/marketplace_json_valid.py	93%
discovery/codex.py	93%
paths.py	100%

Verified good: all five new rules fire on tests/fixtures/codex/broken (confirmed by running the CLI), so TestRuleCoverage.test_every_rule_fires_somewhere genuinely pins them; codex/clean is in CLEAN_FIXTURES; the registration autofix has CLI-level idempotency, --suggest gating, re-lint convergence, BOM/CRLF preservation, symlink refusal, malformed/duplicate-key/non-finite refusal, and non-ASCII preservation; TestDualManifestBackwardCompat is real mutation-pinning (test_claude_only_twin_matches_dual_results_exactly asserts message-set equality, and test_codex_only_twin_gets_the_tightened_checks proves the other half of the conjunction so the negative assertions can't pass vacuously); symlink loops, UTF-8 BOM, unicode names, and RecursionError all have tests. The SAFE-idempotency fixture rule correctly does not apply — the new autofix is SUGGEST, and TestSafeAutofixIdempotency discovers its rule set dynamically, so EXPECTED_SAFE_VIOLATIONS needs no update.

1. SUGGESTION — ten uncovered violation branches in codex-marketplace-json-valid — marketplace_json_valid.py:143

The lowest-covered new rule (93%) has ten violation branches with zero test assertions anywhere. Each confirmed live and reachable by driving it from /tmp:

Line	Message produced (verified)
143	'name' must be a non-empty string
149	'interface' must be an object
184	plugins[0] missing required 'source' field
241	plugins[0] missing required 'name' field
249	plugins[0] plugin name must be a string, got '7'
315	plugins[0].source must be a relative path string or an object
404	plugins[N].source.registry must be a string
450	plugins[0].source.registry: must not have a fragment
464	plugins[0].policy must be an object
428-432	parsed.hostname ValueError fallback (e.g. https://[::1)

These are the plainest schema branches — exactly what a refactor of _check_entry_name / _check_source would silently drop.

Action: add a parametrized test to tests/codex/test_marketplace_json.py driving one document per row through _codex_marketplace_repo and asserting the expected message — e.g. {"name": "", "plugins": []}, {"interface": []}, an entry missing source, an entry missing name, {"name": 7}, {"source": 5}, {"policy": []} — plus registry: 5, registry: "https://r.example.com/#frag", and registry: "https://[::1".

2. SUGGESTION — the positive duck-typing branch of sibling-catalog discovery is never exercised — discovery/codex.py:153

tests/codex/test_catalogs.py:381 covers only the negative case (notamarketplace.json with invalid JSON). Line 152 is covered but line 153 (found.append(candidate)) is not — no test has ever discovered a catalog by content. Deleting lines 152–153 entirely would leave the whole suite green while silently dropping a designed feature. Verified the branch works.

Action: add the positive twin — write a .agents/plugins/catalog-b.json holding a valid catalog shape and assert catalog-b.json appears in RepositoryContext(repo).codex_marketplace_paths() — plus a negative-shape case pinning the docstring's intent (a plugin-versions.json sibling whose entries have no source must not be discovered).

3. SUGGESTION — the autofix's catalog target selection is unpinned — marketplace_registration.py:390

fix() writes to config_nodes[0].path — whichever catalog discovery ordered first — but no test exercises the autofix with more than one catalog. test_registration_in_a_sibling_catalog_counts is check()-only. This is a file-writing fix whose target is unpinned; a change to discover_codex_marketplace_paths ordering would silently redirect writes into api_marketplace.json. Confirmed today's behavior by probe.

Action: add test_the_fix_targets_the_primary_catalog_not_a_sibling (assert the sibling is untouched) and test_a_sibling_only_catalog_is_the_write_target.

4. SUGGESTION — the non-string-type crash fix is only regression-tested through a Codex repo — hooks/json_valid.py:239

The guard is unconditional — it protects every repo type, including plain Claude plugins, from a TypeError on an unhashable type. But its only regression test (tests/codex/test_hooks_mcp.py:418) builds a Codex plugin. Nothing in tests/test_hook_rules.py covers a non-string type. If someone later moves this guard under the adjacent validate_codex_shapes flag, the Claude-repo crash regresses with a green suite.

Action: mirror the test in tests/test_hook_rules.py using _make_hooks_plugin, parametrized over [[], {}, ["command"], 42].

5. SUGGESTION — the dir/** exclusion invariant is only unit-tested — tests/test_context.py:857

Asserted only as assert not self._match(temp_dir, "templates", ["**/templates/**"]). The bug the docstring at context.py:49 describes is user-visible and end-to-end: the built-in **/template/** exclude swallowing Missing SKILL.md. The unit assertion pins _pattern_variants; the real regression path — default excludes → discovery → a whole-directory violation — is not exercised.

Action: add an integration test with skills/template/ containing no SKILL.md, asserting the violation survives a default lint. This is the assertion that would have caught the original bug; the unit one would not.

6. SUGGESTION — no permission-error test anywhere in the new Codex suite — codex/plugin_structure.py:45

The except OSError: continue guard on manifest_dir.iterdir() is untested (grep chmod tests/codex/ → nothing). The same untested pattern appears at discovery/codex.py:138-139 and :240-241. Verified the guard behaves correctly today (chmod 0o111 yields [], not a traceback), but nothing pins it.

Action: add one root-guarded chmod test to tests/codex/test_containment.py.

7. SUGGESTION — two documented guards in _registerable / _check_entries are unreached — marketplace_registration.py:194

Lines 194/511/514-515 (_relative_source(...) is None, the "outside the marketplace root" exclusion the docstring calls out explicitly) and line 332 (the plugin_dir escapes context.root_path early-continue). Every other bullet in that docstring has a dedicated test; these do not.

Action: for 332, use the existing plugins/linked -> outside symlink pattern and assert codex-marketplace-registration produces no message for it. For 194, if genuinely unreachable now that discovery is containment-filtered, mark # pragma: no cover with the reason, matching line 434.

8. NOTE — content-broken-internal-reference blast radius undocumented by test — broken_internal_reference.py:19. The new _URI_SCHEME also swallows top-level file targets whose first path segment contains a colon. Verified: [the helper](src/helper.py:42) is still reported (the / breaks the class) but [notes](notes:draft.md) and [x](README.md:10) are now silently skipped where base reported them. Added tests pin app://, vscode://, ssh://, //host, C:/notes.md — not this asymmetry. Add one assertion pinning whichever behavior is intended.

9. NOTE — encoding failure conflated with syntax failure — plugin_json_valid.py:103. Invalid UTF-8 bytes report Invalid JSON: Failed to read plugin.json — graceful but misleading, and untested. Add a write_bytes(b'{"name": "\xff\xfe"}') case.

10. NOTE — coverage residue. openai_metadata.py:223 and discovery/codex.py:49 are trivial uncovered lines. No action needed.

Working tree left clean; all probe tests written to /tmp.

Technical Writer

Docs coverage for this PR is unusually thorough — five new rule docs, a new rule category page, a new repo-types.md section, THREAT_MODEL.md entry points/threats, plus docs/pre-commit.md, docs/cli.md, docs/configuration.md, docs/autofixing.md, docs/supply-chain-protection.md, mkdocs.yml nav, and Codex sections retro-fitted onto nine existing Claude rule docs.

1. BLOCKING — rule doc misstates which manifest fields get the path-kind check — src/skillsaw/rules/docs/codex-plugin-json-valid.md:61-65 (propagated to docs/rules/codex-plugin-json-valid.md:76-80)

The doc states:

"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 implementation drives this off _EXPECTED_KIND = {"hooks": "file", "mcpServers": "file", "skills": "dir"} (plugin_json_valid.py:35), keyed by the field's base label. So interface.composerIcon/logo/logoDark/screenshots get no kind check; apps gets no kind check; and mcpServers has one the doc never mentions. The source even carries a comment contradicting the doc: "Only fields the tree actually filters on are listed — nothing drops apps or the interface assets silently."

Verified at runtime (temp fixture, restored): a manifest with "apps": "./apps-file.txt" (a file) and "interface": {"logo": "./assets/logo.png"} (a directory) produces zero violations, while "skills": "./skills.txt" correctly reports 'skills': './skills.txt' is a file — this field names a directory.

Rationale for BLOCKING: a reader following this doc will believe skillsaw guards their interface asset kinds and their apps directory. It does not — and the failure mode the doc describes (a component silently dropped by the loader) is exactly the one it promises to catch.

Action: rewrite the paragraph to match _EXPECTED_KIND, and regenerate docs/rules/codex-plugin-json-valid.md via make generate-site-content.

2. SUGGESTION — the .pre-commit-hooks.yaml files: change is a user-visible behavior change with no version marker — docs/pre-commit.md:39-50, 92-101

The published hook's files: regex (a ~25-alternative allowlist) was replaced with files: .. Every existing pre-commit user who bumps rev: will see the hook fire on commits that previously skipped it — a real, unannounced change in daily workflow cost. docs/pre-commit.md describes the new behavior well but reads as though it was always the case. There is no CHANGELOG.md or release-notes page, so the docs site is the only upgrade surface — and the repo already has the convention: !!! note "Removed in 0.15" and !!! note "Breaking changes (0.15)" in docs/autofixing.md:45,70.

Action: add a !!! note "Changed in 0.18" admonition stating the hook previously fired only on a filename allowlist, with the one-line rationale and a pointer to the files: override snippet already in Troubleshooting.

3. SUGGESTION — doc implies an author.name check that does not exist — codex-plugin-json-valid.md:55-56

"author must be a string or an object (an object should carry a name)". The rule only type-checks (plugin_json_valid.py:133-140); author: {} or author: {"email": "x"} passes silently. The parenthetical reads as a documented requirement.

Action: drop it, or reword to make clear it is upstream guidance skillsaw does not enforce.

4. SUGGESTION — Autofix: auto in the metadata table contradicts the page's own "How to fix" — docs/rules/codex-marketplace-registration.md:13 vs :64; generator at scripts/generate-site-content.py:272-274

The table says | **Autofix** | auto |; the body says "skillsaw fix --suggest adds a complete entry". The rule declares autofix_confidence = AutofixConfidence.SUGGEST, so a plain skillsaw fix will not touch it. The generator emits only "auto" for rule.supports_autofix and never surfaces the confidence. Pre-existing (Claude's marketplace-registration has the identical mismatch), but this is the first page where the contradiction sits beside the correct prose, and docs/autofixing.md teaches readers that [*] and [?] differ.

Action: teach the generator to emit suggest for AutofixConfidence.SUGGEST and regenerate — fixes both Codex and Claude registration pages and docs/rules/index.md in one pass.

5. SUGGESTION — uneven depth across the five new Codex rule docs

• codex-openai-metadata.md documents interface, policy, brand_color but omits the dependencies.tools[] entry-key checks the rule performs (type, value, description, transport, url, each string-typed — openai_metadata.py:244-253) and the icon severity split (escape/absolute = error, missing file = warning). Its siblings enumerate every check with its severity.
• Only codex-marketplace-registration.md explains the .codex/plugins/ stand-down. codex-plugin-json-valid, codex-plugin-structure, and codex-openai-metadata all implement the same stand-down but say nothing; the behavior lives only in the docs/repo-types.md table, which skillsaw explain <rule> users never see.

Action: add the missing dependencies.tools/icon-severity paragraph, and a one-sentence "Installed plugins" note to the three silent rule docs.

6. NOTE — docs/lint-tree.md shows no Codex nodes — :21-25. skillsaw tree now emits [codex plugin]/[codex] labels; the sample tree shows only Claude shapes. The page never enumerated node types exhaustively, so nothing is wrong — just no longer representative. Optional.

Verified clean: Generated-file drift: none — ran scripts/generate-docs.py, scripts/generate-site-content.py, skillsaw init, and skillsaw badge --large inside a throwaway copy at /tmp; git status --porcelain empty in every case. The working tree is untouched and all temp copies were removed. APM compiled outputs in sync (.claude/rules/, .cursor/rules/, .github/instructions/, AGENTS.md, .github/copilot-instructions.md all match .apm/instructions/). Documented defaults match code for all five rules (severities, enabled: auto, since = "0.18.0", repo-type sets, config schemas vs .skillsaw.yaml.example). The three behavior changes are documented where an upgrader looks (docs/configuration.md:178-186, docs/rules/content-broken-internal-reference.md:45-48, hooks-json-valid.md/mcp-valid-json.md); the non-string hook type change is a crash→report fix so no doc is owed. The "Adding an ecosystem" recipe is accurate — every symbol it names exists and behaves as described. The nine retro-fitted exemption sections are behaviorally true (plugin-readme is exempt by node typing, verified with a mixed fixture). No page moved, so no redirects owed. New lint targets documented (agents/openai.yaml in docs/repo-types.md; codex-plugin/codex-marketplace in docs/cli.md). THREAT_MODEL.md gained matching entry points, asset, threats T14/T15, and dated provenance.

Ecosystem Reviewer

Verdict: BELONGS IN CORE. No scope blocker.

I went in prepared to recommend redirect-to-plugin given the ~6.6k src/ lines and core-surface cost. The evidence does not support it.

Adoption clears the bar comfortably, by a wide margin over several formats already in core:

• The Codex plugin marketplace shipped 27 March 2026 with 20+ launch partners (Slack, Figma, Notion, Sentry, Linear, Atlassian, Vercel, Expo). A shipped product surface of a major vendor, not a proposal.
• openai/plugins: 4.8k stars, 641 forks, 299 commits, actively maintained.
• agents/openai.yaml — the file codex-openai-metadata validates — has 556 occurrences in openai/plugins alone, and Codex's own bundled skill-creator skill generates it.
• Third-party ecosystem exists independently of OpenAI: hashgraph-online/awesome-codex-plugins, codex-marketplace.com, an Elixir Codex SDK with dedicated Codex.Plugins.Marketplace modules.
• Nothing upstream labels the format experimental or preview.

Standardization favors core, decisively — the point that moved me most. Codex's authoritative manifest lookup is not a private path:

// codex-rs/exec-server-protocol/src/protocol.rs:46
pub const DISCOVERABLE_PLUGIN_MANIFEST_PATHS: &[&str] = &[
    ".codex-plugin/plugin.json",
    ".claude-plugin/plugin.json",
    ".cursor-plugin/plugin.json",
];

plus AGENT_PLUGIN_SCHEMA_URI = "https://agent-plugins.org/schemas/1.0.0/plugin.schema.json". Codex reads Claude's and Cursor's manifests, and the schema is being published at a vendor-neutral domain. My scope file says single-vendor niche formats favor plugins and multi-tool conventions favor core — this is squarely the latter, and PluginProvenance is the correct structural response to one directory claimed by two ecosystems. That abstraction is not Codex-specific overhead; it is what keeps Claude rules from misfiring on shared directories.

The plugin API genuinely cannot deliver this. Verified against the base commit. SKILLSAW_TREE_CONTRIBUTORS runs at the end of build_lint_tree and can only append root-level nodes. A plugin cannot: add a RepositoryType (closed enum), hook RepositoryContext discovery (all discovery runs eagerly in __init__ before plugin fields are initialized), populate context.codex_plugins/skills/plugins, attach nodes under an existing PluginNode, set plugin_owner/provenance_dir() so its nodes participate in scoping, or reuse a single builtin skill/command/hooks/MCP rule against its files. docs/plugins.md:14-19 already states the intended policy: "For common, popular formats it is recommended to contribute repository types, rules, and tree support back to skillsaw itself."

Spec conformance verified against the implementation, not the PR description — checked against openai/codex Rust sources and generated schemas. RawPluginManifest has name: String with every other field Option<_>, exactly the PR's posture (name hard error; version/description configurable warnings; interface type-checked only). All new rules are enabled: auto gated on the new repo types, so existing users are untouched.

1. SUGGESTION — the commands manifest field is unhandled — formats/codex.py:26, _PATH_FIELDS at codex/plugin_json_valid.py:26

commands is a real, implemented Codex manifest field with the same RawPluginManifestPaths shape as skills:

// codex-rs/core-plugins/src/manifest.rs:60
struct RawPluginCommandManifest {
    #[serde(default)]
    commands: Option<RawPluginManifestPaths>,
}
// :177  pub(crate) fn load_plugin_command_paths(...)
// :187  resolve_manifest_paths(&plugin_root, "commands", Some(&commands))

The PR covers skills, hooks, mcpServers, apps but not commands. Two consequences: codex-plugin-json-valid never validates a commands path (an empty value, a root-escaping ../, a missing target, or a missing ./ prefix all go unreported, whereas the identical defect in skills is caught); and there is no codex_declared_command_dirs, so the lint tree attaches commands only by literal directory convention (lint_tree.py:194) and a plugin declaring "commands": "./bundled-commands" gets zero coverage of those command files. This is precisely the failure mode codex_declared_skill_dirs was written to prevent — its own docstring says "The field does not have to say ./skills… nothing else walks them."

Action: add "commands" to _PATH_FIELDS and _EXPECTED_KIND ("dir"), add codex_declared_command_dirs(), attach the resulting CommandBlocks in the single plugin pass, and add a fixture with a non-conventional commands path.

2. SUGGESTION — .cursor-plugin/plugin.json is a Codex-discoverable manifest; document the deliberate scoping — discovery/codex.py:26-32

The module comment explains why .claude-plugin/marketplace.json is left to the Claude rules — a sound call — but does not mention that Codex's lookup covers three locations. Today a Cursor-manifest plugin registered as a local source in a Codex catalog gets a codex claim via _codex_claim_set but is never discovered by discover_codex_plugins (the .codex-plugin marker gate in _add), so codex-plugin-json-valid never runs on it. Behavior is safe (coverage gap, not a false positive), but a future maintainer will not know the third path exists.

Action: extend the module comment to name all three DISCOVERABLE_PLUGIN_MANIFEST_PATHS entries and state that skillsaw keys discovery on .codex-plugin/ only, with the reason. No behavior change needed in this PR.

3. NOTE — cite the generated schema for ON_USE, not the prose spec — marketplace_json_valid.py:40-43. DEFAULT_AUTHENTICATION_VALUES = ["ON_INSTALL", "ON_USE"] is correct — confirmed against codex-rs/app-server-protocol/schema/typescript/v2/PluginAuthPolicy.ts, generated from the implementation: export type PluginAuthPolicy = "ON_INSTALL" | "ON_USE";. ON_FIRST_USE has zero occurrences in openai/codex. Flagging only because developers.openai.com/plugins/build/plugins and several third-party docs say ON_FIRST_USE, so a future reviewer will "correct" this into a regression. Amend the comment to cite the generated schema as the tiebreaker.

4. NOTE — author is not a field Codex reads — plugin_json_valid.py:133-140. RawPluginManifest has no author field; Codex's deserializer ignores it. The check is harmless (fires only on a wrong type, and the plugin-creator prose spec lists author as required), but the code carries no note that it is prose-spec-only and inert to the runtime.

Checked and clean: manifest and marketplace paths both correct, with the api_marketplace.json sibling used by openai/plugins handled by _is_marketplace_filename; marketplace entry schema matches the live openai-curated catalog verbatim; source types local/url/git-subdir/npm and required fields match; policy.installation values match PluginInstallPolicy exactly; agents/openai.yaml fields match openai_yaml.md field-for-field including snake_case (correctly distinct from plugin.json's camelCase); required-vs-optional posture matches the Rust struct's Option<_> layout; rule enablement introduces no existing-user breakage; the .codex/plugins/ stand-down covers both $CODEX_HOME/plugins/{cache,data} and a repo-local CODEX_HOME, so the flat fixture layout is a fidelity nit with no behavioral consequence.

Slopinator Reviewer

Reviewed + lines across src/skillsaw/** comments and docstrings, docs/, src/skillsaw/rules/docs/*.md, README.md, THREAT_MODEL.md, AGENTS.md, the .claude//.apm//.cursor//.github/ instruction mirrors, the new tests/codex/ package, tests/fixtures/codex/**, branch commit subjects, and the PR body.

Overall: unusually clean. Keyword sweeps for the classic residue vocabulary (as requested, per review feedback, now handles, changed from, round N, fixed to) and the AI-prose set (delve, seamless, robust, comprehensive, leverage, not just X but Y) came back essentially empty. Comment density in the new rule files (5–14%) sits inside the existing house range. The long # blocks in discovery/codex.py, docs/extractor.py, and the codex rules are subjunctive "why this code exists" comments — exactly the comments the scope says to leave alone. New fixtures read as plausible real plugin content. The # ── … ── separators are pre-existing house style. The verifiable structural claim in the new instruction text (provenance_dir() overridden only on PluginNode, CommandBlock, AgentBlock) checks out against lint_target.py:231 and blocks/frontmatter.py:316,336.

Four residue findings survived, all clustered in the new test package.

1. BLOCKING — review-history narration in a shipped docstring — tests/codex/test_docs_output_safety.py:324-327

A scaling ratio, not a wall-clock bound — absolute timings are instrumentation-dependent (coverage tracing on Python <=3.11 has no sys.monitoring backend and multiplies per-line cost), and a wall-clock assertion here was once misread as a timing flake while it was correctly reporting quadratic behavior.

The last clause narrates an event inside this PR's review. A stranger opening this file in a year cannot verify it and gains nothing; the first half already justifies the ratio form completely.

Action: cut the trailing clause.

2. BLOCKING — round-scoped measurement plus a phantom bug — tests/codex/test_docs_output_safety.py:344-345

# Truncate-first makes this near-constant (ratio ~1); the old
# quadratic scan lands near 16. 8 leaves headroom for noise.

"the old quadratic scan" describes an implementation that does not exist on main — safe_display is new in this PR, so there is no "old" scan a future reader can find. The number 16 is a measurement frozen from one review round; nothing re-derives it.

Action:

        # The display cap is applied before the scan, so a 4x larger input
        # must not cost 4x the work. 8 leaves headroom for measurement noise.

3. BLOCKING — fix-sequence narration and a change announcement — tests/codex/test_containment.py:279-282

# Formerly a strict xfail: the generic walk had no containment
# boundary. _discover_skills_in_dir now derives one from
# provenance whenever it starts in or descends into a Codex-only
# directory, closing this route too.

"Formerly a strict xfail" is keyed to the state of this branch mid-review, not to anything that ships. "now derives … closing this route too" is a change announcement — git records that. What the test actually asserts never gets stated.

Action:

        # _discover_skills_in_dir derives a containment boundary from
        # provenance whenever it starts in or descends into a Codex-only
        # directory, so the generic walk cannot leave the checkout either.

4. BLOCKING — before/after narration of an optimization that never shipped separately — tests/codex/test_discovery.py:614-616

# One resolve per lookup, for the queried path itself. Before the
# memo it was one per root per lookup as well, which two rules pay
# once per skill.

"Before the memo it was…" describes a pre-memoization state that exists only in this branch's history.

Action:

        # One resolve per lookup, for the queried path itself: the root set
        # is memoized, and two rules pay this lookup once per skill.

5. SUGGESTION — the "Adding an ecosystem" recipe is one 15-line sentence — .claude/rules/development.md, mirrored verbatim in AGENTS.md, .apm/instructions/development.instructions.md, .cursor/rules/development.mdc, .github/copilot-instructions.md, .github/instructions/development.instructions.md

Seven imperative clauses chained across semicolons and three em dashes. Every other entry in this file is a bounded bullet, and a recipe is the one thing that most wants to be an ordered list — this is the shape an agent will follow step by step.

Action: break it into a numbered list, one step per current semicolon clause, keeping the wording; leave "teach in_format_scope nothing" and the closing "prose needs no work / no rule-file visits" as a trailing note. Purely restructuring; no new facts.

6. SUGGESTION — narration of the PR's own authoring — .claude/skills/skillsaw-maintenance/references/codex.md:14-16 (and .agents//.apm/ mirrors)

"the rendered HTML page summarizes poorly and has produced invented constraints (an ON_FIRST_USE value that appears nowhere in either source)."

Narrates something that happened during this PR's authoring; a future maintainer has no diff and cannot check it, and the passive hides who produced it. The actionable half is the instruction to read the .md twin.

Action:

- Spec: https://developers.openai.com/plugins/build/plugins — read the `.md`
  twin at https://developers.openai.com/plugins/build/plugins.md as the
  authoritative text. The rendered HTML page omits values the `.md` states;
  do not take a constraint from it that the `.md` does not carry.

7. NOTE — deictic pointing at a review conversation — marketplace/json_valid.py:88-92: "resurrecting this very false positive on a repository the default invocation calls clean." Nothing above names the false positive. Suggest: "resurrecting the false positive this exemption exists to prevent, on a repository…".

8. NOTE — past indicative for a conditional invariant — tests/test_context.py:857-859: "matching the entry made the built-in template exclude swallow a Missing SKILL.md error…". Suggest "would make".

9. NOTE — branch commit subjects. ~20 of the branch's commits are keyed to review iterations rather than behavior — Address Codex review round thirteen, Address review round ten, Address panel verdict and review round eleven, Close the second panel's findings and the round-fifteen gaps, Trim review-round accretion from the test suite. main uses descriptive imperative subjects with a trailing (#NNN), which is squash-merge shaped, so these should not survive the merge — filed as a NOTE only because they land in permanent history if merged with a merge commit. Confirm the merge strategy before landing; the PR title and body are already in the right register.

10. NOTE — PR body is a version-scoped document and exempt from diff-anchoring rules. One nit: "zero codex-* findings" and "a net false-positive reduction" lean on emphasis where the numbers already carry the point.

---

Panel Synthesis

This is a large, unusually well-executed change. The Ecosystem Reviewer went in expecting to recommend redirect-to-plugin and came back with the opposite conclusion, on strong evidence: Codex's manifest lookup is a published constant that reads Claude's and Cursor's manifests, the schema is headed for a vendor-neutral domain (agent-plugins.org), adoption exceeds several formats already in core, and the plugin API provably cannot add a RepositoryType, hook eager discovery, or attach nodes under an existing PluginNode. Scope is settled: this belongs in core, and PluginProvenance is the right structural response rather than Codex-specific overhead. The test suite passes at 3,482 with 94% coverage (independently verified), generated-file drift is zero, no new dependencies, CI workflows are correctly hardened, and the HTML renderer survived an adversarial XSS sweep across every javascript: spelling the reviewer could construct.

Four blocking items stand between this and merge. Two are substantive; two are cheap but required.

The one that matters most is not about Codex at all. _resolve_lint_paths widens .claude-plugin alongside .codex-plugin, and I reproduced the consequence end-to-end. At base, skillsaw lint <repo>/.claude-plugin/plugin.json roots the context at .claude-plugin, reports repo type unknown, and exits clean. At HEAD it roots at the plugin directory and reports command-frontmatter on commands/deploy.md plus plugin-readme. Worse, _resolve_lint_paths is shared with _fix.py:35 with no post-filter — I confirmed that skillsaw fix <repo>/.claude-plugin/plugin.json now rewrites commands/deploy.md, a file the caller never named. The repo's own development rules make backward compatibility paramount, and CI setups that pipe changed files into skillsaw are common. The Codex half of this widening is new surface and fine; the .claude-plugin half is an unannounced behavior change on a long-standing path that also expands write blast radius. Only the Architecture Reviewer found it, so I verified it myself rather than take it on one report.

The second is a mitigation claim that does not hold. The Security Reviewer demonstrated that skillsaw docs --format markdown passes raw <script> and <img onerror=> from third-party plugin metadata straight through, and that several heading and metadata sinks get no folding at all. I confirmed the root cause directly: _table_cell escapes \ | [ ] ( ) and not < or >. The HTML renderer is clean — the two renderers simply disagree about whether this data is hostile. The sharp edge is that THREAT_MODEL.md T15 is recorded as mitigated against exactly the published_docs asset this reaches, and GitHub sanitizes while MkDocs and Docusaurus do not. Either close the gap or downgrade T15 to partially_mitigated with the Markdown path named — but the threat model must not overstate its coverage.

The remaining two blockers are inexpensive. The codex-plugin-json-valid rule doc tells readers that interface asset paths and apps get kind checks; _EXPECTED_KIND covers only hooks, mcpServers, and skills — I verified both the constant and the doc text. It is a one-paragraph fix, but it promises a guarantee the linter does not provide. And the Slopinator found four comments in the new tests/codex/ package that narrate this PR's own review history ("was once misread as a timing flake", "Formerly a strict xfail", "the old quadratic scan", "Before the memo it was") — each anchored to a branch state that will not exist after merge. Replacement wording is supplied inline for all four.

Corroboration across specialists strengthened four non-blocking items. Architecture and Python independently flagged the tree builder reaching into context._codex_claim_set() and the fact that codex_install_root() never returns None, leaving its early-out dead and costing a resolve() per violation path on every lint. Python and Security independently flagged the .resolve() ratchet — and Python's version is sharper: codex/marketplace_registration.py was created by this PR and is already on the grandfather list, which defeats the ratchet's stated guarantee on day one. That one is worth fixing now, while it is a one-line change.

The highest-value optional item is performance. Python measured context construction on a Codex-free repo going from 94ms to 238ms, with provenance() called once per walked directory accounting for 0.193s of a 0.509s cold build, plus an unconditional realpath() per directory entry that is discarded when contain_within is None. Every existing Claude-only user pays this for a feature they do not use. The proposed one-exists() pre-gate recovered 36% in measurement. This is not blocking — it is a constant-factor regression, not a correctness break, and the repo has a benchmark harness to track it — but it should not be deferred indefinitely.

On disagreements: there were none of substance. No specialist contradicted another; where two looked at the same code they reached the same conclusion. The Ecosystem Reviewer's commands finding (a declared non-conventional commands path gets zero lint coverage, the exact failure mode codex_declared_skill_dirs exists to prevent) is the most substantive of the suggestions and is a genuine gap rather than a nit, but it is a coverage gap in new functionality, not a regression, so it does not block.

Two housekeeping notes for the author: the PR description lists the dir/** exclusion change as one of three behavior changes in this branch, but Architecture found _pattern_variants is semantically identical to base — that change landed before the merge base. And ~20 branch commits are titled by review round (Address review round ten, Close the second panel's findings and the round-fifteen gaps); main's history is squash-merge shaped, so confirm the merge strategy before landing.

---

Required Actions Before Merge

1. Stop widening .claude-plugin in _resolve_lint_paths (src/skillsaw/cli/_helpers.py:66). Restrict the widening to the new surfaces (.codex-plugin, .agents/plugins). Reproduced end-to-end: at base skillsaw lint <repo>/.claude-plugin/plugin.json reports nothing; at HEAD it lints the whole plugin, and skillsaw fix on the same path rewrites commands/deploy.md — a file the caller never named. If the widening is intended, it must be called out as a fourth deliberate behavior change and fix must be gated so writes stay within the named path. Add a regression test pinning the fix blast radius.

2. Close the Markdown-renderer injection gap, or correct T15 (src/skillsaw/docs/markdown_renderer.py:118 and the unguarded sinks at :149, :220, :242, :249-253, :260, :268, :293, :300). Add </> to _table_cell's escape tuple, apply it to the un-guarded heading/meta sinks, and route plugin.description / skill.description / agent.description through the scheme allow-list — or restate THREAT_MODEL.md T15 as partially_mitigated naming the Markdown path as accepted risk. Either way, add a test asserting hostile fixture output contains no raw <script/onerror/javascript:; tests/codex/test_docs_output_safety.py currently covers only the HTML renderer.

3. Fix the codex-plugin-json-valid doc's path-kind paragraph (src/skillsaw/rules/docs/codex-plugin-json-valid.md:61-65). It promises kind checks for interface asset paths and apps that _EXPECTED_KIND = {"hooks": "file", "mcpServers": "file", "skills": "dir"} does not perform, and omits mcpServers which it does. Regenerate via make generate-site-content.

4. Remove the four review-history comments in tests/codex/ — test_docs_output_safety.py:324-327 and :344-345, test_containment.py:279-282, test_discovery.py:614-616. Replacement wording is supplied inline in the Slopinator section.

---

Optional Follow-ups

Ordered by value. None block merge.

• Recover the context-build regression (context.py:1343-1367): 94ms → 238ms on a Codex-free repo. Pre-gate the provenance() consult on a single exists() stat and move safe_resolve(item / "SKILL.md") inside the .exists() branch — measured 36% recovery. Pairs with making codex_install_root() return None when .codex/plugins/ is absent (discovery/codex.py:295-297), a further ~10% and the fix for a dead early-out on the _filter_violations hot path.
• Handle the commands manifest field (plugin_json_valid.py:26, formats/codex.py:26): a plugin declaring "commands": "./bundled-commands" currently gets zero coverage of those files, and the path itself is never validated. Add codex_declared_command_dirs() alongside the skills equivalent, plus a non-conventional-path fixture.
• Un-grandfather codex/marketplace_registration.py:249 from GRANDFATHERED_BARE_RESOLVE and use safe_resolve — a file added by this PR should not be on the ratchet's debt list. Consider a per-function rather than per-file allowlist.
• Reconcile the two provenance spellings: PluginProvenance.installed is written and never read, while is_codex_installed_plugin / is_installed_plugin_skill / Linter._is_vendor_managed are three names for the same question. Pick one.
• Expose _codex_claim_set() publicly and reconcile it with distinct_plugin_dirs() — two definitions of "every plugin directory" currently differ silently between the tree and the badge/stats path (flagged by both Architecture and Python).
• Enforce the discovery/ layering invariant: add discovery/*.py to _core_module_files() and skillsaw.discovery.codex to CYCLE_SENSITIVE_MODULES in tests/test_module_layering.py. The one rule the package exists to hold is currently unchecked.
• Resolve the provenance_scope / container-typing duplication: the "claude" scope is provably inert on all three PluginNode rules, and plugin-readme is exempt purely by tree shape. Two mechanisms, one policy — pick one and document it.
• Test coverage (QA): ten uncovered violation branches in codex-marketplace-json-valid; the positive duck-typing branch at discovery/codex.py:153 (deleting it leaves the suite green); the autofix's multi-catalog target selection; a Claude-repo mirror of the non-string hook type regression test; an end-to-end dir/** exclusion test; one permission-error test.
• Docs: add a !!! note "Changed in 0.18" for the .pre-commit-hooks.yaml files: . change (existing users bumping rev: will see the hook fire far more often); teach the site generator to emit suggest for AutofixConfidence.SUGGEST instead of auto; even out the five Codex rule docs (dependencies.tools checks, the .codex/plugins/ stand-down note).
• Prose: restructure the one-sentence "Adding an ecosystem" recipe into a numbered list across its six mirrors; de-narrate skillsaw-maintenance/references/codex.md:14-16; the marketplace/json_valid.py:88 and test_context.py:857 wording nits.
• Comment accuracy: cite the generated PluginAuthPolicy.ts schema for ON_USE (the published prose docs say ON_FIRST_USE, and a future reviewer will "correct" this into a regression); note that author is prose-spec-only and inert to Codex's deserializer; document that skillsaw deliberately keys discovery on .codex-plugin/ only, of Codex's three discoverable manifest paths.
• PR description: the dir/** exclusion change predates this branch's merge base; and confirm squash-merge so the ~20 review-round commit subjects do not land in permanent history.

---

_{Generated by skillsaw-review-panel}

[not-stbenjam]

Response to the panel round

All four required actions are addressed in 2f173ea (plus fe21887 for the two Qodo-found defects that overlapped).

1. .claude-plugin widening removed (cli/_helpers.py). Widening is Codex-only again — .codex-plugin and .agents/plugins. The panel's reproduction was correct and it is the one finding here that changed behavior for existing Claude users, which is exactly why it should not ship: skillsaw fix <repo>/.claude-plugin/plugin.json could rewrite commands/*.md. The old test that pinned the widening is replaced by test_claude_manifest_file_input_is_not_widened, which asserts the fix blast radius — a command file next to the manifest must be byte-identical after fix.

2. Markdown-renderer injection closed, and T15 restated honestly. _table_cell now entity-encodes </> (and replaces backticks, from the Qodo find — escapes do not work inside a code span), and the previously raw sinks now fold through it: skill and agent headings, command headings, license, compatibility, allowed-tools. TestMarkdownRendererHostileMetadata asserts no raw <script/<img/<b> and no active-scheme link survives into generated Markdown. Description fields are the deliberate exception — they are author prose rendered as Markdown by design — so T15 is now partially_mitigated with that accepted risk named explicitly, rather than claiming a mitigation that does not cover the prose path.

3. codex-plugin-json-valid doc corrected. It now names the fields the rule actually kind-checks (hooks, path-valued mcpServers, skills) and states that apps and the interface asset paths are containment- and existence-checked only. Regenerated via make update.

4. Four review-history comments de-narrated in tests/codex/ at the cited lines.

Also folded in from the Codex round: catalog-claimed directories now join the Codex ownership roots, so codex_plugin_owning() answers correctly for a manifest-less claimed directory — scoped to Codex-exclusive claims so a dual-identity directory stays owned by its Claude manifest and docs keep refusing to publish a claim Codex cannot install.

Declined, with reasons (replied inline): the PEP 604 / Python 3.9 report (the module has from __future__ import annotations; the 3.9 job is green), the workflow python3 -c report (CI step, no project venv), and the whole-catalog autofix reserialization (precedent-consistent with the pre-existing Claude registration rule, and behind AutofixConfidence.SUGGEST).

Validation after the fixes: 3,484 tests green, make lint/make update clean, openai/plugins canary unchanged (973 total, 0 codex-*).

The optional follow-ups are recorded rather than folded in — the perf recovery, the commands manifest field, the layering-invariant test, and the claim-set/distinct_plugin_dirs reconciliation are each a small standalone PR, and #470 already tracks the context.py decomposition they touch.

[not-stbenjam]

@coderabbitai review --dir src/skillsaw

[coderabbitai]

@not-stbenjam: Starting a focused review of src/skillsaw.

✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 2f173ea62f

ℹ️ 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]

Review Panel Verdict

Disposition: REQUEST_CHANGES

---

Specialist Findings

Architecture Reviewer

Reviewed the source surface (context.py, lint_tree.py, discovery/codex.py, formats/codex.py, paths.py, diagnostics.py, lint_target.py, rule.py, linter.py, blocks/*, docs/extractor.py, rules/builtin/codex/*, and the format-scoped rule edits) against .claude/rules/development.md.

The layering is genuinely good: discovery/ is state-free and imports nothing from context; formats/codex.py is a pure reader; paths.py centralises the unsafe-resolve() idiom and is backed by a ratchet test; PluginProvenance really is a single decision point; the one-pass plugin loop in build_lint_tree matches the documented invariant; OpenAIMetadataBlock correctly sits as a direct LintTarget rather than under ContentBlock. No import cycles, no per-ecosystem attach loops, no ContentBlock misuse. Findings below are about cost and encapsulation, not about the shape of the design.

1. BLOCKING — Rule.scoped_find() defeats the memoized-find() contract; ~0.9 s of pure overhead per lint on a mid-size marketplace — src/skillsaw/rule.py:198-212

find() is memoized per node precisely because "every rule re-discovers its targets through find(), and the tree is static while rules run" (lint_target.py:50-64). scoped_find() wraps that memoized call in an un-memoized list comprehension calling context.in_format_scope(n, ...) → provenance(owner_dir) per node, per rule, per run. provenance() calls safe_resolve() before its cache lookup, so even a cache hit costs a realpath syscall.

Measured on a synthetic 200-plugin / 4000-command marketplace:

BASE: tree.find(CommandBlock) x4  = 0.000s   (memoized)
HEAD: scoped_find(CommandBlock) x4 = 0.907s

Four passes is exactly the real load: command-frontmatter, command-naming, command-name-format, command-sections all declare provenance_scope = "claude". Add agent-frontmatter (AgentBlock) and the three PluginNode rules and this is ~1 s added to every lint of a large marketplace — including repositories with zero Codex content, where the filter is a guaranteed no-op.

Recommended action: memoize the scope filter. Either (a) cache (id(node), ecosystem) -> bool on the context alongside _provenance_cache, or (b) short-circuit in in_format_scope when the repository has no Codex evidence at all (not self._codex_claim_set() and not self.codex_catalog_exists() → return True immediately). Also make provenance() cheap on the hit path: key the cache on the unresolved path first and fall back to the resolved key, so a repeat consult costs no syscall. Save a benchmark baseline on main and compare per .claude/rules/development.md.

2. BLOCKING — RepositoryContext.__init__ is 1.5–2.3× slower on repositories with no Codex content — src/skillsaw/context.py:1328-1382 (_discover_skills_in_dir), src/skillsaw/lint_tree.py:216-242 (_add_openai_metadata)

Synthetic repo (120 plugins × 25 skills = 3000 skills, pure Claude, no .codex-plugin, no .agents/):

BASE: context=0.89-0.95s  tree=2.6-2.9s
HEAD: context=1.3-2.1s    tree=3.1-4.2s

cProfile of __init__: base 1.48 s → HEAD 3.91 s; _discover_skills_in_dir 1.14 s → 3.34 s; safe_resolve calls 7 562 → 18 144. Tree build: realpath 18 966 → 31 809, lstat 168 k → 275 k.

Three hot spots, all unconditional work for a Codex-only concern:

• context.py:1365 — entrypoint = safe_resolve(item / "SKILL.md") runs for every directory entry, but the value is only read inside if contain_within is not None. ~3 800 wasted realpath calls.
• context.py:1376 — self.provenance(item) is called for every directory lacking a SKILL.md. Each miss costs ~6 syscalls.
• lint_tree.py:225-235 — _add_openai_metadata performs three safe_resolve() calls before the cheap safe_is_file(path) existence test, once per SkillNode. ~9 000 realpath syscalls to discover nothing.

Recommended action: guard the SKILL.md resolve under if contain_within is not None; gate the provenance(item) descent check on a cheap "this repo has Codex evidence" flag computed once per context; reorder _add_openai_metadata to test safe_is_file(path) / _is_excluded(path) first and resolve only on a hit.

3. SUGGESTION — build_lint_tree reaches into RepositoryContext._codex_claim_set() — src/skillsaw/lint_tree.py:316

A private call across a module boundary is the edge that silently breaks when the caching strategy inside context changes, and it is the exact seam the "Ecosystem provenance" rule is written to protect. Promote to a public codex_claimed_dirs().

4. SUGGESTION — marketplace-json-valid answers an ownership question with a fresh filesystem probe — src/skillsaw/rules/builtin/marketplace/json_valid.py:54-77

.claude/rules/development.md is explicit: "Never answer an ownership question with a fresh filesystem probe in a rule or in the tree builder." _has_claude_plugin re-derives "does any plugin declare Claude intent" rather than reading context.provenance(node.path).claude. The two answers already disagree by construction. Add the distinction to the provenance record (e.g. claude_marker: bool) so ownership evidence stays gathered in one place.

5. SUGGESTION — context.py re-exports discovery/format symbols, blurring the layering the PR just established — src/skillsaw/context.py:17-30, 657-662

merge_plugin_dirs and codex_local_source_path are imported into context solely to be re-exported (the latter is flagged unused by flake8), and four Codex constants are aliased as class attributes on RepositoryContext. This creates a false dependency edge on context for callers that need nothing stateful. Import from skillsaw.formats.codex / skillsaw.discovery at the call sites.

6. SUGGESTION — Linter._is_vendor_managed passes a file path to is_codex_installed_plugin(plugin_dir) — src/skillsaw/linter.py:469-487

Works by ancestor test, but the contract is now "path or directory, whichever" and nothing pins it. Also codex_install_root() returns non-None even when .codex/plugins does not exist, so the install_root is None fast path never fires.

7. SUGGESTION — build_lint_tree has grown into a ~560-line function with six stateful closures — src/skillsaw/lint_tree.py:78-242

Four mutable dedupe sets whose interaction rules are subtle and load-bearing, expressible only as closure state — nothing can unit-test them in isolation. Extract a _TreeBuilder class.

8. NOTE — dead import: src/skillsaw/lint_tree.py:42 imports codex_manifest_is_contained but never uses it.

9. NOTE — LintTarget.print_dot colour map (lint_target.py:152-164) missed CodexMarketplaceConfigNode, CodexPluginConfigNode, OpenAIMetadataBlock.

10. NOTE — LintTarget.plugin_owner (lint_target.py:26) adds a field to every node for a tag written by one producer and read by one consumer; a side-table keyed by node identity would keep the base node type free of a docs-specific concern.

Checked, no issue found: no import cycles (test_module_layering.py correctly widened); one-pass plugin loop honours the "never add a second per-ecosystem plugin loop" invariant; provenance_scope filtered in exactly one place; CodexInlineHooksBlock/CodexInlineMcpBlock correctly override __eq__/__hash__ to identity; config files land under JsonConfigBlock, never ContentBlock; parse failures become violations rather than exceptions; the __init__ ordering hazard is real but handled by the unconditional _provenance_cache.clear() in apply_excludes.

Python Expert

No BLOCKING findings. Full suite passes (3484 passed), black --check clean, no Python 3.10+ syntax reached any new module, all new modules carry from __future__ import annotations where PEP 604 annotations are used, Path.is_relative_to (3.9+) is the newest stdlib API in play.

1. SUGGESTION — a brand-new file was added to the bare-.resolve() ratchet allowlist in the same PR that introduced the ratchet — tests/test_banned_apis.py:44, src/skillsaw/rules/builtin/codex/marketplace_registration.py:249

The ratchet docstring states the contract: "existing debt is grandfathered per issue #463, but any new file reaching for bare .resolve() fails here." Of the 30 allowlist entries, exactly one is a file this PR creates. Its single offender is if plugin_dir.resolve() in registered_dirs: — and the file already imports safe_resolve and uses it in four other places. Live hazard: plugin_dir comes from CodexPluginConfigNode.plugin_dir, so a symlink loop or unreadable parent turns this into a rule-execution-error that discards every finding the rule would have produced for the whole catalog.

Recommended action: use safe_resolve with a None guard and drop the allowlist entry (test_allowlist_has_no_stale_entries will enforce it). Line 244's plugin_node.path.is_file() should be safe_is_file for the same reason.

2. SUGGESTION — O(plugins × nodes) scans with a full list copy per call in the Codex docs extractor — src/skillsaw/docs/extractor.py:437-448, :301-317

LintTarget.find is memoized but returns list(found) — a fresh copy on every call. _extract_codex_plugin calls _owned_blocks five times plus _extract_codex_skills once, all inside a per-plugin loop: 6 × plugins full-list copies plus plugins × nodes comparisons. This is precisely the pattern the neighbouring comment in lint_tree.py:462 warns about. Build the owner index once before the loop — plugin_owner is already recorded at build time, so it is a pure read.

3. SUGGESTION — linear scan of seen_roles inside the extra-content-paths loop — src/skillsaw/lint_tree.py:588-596

seen_roles accumulates one entry for every block attached anywhere in the tree, so this is O(len(matches) × len(all_blocks)) with a tuple unpack per comparison. Maintain a parallel seen_role_paths: Set[Path].

4. SUGGESTION — provenance() papers over an __init__ ordering dependency with getattr — src/skillsaw/context.py:800-804

marketplace_entries is initialised at line 269 but _detect_types() runs at 262 and can reach provenance(). The getattr default silently makes provenance answer "no Claude claim" during that window, and correctness then depends on the end-of-init _provenance_cache.clear(). Two coupled invariants held together by a defensive getattr. Move self.plugin_metadata = {} / self.marketplace_entries = {} above the repo_types assignment.

5. NOTE — lint_tree.py:316 calls private context._codex_claim_set() from a different module.

6. NOTE — discovery/codex.py:236, 243 use bare is_dir() in a module whose whole premise is "never raise during discovery"; every other call site uses safe_is_dir.

7. NOTE — formats/codex.py:136: if safe_is_dir(candidate) if want_dir else safe_is_file(candidate): is correct but hostile; hoist probe = safe_is_dir if want_dir else safe_is_file.

8. NOTE — rules/builtin/mcp/valid_json.py:199-204 triple-negative guard; spell it positively as url_handled_elsewhere.

9. NOTE — diagnostics.py:108 dead branch: if truncated or len(text) > _MAX_DISPLAY: — the second disjunct can never be true when truncated is false.

10. NOTE — cli/_helpers.py:81 uses is not identity comparison on Path value objects.

11. NOTE — blocks/json_config.py:221-258: _InlineJsonPayload requires every subclass to redeclare inline_data; a __init_subclass__ assert turns a silent wrong-payload bug into an import-time error.

Clean: packaging change is version-only (0.17.0 → 0.18.0), matched in __init__.py. safe_resolve's three-exception catch correctly covers the 3.13 symlink-loop exception-type change. is_absolute_path correctly handles drive-relative (C:foo) and root-relative (\Windows) Windows shapes that PureWindowsPath.is_absolute() misses. _redact_userinfo invariants hold with no quadratic blowup. find() returns a copy, so _owned_blocks' extend does not corrupt the memo; set_parents() drops every cache after the build. read_json catching bare ValueError (3.11+ integer-digit-limit) is correct and important. commented_key_line KeyError guard for YAML merge keys is a real bug fix. _URI_SCHEME correctly cannot match a path-shaped target.

Security & Supply Chain Reviewer

Verified the PR's own security claims by exercising them, not by reading the prose. One blocking hole; the rest of the claimed hardening holds up.

1. BLOCKING — Catalog-claimed directory with a symlinked .codex-plugin/ reads and discloses files outside the checkout — src/skillsaw/context.py:812-815, src/skillsaw/lint_tree.py:400-408, src/skillsaw/rules/builtin/codex/plugin_structure.py:42

discovery/codex.py _add() and formats/codex.py:codex_manifest_is_contained() both containment-check the .codex-plugin marker. But provenance() has a second way to set codex=True:

if codex_manifest_is_contained(plugin_dir) or (
    resolved is not None and resolved in self._codex_claim_set()   # <-- no marker containment
):
    ecosystems.add("codex")

_codex_claim_set() is fed by codex_local_sources(), which only checks that the source directory stays inside the repo root — it never looks at the marker. build_lint_tree then creates a CodexPluginConfigNode unconditionally on prov.codex, with no containment check. Everything downstream follows the symlink.

This is exactly the hole the registration rule already patches locally for itself (marketplace_registration.py:346-352: "This rule reads the manifest itself, so it needs its own containment check — discovery's does not cover this path") — seen in one rule, not closed at the tree/provenance layer.

Reproduced (git-committable, relative symlink, fresh repo on this branch):

.agents/plugins/marketplace.json  -> {"plugins":[{"name":"foo","source":"./plugins/foo",...}]}
plugins/foo/.codex-plugin         -> symlink ../../../target   (outside the checkout)

⚠ codex-plugin-json-valid  [plugins/foo/.codex-plugin/plugin.json]: Plugin name 'LEAKED_NAME' should use kebab-case
⚠ codex-plugin-structure   [plugins/foo/.codex-plugin/id_rsa]: 'id_rsa' does not belong in .codex-plugin/

codex-plugin-structure iterdir()s the external directory and emits one violation per filename — an arbitrary directory listing. skillsaw docs on the same repo publishes the out-of-tree metadata into generated pages. Attack path: a fork PR to any repo running skillsaw in CI (the Action posts results as PR comments / SARIF artifacts). This is threat T6 realized through the new codex_plugin_inputs entry point, which THREAT_MODEL still marks mitigated. Reads only — no write path reaches these files.

Recommended action: close it at the single ownership point per the repo's own "Ecosystem provenance" rule. Either (a) in provenance(), require claim-derived codex evidence to also pass marker containment when the marker exists, or (b) in build_lint_tree before CodexPluginConfigNode(path=manifest), skip when the marker exists but contained_resolve(plugin_path / ".codex-plugin", resolved_plugin) is None. Keep the missing-marker case attaching. Add a fixture under tests/fixtures/codex/.

2. SUGGESTION — Three Markdown-renderer sinks bypass _table_cell; THREAT_MODEL T15 overclaims — src/skillsaw/docs/markdown_renderer.py:277, :280, :309

Every other metadata scalar folds through _table_cell(), but hook event names, matchers and rule globs are interpolated raw:

lines.append(f"### {hook.event_type}")           # arbitrary JSON key
lines.append(f"**Matcher:** `{entry.matcher}`")  # arbitrary string
lines.append(f"**Paths:** {', '.join(f'`{g}`' for g in rule.globs)}")

parse_hooks_events() does not constrain event-key content. Reproduced: a hooks.json whose event key is "SessionStart\n\n## Injected Heading\n\n[click me](javascript:alert(1))\n\n<img src=x onerror=alert(2)>" emits a live javascript: link and raw <img onerror> into the generated p1.md; a backtick in matcher/globs closes the code span the same way _table_cell's comment says it exists to prevent. The HTML renderer escapes all three correctly — this is Markdown-only.

T15's mitigation text says "_table_cell() folds newlines and escapes Markdown link/table delimiters in metadata scalars" and scopes the accepted risk to description fields only. These three are not descriptions and not author prose, so the entry is inaccurate as written.

Recommended action: wrap all three in _table_cell() (matches _append_mcp_table). Add the hostile-hooks case to tests/codex/test_docs_output_safety.py. If any are deliberately left raw, amend T15's accepted-risk clause to name them.

3. SUGGESTION — safe_display() is per-callsite discipline with no ratchet; one new sink already skips it — src/skillsaw/rules/builtin/mcp/valid_json.py:207

Redaction happens at message construction, not at any sink. The new Codex-gated MCP check echoes an untrusted manifest key unredacted: f"MCP server '{server_name}' '{required_field}' must be a non-empty string". server_name is an mcpServers object key, reachable from inline mcpServers in a Codex plugin.json — inside T14's codex_plugin_inputs entry point, where T14 claims manifest-derived values have credentials stripped. Contrast: bare Path.resolve() got a proper ratchet; message construction got none.

Recommended action: route server_name through safe_display(). Consider extending test_banned_apis with an AST check on self.violation(...) f-strings under rules/builtin/codex/.

4. NOTE — U+2028/U+2029 survive _safe_url (docs/extractor.py:398) and land literally in the inline <script> JSON (html_renderer.py:406 dumps with ensure_ascii=False). ES2019+ engines are fine; pre-2019 would fail to parse the whole DATA block. Add 

 to _URL_FORBIDDEN.

5. NOTE — cli/_helpers.py:62-89: naming .../.codex-plugin/plugin.json as a file argument roots the run one or two levels up, so skillsaw fix <that file> rewrites siblings the caller did not name. The $HOME/anchor bound is correct and the asymmetry with .claude-plugin is deliberate, but worth stating in docs/cli.md.

6. NOTE — the generic repo walk still follows symlinks out of the checkout (plugins/linked -> /outside). Confirmed identical on the merge base — pre-existing, not a regression — but notable because T6 is marked mitigated on the strength of _resolve_plugin_source() alone.

Checked and clean: pyproject.toml version bump only, no new/loosened/unpinned dependencies. CI adds openai/plugins cloned over HTTPS and pinned to a commit SHA in both workflows; all actions SHA-pinned, persist-credentials: false, pull_request (not pull_request_target), contents: read. paths.py safe_* exception sets correct for 3.9–3.14; escapes_root() fails closed; is_absolute_path() catches both Windows containment bypasses. safe_display() exercised against 16 credential shapes plus ANSI and bidi-override injection — all redacted; timed at 1M chars, < 1 ms, no quadratic blowup. HTML docs XSS: hostile fixture with javascript:/data:/entity-encoded schemes, attribute-breakout, JS-string-breakout and </script> payloads across eight fields — every payload landed inert; escAttr/escJsAttr verified in Node against 7 payloads with no breakout; _get_js() passes node --check. Filename handling tested with ../../evil, /etc/passwd, embedded NUL, con, 400-char names — no traversal, no collision-overwrite. Autofix stand-down for .codex/plugins/ verified end-to-end. Registration autofix declines on symlinked catalog paths, unparseable/duplicate-key JSON, and RecursionError, re-deriving every guard in fix(). Codex marketplace source traversal (../outside, symlink to /outside) both rejected. No subprocess, shell=True, eval/exec, or yaml.load added anywhere in the diff. Lint-report formatters escape correctly.

QA Engineer

Suite verification (run, not trusted): make test → 3484 passed, 0 failed, 20 warnings, 678 s, exit 0. Coverage 94% (13159 stmts / 799 miss). The PR's "3,482 tests, 94% coverage" claim is accurate. New-module coverage: paths.py 100%, codex/plugin_json_valid.py 100%, codex/_helpers.py 100%, codex/openai_metadata.py 99%, codex/marketplace_registration.py 95%, codex/plugin_structure.py 94%, codex/marketplace_json_valid.py 93%, discovery/codex.py 93%, docs/extractor.py 93%.

Verified — the three behavior changes: hooks-json-valid non-string type is regression-protected (tests/codex/test_hooks_mcp.py:419, parametrized over [], {}, ["command"], 42; :520 covers redaction; tests/test_hook_rules.py:170 pins that the Codex matcher tightening does not change Claude results, with an anti-vacuity assertion). content-broken-internal-reference URI schemes covered at tests/test_content_rules.py:1429 including the C:/notes.md two-char boundary. Exclusion dir/** — see NOTE 7.

Verified — new-rule fixtures & autofix invariants: tests/fixtures/codex/{clean,broken,dual-manifest} are realistic and wired into BROKEN_FIXTURES/CLEAN_FIXTURES, so test_every_rule_fires_somewhere gates all five new rules. codex-marketplace-registration is SUGGEST, so EXPECTED_SAFE_VIOLATIONS correctly stays unchanged; its CLI harness covers no-op-without---suggest, idempotency, re-lint-clean, BOM+CRLF preservation. I independently ran skillsaw fix --suggest twice — byte-identical, append-only diff. 369 tests across tests/codex/, including injection-based tests for OSError/RuntimeError/ValueError from Path.resolve() and RecursionError from the parsers.

1. SUGGESTION — nine unexecuted diagnostics in a new 506-line ERROR-severity rule — src/skillsaw/rules/builtin/codex/marketplace_json_valid.py:143 (also 149, 184, 241, 249, 315, 402, 404, 450, 464)

All are reachable — I built a catalog firing every one, producing correct but unasserted messages ('name' must be a non-empty string, 'interface' must be an object, plugins[0] missing required 'source' field, plugins[0].policy must be an object, plugins[1] missing required 'name' field, plugins[2] plugin name must be a string, plugins[3].source must be a relative path string or an object, plugins[4].source.registry must be a string, plugins[5].source.registry: must not have a fragment). These are the rule's required-field messages; a refactor could silently drop any of them today. Add a parametrized test in tests/codex/test_marketplace_json.py.

2. SUGGESTION — new debt added under the "grandfathered" banner in the same commit that introduced the ratchet — tests/test_banned_apis.py:44

marketplace_registration.py:249 — if plugin_dir.resolve() in registered_dirs: inside _unregistered(), reached for every discovered plugin directory. On Python ≤3.12 a symlink-loop plugin dir raises RuntimeError here, degrading the rule to a rule-execution-error. Use safe_resolve (already imported at line 24), drop the allowlist entry, and add a TestResolveFailureModes-style injection test.

3. SUGGESTION — untested load-bearing except OSError guard — src/skillsaw/rules/builtin/codex/plugin_structure.py:45

Genuinely reachable: discover_codex_plugins accepts a regular file at the reserved .codex-plugin name (discovery/codex.py:215 deliberately uses existence-or-symlink, not is_dir()). Verified end-to-end — a repo with plugins/foo/.codex-plugin as a plain file lints cleanly with no traceback. Add:

def test_a_regular_file_at_the_marker_name_does_not_crash(self, tmp_path):
    repo = tmp_path / "r"; (repo / "plugins" / "foo").mkdir(parents=True)
    (repo / "plugins" / "foo" / ".codex-plugin").write_text("not a dir")
    ctx = RepositoryContext(repo)
    assert CodexPluginStructureRule({}).check(ctx) == []
    assert any("Missing .codex-plugin/plugin.json" in m
               for m in messages(CodexPluginJsonValidRule({}).check(ctx)))

4. SUGGESTION — two new symlink-loop guards pass vacuously on the 3.13/3.14 CI legs — src/skillsaw/context.py:75, src/skillsaw/utils.py:43

Both are covered only by tests using a real self-symlink. Path.resolve(strict=False) stopped raising on symlink loops in Python 3.13 — as tests/codex/test_containment.py:486 itself notes — so on those matrix legs deleting either except clause would not fail the tests. The PR already established the correct injection pattern but did not apply it here. Add:

@pytest.mark.parametrize("exc", [RuntimeError, OSError, ValueError])
def test_path_matches_patterns_never_raises(monkeypatch, tmp_path, exc):
    monkeypatch.setattr(Path, "resolve", lambda self, *a, **k: (_ for _ in ()).throw(exc("nope")))
    assert path_matches_patterns(tmp_path / "x", tmp_path, ["**/x"]) is False

5. SUGGESTION — the duck-typing catalog-acceptance branch is never executed — src/skillsaw/discovery/codex.py:153

tests/codex/test_catalogs.py:381 covers only the negative. _looks_like_codex_catalog exists solely to distinguish an arbitrarily-named .agents/plugins/*.json catalog from plugin-versions.json siblings — so the discriminator's positive side is unverified. Add a curated.json case asserting two CodexMarketplaceConfigNodes and a reported defect inside it, paired with a plugin-versions.json negative.

6. SUGGESTION — fix() reserializes the whole catalog with json.dumps(indent=2) — src/skillsaw/rules/builtin/codex/marketplace_registration.py:452

Registering one plugin rewrites every line of a catalog authored at any other indentation, changing line counts. tests/test_integration.py:3742 pins this but frames it as "current behaviour, not endorsing it". Mitigating: the pre-existing Claude marketplace-registration does the same — consistent, not novel — but it now applies to third-party Codex catalogs whose formatting skillsaw does not control. Not a blocker; at minimum add a tab-indented-catalog re-lint-clean test.

7. NOTE — the dir/** "behavior change" is not one — src/skillsaw/context.py:42 (_pattern_variants)

The only delta versus base is list → sorted(set), and the stripped variant can never equal the original. I cross-checked all 8 patterns × 10 relative paths against the base implementation: 0 behavioral differences. tests/test_context.py:861 is a pinning test for pre-existing behavior, not regression protection. Correct the PR description so a future bisect doesn't chase a phantom.

8. NOTE — marketplace_json_valid.py:65: if normalised.startswith("./") is dead — PurePosixPath("./plugins/foo").as_posix() already returns "plugins/foo". Coverage confirms line 66 is never hit. The six-line comment reasons about a hazard the code can't reach.

9. NOTE — tests/test_lint_tree.py:141: test_tree_labels was extended for the two Codex nodes but not OpenAIMetadataBlock; blocks/openai.py:41 is the file's only uncovered line.

10. NOTE — docs/markdown_renderer.py:212: the ## MCP Servers section is never exercised end-to-end (213–215 uncovered). Add .mcp.json to a docs fixture plugin.

No BLOCKING findings. Everything probed behaves correctly at runtime; every item is a missing assertion, dead code, or a convention slip.

Technical Writer

Verification performed (clean). Re-ran scripts/generate-site-content.py against a pristine git archive of HEAD in /tmp: diff -rq of regenerated docs/ against committed docs/ is empty. skillsaw init output is byte-identical and contains all five codex-* rules with parameter comments. Mirrored instruction files are in sync (.claude/rules/, .apm/instructions/, .github/instructions/, .cursor/rules/ identical bodies; AGENTS.md and .github/copilot-instructions.md carry fresh APM build IDs and match sources; the three skillsaw-maintenance copies are sha-identical including the new references/codex.md).

Doc accuracy vs code — verified against source. All five new rules: enabled: auto, severities (error ×4, warning for codex-plugin-structure), since = 0.18.0, repo_types, and config_schema keys/defaults match the generated pages exactly. codex-marketplace-registration is the only autofixer and its prose correctly says skillsaw fix --suggest. Spot-verified every identifier named in references/codex.md and the new AGENTS.md "Ecosystem provenance" section — all exist and behave as described.

Behavior changes — traced end-to-end. docs/configuration.md's dir/** semantics are accurate against path_matches_patterns. Version bumps to 0.18.0 consistent across pyproject.toml, action.yml, docs/ci.md, docs/pre-commit.md — no stale 0.17.0 outside historical Since badges. There is no CHANGELOG in this repo — release notes are generated at tag time by the skillsaw-release skill — so no in-repo changelog convention was skipped.

Completeness. docs/repo-types.md covers both new types; mkdocs.yml nav gains rules/codex.md; docs/rules/index.md count 68 matches len(BUILTIN_RULES); THREAT_MODEL.md adds T14/T15; docs/supply-chain-protection.md adds the Codex hook/MCP rows; docs/autofixing.md documents the .codex/plugins/ autofix stand-down. Self-lint: .venv/bin/skillsaw lint . → exit 0, 0 errors, 0 warnings.

1. SUGGESTION — the pre-commit hook trigger widened with no change signal — docs/pre-commit.md:39

.pre-commit-hooks.yaml replaces a 25-branch files: regex with files: .. Every consumer who bumps rev to v0.18.0 goes from "fires on commits touching agent context" to "fires on every commit" — a real latency and UX change on large repos. The doc describes the new state accurately but nothing marks it as a change, and the Troubleshooting entry explaining the old trigger was silently replaced. This repo already has the convention: docs/autofixing.md:45 and :70 use !!! note "Removed in 0.15" / !!! note "Breaking changes (0.15)". Add a !!! note "Changed in 0.18" admonition pointing at the "run the hook only for selected paths" override below.

2. SUGGESTION — external rule authors have no signal that PluginNode misses Codex plugins — docs/custom-rules.md:125

CodexPluginNode deliberately does not subclass PluginNode (lint_target.py:239). Confirmed empirically: in a repo with both, find(PluginNode) returns only the Claude directory. docs/custom-rules.md names PluginNode as the go-to structural node and docs/lint-tree.md:39 shows it as the canonical example. A third-party rule written against those docs now silently skips every Codex plugin — no error, no deprecation signal. This is the same trap AGENTS.md warns builtin authors about ("the mechanism fails open by node type"); the external-author docs carry no equivalent warning. Add one sentence to both.

3. SUGGESTION — "(no autofix)" misdescribes rules that never autofix — docs/repo-types.md:99

The .codex/plugins/* decision table annotates hooks-dangerous, hooks-prohibited, hooks-json-valid and mcp-valid-json, mcp-prohibited as "Runs (no autofix)". supports_autofix is False unconditionally on all five, in every repository. The parenthetical is only meaningful on the agentskill-* row; as written it implies these rules normally autofix and are being specially restrained. Drop it from the hooks/MCP rows, or replace all three with the sentence already present two paragraphs down.

4. SUGGESTION — the non-Codex half of the hooks-json-valid change is undocumented — src/skillsaw/rules/docs/hooks-json-valid.md:43

rules/builtin/hooks/json_valid.py:239 now reports a non-string handler type instead of raising TypeError inside the set membership test. This affects all repositories, not just Codex ones. The doc addition in this PR covers only the Codex-only matcher-must-be-a-string rule, so a user who sees a brand-new hooks-json-valid error on an unchanged hooks.json has nothing to look it up against. Extend the "How to fix" paragraph and regenerate with make update.

5. NOTE — .apm/skills/skillsaw-maintenance/references/codex.md:70: self-lint reports content-section-length (info) — "Sync notes" is ~1071 tokens against a 500-token recommendation. It already reads as two lists; split into sibling ## sections.

6. NOTE — docs/designs/lint-tree.md:25 does not mention the three new node types. exclude_docs'd from the published site and already stale on several pre-existing node types — flagging only so leaving it is deliberate.

7. NOTE — scripts/generate-site-content.py:515 renders | **Autofix** | auto | identically for SAFE and SUGGEST rules, so codex-marketplace-registration's page metadata does not signal that plain skillsaw fix will not clear it. Pre-existing generator behavior shared with several rules — follow-up issue, not a change in this PR.

No BLOCKING documentation issues.

Ecosystem Reviewer

Verdict: ACCEPT INTO CORE — no redirect to plugin.

Adoption evidence (all checked live):

Signal	Value
openai/codex	101,883 stars, pushed 2026-07-28 (today)
openai/plugins	4,769 stars, 641 forks, official OpenAI org, not archived, pushed 2026-07-14
Catalog size	180 plugin dirs / 182 manifests
GitHub code hits for .codex-plugin/plugin.json	~2,100 — adoption well beyond the official catalog

Core already ships rule packages for OpenClaw, promptfoo, APM, and CodeRabbit — all substantially more niche than a first-party OpenAI format backed by a 100k-star CLI. This is single-vendor rather than a cross-tool standard, which normally favors plugins, but the vendor is OpenAI and the trajectory is clearly ascending. Redirecting this to a plugin would be inconsistent with what core already covers.

Conformance verification. Ran the author's own stated regression check: cloned openai/plugins and linted with this branch. Zero codex-* violations across all 180 plugins. The 973 other violations are pre-existing content/agentskills rules that fire on any repo. No false positives on the de-facto conformance suite.

Spec accuracy — verified against raw upstream, not summarizers. Fetched plugins.md, plugin-json-spec.md, openai_yaml.md, and validate_plugin.py as raw text. Every claim in references/codex.md checks out, including the subtle ones: ON_FIRST_USE appears in neither source (grep count 0 in both) — notably my own WebFetch of plugins.md did report ON_FIRST_USE, the exact hallucination the author documented as a trap; the reference note is correct and my tooling was wrong. installation open-vs-closed divergence is real (plugins.md:370 hedges "values such as", plugin-json-spec.md:177 says "Allowed values") — warning-severity intersection is the right call. authentication = ON_INSTALL/ON_USE exact. _INTERFACE_STRINGS (6 fields) and dependencies.tools keys exact. _BRAND_COLOR ← HEX_COLOR_RE at validate_plugin.py:25, applied at :522-527, correctly citing the skill-level brand_color site. Plain-string local source handled; api_marketplace.json sibling naming handled deliberately with a guard against notamarketplace.json. make test passes.

1. SUGGESTION — skillsaw docs output changed for Claude-only repos, breaking downstream committed artifacts — src/skillsaw/docs/extractor.py:325 (plus docs/html_renderer.py)

The project's pre-push checklist item 5 requires openshift-eng/ai-helpers to exit 0. It does not on this branch. Bisected against the merge base:

• Base (43654b2): exit=0, Grade A+, "All checks passed!"
• Branch: exit=1, Grade B+, 1 ERROR — plugins-doc-up-to-date

Reproducing ai-helpers' exact command (skillsaw docs -o docs/ --theme crimson-red): base regenerates docs/index.html byte-identical to what's committed; the branch produces an 84-line diff. Two intentional changes drive it — new author extraction, and XSS hardening (escAttr→escJsAttr in JS-string contexts, esc→escAttr in attribute contexts).

The change is an improvement, not a defect. The ecosystem-relevant point: ai-helpers contains zero Codex content (0 .codex-plugin, 0 openai.yaml, 41 .claude-plugin) — a Codex PR altered generated-docs output for a pure Claude marketplace. Any downstream repo that commits generated docs and gates CI on them breaks until it re-runs make update.

Recommended action: confirm the docs-output change is an accepted breaking change and call it out in the 0.18.0 release notes. If the author field addition is not needed for the Codex feature, consider splitting it out so the security fixes land on their own.

2. SUGGESTION — the hooks contradiction between upstream sources is not recorded in the Sync notes — references/codex.md (~line 90); rules/builtin/codex/plugin_json_valid.py:24

plugin-json-spec.md:215 states "Validation rejects unsupported manifest fields such as hooks." But plugins.md documents hooks in plugin.json extensively (lines 494, 528, 627-635). skillsaw follows the permissive prose reading, which is correct. The official corpus can't disambiguate — none of the 180 manifests uses hooks. The gap is documentation: the two other upstream divergences each get an explicit "check both on the next sync" note, and this one — arguably the sharpest, since the sources flatly contradict — gets none. A future maintainer syncing against plugin-json-spec.md could reasonably "fix" skillsaw to reject hooks and break every user following the prose docs.

3. SUGGESTION — codex-openai-metadata fires on plain skill repos with no Codex plugin — rules/builtin/codex/openai_metadata.py:37

The rule declares repo_types = SKILL_REPO_TYPES, so it auto-enables on any skill repo. Confirmed empirically — a bare agentskills.io skill (SKILL.md + agents/openai.yaml, no plugin manifest anywhere) produces codex-openai-metadata | error | 'interface.brand_color' must be a #RRGGBB hex color. The diagnostic is substantively right, but agents/openai.yaml is an OpenAI skill convention documented at learn.chatgpt.com/docs/build-skills, not a Codex-plugin construct — and docs/repo-types.md correctly lists it under the agentskills.io section. An agentskills.io author with no Codex involvement gets a new ERROR labeled codex-*, which misattributes the source of the requirement. Rename (e.g. openai-skill-metadata) or document the broader scope. Renaming a rule that ships in the same release is cheap; renaming later is not.

On size and the PluginProvenance abstraction — no finding. ~15.6k insertions and +639 in context.py is large, but the provenance abstraction solves a real problem: two vendors now ship overlapping plugin formats in the same directory tree, and dual-manifest directories genuinely exist. Centralizing "who claims this directory" in one cached record is the right shape, and the declarative provenance_scope gate means adding the next ecosystem doesn't require visiting existing rule files.

Slopinator Reviewer

Scope checked: every added/changed code comment and docstring under src/, all new and modified docs/ and src/skillsaw/rules/docs/ pages, THREAT_MODEL.md, README.md, AGENTS.md and the four generated instruction twins, the new references/codex.md, all 13 files in tests/codex/, the new workflow steps, and the PR body and 40-commit chain.

Method: verified the issue-number convention is pre-existing (src/skillsaw/markdown_doc.py:289 on the base ref already carries see issue #318), so issue #463 in tests/test_banned_apis.py is house convention, not residue. Measured em-dash density in the new docs against existing maintenance references — the new prose is lower (9.7–23.0 per 1k words vs. 29.8–37.8 in references/claude.md / references/openclaw.md), so no formatting finding. Comment density in rules/builtin/codex/ is ~10.5% vs. ~3% on comparable base-ref rules, but all 183 comment lines are load-bearing "why" comments in an adversarial domain — no finding. Grep for conversational filler (note that, as mentioned, this ensures, sycophancy) returned zero hits across the whole 15.6k-line diff. Two prior commits in the chain (3634f6d "Prune review-sediment comments", 2f173ea "De-narrate four review-history comments") clearly did a sweep, and it mostly held.

1. BLOCKING — phantom-bug docstring narrating a regression that never shipped — src/skillsaw/context.py:49-52

    A trailing ``/**`` must NOT be widened to match the directory entry
    itself: doing so suppressed violations addressed to the directory — the
    built-in ``**/template/**`` content exclude swallowed ``Missing
    SKILL.md`` for any skill directory named ``template``.

The base ref's docstring reads "A trailing /** needs no variant" and the base ref code never widened it. The widening was introduced inside this branch by 42016d5 and removed four commits later by 28172ee. The **/template/** / Missing SKILL.md regression is a within-branch artifact of one review round, frozen into a permanent past-tense claim about history that no reader of main can verify. This fails the stranger test: a reader a year out has no diff and will go looking for a released version where the default excludes swallowed skill-structure errors. It is also redundant — docs/configuration.md already states the shipped semantic correctly and without narration.

Recommended action: replace lines 49–52 with a statement of the invariant, no history:

    A trailing ``/**`` is not expanded: it matches strictly inside the
    named directory, never the directory entry itself, so a violation
    addressed to an excluded directory — a missing required file, say —
    is still reported.

2. SUGGESTION — the PR body's lead claim contradicts its own change section — PR #471 body, opening paragraph

The lead sentence claims "zero behavior change for Claude-only repositories," and the body then carries a section headed "Behavior changes affecting existing (non-Codex) repositories" listing three. A Claude-only repository is non-Codex, and all three reach it. The supporting evidence ("openshift-eng/ai-helpers (Claude-only): byte-identical output vs. base") is one repo, not the general claim. Narrow the lead to what the change section supports:

...five new rules, Codex-aware discovery, and docs generation for Codex catalogs — no new Codex rules fire on Claude-only repositories (three deliberate fixes to neutral machinery are listed below).

3. NOTE — references/codex.md:111 (and the three generated twins): the version non-check entry ends "Enforcing it would be defensible." That is an aside answering a reviewer who argued the other side; it tells a future maintainer nothing about what to do and undercuts the section's own framing ("do not 'fix' these without a spec change"). Cut the sentence.

4. NOTE — src/skillsaw/docs/html_renderer.py:1233: the _md_link docstring opens "html.escape above stops the target breaking out of the attribute." The escape that guards this call site is at line 1257, inside _md, defined below _md_link. Replace "above" with "in _md".

5. NOTE — commit-message chain: roughly half the 40 commits are titled by review round rather than by change (Address Codex review round thirteen, Fix the fifth panel's blockers, Close the second panel's findings and the round-fifteen gaps). Harmless if #471 lands squashed — main's history suggests it will. Worth one line of confirmation before merge.

6. NOTE — nine near-identical ## Codex plugins blocks in src/skillsaw/rules/docs/ repeat the same ~40-word definition. Standalone rule pages justify some duplication, so not a finding on its own — but two carry mid-sentence reflow artifacts from patching (plugin-naming.md: "would double up. A dual-manifest / directory keeps this check"; hooks-json-valid.md: "a matcher on a hook / config must be a string"). Reflow those two paragraphs.

Clean: no round-numbered test organization anywhere in tests/codex/ — all 100+ classes are scenario-named and their docstrings describe behavior, not when they were written. No round-scoped benchmark numbers frozen into comments. No signature-echo docstrings, no changelog-in-comment, no chatbot artifacts. The long security comments in markdown_renderer.py, diagnostics.py, and docs/extractor.py earn their length.

---

Panel Synthesis

The feature is right for core and the engineering is above the bar for this repo. The Ecosystem Reviewer independently verified adoption (openai/codex at 101k stars; openai/plugins an active first-party catalog of 180 plugins; ~2,100 GitHub code hits for .codex-plugin/plugin.json), re-derived the schema from raw upstream sources rather than summarizers, and reproduced the author's headline conformance result: zero codex-* findings across the entire official corpus. Core already ships packages for OpenClaw, promptfoo, APM and CodeRabbit — all more niche than this. No plugin redirect. The PluginProvenance abstraction drew explicit approval from both the Architecture and Ecosystem reviewers as the correct response to two vendors sharing one directory tree.

The panel also independently confirmed the PR's own claims rather than accepting them: QA ran the suite (3484 passed, 94% coverage — the stated numbers are accurate), the Technical Writer regenerated the entire docs tree from a pristine archive and found zero drift, and Security exercised the XSS, credential-redaction, containment, and autofix-stand-down hardening against hostile fixtures and found most of it genuinely holds.

Six blocking items, all in scope and all cheap relative to the size of the change. They cluster into three themes:

Security — one real containment escape, one reproduced docs injection. Security found that provenance() has a second path to codex=True (the catalog claim set) that skips the marker-containment check the discovery layer applies, and reproduced arbitrary out-of-tree file disclosure through it: a catalog-claimed directory with a symlinked .codex-plugin/ yields a directory listing of an external path, one violation per filename, published wherever skillsaw reports go (PR comments, SARIF). The PR's own marketplace_registration.py:346-352 comment shows the author saw this gap and patched it in one rule instead of at the ownership layer — which is precisely what the repo's "Ecosystem provenance" rule exists to prevent. Separately, three Markdown-renderer sinks bypass _table_cell(), producing a live javascript: link and raw <img onerror> in generated pages from a hostile hooks.json; the arbiter elevated this from the specialist's SUGGESTION because THREAT_MODEL T15 explicitly claims the mitigation and scopes accepted risk to description fields only — the entry is inaccurate as shipped, and the fix is three lines.

Performance — a measured regression on repositories with no Codex content. Architecture benchmarked both halves. Rule.scoped_find() wraps the memoized find() in an un-memoized per-node provenance consult, turning four cached lookups (0.000 s) into 0.907 s on a 200-plugin marketplace; RepositoryContext.__init__ runs 1.5–2.3× slower with safe_resolve calls up from 7,562 to 18,144 on a pure-Claude repo. This repo's development rules single out exactly these patterns, and the cost falls hardest on users who get no benefit. Fixable with memoization and a cheap "repo has Codex evidence" short-circuit; the rules also require a make benchmark-compare against a main baseline, which does not appear to have been run.

Accuracy — two claims the panel could not reproduce. Ecosystem bisected the pre-push checklist's own gate: openshift-eng/ai-helpers exits 0 on base and 1 on this branch (Grade A+ → B+, plugins-doc-up-to-date ERROR), an 84-line generated-docs diff on a repo with zero Codex content — directly contradicting the PR's "byte-identical output vs. base" line. The underlying change (XSS hardening plus new author extraction) is an improvement; what blocks is that it is an undisclosed break for any downstream repo that commits generated docs and gates CI on them, and backward compatibility is the panel's paramount bias. Independently, QA and the Slopinator converged on the same phantom from opposite directions: QA cross-checked 8 patterns × 10 paths and found 0 behavioral differences in _pattern_variants versus base, while the Slopinator traced the **/template/** regression its docstring describes to a within-branch commit (42016d5) reverted four commits later (28172ee). Behavior change #1 in the PR body does not exist, and a permanent docstring asserts a bug no released skillsaw ever had. Two specialists, two methods, same conclusion — the strongest corroboration in this review.

Notable near-misses that stayed below blocking. Python and QA both landed on marketplace_registration.py:249 — a brand-new file added to the bare-.resolve() grandfather list in the same commit that introduced the ratchet, with safe_resolve already imported four lines away. Not blocking, but it is the one place where this PR's own discipline slipped, and test_allowlist_has_no_stale_entries will enforce the cleanup for free. QA also found two symlink-loop guards that pass vacuously on the 3.13/3.14 CI legs because resolve(strict=False) stopped raising there — the PR already established the right injection pattern elsewhere and simply didn't apply it here.

Filtered as noise or non-actionable: the catalog-reserialization indentation concern (consistent with the pre-existing Claude rule, explicitly pinned as known behavior); the generic symlink walk (confirmed identical on the merge base); the | **Autofix** | auto | generator display (pre-existing, shared with several shipped rules); docs/designs/lint-tree.md staleness (internal, exclude_docs'd, already stale on pre-existing node types).

Nothing here challenges the design. Every blocker is a leak at a seam the architecture already defines correctly — a second path around a single ownership point, three sinks that skipped a helper the other twenty use, a memoization boundary crossed, and two claims that outran their evidence. Land the fixes and this is a strong addition.

---

Required Actions Before Merge

1. Close the provenance containment escape. context.py:812-815 / lint_tree.py:400-408 — require claim-derived codex evidence to pass marker containment when the marker exists, or skip CodexPluginConfigNode creation when contained_resolve(plugin_path / ".codex-plugin", resolved_plugin) is None. Keep the missing-marker case attaching so codex-plugin-json-valid still reports it. Add a tests/fixtures/codex/ fixture with a catalog-claimed dir and an escaping .codex-plugin symlink. (Security Add "Claude was here" to README.md #1 — reproduced)

2. Route the three raw Markdown sinks through _table_cell(). markdown_renderer.py:277, :280, :309 (hook event names, matchers, rule globs). Add the hostile-hooks.json case to tests/codex/test_docs_output_safety.py. Either way, correct THREAT_MODEL T15 — its accepted-risk clause currently names description fields only. (Security [Auto] Sync hooks rules with upstream Claude Code spec #2 — reproduced; elevated by the arbiter on the security-over-ergonomics bias)

3. Memoize the provenance scope filter. rule.py:198-212 — cache (id(node), ecosystem) -> bool, or short-circuit in_format_scope when the repository has no Codex evidence at all. Make provenance() cheap on the hit path by keying the cache on the unresolved path first. (Architecture Add "Claude was here" to README.md #1 — 0.000 s → 0.907 s measured)

4. Remove the unconditional Codex work from the non-Codex init path. context.py:1365 (guard the SKILL.md resolve under if contain_within is not None), context.py:1376 (gate the provenance(item) descent on a once-per-context Codex-evidence flag), lint_tree.py:225-235 (test safe_is_file(path) before the three safe_resolve() calls). Then run make benchmark-save on main and make benchmark-compare on the branch per .claude/rules/development.md. (Architecture [Auto] Sync hooks rules with upstream Claude Code spec #2 — 1.5–2.3× slower on pure-Claude repos)

5. Reconcile the skillsaw docs output change. openshift-eng/ai-helpers goes from exit 0 to exit 1 on this branch — the pre-push checklist's own gate. Confirm with the maintainer that the generated-docs change is accepted, disclose it in the 0.18.0 release notes so downstream repos know to regenerate, and correct the PR body's "byte-identical output vs. base" claim. Consider splitting the author extraction from the XSS hardening if it is not needed for Codex. (Ecosystem Add "Claude was here" to README.md #1 — bisected)

6. Retract the phantom dir/** behavior change. Replace the _pattern_variants docstring at context.py:49-52 with a statement of the invariant and no history (the Slopinator supplies verbatim replacement text), and drop item 1 from the PR body's "Behavior changes" section — QA verified 0 behavioral differences versus base across 8 patterns × 10 paths. (Slopinator Add "Claude was here" to README.md #1 + QA Implement .claude/rules #7 — independently corroborated)

Strongly recommended alongside the above (two specialists each, one-line fixes):

• marketplace_registration.py:249 → safe_resolve with a None guard; drop the entry from GRANDFATHERED_BARE_RESOLVE. Also :244 is_file() → safe_is_file. (Python Add "Claude was here" to README.md #1 + QA [Auto] Sync hooks rules with upstream Claude Code spec #2)
• Route server_name through safe_display() at mcp/valid_json.py:207 — T14 claims this redaction. (Security [Auto] Sync MCP rules with upstream server spec #3)
• Replace the real-symlink coverage for context.py:75 and utils.py:43 with exception-injection tests; they currently pass vacuously on 3.13/3.14. (QA [Auto] Validate agentskills.io optional frontmatter fields #4)

---

Optional Follow-ups

Encapsulation and layering — promote context._codex_claim_set() to a public accessor (Arch #3, Python #5); move the _has_claude_plugin filesystem probe onto PluginProvenance as a claude_marker field (Arch #4); drop the discovery/format re-exports and class-attribute aliases from context.py (Arch #5); tighten the is_codex_installed_plugin parameter contract and make codex_install_root() return None when the directory is absent (Arch #6); initialise marketplace_entries/plugin_metadata before _detect_types() so the defensive getattr at context.py:803 can go (Python #4).

Performance (non-blocking) — build the owner index once in _extract_codex_plugins instead of 6 × plugins list copies (Python #2); replace the seen_roles linear scan with a parallel Set[Path] (Python #3).

Test coverage — parametrized tests for the nine unexecuted codex-marketplace-json-valid diagnostics (QA #1); the regular-file-at-.codex-plugin crash guard (QA #3); the positive duck-typed-catalog branch at discovery/codex.py:153 (QA #5); OpenAIMetadataBlock in test_tree_labels (QA #9); .mcp.json in a docs fixture to exercise markdown_renderer.py:212 (QA #10).

Documentation — a !!! note "Changed in 0.18" for the widened pre-commit files: trigger (Writer #1); a PluginNode ≠ CodexPluginNode warning in docs/custom-rules.md and docs/lint-tree.md (Writer #2); drop the misleading "(no autofix)" from the hooks/MCP rows in docs/repo-types.md (Writer #3); document the non-string handler type change in hooks-json-valid.md (Writer #4); record the plugins.md vs plugin-json-spec.md hooks contradiction in the Sync notes (Ecosystem #2); narrow the PR body's "zero behavior change" lead (Slopinator #2).

Naming — consider renaming codex-openai-metadata, which fires on agentskills.io repos with no Codex plugin (Ecosystem #3). Cheap now, expensive after release.

Refactoring and polish — extract a _TreeBuilder class from the ~560-line build_lint_tree (Arch #7); dead import at lint_tree.py:42 (Arch #8); missing dot-colour entries for three node types (Arch #9); dead startswith("./") branch and its six-line comment at marketplace_json_valid.py:65 (QA #8); U+2028/U+2029 in _URL_FORBIDDEN (Security #4); docs/cli.md note on fix write scope for widened Codex paths (Security #5); the _md_link "above" → "in _md" docstring pointer (Slopinator #4); the two reflow artifacts in plugin-naming.md and hooks-json-valid.md (Slopinator #6); confirm this lands squashed so "review round thirteen" does not enter main's history (Slopinator #5).

---

_{Generated by skillsaw-review-panel}

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 053f1bb1ca

ℹ️ 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".

[chatgpt-codex-connector]

Preserve Codex ownership for marker-only plugins

When a plugin has a contained .codex-plugin/ directory but no plugin.json and the user runs with an explicit non-Codex type such as --type marketplace, Codex discovery is disabled so _codex_claim_set() is empty, and this safe_is_file() requirement makes provenance classify the directory as unclaimed/Claude. The lint tree can then run Claude scoped rules on a broken Codex plugin instead of standing them down; treat the contained marker as Codex evidence and only reject an existing manifest that resolves outside the plugin.

AGENTS.md reference: AGENTS.md:L161-L164

Useful? React with 👍 / 👎.

[chatgpt-codex-connector]

Preserve Codex ownership for marker-only plugins

When a plugin has a contained .codex-plugin/ directory but no plugin.json and the user runs with an explicit non-Codex type such as --type marketplace, Codex discovery is disabled so _codex_claim_set() is empty, and this safe_is_file() requirement makes provenance classify the directory as unclaimed/Claude. The lint tree can then run Claude scoped rules on a broken Codex plugin instead of standing them down; treat the contained marker as Codex evidence and only reject an existing manifest that resolves outside the plugin.

Useful? React with 👍 / 👎.

[chatgpt-codex-connector]

Redact truncated passwordless userinfo

For a long passwordless-userinfo URL such as https://<long-token>@host where the @host falls beyond _MAX_DISPLAY, this truncates before _redact_userinfo() can see the host and only redacts the severed tail when it contains :. That means even callers already using safe_display() can still emit the first 500 characters of token-style credentials into text/JSON/SARIF diagnostics; treat a truncated URL/userinfo-looking tail as sensitive or inspect bounded lookahead before returning the prefix.

Useful? React with 👍 / 👎.

[chatgpt-codex-connector]

Preserve lexical paths for installed declared skills

When an installed plugin is reached through a symlink under .codex/plugins/ and its manifest declares a non-default skills path, returning the resolved candidate stores skill nodes under the symlink target instead of the .codex/plugins/... path. Agent Skill fixers and the linter’s vendor-managed check then no longer recognize those SKILL.md files as installed content, so skillsaw fix can rewrite third-party plugin files despite the diagnostic-only invariant; preserve the lexical path after containment validation or decide fixability from the owning plugin root.

Useful? React with 👍 / 👎.

[chatgpt-codex-connector]

Accept Codex mcp_servers wrappers

When a Codex manifest points mcpServers at a .mcp.json that uses the documented wrapped mcp_servers form, this attaches it to McpBlock, whose parser only unwraps mcpServers or treats the whole object as the direct server map. The actual server entries are then missed by mcp-valid-json, mcp-prohibited, and docs generation while the wrapper key is treated as one malformed server; normalize mcp_servers before attaching or teach McpBlock to accept both wrappers.

Useful? React with 👍 / 👎.

[github-actions]

Review Panel Verdict

Disposition: REQUEST_CHANGES

---

Specialist Findings

Architecture Reviewer

Verified: PluginProvenance is a genuine single source of truth (context.py:807-868, cached under raw + resolved keys, --type-invariant). The single-pass plugin loop (lint_tree.py:306-453) is real — one container per resolved directory, seen_plugin_dirs fully populated before the first _add_plugin_prose. All 8 provenance_scope = "claude" rules go through scoped_find; in_format_scope is the only filter site and fails open for unclaimed/dual directories. discovery/codex.py imports nothing from skillsaw.context and is pinned state-free by TestDiscoveryModuleIsStateFree. No layering cycles; blocks/openai.py correctly subclasses LintTarget rather than ContentBlock/JsonConfigBlock.

1. SUGGESTION — src/skillsaw/rules/builtin/marketplace/json_valid.py:55 — a rule answers an ownership question with its own filesystem probe. _has_claude_plugin() re-derives Claude ownership from safe_is_dir(node.path / ".claude-plugin") instead of context.provenance(node.path).claude — the exact pattern .claude/rules/development.md forbids. The two spellings already disagree. Reproduced with plugins/foo/.claude-plugin as a regular file plus a Codex catalog:

prov foo            PluginProvenance(ecosystems={'codex','claude'})
_has_claude_plugin: False
marketplace-json-valid: []                        # exempted: "no Claude plugin here"
plugin-json-required:  ['Missing plugin.json']    # scoped in: "this IS a Claude plugin"

skillsaw simultaneously demands a Claude manifest and exempts the repo from the marketplace that would publish it. Action: use any(context.provenance(n.path).claude for n in context.lint_tree.find(PluginNode)); if the stricter containment/is_dir semantics are wanted, move them into provenance().

2. SUGGESTION — src/skillsaw/context.py:412 vs :419-422 — _codex_roots escapes the unconditional cache-invalidation block. apply_excludes() unconditionally clears _codex_claims, _codex_evidence, _provenance_cache, _format_scope_cache, but _codex_roots is only reset inside if self.exclude_patterns: and if codex_set_changed: — despite being derived from all three inputs (codex_plugin_roots(), context.py:721-744). Reproduced with a catalog-claimed directory that has no .codex-plugin:

roots(before)  [/tmp/rcheck/plugins/note]
# exclude_patterns = ["plugins/note"]; apply_excludes()
roots(after)   [/tmp/rcheck/plugins/note]   # stale — excluded dir survives

codex_plugin_owning() then attributes content to an excluded plugin, feeding in_codex_only_plugin(). Reachable on the supported two-Linters-over-one-context path (linter.py:74-80). Action: move self._codex_roots = None into the unconditional block at context.py:419.

3. SUGGESTION — src/skillsaw/lint_tree.py:71-646 — build_lint_tree doubled 274 → 575 lines: six nested closures, ten discovery sections, four shared mutable accumulators. The file already establishes the right pattern with module-level _build_promptfoo_nodes(...). Not a request to split the plugin pass in two — the one-pass invariant is correct. Action: extract lint_tree.py:297-453 into _build_plugin_nodes(...) and :248-295 into _build_root_blocks(...).

4. SUGGESTION — src/skillsaw/lint_tree.py:320 — tree builder reaches into private context._codex_claim_set(); every other cross-module consult in the file uses a public accessor, and it duplicates the exclusion filter codex_plugin_roots() already applies. Action: add RepositoryContext.codex_claimed_dirs().

5. SUGGESTION — src/skillsaw/context.py:843 — safe_resolve(self.root_path / ".claude") is a per-context constant recomputed on every provenance cache miss, on the elif (common) branch. _discover_skills_in_dir (:1446-1453) calls provenance() per directory whenever _codex_claims_possible() is true — which returns True whenever _codex_discovery_enabled is False (:789), so --type <non-codex> pays the full walk on a repo with no Codex content. Action: cache in __init__ or lazily via the _UNSET sentinel pattern.

6. SUGGESTION — src/skillsaw/docs/extractor.py:389 and src/skillsaw/docs/html_renderer.py:1227 — _SAFE_URL_SCHEMES and _MD_SAFE_SCHEMES are two independent copies of the same security allowlist. The justifying comment cites import-cycle avoidance, but both modules already import skillsaw.docs.models at load time. Action: move the frozenset to models.py.

7. NOTE — tests/test_banned_apis.py:22-52 — the bare-.resolve() ratchet is file-granular and grandfathers context.py, lint_tree.py, linter.py, docs/extractor.py — the four files this PR changes most. One new call already landed: lint_tree.py:449 (marketplace_dir.resolve(), recomputed once per plugin inside the loop). Action: freeze the per-file count rather than the filename.

8. NOTE — tests/codex/test_discovery.py:722 — the state-free test hardcodes (skillsaw.discovery, skillsaw.discovery.codex), so it won't cover the future discovery/<ecosystem>.py the development rules tell the next contributor to add. Action: glob the package directory.

9. NOTE — src/skillsaw/rules/builtin/agentskills/_helpers.py / src/skillsaw/linter.py:469 — is_installed_plugin_skill is a docstring-only alias for context.is_codex_installed_plugin, which is named/documented for plugin directories (context.py:960, param plugin_dir) but is passed arbitrary violation file paths by _is_vendor_managed. Works, but the signature lies.

10. NOTE — src/skillsaw/rules/builtin/marketplace/registration.py:176 — check() uses scoped_find but fix() re-looks-up with unscoped find(PluginNode) and breaks on the first name match. Harmless today; contradicts "filtering happens in exactly one place".

11. NOTE — src/skillsaw/paths.py:44-113 — safe_resolve/_safe_stat swallow OSError/ValueError/RuntimeError with no logging. Fail-quiet is right for discovery, but permission-denied on a plugin directory now means silent non-discovery with no diagnosis path. context.py already logs comparable skips. Action: add logger.debug(...) in the except branches.

Python Expert

Verification: clean venv, pip install -e ., full suite — 3505 passed in 149s. Also exercised diagnostics.safe_display, paths.is_absolute_path, discovery.codex._is_marketplace_filename directly and ran a live skillsaw lint against a hand-built fixture.

No BLOCKING findings. Quality is high: no mutable default args, no bare except:/except Exception, no resource leaks, correct Optional/FrozenSet/TypeVar usage, correct 3.9 compatibility (from __future__ import annotations wherever PEP 604/585 syntax appears), no negative-lookaround autofix hazards, KEBAB_CASE correctly uses \Z not $, cache lookups correctly use is None rather than truthiness (in_format_scope, find_filtered, _is_vendor_managed), and _InlineJsonPayload.__eq__/__hash__ correctly precede LintTarget's in the MRO.

1. SUGGESTION — src/skillsaw/rules/builtin/mcp/valid_json.py:183 (also :162, :171, :217, :225) — incomplete safe_display() adoption, confirmed empirically. This PR introduces diagnostics.safe_display() and applies it to the analogous case in hooks/json_valid.py:239 with the rationale "A dict value can carry a credentialed URL into text/JSON/SARIF output — redact like every manifest value." Line 208 redacts server_name; the other five interpolations in the same loop were left raw — including the server_type echo that is exactly the dict-valued case the hooks fix was written for. Live repro (skillsaw lint . --format json):

"MCP server 'ev\x1b[31mil' has invalid type '{'u': 'https://user:tok@host/x'}'. Must be one of: ..."

Both the raw C0 escape and the embedded user:tok@host credential reach the text/JSON/SARIF sinks. Action: wrap server_name/server_type in safe_display() at 162, 171, 183, 217, 225.

2. SUGGESTION — src/skillsaw/lint_tree.py:591-596 — the new content-path guard linearly scans seen_roles per matched extra file (any(claimed == extra_resolved for claimed, _ in seen_roles)). seen_roles is populated by every _add_block call, so it grows to one entry per attached block repo-wide → O(extras × blocks). This is the exact shape the file's own comment warns about ("O(skills x plugins)... dominated tree construction on large marketplaces (3.6k skills x 445 plugins)"). Action: maintain a parallel seen_role_paths: Set[Path] and do an O(1) membership test.

3. SUGGESTION — src/skillsaw/docs/extractor.py:437 (_extract_codex_skills), :301 (_owned_blocks) — both re-scan the whole tree per Codex plugin; _owned_blocks is called 5× per plugin. find() is memoized so the traversal is free, but the list() copy and full scan are not → O(plugins × nodes) for skillsaw docs, reintroducing the pattern the lint tree was refactored to remove. Action: build a defaultdict(list) inverted index keyed by node.plugin_owner once in extract_docs.

4. SUGGESTION — src/skillsaw/paths.py:126 — escapes_root() calls root.resolve() on every invocation, once per catalog entry and per path field, though the root is constant and RepositoryContext.__init__ already stores it resolved. One wasted realpath(2) per entry. Action: accept a pre-resolved root (as the sibling contained_resolve(path, root) already does) or lru_cache it.

5. NOTE — src/skillsaw/lint_tree.py:320 — private reach-through to context._codex_claim_set(); everything else in the loop uses public accessors. (Independently found by the Architecture Reviewer.)

6. NOTE — src/skillsaw/lint_target.py:66-86 — find_filtered's docstring requires cache_key to name everything the predicate depends on, but Rule.scoped_find passes ("provenance_scope", ecosystem) while the predicate closes over context. Safe today (one tree per context) but the invariant is implicit and unasserted. Action: add id(context) to the key, or state the invariant in scoped_find's docstring.

7. NOTE — src/skillsaw/discovery/codex.py:236, 243 — bare parent.is_dir()/item.is_dir() where the rest of the module uses safe_is_dir. Runs inside RepositoryContext.__init__, outside the rule-execution-error guard.

8. NOTE — src/skillsaw/docs/extractor.py:230, 301, 431, 502, 571, 658 — type-hint gaps in otherwise well-annotated new code: bare set/list on _remote_entry_docs, _codex_remote_docs, _owned_blocks; missing annotations on _command_docs, _agent_docs, _rule_docs, _string_list while sibling _extract_hooks(blocks: List[HooksBlock]) has them. _remote_entry_docs also mutates the caller's local_names set as load-bearing cross-catalog dedup (:262), which the signature does not state. Action: annotate; rename local_names → claimed_names or document the mutation.

Security & Supply Chain Reviewer

1. SUGGESTION — src/skillsaw/formatters/text.py:60-70 / src/skillsaw/diagnostics.py:83 — safe_display()'s docstring claims "Control characters are replaced so a crafted value cannot smuggle terminal escapes through the text formatter." That holds only for values routed through safe_display, not for the file_path every violation prints — and filenames are attacker-controlled the same way. Verified end-to-end:

mkdir -p "plugins/$(printf 'ev\033[31mil')/.codex-plugin"   # + a plugin.json
$ skillsaw lint | cat -v
  ⚠ WARNING (codex-plugin-json-valid) [plugins/ev^[[31mil/.codex-plugin/plugin.json]: ...

Raw ESC [ 31 m reaches the terminal. A filename containing \r, \x1b[2J, or \x1b[8m is a linter-evasion primitive — it can blank, overwrite, or hide findings in a developer terminal or CI log. JSON/SARIF/HTML sinks are fine (json.dumps escapes C0; html.escape in formatters/html.py:40-67); the text formatter is the only raw one. Pre-existing, but it contradicts a claim this PR makes about the sink it just hardened. Action: route the rendered path (and repo_types_str) through the same _CONTROL_CHARS substitution.

2. SUGGESTION — src/skillsaw/docs/extractor.py:281 — new code in this PR uses a bare .resolve():

claude_dirs = {pn.path.resolve() for pn in context.lint_tree.find(PluginNode) if not _is_codex_only(...)}

_is_codex_only two lines away (:135-137) uses safe_resolve for exactly this reason, and extract_docs at :90 was rewritten in this PR to replace a bare .resolve() with safe_resolve. The new tests/test_banned_apis.py ratchet is file-level and docs/extractor.py is grandfathered, so it cannot catch this. On a symlink loop or unreadable parent, skillsaw docs aborts with a traceback. Action: use safe_resolve(pn.path) and drop Nones (mirroring :265); note in the ratchet docstring that grandfathering is per-file, not a license for new bare calls.

3. NOTE — src/skillsaw/context.py:76 — the new except (ValueError, OSError, RuntimeError): return False in path_matches_patterns fails open (unresolvable path is never excluded), while paths.escapes_root (:128-132) makes the opposite choice and documents failing closed as "the safe direction." Exclusions now also gate skillsaw docs output (discovery/codex.py:118-122), so an excluded catalog could in principle be published anyway. Low reachability; the asymmetry deserves a sentence or a deliberate flip.

4. NOTE — src/skillsaw/docs/html_renderer.py:1258-1261 — _md() runs code-span/**/* substitution before link substitution, so generated tags can land inside an href: _md('[t](https://x/**a**)') → <a href="https://x/<strong>a</strong>">t</a>. Not exploitable — html.escape already removed every ", the tokenizer treats < as literal in the double-quoted attribute-value state, and the scheme allow-list still rejects [t](**javascript**:...) (verified). Fragile invariant. Action: re-escape </> in target inside _md_link.

5. NOTE — tests/fixtures/codex/claim-escape/outside-the-checkout/deploy_key_outside — ships a literal -----BEGIN OPENSSH PRIVATE KEY-----/-----END----- block. Body is obviously fake, but the delimiters alone match gitleaks' private-key rule and GitHub push protection. This PR also edits tests/fixtures/single-plugin/with-secrets/commands/setup.md to append # notsecret for exactly this reason; the new fixture gets no marker. Action: add # notsecret, or drop the PEM delimiters (the test only needs the filename, per the file's own comment).

6. NOTE — src/skillsaw/diagnostics.py:21-73 — _redact_userinfo catches URL/scp userinfo but not free-standing credentials in the same strings: Bearer ghp_..., token=ghp_..., AKIA... all pass through (verified). Reports are uploaded as CI artifacts and ingested as SARIF. The docstring is honest about scope, so this is coverage, not a defect. Action (optional): add a token-prefix pass (ghp_, github_pat_, sk-ant-, AKIA, xox[baprs]-).

7. NOTE — src/skillsaw/docs/markdown_renderer.py:344 — _UNSAFE_FILENAME_CHARS leaves Unicode bidi overrides: a plugin named "‮evil" yields "‮evil.md". diagnostics._CONTROL_CHARS strips these for the same directory-listing-spoofing reason. Cosmetic, filesystem-safe.

Verified clean — the PR's security claims hold up under attack:

• escJsAttr/escAttr (html_renderer.py:1192-1211): correct. Verified by running the emitted JS under Node and re-parsing the produced fragments with an HTML parser. All of a')+alert(1)+(', a\')+…, a\\')+…, a"onmouseover="…, a')+…, a')+…, a</div><img …> decode to inert JS string literals. Backslash-doubling blocks the \'-escape-the-escape attack; &-first ordering blocks entity re-introduction of a quote.
• URL scheme allow-listing (extractor._safe_url:401-428): no bypass across 23 payloads — case variants, leading/embedded whitespace and \xa0, :/:/:/:/:/
, data:, vbscript:, percent-encoded schemes. Bounded 5-round html.unescape + _URL_FORBIDDEN + (/) percent-encoding is sound. javascript: in homepage/repository is dropped at extraction (confirmed on a live hostile fixture).
• <script> data blob: < > → < > over the whole json.dumps output prevents </script> and <!-- breakout. Confirmed on a fully hostile fixture.
• Every other JS interpolation point audited individually (updateStats, renderTypeView, renderPluginGrid, renderCategoryFilter, renderPluginSections, renderSearchResults, showPluginDetail, hi) — all go through esc/escAttr/escJsAttr/encodeURIComponent or are Python-escaped upstream. hi()'s dynamic RegExp escapes all metacharacters (no ReDoS, no $& injection).
• Filename collision / zip-slip: ../../evil → evil.md, /etc/passwd → etc-passwd.md, \x00evil → evil.md, con/nul/com1 → -plugin suffix, readme reserved, 400-char names truncated with SHA-256 tail, case-insensitive collision probing. Nothing escapes the output directory.
• Path traversal / symlink escape: verified on a hostile repo (plugin skills → symlink out, hooks → ../../../outside/creds.env, marketplace source.path → ../outside). All rejected; skillsaw docs leaked neither outside content nor the credential file.
• Malformed input: 20,000-deep JSON → clean "Nesting too deep to parse", exit 1, no traceback. Mutual symlink loops → clean run. YAML billion-laughs in agents/openai.yaml → no blow-up.
• Autofix write safety: marketplace_registration.fix() declines to write when os.path.abspath(...) != safe_resolve(...) (never writes through a symlink); linter._is_vendor_managed drops every fix under .codex/plugins/.
• ReDoS: all five new/changed regexes linear. _redact_userinfo measured flat at 20,000 @s.
• Injection / new I/O: zero new subprocess, os.system, shell=True, socket, requests, urllib.request in the diff. Only urlparse, with all three ValueError sites guarded so scheme/credential checks cannot fail open.
• Supply chain: pyproject.toml changes are the version bump only — no new runtime deps, no loosened pins, no lockfile changes.
• CI: test.yml keeps permissions: contents: read; both new steps pass --no-custom-rules (third-party repo code never executed) and pin openai/plugins to a full SHA. No new pull_request_target usage, no new actions.

QA Engineer

Test suite: PASS — 3505 passed, 20 warnings in 621.89s; TOTAL 13206 stmts 802 miss 94%. Working tree stayed clean.

Coverage on new code: plugin_json_valid.py 100%, _helpers.py 100%, paths.py 100%, openai_metadata.py 99%, marketplace_registration.py 95%, plugin_structure.py 94%, marketplace_json_valid.py 93%, discovery/codex.py 93%.

Verified genuinely covered (all six areas the panel asked about):

• All five rules fire in a fixture. tests/fixtures/codex/broken is in BROKEN_FIXTURES (test_integration.py:1588) and codex/clean in CLEAN_FIXTURES (:1605), so TestRuleCoverage::test_every_rule_fires_somewhere mechanically guarantees each new rule produces a violation, and the clean fixture requires exit 0 with zero errors and warnings. Fixtures are static and realistic per .claude/rules/testing.md.
• The registration autofix (test_integration.py:3672-3751) is exercised through the real CLI: idempotency (byte-identical second run), --suggest gating, re-lint-clean, BOM+CRLF survival. test_a_four_space_catalog_is_reserialised_at_two honestly pins the reformat rather than hiding it. Unit-level coverage adds non-list plugins, duplicate JSON keys, NaN/Infinity, symlinked catalogs, injected RecursionError, injected Path.resolve failures, duplicate names, directory-name fallback, non-ASCII preservation. I separately confirmed empty file, trailing-comma, and list-rooted [] catalogs all report non-fixable and leave the file untouched.
• enabled: auto gating: TestActivation (test_discovery.py:260-288) parametrizes all five rules over default_enabled == "auto" + is_rule_enabled(...) is False on a plain repo. --type invariance has its own suites (:454-650).
• TestDualManifestBackwardCompat (test_provenance.py:640-696) is genuinely mutation-resistant: asserts preconditions so the negative assertions cannot go vacuous, adds a positive control that deletes .claude-plugin/ and requires both tightened violations to appear, and closes with the strongest form — deleting .codex-plugin/ must not change either rule's message set at all.
• All four disclosed neutral-machinery changes are regression-pinned: URI schemes (test_content_rules.py:1429-1452), hooks non-string type/matcher (test_hook_rules.py:170-200, asserts the block reached the tree before asserting absence), exclusion symlink loop (test_containment.py:512, parametrized over all three exception types), docs escaping (test_docs.py:449-460, 758-880).
• Vendored .codex/plugins/ diagnostic-only is tested at the linter level (test_skills.py:267-318) via full fix_and_apply(SUGGEST) plus an authored-twin control — the right place, since it covers rules added later.

No BLOCKING issues. All findings are coverage gaps in defensive branches.

1. SUGGESTION — src/skillsaw/rules/builtin/codex/marketplace_registration.py:394 — fix() picks its write target positionally: marketplace_file = config_nodes[0].path. Today that resolves to the primary .agents/plugins/marketplace.json because discover_codex_catalogs appends the primary before the sorted(glob("*.json")) siblings (discovery/codex.py:125-153) — I confirmed by hand. But no test asserts it. test_registration_in_a_sibling_catalog_counts (test_registration.py:207-236) only asserts zero violations and never runs fix(). If anyone sorts the union in build_lint_tree (alphabetically api_marketplace.json sorts first), the SUGGEST autofix would silently start appending to the wrong published catalog with every test still green. Action:

def test_fix_targets_the_primary_catalog(self, tmp_path):
    repo = copy_fixture("codex/clean", tmp_path)
    sibling = repo / ".agents" / "plugins" / "api_marketplace.json"
    sibling.write_text(json.dumps({"name": "example-api", "plugins": []}), encoding="utf-8")
    _write_plugin(repo / "plugins" / "newcomer",
                  {"name": "newcomer", "version": "1.0.0", "description": "x"})
    before_sibling = sibling.read_bytes()
    results = self._fix(repo)
    assert [r.file_path.name for r in results] == ["marketplace.json"]
    assert sibling.read_bytes() == before_sibling

2. SUGGESTION — src/skillsaw/discovery/codex.py:152-153 — the duck-typing acceptance branch is uncovered. The test that names the feature (test_sibling_catalog_discovered_when_it_looks_like_a_marketplace, test_discovery.py:216-226) only exercises the filename path — api_marketplace.json is accepted at :149 by _is_marketplace_filename, and notes.json exercises only the negative. So "any other *.json must duck-type as a marketplace" has a negative test but no positive one. _looks_like_codex_catalog is shared with codex_catalog_exists, which drives the Claude stand-down — high blast radius. Action: add the positive case (extras.json with a plugins[].source) and the near-miss the docstring calls out (plugin-versions.json), asserting the discovered set is exactly {marketplace.json, api_marketplace.json, extras.json}.

3. SUGGESTION — src/skillsaw/rules/builtin/codex/marketplace_registration.py:194-195 and 507-520 — _registerable's fourth documented exclusion ("lies outside the marketplace root") is entirely uncovered, as are _relative_source's two failure returns. This guards an autofix that writes a published catalog. The file already shows the right convention for a genuinely unreachable line (fix() line 438 carries # pragma: no cover - _registerable already excluded these); lines 194-195 do not, so a reader cannot tell untested-but-reachable from dead. Action: determine which, then pin it (start from tests/fixtures/codex/claim-escape/) or mark it with the same style of note.

4. SUGGESTION — tests/codex/test_provenance.py:881-901 — .claude/rules/development.md warns the provenance-scope mechanism "fails open by node type": provenance_dir() is overridden only on LintTarget, PluginNode, CommandBlock, AgentBlock. A rule declaring provenance_scope = "claude" over any other node type gets silently no filtering. test_claude_format_rules_declare_their_scope pins the eight scoped rule IDs — but the natural response to that test failing is to add the new ID to the set, which passes while leaving the rule unfiltered. Action: pin the override set mechanically so adding a scoped rule over a new node type forces a deliberate decision:

def test_provenance_dir_overrides_are_pinned(self):
    from skillsaw import lint_target as lt
    from skillsaw.blocks import frontmatter as fm
    overriding = {cls.__name__ for mod in (lt, fm) for cls in vars(mod).values()
                  if isinstance(cls, type) and "provenance_dir" in cls.__dict__}
    assert overriding == {"LintTarget", "PluginNode", "CommandBlock", "AgentBlock"}, (
        "scope filtering fails open for node types without the override")

5. NOTE — marketplace_registration.py:73-74 — the non-object-root branch of _mutable_marketplace_data is uncovered. Cheapest fix: parametrize test_malformed_marketplace_is_left_alone over ["", '{"name":"b","plugins":[],}', "[]", '{"name":"b","plugins":{}}'].

6. NOTE — src/skillsaw/docs/extractor.py:340 and :459 — the string-form author coercion (if isinstance(author_val, str)) has no test; every fixture uses the object form. Add "author": "Ada Lovelace" cases to test_docs_rendering.py beside test_an_author_with_no_other_metadata_reaches_the_html (:634), for both plugin and skill call sites.

7. NOTE — src/skillsaw/rules/builtin/codex/plugin_structure.py:45-46 — the except OSError: continue around manifest_dir.iterdir() is uncovered, and the rule has exactly one dedicated test. Add a monkeypatched-iterdir test matching TestRegistrationSurvivesUnresolvablePlugins. A stray directory inside .codex-plugin/ is also untested.

8. NOTE — marketplace_json_valid.py:426-432 — the except ValueError around parsed.hostname appears unreachable on 3.12 (probed https://[abc], https://[::1]x, https://[], https://[v1.x]). Mark # pragma: no cover with an interpreter note, or drop it.

9. NOTE — marketplace_json_valid.py:402, 404 — two uncovered returns in _check_npm_registry: npm source with no registry key, and non-string registry.

Technical Writer

1. BLOCKING — docs/cli.md:35 (source: scripts/generate-site-content.py:569) — the new "Path arguments" section documents manifest-path widening for .claude-plugin/plugin.json, but the implementation deliberately excludes it. src/skillsaw/cli/_helpers.py:62-74 widens only .codex-plugin/ and .agents/plugins/, and the inline comment states the exclusion is intentional: "Codex surfaces only: widening .claude-plugin too would silently expand what an existing skillsaw lint <repo>/.claude-plugin/plugin.json invocation reads — and what skillsaw fix writes — beyond the path the caller named." Verified:

$ skillsaw lint cx4/.claude-plugin/plugin.json
Linting: /tmp/cx4/.claude-plugin
  Repo type: unknown
  Plugins:   0          # manifest rules do NOT run

$ skillsaw lint cx2/.codex-plugin/plugin.json
Linting: /tmp/cx2
  Repo type: codex-plugin
  Plugins:   1          # widened as documented

A user following this doc gets Repo type: unknown / 0 plugins / "All checks passed" and reasonably concludes their Claude manifest is clean when no manifest rule ran. The doc advertises silence as a pass. Action: edit CLI_PATH_SECTION in scripts/generate-site-content.py:564-576 to drop .claude-plugin/plugin.json from the widening list and say explicitly that a Claude manifest path is not widened, naming the reason. Regenerate with make update. Check scripts/generate-docs.py for the same string.

2. SUGGESTION — docs/ci.md:198-205 — the "Changed in 0.18" upgrade note describes the skillsaw docs change as HTML-only (escJsAttr/escAttr + author) and closes with "nothing about the published pages' behavior changes otherwise." The Markdown renderer also changed for every repository: markdown_renderer.py:118-136 now folds every metadata scalar through the new _table_cell(), which backslash-escapes \ | [ ] ( ), replaces backticks with ', and entity-encodes < >. That covers version, license, category, tags, owner.name, author.name, titles/headings, and command names (_append_command, :228). A plugin whose version is 1.0.0 (beta) now renders differently in committed Markdown docs — for a Claude-only repo. (I confirmed the is_marketplace gating change is not a Claude-only regression: MARKETPLACE is first in _TYPE_PRIORITY, so the two conditions are equivalent for Claude repos.) Action: extend the note to cover Markdown output and give it the same "regenerate on upgrade" advice.

3. SUGGESTION — docs/rules/codex.md:6 (source: scripts/generate-site-content.py:100, scripts/generate-docs.py:65) — the category blurb says "The metadata rule auto-enables for Agent Skills; the plugin and marketplace rules auto-enable only when their Codex manifests are present." codex-openai-metadata declares repo_types = SKILL_REPO_TYPES (openai_metadata.py:37) → agentskills, codex-marketplace, codex-plugin, dot-claude, marketplace, single-plugin, so it also auto-enables on plain Claude plugin, Claude marketplace, and .claude/ repos. Verified. The rule's own page has the correct Repo Types row, so this is a category-summary imprecision — but it's the first sentence a reader hits, and it under-sells the blast radius of the one new rule that can fire on a non-Codex repo. Action: reword to "auto-enables anywhere skills are detected — including Claude plugin, marketplace and .claude/ repositories — and fires only when an agents/openai.yaml is present." Update both generator copies.

4. NOTE — src/skillsaw/rules/docs/codex-plugin-json-valid.md:55 — "author must be a string or an object (an object should carry a name)" implies a check that doesn't exist; plugin_json_valid.py:133-140 only rejects non-str/non-dict. An author: {} is never reported.

5. NOTE — docs/rules/codex-marketplace-registration.md:11 — header table shows | **Autofix** | auto | while the body correctly says --suggest is required. The generator's fix column (generate-site-content.py:273-275) only checks supports_autofix and never renders autofix_confidence. Pre-existing across marketplace-registration, command-naming, content-unclosed-fence — not introduced by this PR. Worth a follow-up issue.

Checked and correct:

• Five new rule docs vs implementation — severity (error ×4, warning for codex-plugin-structure), enabled: auto, repo-type gating, and config schemas all match. I ran the codex-plugin-json-valid "Bad" example and got exactly the documented mix; the codex-marketplace-registration --suggest fix produced exactly the documented entry shape.
• Detailed prose claims verified against code: _RECOMMENDED_ENTRY_FIELDS + _check_policy; _SOURCE_REQUIRED_FIELDS; _BRAND_COLOR uses fullmatch (so "no shorthand" is true); hooks excluded from _PATH_FIELDS and unpacked separately with drop_objects=True; _EXPECTED_KIND file/dir reporting; installed-plugin stand-downs.
• plugin-readme.md's new Codex exemption — correct via node typing (CodexPluginNode is not a PluginNode), confirmed by lint run.
• docs/repo-types.md — discovery probe list, the .codex/plugins/* runs-vs-stands-down table, the sibling-catalog separator rule (matches lowered[-17] in "-_."), and the docs metadata list are all accurate.
• Generated files are current. Copied the repo to /tmp and ran both generator scripts; git status empty in both cases. .skillsaw.yaml.example has all five new rules with correct defaults. Rule counts (68 total, 5 Codex) consistent.
• Behavior-change disclosure — Add "Claude was here" to README.md #1 at docs/rules/content-broken-internal-reference.md:45-49; [Auto] Sync hooks rules with upstream Claude Code spec #2 covered generically plus the Codex-only matcher paragraph; [Auto] Sync MCP rules with upstream server spec #3 is internal robustness with docs/configuration.md:181-184 matching _pattern_variants exactly; [Auto] Validate agentskills.io optional frontmatter fields #4 at docs/ci.md:191-205 (incomplete per finding 2).
• Agentic-context drift — the "Ecosystem provenance" section and "Adding an ecosystem" recipe exist at .claude/rules/development.md:135-197 and are accurate; every symbol they name exists. All three APM-compiled copies of references/codex.md are byte-identical.
• Version — 0.18.0 consistent across pyproject.toml, .skillsaw.yaml.example, docs/ci.md, docs/pre-commit.md, action.yml; no stale 0.17 outside correct Since rows. No CHANGELOG file exists, so no entry expected. THREAT_MODEL.md T14/T15 consistent with code. Self-lint clean (A+, 0 errors/warnings). No published-URL breakage.

Ecosystem Reviewer

Bottom line: BELONGS IN CORE. No blocking findings. Adoption clears the bar by a wide margin, the implementation's field expectations match the real spec with unusual fidelity, and a redirect-to-plugin is architecturally infeasible for the majority of this change's measured value.

1. Adoption — verified, well above the bar

Signal	Evidence
Target tool	openai/codex — 102,143 stars, pushed 2026-07-28
Reach	Codex reported at 5M+ weekly users (~6x since the Feb 2026 desktop launch)
Format publisher	OpenAI, first-party. Spec at developers.openai.com/plugins/build/plugins (now serving via learn.chatgpt.com/docs/*)
Marketplace launch	Soft rollout 2026-03-27; past 90 integrations by 2026-04-16
Reference corpus	openai/plugins — 4,791 stars, 645 forks, 180 plugins, 588 agents/openai.yaml files
Corpus activity	Multiple merges/week through 2026-07-13
Third-party ecosystem	hashgraph-online/awesome-codex-plugins, codex-marketplace.com, enterprise policy-file governance

I cloned openai/plugins and confirmed HEAD is exactly 11c74d6ba24d3a6d48f54a194cd00ef3beea18f9 (2026-07-13) — the PR's CI pin is the real upstream tip, not a cherry-picked stale commit.

Precedent: core already ships rules for openclaw, coderabbit, promptfoo (23.7k stars), apm, Kiro, Cursor, Copilot. Codex is larger than every one by any available measure. docs/plugins.md states the policy directly: "For common, popular formats it is recommended to contribute repository types, rules, and tree support back to skillsaw itself... Plugins are the right home for private/organization-specific conventions."

2. Spec fidelity — all five rules checked against the live spec and the 180-plugin corpus. Ran the branch's linter against the real corpus: 973 total violations, zero codex-* findings, with all five rules confirmed enabled via --verbose (so the zero is real coverage, not disabled rules).

• plugin.json — requires only name; version/description are configurable WARNINGs (present in 180/180 real manifests). _INTERFACE_PATH_FIELDS matches corpus reality exactly (logo 179, composerIcon 177, screenshots 172, logoDark 42). Unknown keys ignored.
• marketplace.json — unknown source types warn rather than error; policy.* enums configurable. Correctly forward-compatible.
• agents/openai.yaml — _INTERFACE_STRINGS is precisely the documented set. I extracted the union of keys across all 588 corpus files: no key outside what the rule models, no type mismatch. All 119 brand_color values are #RRGGBB. No required fields enforced — correct, since the spec says all sections are optional.
• codex-plugin-structure — "Only plugin.json belongs in .codex-plugin/" is verbatim upstream language; corpus confirms zero non-plugin.json files in any .codex-plugin/.
• api_marketplace.json — correctly recognized as a sibling catalog, with a deliberate guard against matching notamarketplace.json. Without this, 29 API-marketplace entries would have produced spurious findings.

3. Core-vs-plugin — redirect is infeasible. A narrow Codex rule plugin is technically buildable (PluginRepoType, SKILLSAW_TREE_CONTRIBUTORS exist). But three things a plugin cannot do carry most of this PR's measured value: (a) change builtin rules' scoping — provenance_scope has no plugin hook; (b) extend skillsaw docs — no renderer/extractor extension point; (c) nest prose under a plugin container with containment semantics — tree contributors attach at the root.

I measured (a) empirically by running the base commit 43654b2 against the same corpus:

	Base 43654b2	Branch
Total violations	1,644	973
codex-*	n/a	0
agentskill-unreferenced-files	820	180
plugin-json-required (Claude)	6	0
command-frontmatter (Claude)	10	0
agent-frontmatter (Claude)	6	0
command-naming / marketplace-json-valid	2 / 1	0 / 0

Net −671 false positives — the PR's "~700 fewer" claim is verified, not asserted. Base was reporting Missing plugin.json at plugins/figma/.claude-plugin/plugin.json and Missing frontmatter on Codex agent files — pure Claude-convention FPs on OpenAI's own repository. A rule plugin could not have fixed a single one of those 25. I also confirmed openshift-eng/ai-helpers: 1 violation total, 0 codex-* — and that one is the repo's own plugins-doc-up-to-date custom rule, exactly as disclosed.

SUGGESTION — plugin-root hooks.json is invisible to the lint tree, including in OpenAI's own plugins — src/skillsaw/lint_tree.py:387,424 (and formats/codex.py:149-191). The openai/plugins README lists the documented companion surfaces as "skills/, .app.json, .mcp.json, plugin-level agents/, commands/, hooks.json, assets/" — i.e. hooks.json at the plugin root. skillsaw discovers Codex hooks only from manifest-declared paths and the conventional hooks/hooks.json. Two official plugins ship a plugin-root hooks.json with no hooks field in the manifest — plugins/figma and plugins/replayio. plugins/figma/hooks.json:

{"hooks":{"PostToolUse":[{"matcher":"Write|Edit","hooks":[{"type":"command","command":"./scripts/post_write_figma_parity_check.sh"}]}]}}

skillsaw tree /tmp/oaiplugins returns zero hooks.json nodes across the entire 180-plugin corpus. hooks-dangerous, hooks-prohibited, and hooks-json-valid therefore never see a shell command Codex may execute. This is why the corpus gate is clean on hooks — nothing was examined, not nothing was wrong. Upstream is ambiguous: openai/codex #16430 reports plugin-local hooks may not execute at all, and the manifest spec says hooks defaults to ./hooks/hooks.json. Rated SUGGESTION because it's a false negative, not an FP source — but the security reviewer should weigh it independently. Action: either attach plugin-root hooks.json as a HooksBlock, or state the omission explicitly in docs/repo-types.md citing openai/codex#16430, so the corpus-gate result is not read as coverage.

SUGGESTION — Codex-schema catalogs at .claude-plugin/marketplace.json are a documented, unmitigated FP path — docs/repo-types.md:152. The PR documents this honestly: "a Codex-schema catalog written to the legacy path will be checked against the Claude schema — a missing owner, and an unknown local source type on every entry." Codex officially reads .claude-plugin/marketplace.json for backward compatibility, so this is user-reachable, and the failure mode is loud (one error per entry). Action: detect the Codex shape at the legacy path (a plugins[].source object carrying a source discriminator, or a top-level interface with no owner) and emit one clear "move it to .agents/plugins/marketplace.json" violation instead of a per-entry error cascade. A follow-up issue is acceptable.

NOTE — docs/rules/codex-plugin-structure.md:42-50 — rule message and doc both say to keep hooks at hooks/hooks.json; upstream documents plugin-root hooks.json, and the manifest spec documents ./hooks/hooks.json as the default. Both valid; the rule enforces neither. Reword to "at the plugin root (hooks.json, or hooks/hooks.json — the manifest default)".

NOTE — spec churn is real but the rules are built for it. The format moved during the review window: logoDark added ~2026-06-17, MCP OAuth declarations 2026-06-22, spec pages migrated hosts (308). This is a ~4-month-old format on an active first-party roadmap. The mitigation is already in the design and belongs on the record as a positive: unknown keys ignored, unknown source types and policy.* enums warn-not-error and are configurable, only name hard-required. Action: schedule a periodic refresh of the CI-pinned corpus commit — the pin guarantees reproducibility but also guarantees staleness.

Sources: openai/plugins · openai/codex · Build plugins (OpenAI) · Build skills · Plugins overview · openai/codex#16430

Slopinator Reviewer

Baseline note: this branch already contains dedicated cleanup commits (Prune review-sediment comments; keep load-bearing invariants (#468), Trim review-round accretion from the test suite, State the trailing-/** invariant without narrating history). The comment corpus is genuinely good — nearly every added comment is a "why" comment that passes the stranger test. Comment density in the new codex/ package (12–17%) matches the existing repo (8–17%). Test class names are behavior-scoped, not round-scoped. Findings are few.

1. BLOCKING — benchmarks/genrepo.py:20-22 — phantom-bug narration in a shipped comment

    # An openai/plugins-shaped catalog: every plugin Codex-claimed with a
    # local source, so repository detection consults the claim set once per
    # child — the shape whose discovery cost was once quadratic.

"the shape whose discovery cost was once quadratic" describes a state that never existed in any released skillsaw. main has no Codex support at all; the quadratic claim-set probe was introduced inside this branch (comment added in 28172ee) and fixed later in the same branch (852a8b5 Take the Codex ownership work off the hot paths). A reader on main a year from now has no diff to check it against and will go looking for a version where Codex discovery was quadratic. This is the "reviewer's hypothetical frozen into a permanent claim about history" pattern verbatim. The rest of the comment is good and should stay — only the final clause is residue. Action: replace the last clause with the live invariant — — the shape that stresses claim-set memoization.

2. SUGGESTION — .apm/instructions/development.instructions.md:186-199 (mirrored into .claude/rules/development.md:182, AGENTS.md, .cursor/rules/development.mdc, .github/copilot-instructions.md, .github/instructions/, .agents/) — a 150-word run-on where the file's own style is bullets. Six sequential steps packed into one semicolon-chained sentence with four em dashes and three parentheticals, inside a file whose every other directive of this shape is a - **Term**: … bullet. It is roughly 3x the longest other prose block in the file, and it is an ordered procedure whose audience is an agent executing it. The closing sentence ("Prose needs no work… existing rule files need no visits") reads as reassurance to a reviewer rather than instruction to an implementer. Action: restructure to the file's own bullet style with no facts added or removed; fix in .apm/instructions/development.instructions.md and re-run make update so the six mirrors regenerate.

3. NOTE — three ragged mid-paragraph line wraps left by in-place edits — text spliced into existing paragraphs without re-flowing, leaving a short line mid-sentence in files otherwise wrapped at ~65–70 columns: src/skillsaw/rules/docs/plugin-naming.md:34 (32 cols), hooks-json-valid.md:46 (54 cols), mcp-valid-json.md:38 (52 cols). The paragraph shape records the edit. Cosmetic in rendered output, but these ship to skillsaw.org via docs/rules/.

Checked and clean:

• Review-history residue: zero hits across the diff for as requested, per review, review feedback, addresses #NNN, previously, we now, changed from, was moved, round-numbered test/section names, or references to PRs Add first-class OpenAI Codex plugin and marketplace linting #451/Split tests/test_codex_rules.py into a tests/codex/ package #464–Break up context.py: finish the discovery/orchestrator decomposition #470. The only issue reference in shipped code — tests/test_banned_apis.py:8, "grandfathered per issue Retire the symlink-crash bug class: sweep bare Path.resolve() behind safe_resolve() and enforce with a banned-API test #463" — points at a real open tracking issue and matches house style (.claude/rules/development.md already cites "issue SAFE autofixes corrupt files: unlinked-ref path truncation, and agentskill-valid duplicate name: keys (non-convergent) #321").
• Round-scoped measurements: no benchmark numbers, percentages, or timings frozen into comments. The only complexity claims (context.py:799, :1103-1107) are live invariants justifying caches, not captured measurements.
• Test organization: all ~90 new test classes in tests/codex/ are named for the scenario (TestCrossedRegistration, TestSiblingOnlyCatalogExempts, TestForcedSeedsRespectContainment). No round/panel/follow-up naming.
• AI vocabulary tells: no hits for comprehensive, robust (as filler), seamless, leverage, delve, crucial, tapestry, landscape, testament, showcase, it's worth noting, important to note, serves as, boasts, this ensures that. silently appears 58 times but is established house vocabulary (18 pre-existing uses in src/) and is the linter's actual domain.
• Em-dash density: codex-marketplace-registration.md runs 1 per 40 words vs ~1 per 56–93 in comparable existing rule docs — denser, but every instance does ordinary parenthetical or causal work, and the repo's house style is em-dash-heavy. Not flagged.
• Docstrings: no signature-echo. Trivial helpers get one-liners; the long ones (paths.py _safe_stat, safe_resolve, is_absolute_path) each earn their length by documenting a cross-version pathlib failure mode with the interpreter range spelled out.
• Commit messages: narrate change, which is what commit messages are for; no sycophancy, chatbot artifacts, or generic send-offs. Several are unusually good (053f1bb discloses a downstream upgrade cost with the verification method).
• Fixtures: tests/fixtures/codex/** prose is realistic authored content, not one-liner stubs.

---

Panel Synthesis

All seven specialists completed. This is a large change (206 files, ~16k insertions) and it holds up unusually well under scrutiny — the panel found no correctness regressions, no security vulnerabilities, and no architectural faults that compound. Two BLOCKING findings survived arbitration; both are small, in-scope edits.

Ratified: this belongs in skillsaw core. The Ecosystem Reviewer's adoption case is not merely argued, it is measured. openai/codex at 102k stars and 5M+ weekly users is larger than every ecosystem skillsaw already supports in core (openclaw, coderabbit, promptfoo, apm, Kiro, Cursor, Copilot), and docs/plugins.md's own stated policy points first-party popular formats at core rather than plugins. Critically, the reviewer independently reproduced the PR's headline claim by running base 43654b2 against the pinned corpus: 1,644 violations → 973, a net −671 false-positive reduction, including 25 pure Claude-convention FPs on OpenAI's own repository (Missing plugin.json at plugins/figma/.claude-plugin/plugin.json). A rule plugin could not have fixed one of those — provenance_scope suppression of builtin rules has no plugin hook, and skillsaw docs has no extractor/renderer extension point. Redirect-to-plugin is architecturally infeasible, not merely undesirable.

The security claims survived adversarial testing. The Security Reviewer did not take the T15 threat-model work on faith: escJsAttr/escAttr were verified by running the emitted JavaScript under Node and re-parsing the resulting fragments with an HTML parser (backslash-doubling defeats the \'-escape-the-escape attack; &-first ordering defeats entity re-introduction of a quote); the URL scheme allow-list held against 23 payloads including multiply-nested entity encodings; filename collision handling, symlink escape, and zip-slip were exercised on a live hostile repository. No new subprocess, socket, or network calls; pyproject.toml carries a version bump and nothing else; CI keeps contents: read and passes --no-custom-rules on the third-party corpus. This is the strongest part of the change.

Corroboration across specialists. Two independent findings landed on the same seams and are weighted accordingly. Architecture and Python both flagged the private reach-through at lint_tree.py:320 (context._codex_claim_set()) on the one seam the design doc says must stay narrow. Architecture and Security both flagged that the new bare-.resolve() ratchet (tests/test_banned_apis.py) is file-granular and grandfathers the four files this PR touches most — and Security then found a new bare .resolve() that landed inside a grandfathered file (docs/extractor.py:281), two lines from a safe_resolve call doing the same job. That is the ratchet failing at its stated purpose within the same PR that introduced it; it is the highest-value non-blocking item here.

A theme worth naming: partially-adopted new helpers. This PR introduces three safety helpers and adopts each incompletely. safe_display() is applied to hooks/json_valid.py:239 with an explicit credential-redaction rationale but skipped on five sibling interpolations in mcp/valid_json.py — where the Python Expert reproduced a credentialed URL and a raw C0 escape reaching the JSON sink. safe_resolve is skipped at docs/extractor.py:281. The text formatter's file_path column bypasses _CONTROL_CHARS entirely, so a crafted directory name can still smuggle terminal escapes — pre-existing, but it contradicts safe_display's own docstring. Individually these are SUGGESTIONs; together they say the helpers need one more adoption sweep before the next ecosystem lands on top of them.

Test quality is high and honestly reported. 3,505 tests pass, 94% coverage, new modules at 93–100%. QA verified the claims rather than restating them: TestDualManifestBackwardCompat is genuinely mutation-resistant (asserts preconditions so negatives cannot go vacuous, carries a positive control, and closes by requiring that deleting .codex-plugin/ change no message at all), the vendor-managed stand-down is drawn at the linter level so it covers rules added later, and test_a_four_space_catalog_is_reserialised_at_two pins the reformat rather than hiding it. The residual gaps are in defensive branches. The one that matters most is marketplace_registration.py:394: fix() selects its write target positionally (config_nodes[0].path) and nothing asserts it — if anyone sorts the union in build_lint_tree, a SUGGEST autofix would silently begin appending entries to the wrong published catalog with the suite still green.

One finding deserves the author's judgment rather than the panel's. The Ecosystem Reviewer found that plugin-root hooks.json — a surface the openai/plugins README documents explicitly — is invisible to the lint tree. Two official plugins (figma, replayio) ship one with no manifest hooks field, and plugins/figma/hooks.json registers a PostToolUse shell command. skillsaw tree returns zero hooks.json nodes across all 180 corpus plugins, so hooks-dangerous and hooks-prohibited never examined them. The clean corpus gate on hooks means nothing was looked at, not nothing was wrong. Upstream is genuinely ambiguous (openai/codex#16430 reports plugin-local hooks may not execute at all), so the panel does not treat this as blocking — it is a false negative, not a false-positive source. But it should be a deliberate, documented decision rather than an implicit one, and the corpus-gate result should not be read as hook coverage.

Disagreement resolution. No specialist contradicted another. Two findings were filtered as out of scope: the Technical Writer's Autofix | auto column issue is pre-existing across marketplace-registration, command-naming, and content-unclosed-fence (follow-up issue, not this PR), and the Slopinator's em-dash-density observation was self-filtered against the repo's established house style — correctly, per the baseline discipline the panel asks for.

On the Slopinator blocker. The benchmarks/genrepo.py comment is one clause in a benchmark helper, and the branch already carries three dedicated prose-cleanup commits — this is a near-miss, not a pattern. It is held BLOCKING because the claim is false on main: no released skillsaw ever had quadratic Codex discovery, so a future reader has no diff to check it against and will hunt for a version that never existed. Confirmed verbatim at benchmarks/genrepo.py:21. One-line fix.

Backward compatibility. The four disclosed behavior changes to neutral machinery are each regression-pinned, and the Technical Writer confirmed the is_marketplace gating change is equivalent for Claude repos (MARKETPLACE is first in _TYPE_PRIORITY) rather than a silent regression. The one disclosure gap — Markdown renderer output also changing for Claude-only repos via the new _table_cell() escaping — is a documentation completeness issue, not a compatibility break, and is listed below.

Disposition is REQUEST_CHANGES on the strength of the two BLOCKING items. Both are edits to a comment and a generated doc string; neither touches shipped logic. Everything else is optional.

---

Required Actions Before Merge

1. docs/cli.md:35 documents path widening that does not exist. Edit CLI_PATH_SECTION in scripts/generate-site-content.py:564-576 to drop .claude-plugin/plugin.json from the widened-manifest list, and state explicitly that a Claude manifest path is not widened — naming the reason already in cli/_helpers.py:62-74 (it would expand what fix writes beyond the path the caller named). Check scripts/generate-docs.py for the same string, then make update. Rationale: a user who follows this doc sees Repo type: unknown / 0 plugins / "All checks passed" and concludes their manifest is clean when no manifest rule ran — the doc advertises silence as a pass.

2. benchmarks/genrepo.py:21 — remove the phantom-history clause. Replace — the shape whose discovery cost was once quadratic. with — the shape that stresses claim-set memoization. Keep the rest of the comment. Rationale: the quadratic state existed only inside this branch (introduced 28172ee, fixed 852a8b5); on main the claim is false and unverifiable.

---

Optional Follow-ups

Highest value first:

1. Complete the safe_display() sweep — mcp/valid_json.py:162, 171, 183, 217, 225 (empirically reproduced: a credentialed URL and a raw C0 escape reach the JSON/SARIF sinks). Consider also routing the text formatter's file_path column through _CONTROL_CHARS (formatters/text.py:60-70), since a crafted directory name is a linter-evasion primitive today.
2. Fix the .resolve() ratchet's own escape hatch — docs/extractor.py:281 uses a bare .resolve() inside a grandfathered file, two lines from a safe_resolve doing the same job. Switch it, and make the ratchet freeze a per-file count rather than a filename (tests/test_banned_apis.py:22-52) so the four files this PR touches most are actually covered.
3. Pin the registration autofix's write target — marketplace_registration.py:394 selects config_nodes[0].path positionally with no test; QA supplied a ready-to-paste test_fix_targets_the_primary_catalog.
4. Decide and document plugin-root hooks.json — either attach it as a HooksBlock alongside hooks/hooks.json, or state the omission in docs/repo-types.md citing Plugin docs/examples imply plugin-local hooks, but runtime only executes global hooks.json openai/codex#16430 so the clean corpus gate is not misread as hook coverage.
5. Move _codex_roots = None into the unconditional invalidation block — context.py:412 → :419; reproduced stale-root case where an excluded directory survives apply_excludes().
6. Read Claude ownership from provenance() in marketplace/json_valid.py:55 — the last rule-level filesystem probe for ownership; the two spellings already disagree in a reproducible case.

Documentation:

7. Extend the docs/ci.md:198-205 0.18 upgrade note to cover Markdown output changes (_table_cell() escaping affects version, license, category, tags, author, owner, command names for every repo, Claude-only included).
8. Reword the docs/rules/codex.md:6 category blurb — codex-openai-metadata auto-enables on all SKILL_REPO_TYPES, including Claude plugin/marketplace/.claude/ repos.
9. Restructure the "Adding an ecosystem" recipe in .apm/instructions/development.instructions.md:186-199 into the file's own bullet style, then make update to regenerate the six mirrors.
10. Minor doc fixes: the author parenthetical at codex-plugin-json-valid.md:55; the hook-location wording at codex-plugin-structure.md:42-50; three ragged line wraps (plugin-naming.md:34, hooks-json-valid.md:46, mcp-valid-json.md:38).

Code quality / performance:

11. Extract _build_plugin_nodes and _build_root_blocks from the 575-line build_lint_tree (tracked alongside Break up context.py: finish the discovery/orchestrator decomposition #470); add a public codex_claimed_dirs() so lint_tree.py:320 stops reaching into context._codex_claim_set().
12. O(1)-ify the three scans the Python Expert found: seen_roles linear scan (lint_tree.py:591-596), per-plugin tree re-scans in docs/extractor.py:301, 437, and the repeated root.resolve() in paths.py:126.
13. Cache safe_resolve(root_path / ".claude") per context (context.py:843) — it sits on the common branch of the provenance hot path.
14. De-duplicate the URL scheme allow-list into docs/models.py (extractor.py:389 / html_renderer.py:1227) — a security allowlist kept in two files will drift.
15. Add # notsecret to tests/fixtures/codex/claim-escape/outside-the-checkout/deploy_key_outside (matches the treatment this PR gives single-plugin/with-secrets/commands/setup.md; the PEM delimiters trip gitleaks and GitHub push protection).
16. Remaining QA coverage items: duck-typed sibling catalogs (discovery/codex.py:152-153), _registerable's outside-root exclusion, the provenance_dir() override pin, and the string-form author coercion.

Separate issues (pre-existing, not this PR):

17. The rule-doc Autofix column renders auto for SUGGEST-confidence rules (generate-site-content.py:273-275) — affects marketplace-registration, command-naming, content-unclosed-fence, and now codex-marketplace-registration.
18. Codex-schema catalogs at the legacy .claude-plugin/marketplace.json path produce a per-entry Claude-schema error cascade; a single "move it to .agents/plugins/marketplace.json" violation would be kinder.
19. Schedule a periodic bump of the CI-pinned openai/plugins corpus commit — the pin guarantees reproducibility and also guarantees staleness on a 4-month-old format still on OpenAI's active roadmap.

---

_{Generated by skillsaw-review-panel}

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 74176cf341

ℹ️ 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".

[chatgpt-codex-connector]

Report the missing Claude marketplace for unclaimed plugins

When a repo has a Codex catalog plus an unclaimed legacy plugin such as plugins/legacy/commands/foo.md, _plugins_dir_suggests_claude_marketplace() still infers MARKETPLACE, but this exemption returns clean solely because no .claude-plugin marker exists. That leaves the unclaimed PluginNode in Claude scope and plugin-json-required can still fail, while the companion .claude-plugin/marketplace.json error is incorrectly suppressed; base the exemption on provenance/scoped plugin ownership rather than the marker probe.

AGENTS.md reference: AGENTS.md:L148-L153

Useful? React with 👍 / 👎.

[chatgpt-codex-connector]

Keep Codex ownership checks independent of discovery

With an explicit non-Codex override such as --type marketplace, a plugins/foo/.codex-plugin/plugin.json directory that is discovered via the legacy plugins scan can still be classified as Codex-only by provenance() and have its hooks/MCP blocks attached, but codex_plugin_owning() builds roots from the disabled Codex discovery path and returns None. In that context hooks-json-valid and mcp-valid-json skip the Codex-only matcher/non-empty-command checks for those blocks, so manifest evidence should also feed this ownership predicate or it needs a provenance ancestor fallback.

AGENTS.md reference: AGENTS.md:L161-L164

Useful? React with 👍 / 👎.

[github-actions]

Review Panel Verdict

Disposition: REQUEST_CHANGES

---

Specialist Findings

Architecture Reviewer

Full suite run on the branch: 3505 passed. No import cycles among the new modules; tests/codex/test_discovery.py::test_discovery_never_imports_context genuinely pins the discovery→context layering the design claims.

1. BLOCKING — src/skillsaw/context.py:1177 — symlinked Claude plugin directories are silently dropped

_discover_from_plugins_dir changed resolved_path = item.resolve() → contained_resolve(item, self.root_path). That removes an entire Claude plugin from discovery with no diagnostic, no test, and no documentation. The sibling containment path (_resolve_plugin_source) already had this check on 43654b2 and logs "Plugin 'x' source './plugins/x' escapes repository root. Skipping." — the plugins/ walk emits nothing.

Reproduced: repo with plugins/foo -> ../../outside/foo, where outside/foo carries .claude-plugin/plugin.json and commands/BadName.md:

• 0.17 (43654b2): Plugins: 1, 2 errors + 5 warnings (command-frontmatter, command-naming, plugin-json-valid ×2, plugin-naming, plugin-readme), Grade C+
• This branch: Plugins: 0, 1 violation (marketplace-json-valid: Marketplace file not found), Grade B

This is a Claude-only repository with no Codex content anywhere, so it sits outside the "dual-manifest repos keep exact 0.17 results" guarantee, and it violates .claude/rules/development.md's "Never break existing rules for users of skillsaw." It is also exactly the failure mode the PR's own architecture note names: "how a directory falls between per-ecosystem attach paths and loses its content silently." No test covers a symlinked plugins/* child in a Claude repo — test_marketplace_source_through_a_symlink_is_rejected covers the Codex catalog path only.

Recommended action. Keep the containment (the security rationale is sound — autofix must not write outside the checkout), but do not lose the content silently. Either (a) emit the same logger.warning the marketplace-source path emits and file a violation so the drop is visible in --format json/SARIF, or (b) if the narrowing is intentional as-is, add a fixture-backed integration test pinning it and document it in docs/repo-types.md alongside the other 0.18 behavior disclosures (053f1bb established that pattern).

2. SUGGESTION — src/skillsaw/rules/builtin/marketplace/json_valid.py:54-75 — last rule-level filesystem probe for ownership

_has_claude_plugin answers "does this repository declare a Claude plugin" with safe_is_dir(node.path / ".claude-plugin") — what the development rules forbid. Re-raising with a concrete divergence the prior panel did not name: provenance() treats <root>/.claude as Claude by definition (context.py:843-848), specifically so a Codex catalog listing "./.claude" as a local source cannot turn the repo's own commands Codex-only. _has_claude_plugin has no such rule, so on a DOT_CLAUDE repo whose .claude/ is claimed by a Codex catalog the two disagree and the rule stands down where provenance() says it should fire. Replace the body with any(context.provenance(n.path).claude for n in context.lint_tree.find(PluginNode)).

3. SUGGESTION — src/skillsaw/context.py:307-308 — rebuild_lint_tree() and apply_excludes() have diverged as invalidation surfaces

apply_excludes() clears six pieces of derived state; rebuild_lint_tree() clears only _lint_tree. The multi-pass autofix loop (linter.py:767-768) calls invalidate_read_caches() + rebuild_lint_tree() after writing to disk, so the next pass re-runs every scoped_find rule against provenance computed from the pre-fix filesystem. Latent today — no shipping fix creates or removes an ownership marker — but codex-marketplace-registration.fix() already appends catalog entries that feed _codex_local_sources() → _codex_claim_set(), which is memoized and not cleared. Give both one _invalidate_derived() helper.

4. SUGGESTION — src/skillsaw/docs/extractor.py:281 — new code reaches for the banned bare .resolve()

_extract_codex_plugins is added by this PR and uses pn.path.resolve(); tests/test_banned_apis.py grandfathers the file by name, so the ratchet cannot see a new offender. _is_codex_only, three lines later, uses safe_resolve. A PluginNode path traversing a symlink loop raises RuntimeError/OSError and aborts skillsaw docs for the whole repository — the exact abort skillsaw.paths exists to prevent.

5. SUGGESTION — src/skillsaw/lint_tree.py:71, :320 — build_lint_tree is now the only function in the codebase over 100 lines, at 575 (AST-verified), with eight closures and nine discovery sections; line 320 reaches into context._codex_claim_set(). Both tracked in #470; the function grew ~200 lines in this PR and each further ecosystem grows it again.

6. SUGGESTION — src/skillsaw/context.py:411-412 — _codex_roots = None still scoped under if codex_set_changed: while everything it derives from is cleared unconditionally at 419-422. Latent (no CLI path consults codex_plugin_roots() before apply_excludes() may re-run). Move it to line 419.

NOTES. _SAFE_URL_SCHEMES/_MD_SAFE_SCHEMES duplicated across two files; is_installed_plugin_skill is a no-value indirection over context.is_codex_installed_plugin; marketplace_registration.py:17-23 mixes the skillsaw.context re-export shim with skillsaw.formats.codex in adjacent import statements; _codex_listed_docs mutates doc.category on shared PluginDoc objects — a hidden side effect for a function named "listed docs".

Verified clean. Provenance is a genuine single source of truth everywhere except marketplace/json_valid.py. All 8 provenance_scope = "claude" rules go through scoped_find, and each iterates a node type overriding provenance_dir(). plugin-readme/plugin-json-valid/mcp-* correctly need no scope declaration — a Codex-only directory never produces a PluginNode, so their exemption is structural. find_filtered's cache key names everything its predicate depends on, and every path dropping the find() memo drops it too. The __init__ ordering hazard around marketplace_entries is real but correctly handled. _InlineJsonPayload MRO and identity __eq__/__hash__ are correct. Claude skill discovery is genuinely unchanged — the regression above is confined to the plugins/ plugin walk.

Python Expert

All 3505 tests pass; black --check src/ tests/ clean; no Python 3.10+ syntax or APIs — the Path | None annotations are all under from __future__ import annotations and never evaluated. MRO of _InlineJsonPayload correctly wins __eq__/__hash__ over LintTarget's path-based equality. Walrus usage in comprehension conditions is evaluation-ordered correctly. KEBAB_CASE uses \Z; _URI_SCHEME is ^-anchored with - last in the class. safe_display/_redact_userinfo/is_absolute_path match their docstrings under adversarial input. _vendor_managed_cache and _format_scope_cache both handle a cached False correctly. No blocking correctness or packaging defects.

1. SUGGESTION — context.py:807, formats/codex.py:209, context.py:786 — provenance() costs 7 Path.resolve() calls per directory, and --type makes every directory pay it

Measured on a synthetic 1800-directory repo:

default             0.50 / 0.48 / 0.47 s
--type agentskills  1.13 / 1.11 / 1.01 s     (~2.2x)

cProfile on the --type run: _discover_skills_in_dir = 1.55s, of which provenance() = 1.29s across 1862 calls, driving 13041 Path.resolve() calls. Instrumented breakdown (1862 each): context.py:831; context.py:843 (safe_resolve(root_path / ".claude") — a constant, recomputed on every cache miss); formats/codex.py:221 (resolves plugin_dir again); formats/codex.py:225+:228 (two contained_resolve calls that run even when .codex-plugin does not exist, because resolve(strict=False) returns a nonexistent path and is_relative_to then passes — 0.58s of the 1.29s); formats/codex.py:322 (a third resolve of the same path).

Only one of the seven is unavoidable. codex_marker_escapes already does the cheap existence probe first; codex_manifest_is_contained is the outlier. The --type amplification comes from _codex_claims_possible() returning not self._codex_discovery_enabled or ... — unconditionally True whenever --type names a non-Codex type, so a repo with zero Codex content pays the full per-directory consult. The default path is correctly gated.

Fix: cache the .claude resolve; short-circuit codex_manifest_is_contained on safe_is_file(...) first (a pure reorder — the body already ANDs that condition at the end); thread the already-computed resolved through instead of re-resolving; and replace the blanket not self._codex_discovery_enabled with a cheap --type-invariant probe.

2. SUGGESTION — context.py:411-421 — a cache built on cleared caches survives them

_codex_claims, _codex_evidence, _provenance_cache, _format_scope_cache clear unconditionally; _codex_roots — derived from both — clears only inside if self.exclude_patterns: and then only when codex_set_changed. Failure scenario: two Linters over one shared context (a mode the code contemplates at context.py:290-292). Linter B adds plugins/secret/** where plugins/secret is claim-only (catalog local source, no marker) — not in codex_plugins so codex_paths_changed is False, catalog file not excluded so codex_catalog_changed is False → _codex_roots keeps the excluded directory, and the Codex-tightened hooks/MCP checks fire against an excluded plugin.

3. SUGGESTION — lint_tree.py:595-597 — seen_roles gains an entry from every _add_block, so on a large marketplace it holds thousands of tuples; the linear scan runs once per file matched by content-paths, making the extra-content pass O(files × blocks). Only bites users who configure content-paths (default empty), which is why the default benchmark misses it. Keep a parallel Set[Path].

NOTES. lint_tree.py:448-450 evaluates marketplace_dir.resolve() inside the per-plugin loop — 445 identical realpath calls on a 445-plugin marketplace. paths.py:126 re-resolves the same constant root on every escapes_root call. context.py:836's getattr(self, "marketplace_entries", {}) is load-bearing on __init__ attribute ordering and invisible at the call site — initialize the attribute before repo_types instead. discovery/codex.py:236,243 use bare parent.is_dir()/item.is_dir() while the module's own docstring promises "pathlib calls that never raise".

Security & Supply Chain Reviewer

1. BLOCKING — src/skillsaw/cli/_helpers.py:59-88 — Codex path-widening silently expands what skillsaw fix writes beyond the path the caller named

_resolve_lint_paths widens <x>/.agents/plugins/<file> two directory levels to <x>, bounded only by Path.home() and the filesystem anchor. Both lint and fix go through it (cli/_fix.py:35, cli/_lint.py:80).

Reproduced — temp workspace with .agents/plugins/marketplace.json, plugins/note-taker/, and an unrelated sibling private-project/skills/helper/SKILL.md:

$ skillsaw lint <ws>/.agents/plugins/marketplace.json
Linting: <ws>                                        # not the named path
  ✗ ERROR (content-embedded-secrets) [private-project/skills/helper/SKILL.md:8]: ...

$ skillsaw fix --suggest <ws>/.agents/plugins/marketplace.json
  ✓ [private-project/skills/helper/SKILL.md] Wrap 1 bare path(s) in markdown link syntax

A file two directories from the named path was rewritten. The same code block's own comment names exactly this hazard as the reason .claude-plugin is deliberately not widened. I confirmed the asymmetry: skillsaw lint <t>/.claude-plugin/plugin.json roots at .claude-plugin/ and finds nothing outside it. The $HOME/anchor bound covers only the documented ~/.agents/plugins/marketplace.json case — ~/work/.agents/plugins/marketplace.json widens to ~/work and rewrites every project under it. The trigger is also looser than intended: resolved.name == "plugins" and resolved.parent.name == ".agents" fires for any file under .agents/plugins/, not just marketplace.json.

Calibration note: this is user-initiated, not an attacker-controlled primitive. Rated blocking because it is an unbounded, silent write-scope expansion that the repo's own stated invariant forbids.

Recommended action. Don't widen at all in the fix path; for lint, bound the .agents/plugins widening to a directory carrying repo-root evidence (.git, a manifest) and restrict the trigger to the marketplace.json filename. The one-level .codex-plugin widening is defensible as-is.

2. SUGGESTION — mcp/valid_json.py:162,171,183,217,225,233,241,249,261,271,281,291; formatters/text.py:60-70 — the safe_display() sweep is incomplete and raw terminal control sequences reach the report

Reproduced. A .mcp.json server name of ev\x1b[31mIL\x07 renders verbatim through both the text and JSON sinks (only line 208, added by this PR, is wrapped). Separately, a filename containing \n ✓ ERROR (fake) [x]: nothing to see\x1b[2K\r is echoed unsanitized into both the path column and command-naming's message — forging report lines and erasing real ones. Pre-existing debt, not a regression, but this PR ships safe_display() as the mechanism and materially widens the untrusted corpus feeding it (.codex/plugins/ vendor installs, the ~180-plugin third-party catalog now in CI). Sanitize once at the formatter boundary rather than chasing per-rule f-strings.

3. SUGGESTION — docs/html_renderer.py:1220-1265, docs/markdown_renderer.py:337-340 — the docs sink doesn't strip the bidi/invisible characters safe_display() strips

diagnostics._CONTROL_CHARS covers U+202A–202E and U+2066–2069; the docs path does not:

_md("Safe plugin ‮detsurt‬")  -> 'Safe plugin ‮detsurt‬'   # passed through
safe_display(same)            -> 'Safe plugin �detsurt�'   # stripped

_UNSAFE_FILENAME_CHARS maps only <0x20 and 0x7F, so a plugin named ‮evil produced an on-disk page named ‮evil.md in the fuzz run. security-invisible-unicode doesn't cover manifest JSON, so nothing else catches it. For a marketplace generating public pages from third-party metadata this is a display-spoofing vector.

4. SUGGESTION — tests/test_banned_apis.py:22-52 — the ratchet freezes filenames, not per-file counts, and this PR slipped a new bare .resolve() past it

AST-counted at 43654b2 vs HEAD: cli/_helpers.py 1 → 2; context.py 14 → 12; lint_tree.py 13 → 11. The new one is Path.home().resolve() at cli/_helpers.py:82 — harmless in itself, but it proves the mechanism. The four files this PR churned most are all grandfathered, so the ratchet covers none of the code that actually moved. Store per-file counts so a file can only shrink.

5. SUGGESTION — docs/extractor.py:389 / docs/html_renderer.py:1225 — the URL scheme allow-list is duplicated with a comment saying the copy is deliberate. The extractor's copy already carries entity-decoding hardening the renderer's does not. Move it to a leaf module, or pin _SAFE_URL_SCHEMES == _MD_SAFE_SCHEMES in a test.

NOTE. tests/fixtures/codex/claim-escape/outside-the-checkout/deploy_key_outside:1 — PEM delimiters with no # notsecret marker; this same branch added exactly that marker to single-plugin/with-secrets/commands/setup.md.

Verified clean — what I actually exercised. Claim-escape containment holds across four variants beyond the shipped fixture (relative marker symlink, absolute marker symlink, real .codex-plugin/ with symlinked plugin.json, whole plugin directory symlinked out) — no external manifest name, filename, or directory listing appeared in any violation or in skillsaw docs. Vendor stand-down holds, drawn in two places (linter.py:519, :653); the destination-only guard is sound because the only rename-producing rule uses with_name(). Docs filename traversal: fuzzed 11 hostile plugin names (../../pwned, C:\pwned4, ....//....//pwned5, con, ., .., 300 chars, bidi) — every page landed inside the output directory. URL allow-list: javascript:, javascript:, tab/newline-split schemes, data:, vbscript:, attribute breakout all neutralized; </script> breakout blocked at html_renderer.py:403-407. safe_display has no ReDoS — ~1.4 ms worst case for a 500-char adversarial value, flat to 1 MB. Codex rules: AST-swept every f-string; every untrusted manifest value routes through safe_display(). Ecosystem gating doesn't weaken security coverage — security-hidden-instructions, security-invisible-unicode, and hooks-dangerous all fire identically on Codex-only content. Supply chain: no new runtime dependencies; every uses: pinned to a 40-char SHA; openai/plugins pinned in both workflows; third-party lints pass --no-custom-rules; rule-impact.yml is pull_request (not pull_request_target) with persist-credentials: false; no subprocess/eval/exec/pickle/yaml.load/network call added anywhere in src/.

QA Engineer

make test at 74176cf: 3505 passed, 0 failed, 631.80s, exit 0. Coverage 93% (13206 statements, 892 missed) — the PR's "94%" is off by a point; the test count matches exactly. New-surface coverage is genuinely high: paths.py 100%, codex/plugin_json_valid.py 100%, codex/_helpers.py 100%, codex/openai_metadata.py 99%, formats/codex.py 97%, context.py 95%, discovery/codex.py 93%. Every finding below was verified by execution. No BLOCKING issues.

1. SUGGESTION — discovery/codex.py:149-153 — the positive duck-typing branch is untested

Line 153 is uncovered in the full run. Only the negatives are tested (invalid JSON, {}). This is the higher-consequence half: confirmed by execution that a file named curated.json with {"plugins": [{"source": ...}]} is discovered as a Codex catalog, flips the repo to codex-marketplace + codex-plugin, and — with no marketplace.json present — becomes config_nodes[0], i.e. the file codex-marketplace-registration --suggest writes into. A ready-to-paste TestDuckTypedSiblingCatalog was supplied and verified passing.

2. SUGGESTION — codex/marketplace_registration.py:394 — marketplace_file = config_nodes[0].path picks the write target positionally with no multi-catalog test. Behavior is correct today (primary first when present, sole sibling otherwise), so this is a pin, not a bug — but a reordering in enumerate_codex_catalogs would silently move where --suggest writes and nothing would fail. test_fix_targets_the_primary_catalog supplied and verified passing.

3. SUGGESTION — agentskills/evals.py:70-78 — the shape violation gained a line number, silently invalidating existing baselines

fingerprint_violation (baseline.py:93-102) hashes rule_id + path + line_content when a line is present and falls back to rule_id + path + message when not. Pre-0.18 this violation had no line, so it hashed on the message; now it hashes on line content. Verified by execution — the two fingerprints differ. The code comment at that site claims the opposite protection ("a message rewording cannot un-suppress old baselines") — true for future rewordings, not this one. A repo with an openai/plugins-style top-level-array evals/evals.json that baselined the 0.17 violation gets an ERROR back on 0.18 with no release-note explanation. The analogous docs-output change was disclosed (053f1bb, docs/ci.md); this one wasn't. Add the docs/ci.md note plus a test pinning that the violation carries a line.

4. SUGGESTION — codex/marketplace_registration.py:342-349 — the guard is if not safe_is_dir(plugin_dir), so a local source pointing at an existing file is reported as source './plugins/x.txt' does not exist. codex-marketplace-json-valid says nothing either, so "does not exist" is the only thing the author sees about a file plainly right there. The only test of this branch uses a genuinely absent path. Test supplied; it fails on current HEAD, confirming the defect.

5. SUGGESTION — cli/_helpers.py:62-89 — the $HOME and filesystem-anchor bounds and the RuntimeError/OSError no-home fallback have no tests. tests/test_path_resolution.py, the dedicated unit-test home for _resolve_lint_paths, was not touched by this PR at all; the only coverage is two happy-path integration tests. Four tests supplied, all verified passing.

6. SUGGESTION — tests/codex/test_provenance.py:849-880 — TestProvenanceScopeMechanism pins scoped_find for CommandBlock and PluginNode. AgentBlock is the third and only other type with a provenance_dir() override and is what agent-frontmatter (a declared provenance_scope = "claude" rule) iterates — no scope test, and no fixture under tests/fixtures/codex/ ships an agents/*.md file at all.

NOTES. blocks/openai.py:41 and blocks/json_config.py:240,261,333 — three tree_label()s and estimate_tokens() uncovered by the entire suite while test_tree_labels was updated for the new node types; skillsaw tree renders all three correctly, so it's pure coverage (4 lines close it). agentskills/structure.py:11 — _HOST_METADATA_DIRS = {"agents"} exempts the directory wholesale regardless of whether it holds openai.yaml; blast radius is small (agentskill-unreferenced-files still catches strays inside it) but the negative case deserves a line.

PR description claims vs. the tree. "3,505 tests" ✅ exact. "94% coverage" — measured 93%. "docs byte-stability … test-pinned" — not accurate: there is no golden/snapshot test of docs output anywhere in tests/, and 053f1bb/docs/ci.md document that the output changed for every repository. "dual-manifest repos keep their exact 0.17 results" — overstated; TestDualManifestBackwardCompat pins that two specific tightened messages don't appear plus the codex-only twin conjunct — a good conjunctive pin, not "exact 0.17 results". "--type invariance test-pinned" ✅ accurate and well covered. "registration autofix appends missing entries" ✅ accurate — additionally verified multi-plugin, root-level "./" source, BOM+CRLF preservation, and idempotency end-to-end through the real CLI.

Prior follow-ups, current state. Plugin-root hooks.json — resolved by documentation; codex-plugin-structure.md names hooks/hooks.json as canonical, so the omission is deliberate and stated. _registerable's outside-root exclusion and the fail-closed branches in formats/codex.py:130,223,246 — still uncovered, low value. String-form author coercion (docs/extractor.py:341,460) — still untested on both paths.

Technical Writer

Prior BLOCKING finding — confirmed fixed. docs/cli.md "Path arguments" now correctly states .claude-plugin/plugin.json is not widened. Verified three ways: rendered docs/cli.md:31-44 is byte-exact against CLI_PATH_SECTION in scripts/generate-site-content.py:564; scripts/generate-docs.py has no CLI section, so no third copy can drift; and the text matches cli/_helpers.py:60-89 and reproduces empirically.

1. SUGGESTION — docs/ci.md:198-205 — the 0.18 note enumerates only HTML changes; Markdown changed too, for every repository

The first sentence is right; everything it enumerates is HTML-only (escJsAttr, escAttr, the author field). A reader generating only --format markdown will conclude it doesn't affect them. Verified on a plain Claude-only marketplace fixture with no Codex content:

**Owner:** Acme \(Inc\)
**Version:** 1.0.0 | **Author:** Jane Doe \(Acme\)
[Homepage](https://x.test/a?b=1&c=2)
**Tags:** `a\|b`

Three distinct Markdown changes: every metadata scalar now folds through _table_cell; _link_dest re-encodes & → &; and per-plugin page filenames can change (_plugin_filename now maps \:<>"|?*, collapses .., appends -plugin to Windows reserved names, hash-truncates over 240 bytes; _unique_filenames case-folds collisions). A filename change is worse than a content change for a committed-docs repo — regeneration adds the new page and orphans the old one, which a "did the tree change" CI gate won't flag as stale.

2. SUGGESTION — docs/markdown_renderer.py:186, :285, :317 — _table_cell escapes leak visibly inside code spans

_table_cell backslash-escapes \ | [ ] ( ) — correct for GFM table cells. But three sinks wrap its output in a code span outside any table, where CommonMark treats the backslash as literal: **Tags:** …, `**Matcher:** `…, **Paths:** …. A `"matcher": "Edit|Write"` — a common hook matcher — now renders as Edit\|Write `` on every plugin page. New regression from 094c4ec, hits ordinary existing Claude repos. Split the escaper: keep `_table_cell` for table cells, add a code-span-safe variant for the three non-table sinks.

3. SUGGESTION — scripts/generate-site-content.py:100 and scripts/generate-docs.py:65 (rendered docs/rules/codex.md:6) — the blurb says the metadata rule "auto-enables for Agent Skills", but CodexOpenAIMetadataRule.repo_types = SKILL_REPO_TYPES = {agentskills, single-plugin, marketplace, dot-claude, codex-plugin, codex-marketplace} — an error-severity rule auto-enabling in every existing Claude plugin, marketplace, and .claude/ repo. The rule's own page gets this right; only the category blurb, the reader's first contact, is narrow. Both generators carry duplicate copies.

4. SUGGESTION — docs/pre-commit.md:39-51 — .pre-commit-hooks.yaml replaced a 20-branch files regex with files: . in this PR. Anyone bumping rev: to v0.18.0 gets a full-repository lint on every commit where previously only matched paths triggered it — slower commits, and pre-existing violations in untouched files now block commits that would have skipped. The doc describes the new behavior accurately but reads as if it were always this way; docs/ci.md and docs/autofixing.md both use the !!! note "Changed in N" idiom for exactly this.

5. SUGGESTION — .apm/instructions/development.instructions.md:186-199 (+2 mirrors) — two issues. Style: the "Adding an ecosystem" recipe is a ~14-line semicolon-chained paragraph in a file whose every other section is a tight - **Bold lead.** bullet list. Completeness: it covers src/ placement thoroughly but says nothing about tests, while this PR established a new convention (a per-ecosystem package at tests/codex/, 14 modules plus _helpers.py) — and DEVELOPMENT.md:115 still says "Add tests in tests/test_<category>.py" while .claude/rules/testing.md doesn't mention per-ecosystem packages. The repo now has both tests/codex/ and a top-level tests/test_codex_marketplace.py, which doesn't help.

NOTES. codex-plugin-json-valid.md:55 — "author must be a string or an object (an object should carry a name)" reads as a documented check, but plugin_json_valid.py:133-140 never reports an object without name. codex-plugin-structure.md:34 — "Move the reported file to the plugin root — hooks/hooks.json for lifecycle hooks" is self-contradictory (hooks/hooks.json is a subdirectory). generate-site-content.py:273-275 — the Autofix column reads auto for SUGGEST-confidence rules; pre-existing, affects 5 rules including codex-marketplace-registration.

Verified clean. All five codex-* rules have hand-written doc sources, render correctly, are grouped in both generators, appear in mkdocs.yml nav, are counted correctly in docs/rules/index.md (68 total; category counts sum to 68), and resolve via skillsaw explain; all four config_schema params appear in both the rule pages and .skillsaw.yaml.example. No doc renames or deletions — only additions, so no redirect entry is needed. docs/repo-types.md's Codex sections match the code (discovery probe set vs discovery/codex.py:164-235, the .codex/plugins/* stand-down table vs the linter-level fix guard, sibling-catalog rules, skillsaw docs field list). Behavior-change docs on shared rules are accurate. Claude-format exemption claims reproduced end-to-end on a mixed fixture. Version references consistently 0.18.0; "more than 60 rules" accurate at 68. Instruction/skill mirrors byte-identical across .apm/, .claude/, .agents/, .github/instructions/ — make update would be a no-op. Repo self-lint: 0 errors, 0 warnings, A+. Bonus: the old agentskill-evals "Good" example would have failed the rule it documents; the new one matches evals.py:118-140.

Ecosystem Reviewer

Verdict: IN SCOPE FOR CORE — no blocking objection. I looked hard for a redirect-to-plugin case and the evidence runs decisively the other way. Two independent lines each suffice on their own.

Adoption evidence (verified independently; as of 2026-07-28). Cloned openai/plugins at the CI-pinned 11c74d6 (committed 2026-07-13): 180 top-level plugin directories, 182 .codex-plugin/plugin.json manifests, 607 SKILL.md files, with both .agents/plugins/marketplace.json and api_marketplace.json present. Under the official /openai org: 4.8k stars, 646 forks, 299 commits. The PR's "~180 plugins / ~580 skills" is accurate. Vendor-backed and ascending: the marketplace launched 2026-03-27 with 20+ launch-day partners (Slack, Figma, Notion, Sentry), 90+ plugins by April 2026, ~4M weekly Codex actives cited mid-2026, enterprise deployments at Cisco, Nvidia, Ramp, Rakuten, Harvey. A third-party ecosystem exists independently of OpenAI: coderabbitai/codex-plugin (notably — CodeRabbit is a vendor skillsaw already supports in core), duyet/codex-claude-plugins, openai/role-specific-plugins, hashgraph-online/awesome-codex-plugins, plus an independent registry at codex-marketplace.com.

Comparative bar. Claude Code's official built-in marketplace held ~101 plugins as of March 2026; openai/plugins holds 180 as of July 2026. skillsaw is being asked to add a format whose flagship catalog is larger than one it already carries in core. Core already ships rule packages for openclaw, promptfoo, apm, and coderabbit — an OpenAI first-party format is not narrower than any of these. The repo's own stated policy governs and points to core (docs/plugins.md:14-19: "For common, popular formats it is recommended to contribute repository types, rules, and tree support back to skillsaw itself… Plugins are the right home for private/organization-specific conventions"). Carrying Anthropic's single-vendor plugin format in core while redirecting OpenAI's would be an incoherent boundary.

The structural argument is decisive on its own. The plugin surface is exactly three hooks (SKILLSAW_RULES, SKILLSAW_REPO_TYPES, SKILLSAW_TREE_CONTRIBUTORS). The five new codex-* rules would fit a plugin — but they aren't where this PR's value is. Roughly 2,100 lines change 23 existing core rules so they behave correctly in the presence of Codex content: Rule.provenance_scope + RepositoryContext.in_format_scope (a new core primitive) with eight existing Claude rules declaring provenance_scope = "claude"; in_codex_only_plugin conditional strictness in hooks/json_valid.py and mcp/valid_json.py; nine agentskills/* files; docs/extractor.py, both renderers, and all four formatters. None of that is reachable from a plugin. A plugin can add rules; it cannot make core's existing Claude rules stop firing on a .codex-plugin/ directory. A redirect would ship the Codex rules plus a permanent, unsuppressable false-positive tail from core. That's a strictly worse product, not a smaller core. I also verified the CI gate is a genuine empirical conformance check, not a claim: .github/workflows/test.yml:276-300 clones the corpus at the pin, runs exactly the 5 codex-* rules, and asserts violations == [] plus codex-marketplace in detected types.

1. SUGGESTION — docs/repo-types.md:152, discovery/codex.py:28 — skillsaw emits contradictory false positives on a marketplace path OpenAI's own spec sanctions

I fetched developers.openai.com/codex/plugins/build.md: Codex reads "a legacy-compatible marketplace at $REPO_ROOT/.claude-plugin/marketplace.json". skillsaw deliberately leaves that path to the Claude marketplace-* rules, and docs/repo-types.md:152 discloses the consequence — a Codex-schema catalog there "will be checked against the Claude schema — a missing owner, and an unknown local source type on every entry." So an author following the vendor's documentation gets an error on every single entry, and the documented remedy has the linter overriding the vendor's compatibility path. The disclosure is honest and schema disambiguation is genuinely hard, so this isn't blocking — but it's the one place where core Codex support makes a Codex user's life worse than no support at all. Sniff for Codex-exclusive markers (policy, category, interface, or any source.type == "local") and emit one "this looks like a Codex catalog at the legacy path" violation instead of N contradictory schema errors. If deferred, file it as an issue rather than leaving it in prose.

2. SUGGESTION — .apm/skills/skillsaw-maintenance/references/codex.md:134-137, test.yml:281, rule-impact.yml:88 — the prior panel's pin-staleness follow-up is still unclosed. The pin is currently fresh — 11c74d6 is origin/main HEAD of openai/plugins, 0 commits behind today — but nothing schedules the bump. The maintenance SKILL.md contains no mention of CI, workflows, or the regression check, and the reference's "Regression check" section never says CI pins a SHA in two files that must advance together, so a partial bump is possible. On a format still on OpenAI's active roadmap this guarantees the conformance gate decays into testing a frozen July-2026 snapshot.

Slopinator Reviewer

Prior finding verified fixed. benchmarks/genrepo.py:20-21 now reads # child — the shape that stresses claim-set memoization. — the unverifiable "discovery cost was once quadratic" claim is gone (74176cf). Clean.

Checked: every added # comment in src/, benchmarks/, scripts/ (~230 lines, read in full); every added comment and docstring in tests/ (~130); all added docstring first-lines in src/ (~120); all added prose in README.md, docs/, THREAT_MODEL.md, DEVELOPMENT.md, AGENTS.md, the five new rule doc pages, and the shipped maintenance skill reference; all 895 commit subjects; every added class Test* name (~80); grep sweeps for the full Part A phrase set and Part B vocabulary set, plus TODO/FIXME/HACK.

1. SUGGESTION — THREAT_MODEL.md:105 — a within-branch correction logged as if it were between-release

- updated: 2026-07-28 — T15 evidence corrected: hook event names, hook matchers, and rule globs reached the Markdown renderer raw, so the accepted risk was wider than descriptionprose. All three now fold through_table_cell(); the accepted risk is again description fields alone.

T15 itself is added by this same PR one line above (:104, dated 2026-07-27). All three entries land on main simultaneously, so a reader sees an entry correcting evidence that was never published — "reached the Markdown renderer raw" describes a skillsaw state that existed only inside this branch, and anyone hunting for the release where hook matchers rendered raw won't find one. The provenance log's genre (a dated changelog) normally exempts it, which is why this is SUGGESTION not BLOCKING. Fold :104-105 into the single state that ships.

2. NOTE — squash-merge commit message (not a tracked file) — the branch's subjects are heavily round-keyed (Address review round ten, Address Codex review round thirteen, Fix the fifth panel's blockers, Close the second panel's findings and the round-fifteen gaps). Not a defect as things stand: main is fully linear over its last 200 commits (zero multi-parent commits; subjects carry the (#NNN) squash suffix), so all 895 subjects collapse into one squash message and no round number reaches main. Just ensure the squash body describes the feature rather than concatenating the round-numbered subjects.

No findings on — the code comments are the strongest part of this branch; the three prior cleanup passes appear thorough and I could not find a fourth pass's worth of residue. Deliberately not flagged: markdown_renderer.py:28-30 and :63-66 (present-tense counterfactuals about the code as shipped, verifiable by reading the adjacent branch — the correct form of the pattern); tests/codex/test_docs_output_safety.py:342-343 (a ratio derived from algorithmic shape, not a frozen wall-clock measurement); context.py:579-582 (a backward-compat claim checkable against main); the maintenance skill's ON_FIRST_USE note (a source-reliability warning, checkable upstream — passes the stranger test). Comment density in the new Codex modules runs 10-17%, high in absolute terms, but the content is spec citation and invariant rationale rather than line restatement. Zero AI-vocabulary hits across the entire diff — no delve, crucial, seamless, leverage, comprehensive, serves as, boasts, negative parallelism, or signposting, in markdown or in Python. Zero round-numbered test classes; all ~80 new Test* names describe scenarios.

---

Panel Synthesis

All seven specialists completed. This is the fourth panel on this branch, and the two BLOCKING findings from the third (docs/cli.md path-widening prose, benchmarks/genrepo.py phantom-history comment) are confirmed fixed by fa3172e and 74176cf — verified independently, not taken on trust.

The change continues to hold up unusually well for its size. The Ecosystem Reviewer's scope verdict is unambiguous and well-evidenced: Codex belongs in core, and the structural argument settles it independently of adoption numbers — ~2,100 lines here modify existing core rules so they stand down on Codex content, and no plugin hook can do that. The security surface was probed hard and empirically: claim-escape containment held across four symlink variants beyond the shipped fixture, docs filename traversal survived 11 hostile plugin names, the URL allow-list neutralized every scheme-obfuscation attempt, safe_display has no ReDoS, and the supply-chain posture is clean (no new runtime deps, SHA-pinned actions, pull_request not pull_request_target, third-party lints run --no-custom-rules). The Slopinator found the prose essentially clean after three cleanup passes, with zero AI-vocabulary hits across 16k insertions.

Two BLOCKING findings survived arbitration. I reproduced both myself rather than relying on the specialists' write-ups.

On the plugins/ containment regression (context.py:1177). Reproduced exactly as reported. A Claude-only repo with plugins/foo -> ../../outside/foo:

• 43654b2: Plugins: 1, 2 errors + 6 warnings, Grade C+
• this branch: Plugins: 0, 1 violation, Grade B

An entire plugin — its manifest, its commands, its naming and frontmatter — reaches no rule at all, and the grade improves, which is the worst possible signal. Symlinked plugin directories are a normal monorepo layout, and this repo has no Codex content anywhere, so it falls outside the "dual-manifest repos keep exact 0.17 results" guarantee. .claude/rules/development.md states "Never break existing rules for users of skillsaw", and the arbiter bias is explicit that backward compatibility is paramount. The containment itself is right — autofix must not write outside the checkout — so the fix is to make the drop visible, not to revert it. The sibling _resolve_plugin_source path already logs exactly the warning that's needed; this is a consistency gap, not a design error.

On the fix write-scope expansion (cli/_helpers.py:59-88). Reproduced end-to-end. Naming <ws>/.agents/plugins/marketplace.json — one file — caused skillsaw fix --suggest to rewrite private-project/skills/helper/SKILL.md, two directories away in an unrelated project. What makes this blocking rather than a rough edge is that the code block's own comment states this exact invariant as the reason .claude-plugin is deliberately not widened, and fa3172e — a commit on this branch, landed hours ago in response to the previous panel — went out of its way to document that invariant in docs/cli.md. The Codex path violates the rule the Claude path was just documented as obeying. The $HOME bound doesn't help: ~/work/.agents/plugins/marketplace.json widens to ~/work. The trigger is also broader than intended — any file under .agents/plugins/, not just marketplace.json.

Neither blocker is a design flaw; both are narrow and in scope. Together they're a coherent theme worth naming: the containment work correctly identified that path handling needed bounds, and got the Codex side's bounds slightly wrong in both directions — too tight in discovery (silently dropping content), too loose in the CLI (silently widening writes).

Filtered out. Several specialists re-raised open follow-ups from prior panels (_codex_roots invalidation placement, marketplace/json_valid.py's ownership probe, the .resolve() ratchet's filename-vs-count design, the duplicated URL allow-list, build_lint_tree's size). All are real and all remain optional — they're carried below rather than re-argued. Where a specialist added new evidence to an old finding (Architecture's concrete provenance() divergence on .claude; Security's AST count showing cli/_helpers.py went 1 → 2 bare resolves) I kept the sharper version.

Corroboration. The _codex_roots invalidation gap was found independently by Architecture and the Python Expert, with different failure scenarios — that strengthens it, though both agree it's latent today. The incomplete safe_display() sweep and the ratchet's filename weakness were each raised by two specialists.

On the PR description. QA verified the test count exactly (3505) but measured 93% coverage, not 94%, and found two claims overstated: "docs byte-stability … test-pinned" is not true (there is no golden/snapshot test of docs output anywhere in tests/), and "dual-manifest repos keep their exact 0.17 results" is a good conjunctive pin on two specific messages, not an exactness guarantee. Worth correcting in the body — the surrounding claims are accurate and the description shouldn't be undermined by three that aren't.

One item I'd weight above the other optional work: the agentskill-evals baseline invalidation (QA #3). It's rated SUGGESTION and I've left it there, but it is a silent CI break on upgrade for exactly the openai/plugins-shaped repos this PR targets, and the in-code comment at that site currently asserts the opposite protection. The project already set the precedent for disclosing this class of change in docs/ci.md (053f1bb); this one just needs the same treatment.

Disposition is REQUEST_CHANGES on the strength of the two reproduced BLOCKING items. Both are contained fixes — a log line plus a violation, and a narrowed widening rule. Everything else is optional.

---

Required Actions Before Merge

1. src/skillsaw/context.py:1177 — make the dropped plugin visible. _discover_from_plugins_dir now silently discards any plugins/* directory whose contained_resolve fails, taking a Claude-only repo from Grade C+ / 8 violations to Grade B / 1 (reproduced against 43654b2). Keep the containment; emit the same logger.warning that _resolve_plugin_source already emits for escaping marketplace sources, and file a violation so the drop appears in --format json/SARIF. Add a fixture-backed test for a symlinked plugins/* child in a Claude repo — tests/codex/test_containment.py covers only the Codex catalog path. Rationale: a plugin losing all rule coverage while the grade improves is a silent backward-compatibility break, and .claude/rules/development.md treats breaking existing users as never acceptable.

2. src/skillsaw/cli/_helpers.py:59-88 — stop fix writing outside the named path. Naming <ws>/.agents/plugins/marketplace.json caused skillsaw fix --suggest to rewrite private-project/skills/helper/SKILL.md two directories away (reproduced). Don't widen in the fix path at all; for lint, require repo-root evidence (.git, a manifest) at the widening target and restrict the trigger to the marketplace.json filename rather than any file under .agents/plugins/. The one-level .codex-plugin widening is fine as-is. Add tests for the $HOME bound, the anchor bound, and the no-home RuntimeError/OSError fallback in tests/test_path_resolution.py (QA supplied four, verified passing). Rationale: this violates the invariant the same code block's comment states, and that fa3172e documented in docs/cli.md for the Claude path, hours ago, in response to the previous panel.

---

Optional Follow-ups

Highest value first:

1. Disclose the agentskill-evals baseline invalidation — the shape violation gained a line number, moving its fingerprint from message-hash to line-content-hash (verified: the two differ), so existing baselines silently stop suppressing it. Add a "Changed in 0.18" note to docs/ci.md beside the docs-output one, and pin the transition with a test that the violation carries a line. The comment at agentskills/evals.py:70-78 currently claims the opposite protection.
2. Take provenance() off 7 resolves per directory — --type runs measured ~2.2x slower on an 1800-directory repo (1.29s of 1.55s inside provenance(), 13041 resolve() calls). Cache safe_resolve(root_path / ".claude"); short-circuit codex_manifest_is_contained on the safe_is_file check first (a pure reorder that drops 3 of 7); thread the computed resolved through instead of re-resolving; and give _codex_claims_possible() a cheap --type-invariant probe instead of not self._codex_discovery_enabled.
3. Fix the three Markdown regressions on existing Claude repos — _table_cell output inside code spans renders visible backslashes (`Edit\|Write` for a common hook matcher) at markdown_renderer.py:186, 285, 317; and extend the docs/ci.md 0.18 note to name the Markdown changes and the fact that per-plugin page filenames can change, orphaning old pages that a tree-diff CI gate won't flag.
4. Complete the safe_display() sweep — mcp/valid_json.py (12 sites) and the text formatter's file_path column; a crafted filename forges report lines and erases real ones today. Sanitize once at the formatter boundary. Add the bidi/invisible ranges to the docs sink (html_renderer.py, markdown_renderer.py:338) — _CONTROL_CHARS strips them for diagnostics but the docs path passes them through, including into on-disk filenames.
5. Unify the invalidation surfaces — move _codex_roots = None to context.py:419 (found independently by two specialists), and give rebuild_lint_tree() and apply_excludes() one _invalidate_derived() helper so the multi-pass autofix loop can't reason over pre-fix provenance.
6. Fix the .resolve() ratchet's own escape hatch — docs/extractor.py:281 is new code in this PR using a bare .resolve() inside a grandfathered file, two lines from a safe_resolve doing the same job; cli/_helpers.py went 1 → 2 bare calls. Freeze per-file counts rather than filenames so the four files this PR churned most are actually covered.
7. Read Claude ownership from provenance() in marketplace/json_valid.py:55 — the last rule-level filesystem probe, and the two spellings already disagree on a DOT_CLAUDE repo whose .claude/ is catalog-claimed.
8. Close the QA coverage gaps — the positive duck-typed-catalog branch (discovery/codex.py:153, uncovered, and it selects the autofix write target); test_fix_targets_the_primary_catalog for the positional config_nodes[0]; an AgentBlock scope test (the third provenance_dir() override, with no Codex fixture shipping agents/*.md at all); the three uncovered tree_label()s. All supplied ready-to-paste and verified passing.
9. Split the "does not exist" message — marketplace_registration.py:342-349 reports a local source pointing at an existing file as missing, and nothing else says otherwise.

Documentation:

10. Fold THREAT_MODEL.md:104-105 into the single state that ships — the correction entry describes a raw-Markdown state that existed only inside this branch.
11. Reword the docs/rules/codex.md:6 blurb in both generators — codex-openai-metadata auto-enables on all SKILL_REPO_TYPES, including every Claude plugin/marketplace/.claude/ repo.
12. Add a !!! note "Changed in 0.18" to docs/pre-commit.md for the .pre-commit-hooks.yaml files: . broadening — every commit now triggers a full-repo lint on rev: bump.
13. Restructure the "Adding an ecosystem" recipe (.apm/instructions/development.instructions.md:186-199) into the file's bullet style, and add the test-layout half — DEVELOPMENT.md:115 and .claude/rules/testing.md still point at tests/test_<category>.py while this PR established tests/codex/. Then make update for the mirrors.
14. Correct three PR-body claims: coverage is 93% not 94%; docs byte-stability is not test-pinned (no golden test exists); "exact 0.17 results" for dual-manifest is a two-message conjunctive pin, not exactness.
15. Minor: the author parenthetical at codex-plugin-json-valid.md:55 documents a check that isn't implemented; the hooks/hooks.json-is-not-the-root contradiction at codex-plugin-structure.md:34.

Code quality:

16. Extract _build_plugin_nodes / _build_root_blocks from build_lint_tree — now the only function in the codebase over 100 lines, at 575 (tracked in Break up context.py: finish the discovery/orchestrator decomposition #470) — and add a public codex_claimed_dirs() so lint_tree.py:320 stops reaching into context._codex_claim_set().
17. De-duplicate the URL scheme allow-list (extractor.py:389 / html_renderer.py:1225) — the extractor's copy already carries entity-decoding hardening the renderer's lacks, so it has already drifted. At minimum pin equality in a test.
18. O(1)-ify the remaining scans: seen_roles linear scan (lint_tree.py:595), marketplace_dir.resolve() hoisted out of the per-plugin loop (:448), root.resolve() in paths.py:126.
19. Initialize marketplace_entries before repo_types in __init__ so context.py:836's defensive getattr and its ordering invariant can go; use safe_is_dir at discovery/codex.py:236, 243 to match the module's stated never-raises contract.
20. Add # notsecret to tests/fixtures/codex/claim-escape/outside-the-checkout/deploy_key_outside — this branch added exactly that marker to single-plugin/with-secrets/commands/setup.md.

Separate issues (not this PR):

21. Codex catalogs at the legacy .claude-plugin/marketplace.json path — OpenAI's own spec sanctions that location, and skillsaw currently errors on every entry against the Claude schema. Sniff for Codex markers (policy, category, source.type == "local") and emit one "move it to .agents/plugins/marketplace.json" violation instead of N contradictory ones. File as an issue if not fixed here.
22. Schedule the openai/plugins pin bump — the pin is fresh today (11c74d6 is upstream HEAD, 0 commits behind), but nothing schedules the advance, and it's duplicated across test.yml:281 and rule-impact.yml:88 so a partial bump is possible. Extend the maintenance skill's "Regression check" section to name both sites.
23. Rule-doc Autofix column renders auto for SUGGEST-confidence rules (generate-site-content.py:273-275) — pre-existing, affects 5 rules including codex-marketplace-registration.
24. Squash-merge hygiene — main is fully linear, so all 895 round-numbered subjects collapse into one message; just ensure the squash body describes the feature rather than concatenating them.

---

_{Generated by skillsaw-review-panel}

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: df339706f9

ℹ️ 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".

[chatgpt-codex-connector]

Reject credentialed generated-docs URLs

When a Codex plugin manifest accidentally sets homepage or repository to an HTTP(S) URL with userinfo, such as https://user:token@example.com/repo, _safe_url() returns it unchanged because the scheme is allowed. The Markdown and HTML renderers then embed that exact value in generated docs, so publishing skillsaw docs output leaks the credential in links and page source; reject or strip username/password before returning accepted URLs.

Useful? React with 👍 / 👎.

[chatgpt-codex-connector]

Report escaping evals files as missing

For a Codex-owned skill where evals/evals.json is a symlink that resolves outside the owning plugin, contained_eval_file() correctly refuses to read it, but this branch then treats the lexical file's existence as a reason to emit no violation at all. That leaves an evals/ directory with no bundled usable evals.json passing the default agentskill-evals check; report this as missing or as a containment violation instead of continuing silently.

Useful? React with 👍 / 👎.