feat(codex): make Codex skills-only and retire managed custom prompts#1283
feat(codex): make Codex skills-only and retire managed custom prompts#1283showms wants to merge 7 commits into
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughCodex is moved to a skills-only surface. The Codex adapter is removed, generation and cleanup now use capability-aware logic, managed global Codex prompt files are detected and cleaned up through ChangesCodex Skills-Only Migration
Estimated code review effort: 4 (Complex) | ~60 minutes Possibly related issues
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
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}
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
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
TabishB
left a comment
TabishB
left a comment
There was a problem hiding this comment.
Thanks for pushing this forward. The direction makes sense: Codex should move to skills and stop generating global prompt files. I think we need to tighten the migration safety before merging, mostly so we do not accidentally delete user prompts or leave older Codex users without a replacement. I left the specific spots inline.
showms
force-pushed
the
codex/make-codex-skills-only
branch
from
e0781f2 to
a29c458
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
src/core/legacy-cleanup.ts (1)
650-666: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick winRe-validate managed global prompt paths before unlinking (still unaddressed).
cleanupLegacyArtifacts()is exported and this loop unlinks every entry indetection.globalSlashCommandFilesunconditionally; the validatedglobalPromptMatchesByPathmap is only consulted for replacement labels, not to gate deletion. A stale/malformed detection object could still delete arbitrary absolute paths. Guard eachfilePathwithgetManagedGlobalLegacyPromptMetadata(filePath)beforefs.unlink.🤖 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/legacy-cleanup.ts` around lines 650 - 666, The managed global slash command cleanup loop in cleanupLegacyArtifacts currently deletes every entry in detection.globalSlashCommandFiles without re-checking that each path is still a valid managed global prompt. Update this loop to validate each filePath with getManagedGlobalLegacyPromptMetadata(filePath) before calling fs.unlink, and only delete when the metadata confirms it is a managed global legacy prompt; keep the existing globalPromptMatchesByPath logic only for replacement labels.
🤖 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 `@openspec/changes/make-codex-skills-only/design.md`:
- Line 47: Use openspec-* consistently in this section instead of opsx-* to
match the rest of the design and the .codex/prompts/openspec-*.md naming. Update
the wording in the global cleanup/allowlist explanation so the referenced prompt
filenames and examples use the openspec prefix everywhere, including any
mentions tied to workflow ID inference or unmanaged user-authored files.
---
Duplicate comments:
In `@src/core/legacy-cleanup.ts`:
- Around line 650-666: The managed global slash command cleanup loop in
cleanupLegacyArtifacts currently deletes every entry in
detection.globalSlashCommandFiles without re-checking that each path is still a
valid managed global prompt. Update this loop to validate each filePath with
getManagedGlobalLegacyPromptMetadata(filePath) before calling fs.unlink, and
only delete when the metadata confirms it is a managed global legacy prompt;
keep the existing globalPromptMatchesByPath logic only for replacement labels.
🪄 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: 01e50aea-e1e5-4b79-86ff-e78ba840ff8c
📒 Files selected for processing (16)
.gitignoredocs/migration-guide.mddocs/troubleshooting.mdopenspec/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/tasks.mdsrc/core/init.tssrc/core/legacy-cleanup.tssrc/core/update.tstest/core/init.test.tstest/core/legacy-cleanup.test.tstest/core/update.test.tstest/helpers/legacy-codex-prompt.ts
✅ Files skipped from review due to trivial changes (6)
- test/helpers/legacy-codex-prompt.ts
- .gitignore
- docs/migration-guide.md
- docs/troubleshooting.md
- openspec/changes/make-codex-skills-only/tasks.md
- openspec/changes/make-codex-skills-only/specs/ai-tool-paths/spec.md
🚧 Files skipped from review as they are similar to previous changes (3)
- openspec/changes/make-codex-skills-only/proposal.md
- openspec/changes/make-codex-skills-only/specs/cli-init/spec.md
- openspec/changes/make-codex-skills-only/specs/cli-update/spec.md
|
Thanks for the careful review. I addressed the migration-safety issues in the latest update and added targeted tests for the new global prompt detection, deferred cleanup, and legacy workflow inference paths. @TabishB |
…ills-only # Conflicts: # .gitignore # docs/commands.md # docs/how-commands-work.md # docs/troubleshooting.md # test/core/init.test.ts
2 participants
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 (both,skills,commands).opsx-*prompt/command artifacts; legacy global cleanup is replacement-gated and safer in non-interactive runs (warns unless--forceis used).