fix(clients): dedupe skills in composer menus - #8043
Conversation
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
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 |
| } | ||
|
|
||
| const skillItems = (selectedProviderStatus?.skills ?? []) | ||
| const skillItems = dedupeProviderSkillsByName(selectedProviderStatus?.skills ?? []) |
There was a problem hiding this comment.
🟡 Medium threads/ThreadComposer.tsx:435
The / menu drops an enabled skill when a disabled duplicate with the same normalized name appears first: dedupeProviderSkillsByName retains the disabled definition, matchesSlashSkillQuery removes it, and the enabled duplicate is never shown. Filter to enabled skills before deduplicating, as the $ branch does.
🤖 Copy this AI Prompt to have your agent fix this:
In file @apps/mobile/src/features/threads/ThreadComposer.tsx around line 435:
The `/` menu drops an enabled skill when a disabled duplicate with the same normalized name appears first: `dedupeProviderSkillsByName` retains the disabled definition, `matchesSlashSkillQuery` removes it, and the enabled duplicate is never shown. Filter to enabled skills before deduplicating, as the `$` branch does.
There was a problem hiding this comment.
gpt-5.6-sol - responding
Fixed in eb99052. Mobile / now uses the shared helper that filters disabled skills before deduplicating, matching web behavior. Added regression coverage for a disabled definition preceding an enabled duplicate; 16 focused tests and the client-runtime/mobile typechecks pass.
There was a problem hiding this comment.
Sorry, I'm unable to act on this request because you do not have permissions within this repository.
ApprovabilityVerdict: Would Approve Macroscope's review found this PR approvable — This is a small, self-contained composer-menu bug fix: enabled skills are filtered before case-insensitive name deduplication, removing duplicate rows without changing provider or server behavior. The listed medium finding concerned a disabled-before-enabled duplicate, which the current head addresses and covers with regression tests. Not approved because:
Adjust the Minimum Blocking Severity for this repo — including turning it Off — in Settings. You can add or adjust custom eligibility rules. Learn more. |
What Changed
/and$menus on web and mobile.Why
Codex can report the same skill name from both
.codexand.agents. The composer rendered every definition with the same row ID, so duplicate rows highlighted together. Hovering a later duplicate also scrolled the menu to the first row with that ID, skipping entries between them.Skill invocation is name-based, so multiple same-name rows are not independently selectable. Keeping one row per name makes the menu match what the user can invoke and restores one-to-one highlight and scroll targets.
UI Changes
The after image uses the same duplicated
.codexand.agentsskill setup. Each skill now has one row and one hover target.Validation
vp test run packages/client-runtime/src/providerSkills.test.ts apps/web/src/providerSkillSearch.test.ts(16 tests passed)vp run --filter @t3tools/client-runtime typecheckvp run --filter @t3tools/web typecheckvp run --filter @t3tools/mobile typecheckvp lintandvp fmton all five changed filesBabysit PrandBranch Auditeach render onceBranch AudittoBabysit Prwithout highlighting another row or jumping the menu$skill menu applies the same deduplicationChecklist
Model: GPT-5.6 Sol
Harness: Codex
Note
Low Risk
UI-only filtering of already-enabled skills; invocation remains name-based and tests cover first-wins plus disabled-then-enabled duplicates.
Overview
Stops composer
/and$skill menus from listing the same skill twice when a provider reports it from multiple paths (e.g..codexand.agents). Duplicate names used the same row ID, so hover/highlight jumped between copies.Adds
dedupeProviderSkillsByName, which keeps the first enabled definition (case-insensitive) and order of other skills. Slash menus go throughgetProviderSkillsForSlashMenu; web$search and mobile$menus apply the same helper.Reviewed by Cursor Bugbot for commit eb99052. Bugbot is set up for automated code reviews on this repo. Configure here.
Note
Dedupe skills by name in composer slash menus and
searchProviderSkillsdedupeProviderSkillsByNameto providerSkills.ts, which keeps only the first occurrence per case-insensitive, trimmed skill name.getProviderSkillsForSlashMenuandsearchProviderSkillsnow dedupe enabled skills before returning or ranking them.getProviderSkillsForSlashMenu(..., true)for skill menu items anddedupeProviderSkillsByNamefor enabled skills in the skill-trigger branch.Macroscope summarized eb99052.