feat(codex): make Codex skills-only and retire managed custom prompts#1283
Open
showms wants to merge 2 commits into
Open
feat(codex): make Codex skills-only and retire managed custom prompts#1283showms wants to merge 2 commits into
showms wants to merge 2 commits into
Conversation
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughCodex is changed to a skills-only surface. The Codex adapter is removed, generation logic now uses capability-aware decisions, and managed global Codex prompt files are detected and cleaned up through ChangesCodex Skills-Only Conversion
Sequence Diagram(s)sequenceDiagram
participant CLI as openspec init/update
participant Surface as command-surface.ts
participant Registry as CommandAdapterRegistry
participant Runtime as init.ts / update.ts
participant Cleanup as legacy-cleanup.ts
participant FS as File System
CLI->>Surface: resolveCommandSurfaceCapability('codex')
Surface->>Registry: has('codex')
Registry-->>Surface: false
Surface-->>Runtime: skills-invocable
Runtime->>FS: write .codex/skills/openspec-*/SKILL.md
Runtime->>FS: skip $CODEX_HOME/prompts/opsx-*.md generation
CLI->>Cleanup: detectLegacyArtifacts()
Cleanup->>FS: scan getCodexPromptDir() for managed prompt files
FS-->>Cleanup: matched global prompt files
Cleanup->>FS: unlink managed global prompt files
Cleanup-->>CLI: cleanup summary with replacement label
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related issues
Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Author
|
One additional note for reviewers: the Windows interactive input bug after legacy cleanup still appears to exist. I can still reproduce the case where, after running I previously opened #1175 for this issue: That PR isolates the problem to the welcome-screen/input lifecycle after cleanup and switches the Enter wait flow to Inquirer-managed input handling. I’m mentioning it here because this Codex PR touches nearby init/update/legacy-cleanup behavior, but this known Windows input bug is a separate issue and may still need to be addressed independently. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
openspec/changes/make-codex-skills-only/specs/ai-tool-paths/spec.md (1)
34-34: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueFix hyphenation: "Codex-managed legacy prompt file names."
"Codex managed" is a compound modifier and should be hyphenated: "Codex-managed legacy prompt file names use prefix-based glob patterns."
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@openspec/changes/make-codex-skills-only/specs/ai-tool-paths/spec.md` at line 34, The requirement title in the spec uses incorrect hyphenation; update the text for the Codex-managed legacy prompt file names requirement to use the compound modifier form. In the relevant spec entry, adjust the wording around the requirement heading so it reads with a hyphenated “Codex-managed” while keeping the rest of the phrase about legacy prompt file names and prefix-based glob patterns unchanged.src/core/update.ts (1)
238-240: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueNo-op
else ifbranch; no skills-invocable messaging parity withinit.ts.This branch only contains a comment and performs no action.
init.tstracksskillsInvocableCommandSkipsand emits aCommands skipped for: … (uses skills)line, but the update flow stays silent here. Consider either dropping this empty branch or emitting an equivalent notice so Codex users see that commands were intentionally skipped in favor of skills (task 4.4).🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/core/update.ts` around lines 238 - 240, The `update.ts` command-surface handling has an empty `else if` branch for `resolveCommandSurfaceCapability(tool.value) === 'skills-invocable'`, so it silently does nothing while `init.ts` reports skipped commands. Either remove the no-op branch or add the same skills-skipped tracking/message behavior used by `init.ts` (for example, the `skillsInvocableCommandSkips`/“Commands skipped for … (uses skills)” flow) so `update` gives parity and users see the intentional skip.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/core/legacy-cleanup.ts`:
- Around line 495-504: The unlink loop in cleanupLegacyArtifacts() is deleting
every entry from detection.globalSlashCommandFiles without re-checking that each
path is still a managed global legacy path. Before calling fs.unlink for each
filePath, validate it against getManagedGlobalLegacyPattern() and only proceed
when the entry matches the managed pattern; otherwise record an error and skip
deletion. Keep the change localized to the global slash command cleanup branch
and preserve the existing result.deletedFiles/result.errors behavior.
---
Nitpick comments:
In `@openspec/changes/make-codex-skills-only/specs/ai-tool-paths/spec.md`:
- Line 34: The requirement title in the spec uses incorrect hyphenation; update
the text for the Codex-managed legacy prompt file names requirement to use the
compound modifier form. In the relevant spec entry, adjust the wording around
the requirement heading so it reads with a hyphenated “Codex-managed” while
keeping the rest of the phrase about legacy prompt file names and prefix-based
glob patterns unchanged.
In `@src/core/update.ts`:
- Around line 238-240: The `update.ts` command-surface handling has an empty
`else if` branch for `resolveCommandSurfaceCapability(tool.value) ===
'skills-invocable'`, so it silently does nothing while `init.ts` reports skipped
commands. Either remove the no-op branch or add the same skills-skipped
tracking/message behavior used by `init.ts` (for example, the
`skillsInvocableCommandSkips`/“Commands skipped for … (uses skills)” flow) so
`update` gives parity and users see the intentional skip.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 71241810-dd7a-49e6-be95-4d555b5bcd7a
📒 Files selected for processing (27)
docs/commands.mddocs/how-commands-work.mddocs/migration-guide.mddocs/supported-tools.mddocs/troubleshooting.mdopenspec/changes/make-codex-skills-only/.openspec.yamlopenspec/changes/make-codex-skills-only/design.mdopenspec/changes/make-codex-skills-only/proposal.mdopenspec/changes/make-codex-skills-only/specs/ai-tool-paths/spec.mdopenspec/changes/make-codex-skills-only/specs/cli-init/spec.mdopenspec/changes/make-codex-skills-only/specs/cli-update/spec.mdopenspec/changes/make-codex-skills-only/specs/command-generation/spec.mdopenspec/changes/make-codex-skills-only/tasks.mdsrc/core/command-generation/adapters/codex.tssrc/core/command-generation/adapters/index.tssrc/core/command-generation/registry.tssrc/core/command-generation/types.tssrc/core/command-surface.tssrc/core/init.tssrc/core/legacy-cleanup.tssrc/core/profile-sync-drift.tssrc/core/update.tstest/core/command-generation/adapters.test.tstest/core/command-generation/registry.test.tstest/core/init.test.tstest/core/legacy-cleanup.test.tstest/core/update.test.ts
💤 Files with no reviewable changes (3)
- src/core/command-generation/adapters/codex.ts
- src/core/command-generation/adapters/index.ts
- src/core/command-generation/registry.ts
showms
force-pushed
the
codex/make-codex-skills-only
branch
from
3e5d111 to
ddbcf16
Compare
showms
force-pushed
the
codex/make-codex-skills-only
branch
from
ddbcf16 to
d244ec3
Compare
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
test/core/legacy-cleanup.test.ts (1)
1050-1163: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winAdd a backslash-path regression for
getToolsFromLegacyArtifacts().This change added Windows-specific normalization in the production matcher, but this section still only feeds POSIX-style
slashCommandFilesinto the extractor. A case like'.cursor\\commands\\openspec-proposal.md'would catch the exact cross-platform break this code is trying to prevent.As per coding guidelines, "When touching path behavior, add coverage that would fail on Windows path separators."
🧪 Example test
+ it('should extract cursor from Windows-style legacy artifact paths', () => { + const detection = { + configFiles: [], + configFilesToUpdate: [], + slashCommandDirs: [], + slashCommandFiles: ['.cursor\\commands\\openspec-proposal.md'], + globalSlashCommandFiles: [], + hasOpenspecAgents: false, + hasProjectMd: false, + hasRootAgentsWithMarkers: false, + hasLegacyArtifacts: true, + }; + + expect(getToolsFromLegacyArtifacts(detection)).toContain('cursor'); + });🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@test/core/legacy-cleanup.test.ts` around lines 1050 - 1163, Add a Windows-path regression test for getToolsFromLegacyArtifacts by extending the existing getToolsFromLegacyArtifacts describe block with a case that passes backslash-separated legacy paths (for example in slashCommandFiles) and asserts the same tool is detected. Use the existing getToolsFromLegacyArtifacts helper and related detection shape, and verify the matcher normalizes separators so the test would fail without the Windows path fix.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@test/core/update.test.ts`:
- Around line 1617-1639: The update test is using a prompt name that is not
recognized by detectLegacyArtifacts(), so it never exercises the managed global
prompt preserve path. Change the legacy prompt setup in the
updateCommand.execute test to use an opsx-*.md prompt name, and keep the
assertions verifying both the prompt and SKILL.md remain after the
non-interactive no-force update. This should be fixed in the test case around
updateCommand.execute, detectLegacyArtifacts, and the promptDir/legacyPrompt
setup.
---
Outside diff comments:
In `@test/core/legacy-cleanup.test.ts`:
- Around line 1050-1163: Add a Windows-path regression test for
getToolsFromLegacyArtifacts by extending the existing
getToolsFromLegacyArtifacts describe block with a case that passes
backslash-separated legacy paths (for example in slashCommandFiles) and asserts
the same tool is detected. Use the existing getToolsFromLegacyArtifacts helper
and related detection shape, and verify the matcher normalizes separators so the
test would fail without the Windows path fix.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 3f4eacb2-72a3-4738-9070-0ce5ccd33f44
📒 Files selected for processing (27)
docs/commands.mddocs/how-commands-work.mddocs/migration-guide.mddocs/supported-tools.mddocs/troubleshooting.mdopenspec/changes/make-codex-skills-only/.openspec.yamlopenspec/changes/make-codex-skills-only/design.mdopenspec/changes/make-codex-skills-only/proposal.mdopenspec/changes/make-codex-skills-only/specs/ai-tool-paths/spec.mdopenspec/changes/make-codex-skills-only/specs/cli-init/spec.mdopenspec/changes/make-codex-skills-only/specs/cli-update/spec.mdopenspec/changes/make-codex-skills-only/specs/command-generation/spec.mdopenspec/changes/make-codex-skills-only/tasks.mdsrc/core/command-generation/adapters/codex.tssrc/core/command-generation/adapters/index.tssrc/core/command-generation/registry.tssrc/core/command-generation/types.tssrc/core/command-surface.tssrc/core/init.tssrc/core/legacy-cleanup.tssrc/core/profile-sync-drift.tssrc/core/update.tstest/core/command-generation/adapters.test.tstest/core/command-generation/registry.test.tstest/core/init.test.tstest/core/legacy-cleanup.test.tstest/core/update.test.ts
💤 Files with no reviewable changes (3)
- src/core/command-generation/adapters/index.ts
- src/core/command-generation/adapters/codex.ts
- src/core/command-generation/registry.ts
✅ Files skipped from review due to trivial changes (9)
- docs/commands.md
- docs/how-commands-work.md
- openspec/changes/make-codex-skills-only/.openspec.yaml
- src/core/command-generation/types.ts
- docs/supported-tools.md
- docs/migration-guide.md
- openspec/changes/make-codex-skills-only/proposal.md
- docs/troubleshooting.md
- openspec/changes/make-codex-skills-only/tasks.md
🚧 Files skipped from review as they are similar to previous changes (9)
- test/core/command-generation/registry.test.ts
- src/core/command-surface.ts
- openspec/changes/make-codex-skills-only/specs/cli-init/spec.md
- test/core/command-generation/adapters.test.ts
- openspec/changes/make-codex-skills-only/specs/ai-tool-paths/spec.md
- openspec/changes/make-codex-skills-only/specs/command-generation/spec.md
- src/core/init.ts
- src/core/profile-sync-drift.ts
- src/core/update.ts
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
1 participant
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.
Summary
This PR removes Codex from OpenSpec's command/prompt-file generation path and makes Codex a skills-only integration.
Specifically, this change:
openspec initandopenspec updatecommandsCODEX_HOME/promptsor~/.codex/promptsWhy
This aligns OpenSpec with Codex's current official guidance.
OpenAI's Codex changelog states that custom prompts were deprecated on January 22, 2026, and recommends using skills for reusable instructions and workflows instead. The Codex custom prompts documentation also marks the feature as deprecated and explains that custom prompts are local to the Codex home directory, require explicit invocation, and should be replaced by skills for reusable/shared behavior.
Codex changelog: developers.openai.com/codex/changelog
Codex custom prompts docs: developers.openai.com/codex/custom-prompts
In practice, continuing to generate global Codex prompt files creates a poor fit for OpenSpec:
OpenSpec already has a skills-based Codex workflow surface, so this PR makes that the supported and explicit path.
Behavior changes
Before
openspec init/openspec updatecould generate managed Codex prompt files inCODEX_HOME/promptsor~/.codex/promptsAfter
commandsdelivery does not remove Codex skillsImplementation notes
skills-invocablecommand-surface capability pathboth/skills/commandsboth/skills/commandsBreaking change
Codex users who previously relied on OpenSpec-generated custom prompt files must use the OpenSpec-generated Codex skills workflow after updating.
Validation
openspec validate make-codex-skills-only --strictpnpm buildSummary by CodeRabbit
.codex/skills/openspec-*across delivery modes.opsx-*artifacts are cleaned up safely (non-interactive warns unless--force).