💭 I'm reviewing the in-scope files for this PR—ChatComposer.tsx and providerSkillSearch.ts—to understand how the slash menu filters skills, particularly how isProviderSkillMentionable excludes userInvocationOnly skills. The SlashMenu in client-runtime filters only by enabled, which seems intentional. I'm checking the insertion format change in ChatComposer where $${item.skill.name} was replaced with a call to formatProviderSkillInsertion using trigger.kind, and verifying trigger.kind is actually available at that point in the code. The mobile ThreadComposer uses a similar pattern but is out of scope here.

1. git_tools — formatProviderSkillInsertion|getProviderSkillsForSlashMenu|searchProviderSkills|isProviderSkillMentionable

2. browse_code — packages/client-runtime/src/providerSkills.ts

💭 Let me look at ChatComposer's relevant line ranges to see what's happening there.

3. browse_code — apps/web/src/components/chat/ChatComposer.tsx

4. browse_code — apps/web/src/components/chat/ChatComposer.tsx

💭 I'm noticing a mismatch between the slash menu label and the actual insertion text — the label displays as /skill:name but the code now inserts /name instead of the previously used $name, so the displayed label no longer matches what gets inserted. Since the dedup logic filters out provider slash commands sharing a name with a visible skill, this new /name insertion should now correctly resolve to the skill rather than colliding with a provider command. That said, I'm unsure how this behaves for skills not registered as provider slash commands — that's more of a functional question than a UI-consistency one, so it might fall outside what these review rules actually cover. I want to compare against the mobile version's skill item labels for consistency, and check whether userInvocationOnly/enabled filtering is applied the same way elsewhere in the web app, like in a skills settings list.

5. git_tools — skill.enabled|skills.*filter|.skills ??

6. browse_code — apps/mobile/src/features/threads/ThreadComposer.tsx

💭 I'm checking whether the editor still highlights $name mentions for skills that are userInvocationOnly, since they're using an enabled flag in a memo key for highlighting—wondering if this creates a minor consistency gap with the menu behavior.

7. browse_code — apps/web/src/components/ComposerPromptEditor.tsx

💭 I'm noticing something odd: the slash-menu label shows /skill:${skill.name} but selecting it inserts /${skill.name} into the composer, dropping the "skill:" prefix. I want to check whether that prefix was ever actually supported by the provider or if this is a genuine mismatch worth flagging.

8. git_tools — skill:${|/skill:|"skill:"

9. git_tools — packages/client-runtime/src/providerSkills.ts

💭 I'm checking the composer docs to confirm how skill results should be labeled and inserted, since it mentions the /skill:Skill Name prefix convention.

10. browse_code — docs/user/composer.md

💭 I notice the slash-menu skill selection now inserts /name instead of $name, a deliberate behavior change, but composer.md still documents the old $name token while providers-claude.md was updated—leaving a doc inconsistency that falls outside the current scope. The bigger concern is functional: skills that the provider doesn't report as native slash commands (e.g., some file-discovered Claude skills) may not resolve correctly when the menu inserts /name, since the CLI could treat it as an unrecognized command rather than the mention-based invocation that $name previously provided.

11. git_tools — slashCommands

💭 I'm reasoning that Claude Code likely registers skills as native slash commands, so /name should be valid there, but for other providers where skills come from a different discovery path, that same syntax might not resolve—though this seems like a plausible but unverified regression the PR author already considered. Then I check whether the menu's displayed /skill:name label mismatches the actual inserted text, but I realize that mismatch already existed before this change with the old $name syntax, so it's not a new issue worth flagging. Now I'm digging into whether the slash trigger's rangeStart is only zero at the very beginning of a message, or whether it also allows a slash after a newline within detectComposerTrigger. The real concern is that when the trigger isn't at position zero, the menu only offers built-in commands, and if the user's query doesn't match any of those, the menu could end up empty while still technically open — I need to check whether ComposerCommandMenu handles that empty-items case gracefully or just pops up blank.

12. browse_code — apps/web/src/components/chat/ComposerCommandMenu.tsx

13. git_tools — detectComposerTrigger

💭 I realize rangeStart being 0 means the slash trigger sits at the very start of a line, so a command on line two would have a positive rangeStart and correctly get filtered out since expansion should only happen at the message's true start. Now I'm second-guessing whether that offset refers to position within the full prompt text or just the visible editor value in the web composer, since that distinction matters for how trigger detection actually works there. I'm also checking whether the web / menu still displays consistently after this change — it labels items as /skill:Name but now inserts as /name, so I need to confirm this doesn't conflict with how the editor decorates or highlights those tokens. Since skill chips are only rendered for $name mentions and not slash insertions, this should stay fine, and I'm now looking at how the $ skill trigger's insertion formatting compares. I'm now considering whether trigger.kind could resolve differently at selection time — if it changes to something like "path" or "slash-model," the code falls back to inserting $name instead, which seems like the safe default. The bigger question is whether skills discovered outside of the provider's own slash-command list should even appear as /name items, so I'm looking into how Claude Code's initialization parses and exposes skills, and whether its docs confirm skills are meant to be listed in the / menu as slash commands. This confirms /name is correct for skills the provider does report natively, since the slash menu deduplicates the skill row against the native command row. But that leaves a gap: an enabled, model-invocable skill that the provider doesn't expose as a native slash command would now get inserted as /name, which the CLI likely can't interpret, whereas previously $name worked correctly for that case — so this looks like a real regression risk worth flagging.

14. browse_code — For each provider driver (Claude, Codex, Grok, etc.), how is skills populated in the provider status snapshot, and does the provider's slashCommands list include those same skills (i.e. can a discovered skill be invoked as /skill-name)? Point to the code that builds skills and slashCommands per provider.

💭 I'm realizing that for non-Claude providers like Codex, Grok, and OpenCode, skills aren't invoked as slash commands—they use different mechanisms like $name mentions. So this change would break those providers by converting a working invocation into literal, non-functional text. The concrete regression is in ChatComposer.tsx at line 1866, which is in scope for my comment. I'm verifying whether showSkillsInSlashMenu really defaults to on, confirming that selecting a skill from the / menu now inserts /release-version instead of the previous $release-version mention syntax, which Codex won't recognize as a valid command. This also affects the chip rendering in the composer editor, since ComposerSkillNode only decorates $name tokens as skill chips, so /name loses that visual treatment too. I want to double-check how Grok handles skill invocation by looking at GrokSkills.ts to see how userInvocable maps to userInvocationOnly and confirm the client's handling for that provider.

15. browse_code — apps/server/src/provider/Drivers/GrokSkills.ts

16. git_tools — showSkillsInSlashMenu

💭 I realize the discrepancy is because that earlier grep used the tree URL while this one queried the plain repo, which defaults to main—so it's just showing main's version of the file, not a conflict. I also note the default value for showSkillsInSl appears to be false. Docs claiming skills appear by default in the slash menu may just be outdated, since the setting seems opt-in. Even so, if a user enables it with a non-Claude provider, the /name insertion behavior likely breaks—though I need to check whether Codex actually treats $name as a real invocation or just as prose the agent's skill tool has to interpret, since that ambiguity is central to whether this counts as broken. I'm now weighing whether this is actually in scope for the review. The rules focus on UI consistency—shared components, Tailwind, defined behavioral constraints—not cross-provider correctness of composer insertion logic. There's a rule requiring focused tests for behavior-altering changes to composition, state, or keyboard handling, so I'm checking whether this insertion change falls under that. I'm scanning the actual diff scope: ChatComposer.tsx logic and the skill search filter, plus a test file—no CSS, no primitive components, no environment routing touched, so this may not trigger a violation under the check's rules at all. Still, I'm considering whether the menu label now mismatches the inserted text in a way that could confuse users, since the label shows one format while the actual inserted text uses another, which is a subtle but real UI consistency concern introduced by these changed lines. I keep coming back to the cross-provider risk: for providers like Codex, Grok, or OpenCode that don't support skills as slash commands, inserting /name produces literal text rather than a working command, unlike the previous $name mention approach. That feels like a genuine, code-supported regression worth flagging as a precise inline comment on the relevant line, suggesting the slash-form insertion be gated on whether the skill is actually invocable as a provider slash command. Given the schema is mandatory, I'll fold "All clear" into the title/summary if the review turns out clean. Checking the ChatComposer menu behavior for empty query matches confirms it's unchanged from prior behavior, so that's fine. I still want to verify whether another call site referencing showSkillsInSlashMenu around line 1116 was left unsynced with the rest of the update.

17. git_tools — showSkillsInSlashMenu

💭 I'm weighing whether to flag the cross-provider /name insertion regression, but since the check is scoped to UI-consistency rules and verifying provider-specific expansion behavior (Codex/Grok) is outside what I can confirm, I'm leaning toward not reporting it since only concrete violations from changed lines should be flagged. Before deciding, I want to double-check whether there's an outbound transformation that converts a leading /name into a skill invocation before sending the prompt, which would mean this isn't actually a bug at all.

18. browse_code — When a composer message is sent, is there any client- or server-side transformation of a leading slash command (e.g. "/release-version ...") or of a "$skill-name" mention in the prompt text? Specifically: does the server or provider driver detect a leading "/name" and map it to a discovered skill invocation (for Codex, Grok, OpenCode), or is the text passed through verbatim to the CLI/agent? Show the relevant code.

💭 This confirms a real cross-provider regression: since text is inserted verbatim, switching from $name to /name breaks Codex/Grok/OpenCode, since those CLIs don't expand slash commands and would just treat it as literal text instead of a usable skill mention. I'll flag this as an inline comment on the ChatComposer.tsx change. gating, action dispatch, and labels must agree" and "suppress rather than guess when scope is ambiguous." Given the check defaults to failure and this is a concrete regression introduced by changed lines, flagging it once seems defensible. I'm now double-checking whether Codex actually supports /skill-name invocation — recalling that Codex's TUI has slash-command expansion for custom prompts, but that's TUI-level, not something the app-server itself handles. Since this integration talks to the app-server directly via turn/start, that TUI expansion wouldn't apply, and the only slash command the app-server itself recognizes here is a hardcoded /feedback, meaning no real expansion would occur. The same question applies to Claude's provider-reported slash commands like /compact — those come from Claude's own init response, so they're fine. For Claude skills specifically, the dedup logic works because Claude registers skills as commands during init, but if a skill were discovered on disk without appearing in init's command list, /name would silently fail there too — which is exactly the scenario this fix needs to guard against. The precise fix, then, is to gate whether /name gets inserted on whether the provider actually reported that slash command, falling back to the $name mention form otherwise; since the collision-detection helper already computes this, the minimal change is to pass a flag on the skill item. I also want to check whether $name is even functional for Codex skills, or if this whole distinction is moot. Codex's skills mechanism lets the model detect skill invocations from prose, so $release-version and /release-version might both work as textual hints either way — but there's a UI-level regression too, since $name triggers the editor's skill chip rendering while /name doesn't, so users lose that visual chip even if the underlying invocation still works. I'm now weighing severity and how confident to be given the uncertainty about whether Codex/Grok/OpenCode actually expand /name tokens, trying to decide if this rises to a failing-level finding or just a moderate note. I've verified there's no server-side mapping and only Claude surfaces skills as slash commands, which feels like strong enough evidence to justify a failure state rather than staying neutral. Still, I'm second-guessing whether raising this at all risks being a false positive, since the PR itself is explicitly trying to preserve slash skill invocation. Then I dug into composer.md and found the previous behavior — inserting $name in the / menu — was intentionally documented for non-Claude providers, and the PR changes that behavior across all providers without touching the docs. That combination of documented intent plus stale documentation gives me confidence this is an unintended cross-provider regression, so I'll cite that doc reference in my comment. I'm locating the exact target line in ChatComposer.tsx to anchor the review comment before posting it.

19. modify_pr — One finding on the web composer: the / menu now inserts the slash form of a skill for every provider, but only Claude reports skills as native slash commands. See the inline comment.