[riccardovandra]

Problem

A hands-on pass of Global Skills (Settings → Tools) found three bugs:

1. In the Library, clicking Install on any skill crashed the whole app with "Base UI: MenuGroupContext is missing".
2. Removing a skill from its page put it in Trash correctly, but the "Moved to the trash" notice with Restore never appeared.
3. The agent tools t3_skills_remove and t3_skills_restore returned {}, so an agent could not confirm what happened.

Root causes and fixes

1. SkillLibrarySection rendered MenuGroupLabel straight inside MenuPopup. It is now wrapped in MenuGroup, as in IntegrationsSettings. No other MenuGroupLabel added by feat: port upstream skills and Tools settings (#17691) #39, feat(skills): skill library sources and per-instance Claude homes #46 or feat(skills): move removed skills to a trash with restore #47 had this problem.
2. SettingsContentLayout keys each settings page by JSON.stringify(search). In TanStack Router, a parent route's search carries every URL param, including a child page's own skill. Closing a skill's page changed the key and remounted the Skills panel. That dropped the useSkillTrash state, and setRemoved then ran on the unmounted instance after the removal returned. The key now comes from the scope params only (settingsScopeKey). Opening or closing a skill, task or server keeps the page mounted, and changing scope still remounts it. Every remove entry point goes through the same dialog, so all of them now show the notice. Dismiss and Restore are unchanged.
3. SkillLibrary.remove and restoreFromTrash now return the trash entry. t3_skills_remove returns { id, name, paths } and t3_skills_restore returns { name, paths }. The RPC handlers still return {}, so there is no contract change.

Verification

• vp test run passes for settingsScopeNavigation.test.ts (new router test: opening and closing a skill keeps the key, changing scope changes it), the tools and skill-trash MCP handler tests (new) and SkillLibrary.test.ts (asserts the returned entries).
• Typecheck is clean for apps/web, apps/server and packages/contracts, apart from the known ChatView.tsx:1908 error. vp run check:changed fails only on things that were already failing: prefer-effect-fn and test-timeout lint on lines this PR does not touch, and knip unused exports from the Docs work.
• No browser pass yet. The orchestrator re-tests in a real client.

Model: Claude Opus 5.5 (claude-opus-5-5), harness: Claude Code in T3 Code.

🤖 Generated with Claude Code