From 402a984c4c8e810bcf64d41e4d38024c4b625bea Mon Sep 17 00:00:00 2001 From: "@daniel-lxs" <57051444+daniel-lxs@users.noreply.github.com> Date: Wed, 9 Sep 2026 20:24:40 +0000 Subject: [PATCH] feat: restore environment skill management --- .../restore-environment-skill-management.md | 5 + apps/docs/skills.mdx | 20 +- .../EnvironmentSkills.client.test.tsx | 223 ++++++++ .../components/settings/EnvironmentSkills.tsx | 498 ++++++++++++++++++ .../settings/InstanceSkills.client.test.tsx | 9 +- .../settings/pages/SkillsSettingsPage.tsx | 4 + 6 files changed, 747 insertions(+), 12 deletions(-) create mode 100644 .changeset/restore-environment-skill-management.md create mode 100644 apps/web/src/components/settings/EnvironmentSkills.client.test.tsx create mode 100644 apps/web/src/components/settings/EnvironmentSkills.tsx diff --git a/.changeset/restore-environment-skill-management.md b/.changeset/restore-environment-skill-management.md new file mode 100644 index 000000000..61f9ab008 --- /dev/null +++ b/.changeset/restore-environment-skill-management.md @@ -0,0 +1,5 @@ +--- +'@roomote/web': patch +--- + +Restore admin management of environment-scoped skills under a collapsed Settings section. diff --git a/apps/docs/skills.mdx b/apps/docs/skills.mdx index 18e50ca84..cefceccb0 100644 --- a/apps/docs/skills.mdx +++ b/apps/docs/skills.mdx @@ -38,18 +38,22 @@ Instance skills are stored once for the Roomote instance, independently of environments. Any member can create or read them. Only the creator or an admin can edit or delete them; these permissions also apply to conversational creation. -Environment skills and marketplace installation controls are not shown in -**Settings > Skills**. Existing environment skills remain stored and available -to Roomote within their authorized environment scope. Admins manage them in -the environment editor's **YAML** view: use `manualSkills` for inline custom -instructions and `skills` for published skills. See the -[environment definition reference](/environments/definition#skills) for examples. -They are not automatically migrated, copied, or removed. +Admins can expand **Environment skills** below the shared catalog to create, +view, edit, or delete inline skills and choose the environments where they are +available. The section stays hidden from members and when no environments are +configured. Marketplace search and installation are separate from this +environment management section. + +Admins can also manage environment skills in an environment's **YAML** view: +use `manualSkills` for inline custom instructions and `skills` for published +skills. See the [environment definition reference](/environments/definition#skills) +for examples. Existing environment skills are not automatically migrated, +copied, or removed. | Skill type | How it is added | Availability | | ----------------- | ---------------------------------------------------- | ---------------------------------------------------------------------- | | Instance skill | Ask in conversation or use Add Skill in Settings | All Sessions and coding tasks on this instance | -| Environment skill | Admins edit `manualSkills` or `skills` in environment YAML | Selected environments only | +| Environment skill | Admins use Settings or edit `manualSkills` in environment YAML | Selected environments only | | Repository skill | Maintain a skill in the repository | The authorized repository or environment scope | Instance skills do not have an environment selection. Environment skills apply diff --git a/apps/web/src/components/settings/EnvironmentSkills.client.test.tsx b/apps/web/src/components/settings/EnvironmentSkills.client.test.tsx new file mode 100644 index 000000000..dee48962c --- /dev/null +++ b/apps/web/src/components/settings/EnvironmentSkills.client.test.tsx @@ -0,0 +1,223 @@ +import { QueryClient, QueryClientProvider } from '@tanstack/react-query'; +import { + fireEvent, + render, + screen, + waitFor, + within, +} from '@testing-library/react'; + +const { state, saveMock, removeMock } = vi.hoisted(() => ({ + state: { + environments: [ + { id: '00000000-0000-4000-8000-000000000001', name: 'Alpha' }, + { id: '00000000-0000-4000-8000-000000000002', name: 'Beta' }, + ], + installed: [ + { + kind: 'manual' as const, + source: 'manual', + name: 'release-checklist', + skillId: 'manual@release-checklist#1234567890ab', + isAllSelection: false, + installsLabel: null, + url: null, + description: 'Use when preparing a release.', + content: '# Release checklist', + environments: [ + { id: '00000000-0000-4000-8000-000000000001', name: 'Alpha' }, + ], + }, + { + kind: 'marketplace' as const, + source: 'vercel-labs/agent-skills', + name: 'vercel-react-best-practices', + skillId: 'vercel-labs/agent-skills@vercel-react-best-practices', + isAllSelection: false, + installsLabel: null, + url: null, + description: 'Marketplace skill', + content: null, + environments: [ + { id: '00000000-0000-4000-8000-000000000001', name: 'Alpha' }, + ], + }, + ], + }, + saveMock: vi.fn(async (_input: unknown) => ({ success: true })), + removeMock: vi.fn(async (_input: unknown) => ({ success: true })), +})); + +vi.mock('sonner', () => ({ + toast: { success: vi.fn(), error: vi.fn() }, +})); +vi.mock('@/trpc/client', () => ({ + useTRPC: () => ({ + customSkills: { + list: { + queryKey: () => ['customSkills', 'list'], + queryOptions: () => ({ + queryKey: ['customSkills', 'list'], + queryFn: async () => ({ + deploymentName: 'this deployment', + environments: state.environments, + installed: state.installed, + }), + }), + }, + saveManual: { + mutationOptions: (options = {}) => ({ + mutationFn: saveMock, + ...options, + }), + }, + remove: { + mutationOptions: (options = {}) => ({ + mutationFn: removeMock, + ...options, + }), + }, + }, + }), +})); + +import { EnvironmentSkills } from './EnvironmentSkills'; + +function renderEnvironmentSkills() { + const queryClient = new QueryClient({ + defaultOptions: { queries: { retry: false }, mutations: { retry: false } }, + }); + return render( + + + , + ); +} + +beforeEach(() => { + vi.clearAllMocks(); + state.environments = [ + { id: '00000000-0000-4000-8000-000000000001', name: 'Alpha' }, + { id: '00000000-0000-4000-8000-000000000002', name: 'Beta' }, + ]; +}); + +it('is collapsed by default and excludes marketplace skills and controls', async () => { + renderEnvironmentSkills(); + + const summary = await screen.findByText('Environment skills'); + expect(summary.closest('details')).not.toHaveAttribute('open'); + expect(screen.getByText('release-checklist')).not.toBeVisible(); + expect( + screen.getByRole('button', { name: 'Add environment skill' }), + ).not.toBeVisible(); + expect( + screen.queryByText('vercel-react-best-practices'), + ).not.toBeInTheDocument(); + expect(screen.queryByText(/marketplace/i)).not.toBeInTheDocument(); + + fireEvent.click(summary); + expect(summary.closest('details')).toHaveAttribute('open'); + expect(screen.getByText('release-checklist')).toBeVisible(); + expect( + screen.getByRole('button', { name: 'Add environment skill' }), + ).toBeVisible(); +}); + +it('hides environment management when there are no environments', async () => { + state.environments = []; + const { container } = renderEnvironmentSkills(); + + await waitFor(() => expect(container).toBeEmptyDOMElement()); +}); + +it('views and edits an existing environment skill', async () => { + renderEnvironmentSkills(); + fireEvent.click(await screen.findByText('Environment skills')); + + fireEvent.click( + screen.getByRole('button', { name: 'View release-checklist' }), + ); + const viewDialog = screen.getByRole('dialog'); + expect( + within(viewDialog).getByText('# Release checklist'), + ).toBeInTheDocument(); + expect(within(viewDialog).getByText('Enabled in Alpha')).toBeInTheDocument(); + fireEvent.click( + within(viewDialog).getAllByRole('button', { name: 'Close' })[0]!, + ); + + fireEvent.click( + screen.getByRole('button', { name: 'Edit release-checklist' }), + ); + const editDialog = screen.getByRole('dialog'); + expect(within(editDialog).getByLabelText('Slug')).toHaveValue( + 'release-checklist', + ); + expect(within(editDialog).getByLabelText('Alpha')).toBeChecked(); + expect(within(editDialog).getByLabelText('Beta')).not.toBeChecked(); + fireEvent.change(within(editDialog).getByLabelText('Content'), { + target: { value: '# Updated checklist' }, + }); + fireEvent.click(within(editDialog).getByLabelText('Beta')); + fireEvent.click( + within(editDialog).getByRole('button', { name: 'Save Skill' }), + ); + + await waitFor(() => expect(saveMock).toHaveBeenCalled()); + expect(saveMock.mock.calls[0]?.[0]).toEqual({ + name: 'release-checklist', + description: 'Use when preparing a release.', + content: '# Updated checklist', + environmentIds: [ + '00000000-0000-4000-8000-000000000001', + '00000000-0000-4000-8000-000000000002', + ], + previousSkillId: 'manual@release-checklist#1234567890ab', + }); +}); + +it('creates and deletes environment skills through the existing mutations', async () => { + renderEnvironmentSkills(); + fireEvent.click(await screen.findByText('Environment skills')); + fireEvent.click( + screen.getByRole('button', { name: 'Add environment skill' }), + ); + + const createDialog = screen.getByRole('dialog'); + fireEvent.change(within(createDialog).getByLabelText('Slug'), { + target: { value: 'my new/skill' }, + }); + fireEvent.change(within(createDialog).getByLabelText('Description'), { + target: { value: 'Use for a new workflow.' }, + }); + fireEvent.change(within(createDialog).getByLabelText('Content'), { + target: { value: '# New instructions' }, + }); + fireEvent.click(within(createDialog).getByLabelText('Beta')); + fireEvent.click( + within(createDialog).getByRole('button', { name: 'Save Skill' }), + ); + + await waitFor(() => expect(saveMock).toHaveBeenCalled()); + expect(saveMock.mock.calls[0]?.[0]).toEqual({ + name: 'mynewskill', + description: 'Use for a new workflow.', + content: '# New instructions', + environmentIds: ['00000000-0000-4000-8000-000000000002'], + previousSkillId: undefined, + }); + + fireEvent.click( + screen.getByRole('button', { name: 'Delete release-checklist' }), + ); + const deleteDialog = screen.getByRole('dialog'); + expect(removeMock).not.toHaveBeenCalled(); + fireEvent.click( + within(deleteDialog).getByRole('button', { name: 'Delete Skill' }), + ); + await waitFor(() => expect(removeMock).toHaveBeenCalled()); + expect(removeMock.mock.calls[0]?.[0]).toEqual({ + skillId: 'manual@release-checklist#1234567890ab', + }); +}); diff --git a/apps/web/src/components/settings/EnvironmentSkills.tsx b/apps/web/src/components/settings/EnvironmentSkills.tsx new file mode 100644 index 000000000..73196c2e7 --- /dev/null +++ b/apps/web/src/components/settings/EnvironmentSkills.tsx @@ -0,0 +1,498 @@ +'use client'; + +import { useState } from 'react'; +import { useMutation, useQuery, useQueryClient } from '@tanstack/react-query'; +import { toast } from 'sonner'; + +import { useTRPC } from '@/trpc/client'; +import { + BasicTooltip, + Button, + Card, + CardContent, + Checkbox, + ChevronDown, + Dialog, + DialogContent, + DialogDescription, + DialogFooter, + DialogHeader, + DialogTitle, + Eye, + Input, + Label, + Pencil, + Plus, + Skeleton, + Textarea, + Trash2, + VectorSquare, +} from '@/components/system'; + +const DEFAULT_SKILL = { + name: 'my-environment-skill', + description: 'Adds custom Roomote behavior.', + content: + '# My Environment Skill\n\nUse this skill when you need custom instructions.', + selectedEnvironmentIds: [] as string[], +}; +const INVALID_NAME_CHARACTER = /[/\s]+/g; + +type Environment = { id: string; name: string }; +type EnvironmentSkill = { + kind: 'manual'; + source: string; + name: string; + skillId: string; + isAllSelection: boolean; + installsLabel: string | null; + url: string | null; + description: string | null; + content: string | null; + environments: Environment[]; +}; +type EditorState = { + previousSkillId?: string; + name: string; + description: string; + content: string; + selectedEnvironmentIds: string[]; +}; + +function sameEditorState(left: EditorState | null, right: EditorState | null) { + if (!left || !right) return left === right; + return ( + left.previousSkillId === right.previousSkillId && + left.name === right.name && + left.description === right.description && + left.content === right.content && + left.selectedEnvironmentIds.length === + right.selectedEnvironmentIds.length && + left.selectedEnvironmentIds.every( + (environmentId, index) => + environmentId === right.selectedEnvironmentIds[index], + ) + ); +} + +function sortedEnvironmentIds(ids: string[], environments: Environment[]) { + const names = new Map( + environments.map((environment) => [environment.id, environment.name]), + ); + return [...ids].sort((left, right) => + (names.get(left) ?? left).localeCompare(names.get(right) ?? right), + ); +} + +export function EnvironmentSkills() { + const trpc = useTRPC(); + const queryClient = useQueryClient(); + const list = useQuery(trpc.customSkills.list.queryOptions()); + const environments = list.data?.environments ?? []; + const skills = (list.data?.installed ?? []).filter( + (skill): skill is EnvironmentSkill => skill.kind === 'manual', + ); + const [viewing, setViewing] = useState(null); + const [editing, setEditing] = useState(null); + const [initialEditor, setInitialEditor] = useState(null); + const [deleting, setDeleting] = useState(null); + + const save = useMutation( + trpc.customSkills.saveManual.mutationOptions({ + onSuccess: async () => { + await queryClient.invalidateQueries({ + queryKey: trpc.customSkills.list.queryKey(), + }); + toast.success( + editing?.previousSkillId + ? 'Environment skill updated' + : 'Environment skill created', + ); + setEditing(null); + setInitialEditor(null); + }, + onError: (error) => toast.error(error.message), + }), + ); + const remove = useMutation( + trpc.customSkills.remove.mutationOptions({ + onSuccess: async () => { + await queryClient.invalidateQueries({ + queryKey: trpc.customSkills.list.queryKey(), + }); + toast.success('Environment skill deleted'); + setDeleting(null); + }, + onError: (error) => toast.error(error.message), + }), + ); + + const openEditor = (skill?: EnvironmentSkill) => { + const nextState = skill + ? { + previousSkillId: skill.skillId, + name: skill.name, + description: skill.description ?? '', + content: skill.content ?? '', + selectedEnvironmentIds: sortedEnvironmentIds( + skill.environments.map((environment) => environment.id), + environments, + ), + } + : { ...DEFAULT_SKILL }; + setInitialEditor(nextState); + setEditing(nextState); + }; + + const closeEditor = () => { + if (!editing || save.isPending) return; + if ( + !sameEditorState(editing, initialEditor) && + !window.confirm('Discard unsaved changes to this environment skill?') + ) { + return; + } + setEditing(null); + setInitialEditor(null); + }; + + if (list.isSuccess && environments.length === 0) return null; + + if (list.isPending) { + return ( +
+ +
+ ); + } + + if (list.isError) { + return ( +

+ Failed to load environment skills. +

+ ); + } + + return ( + <> +
+ + + Environment skills + +
+
+

+ Admin-managed skills available only in selected environments. +

+ +
+ + {skills.length > 0 ? ( + + +
    + {skills.map((skill) => ( +
  • +
    +

    + {skill.name} +

    + {skill.description ? ( +

    + {skill.description} +

    + ) : null} +

    + + {skill.environments + .map((environment) => environment.name) + .join(', ')} +

    +
    +
    + + + + + + + + + +
    +
  • + ))} +
+
+
+ ) : ( +

+ No environment skills yet. +

+ )} +
+
+ + !open && setViewing(null)} + > + + + {viewing?.name} + {viewing?.description} + +

+ Enabled in{' '} + {viewing?.environments + .map((environment) => environment.name) + .join(', ')} +

+
+            {viewing?.content}
+          
+ + + +
+
+ + { + if (!open) closeEditor(); + }} + > + + {editing ? ( + <> + + + {editing.previousSkillId + ? 'Edit Environment Skill' + : 'Add Environment Skill'} + + + Choose which environments can use these instructions. + + +
+
+ + + setEditing({ + ...editing, + name: event.currentTarget.value.replace( + INVALID_NAME_CHARACTER, + '', + ), + }) + } + autoCapitalize="off" + autoCorrect="off" + spellCheck={false} + disabled={save.isPending} + /> +
+
+ +

+ Tell the agent when to use this skill. +

+