Skip to content

Expose Codex skills as ACP commands#246

Open
lgmoneda wants to merge 3 commits into
zed-industries:mainfrom
lgmoneda:lm/skills-acp-commands
Open

Expose Codex skills as ACP commands#246
lgmoneda wants to merge 3 commits into
zed-industries:mainfrom
lgmoneda:lm/skills-acp-commands

Conversation

@lgmoneda
Copy link
Copy Markdown

References #190

This exposes Codex skills to ACP clients as skills:<name> commands, maps invocations to UserInput::Skill, and refreshes the available command list when skills change.

Validation:

  • cargo test
  • cargo clippy --all-targets -- -D warnings

@priyashpatil
Copy link
Copy Markdown

priyashpatil commented Apr 25, 2026

Hi, thanks for this. Can you fix the merge conflicts?

@lgmoneda lgmoneda force-pushed the lm/skills-acp-commands branch from c6af44a to 6fa61db Compare April 25, 2026 11:00
@priyashpatil
Copy link
Copy Markdown

priyashpatil commented Apr 25, 2026

Thanks for rebasing this. The updated shape looks much better and matches the ACP bridge approach.

A few small changes I think would make this line up with Codex's own skill behavior and keep it correct for ACP clients generally:

  1. Only advertise enabled skills in skill_commands() / available_commands().
    skills/list can return disabled skills with enabled: false, and Codex's TUI filters those before exposing skill mentions. If ACP advertises disabled skills, a client can offer a command that core later skips, which is confusing UX.

  2. Avoid falling back to entries.first() in skills_for_cwd().
    If the requested cwd is not present, returning no skills is safer than exposing commands from a different cwd. This also matches the TUI helper behavior.

  3. Use force_reload: true for the initial session load.
    Since this drives ACP available commands, it is better to read the current skill state on load and keep SkillsUpdateAvailable as the later invalidation path.

Duplicate names are a separate edge case because /skills:<name> cannot distinguish two enabled skills with the same name, but I would not block this first pass on that if the enabled/cwd behavior is fixed.

@priyashpatil
Copy link
Copy Markdown

Working!

Screenshot 2026-04-26 at 4 54 01 PM

@benbrandt
Copy link
Copy Markdown
Member

Hey all it looks like they removed this op/event and now have EventMsg::SkillsUpdateAvailable

So this will likely have to be relooked at again

@lgmoneda lgmoneda force-pushed the lm/skills-acp-commands branch from 5befd50 to 11721ae Compare May 9, 2026 16:56
@lgmoneda
Copy link
Copy Markdown
Author

lgmoneda commented May 9, 2026

@benbrandt, let me know if adjustments are needed. Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants