feat(review): codex runs real skills — on-disk $name discovery + native invocation - #1669
Merged
Conversation
expandCodexBuiltinReview silently REPLACED a configured /review skill with a generic 28-word instruction before composing the prompt — the configured skill never ran. This is the codex sibling of the claude -p slash-expansion bug: in both cases the child executed something other than what the user configured. Rewrite slash-form skill invocations (the agent-portable form profiles are configured with) into codex's native $name form, which codex's skill system resolves against its installed-skill catalog and loads the matching SKILL.md. Non-slash entries (plain instruction text) pass verbatim; PromptOverride is untouched. Extracted from PR #1370 (codex review correctness), which is closed in favor of this narrower fix: its live-token tailing remains queued as a separate follow-up, its per-spawn reasoning_effort was dropped by product decision (entire does not alter how skills run), and its skilldiscovery refactor is superseded. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Entire-Checkpoint: 281a46a17694
Codex's DiscoverReviewSkills was a Phase-1 stub returning nil, while spawn-time validation (VerifyConfiguredSkillsInstalled) rejects any configured skill that is neither a curated builtin nor discovered — so every on-disk codex skill configured in $name form hard-failed with 'configured review skill(s) not installed'. This repo's own saved $code-reviewer config hit it on every run. Ports the discovery slice from closed #1370 (applies verbatim — the touched files did not diverge and no open PR overlaps them): - skilldiscovery gains the generic SKILL.md scanners (skills dir, versioned plugin cache with semver pick, frontmatter parse, dedupe) parameterized by invocation form (SlashForm/DollarForm). - claude-code's discovery delegates to the shared scanners; its own discovery tests pass unchanged, pinning identical behavior. - codex discovers ~/.codex/{skills,plugins/cache,superpowers} in $name / $plugin:name form — the literal token codex's skill system resolves. - codex's curated '/review' builtin is removed: the interactive TUI slash command never fires through codex exec, so validating it was a lie. Configs still naming '/review' for codex now fail validation with the reconfigure hint (honest — that path never invoked a real skill); on-disk skills like $code-reviewer validate and run. Verified against the real ~/.codex on this machine: $code-reviewer, $review-swarm, $superpowers:* et al discovered correctly. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Entire-Checkpoint: 01KWYW70EFC8JERS2Q1FRNFXKY
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 2 potential issues.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Comment @cursor review or bugbot run to trigger another review on this PR
Reviewed by Cursor Bugbot for commit ed5f528. Configure here.
Contributor
There was a problem hiding this comment.
Pull request overview
Implements Codex on-disk review skill discovery so $name / $plugin:name skills can pass spawn-time validation, while refactoring Claude Code discovery to share the same generic scanners in skilldiscovery.
Changes:
- Add shared
skilldiscoveryscanners (SKILL.md + flat markdown scanning, plugin-cache version selection, frontmatter parsing, invocation dedupe) parameterized by invocation form (SlashForm/DollarForm). - Implement Codex
DiscoverReviewSkillsto scan~/.codex/{skills,plugins/cache,superpowers}and emit$...invocation tokens; remove Codex curated/reviewbuiltin. - Update tests/docs to reflect Codex’s on-disk
$skillbehavior (including seeding a SKILL.md in tests under a controlledHOME).
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| cmd/entire/cli/review/cmd_test.go | Seeds a real Codex SKILL.md under a controlled HOME and updates expectations to $code-review. |
| cmd/entire/cli/agent/skilldiscovery/scan.go | New shared skill scanning utilities (plugin cache walking, SKILL.md scanning, frontmatter parsing, dedupe, invocation forms). |
| cmd/entire/cli/agent/skilldiscovery/registry.go | Removes Codex curated builtins; adjusts install-hint messaging to avoid backticks. |
| cmd/entire/cli/agent/skilldiscovery/registry_test.go | Updates registry expectations for Codex (no curated built-ins). |
| cmd/entire/cli/agent/codex/discovery.go | Replaces Phase-1 stub with real Codex on-disk discovery emitting $... invocations. |
| cmd/entire/cli/agent/codex/discovery_test.go | Adds test coverage for user/plugin/superpowers skill discovery and filtering behavior. |
| cmd/entire/cli/agent/codex/AGENT.md | Documents review integration specifics for codex exec and feature-flag rename to codex_hooks. |
| cmd/entire/cli/agent/claudecode/discovery.go | Refactors Claude Code discovery to delegate to shared skilldiscovery scanners. |
…on' into feat/codex-skill-discovery
peyton-alt
changed the title
…trail findings) Trail findings on this branch: - Removing codex's curated /review orphaned configs the old picker auto-preselected, and in the multi-agent path one worker's failed skill validation returned a SilentError that aborted the ENTIRE review — claude-code and all — until the user reconfigured codex. Validation failure now excludes just that worker with a loud 'skipping reviewer <name>: ...' warning and the rest of the crew runs; the run still hard-fails when every worker is excluded, so a fully-stale profile can't silently review with nobody. - The codex install hint's suppression fingerprint was slash-form (/codex:adversarial-review) while codex discovery emits $-form invocations; the exact-match comparison could never intersect, so the 'install codex-review-pack' hint showed forever even with the plugin installed. Fingerprint updated to $-form. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Entire-Checkpoint: 01KX0QBFH472YTJMEF7SXTA46S
Discovery walked ~/.codex via UserHomeDir while every other codex-agent path resolves through resolveCodexHome, which honors CODEX_HOME — so skills installed under a custom codex home were invisible to discovery and spawn-time validation rejected saved $skills as 'not installed' even though codex itself finds and runs them. Discovery now uses the canonical resolution; the fake-home test helper also pins CODEX_HOME empty so a dev shell's value can't leak into the hermetic tests. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Entire-Checkpoint: 01KX0Y35F2WQNBG14701C9WNWF
computermode
approved these changes
Jul 13, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
https://entire.io/gh/entireio/cli/trails/791
Combines the two halves of the codex-skills story (zero file overlap, verified together in a live full-crew run):
Discovery + validation (was this PR)
Codex's
DiscoverReviewSkillswas a Phase-1 stub returning nil, while spawn-time validation rejects any configured skill that is neither curated nor discovered — so every on-disk codex skill configured in$nameform hard-failed ("configured review skill(s) not installed"). This repo's own saved$code-reviewerconfig hit it on every run.skilldiscoverygains the generic SKILL.md scanners (user skills dir, versioned plugin cache with semver pick, frontmatter parse, invocation dedupe), parameterized by invocation form (SlashForm/DollarForm). Ported verbatim from closed feat(review): codex review correctness (PR 2 of 3) #1370.~/.codex/{skills, plugins/cache, superpowers}in$name/$plugin:nameform — the literal token codex's skill system resolves to the real SKILL.md./reviewbuiltin is removed: the interactive TUI slash command never fires throughcodex exec, so validating it was a lie. Configs still naming/reviewfor codex now fail validation with the reconfigure hint; on-disk skills validate and run.Native invocation (was #1655)
Configured skills reach codex verbatim — no paraphrase. An older version replaced
/reviewwith 28 words of generic instruction, which obscured the skill signal; now the composed prompt names the skill exactly as the user typed it ($code-reviewer), and codex's skill system loads the real SKILL.md.Together: discovery makes
$nameconfigs valid, invocation makes them run for real.Verification
~/.codex:$code-reviewer,$review-swarm,$superpowers:*et al discovered in correct form.entire reviewrun on the integrated stack spawned codex from the real saved$code-reviewerconfig (the exact config that previously hard-failed) and completed cleanly.Replaces #1655 (folded in unchanged). Found during #1666's live dogfood runs.
🤖 Generated with Claude Code