From 6498c5776d36faaf91a318c9830c1390278f9ad9 Mon Sep 17 00:00:00 2001 From: Michael Suchacz <203725896+ibetitsmike@users.noreply.github.com> Date: Tue, 13 Oct 2026 18:57:41 +0200 Subject: [PATCH] feat(site): add organization skill settings with a shared skills UI (#30612) ## Stack Part of [CODAGT-1341](https://linear.app/codercom/issue/CODAGT-1341/support-organization-level-skills-in-coder-agents). This stack adds organization skills to Coder Agents: admins publish a skill once per organization, share it with groups or members, and chats in that organization load it next to personal and workspace skills. It replaces the 21-PR draft stack #30584 to #30604 with five PRs, bottom to top: 1. #30609 data model: one migration, queries, RBAC and dbauthz 2. #30610 organization skill API routes and the scope-neutral SDK 3. #30611 sharing ACL routes 4. **#30612** settings UI: shared skills table, admin page, member section (this PR) 5. #30613 chats: chatd loading, `/` menu, docs ## What changes This PR turns the personal skills page into a skills table that works for any owner, then uses it for an organization admin page and a read-only member section. It changes only `site/`. **Shared skills table.** The personal page's files move to owner-neutral paths (kept as git renames, so the diff shows edits): `AgentSettingsPersonalSkillsPageView.tsx` becomes `components/SkillsTableView.tsx`, `AgentSettingsPersonalSkillsPage.tsx` becomes `components/SkillsTable.tsx`, `PersonalSkillEditor` becomes `SkillEditor`, and `utils/personalSkills` becomes `utils/skills`. `queries/userSkills.ts` keeps main's query names and keys for personal skills, and the new `queries/organizationSkills.ts` holds the organization and ACL queries. The table takes a `SkillsScope` (list, detail, create, update and delete queries), built by `userSkillsScope` or `organizationSkillsScope` in `utils/skills.ts`, plus an `access` object (`create`, `update`, `delete`) and its copy as props. - **Enabled toggle:** `SkillEnabledSwitch` runs one update mutation per row from the table's scope, is disabled while it is pending, and invalidates the list when it settles, which also drops a skill deleted elsewhere. - **Read-only status:** a table without update access (the member section, and auditors on the admin page) shows a green check ("Enabled") or a gray cross ("Disabled") in the Enabled column instead of a disabled switch. Editable tables keep the switch. - **Read-only view:** when the caller can't update a skill, the row opens `SkillEditor` in read-only mode with a Close button, replacing the old View dialog. It omits the naming hint and starts focus on Close. - Dialogs return focus to the control that opened them (`useDialogFocusReturn`). - Create, update and delete invalidate the skill list instead of patching it by hand. Update also writes the PATCH response to the skill's detail, so a reopened editor shows the new content. - Saving an ACL refreshes the organization skill list and the skill's detail. Deleting a skill removes its detail and the ACL queries cached under it, so a skill recreated with the same name doesn't inherit them. **Admin page** at `/ai/settings/skills?org=` (`AISettingsPage/SkillsPage`): lists, creates, edits, toggles and deletes organization skills with the caller's per-organization permissions, and opens `OrganizationSkillSharingDialog` for users with share. A requested organization the caller can't read shows the permission error instead of falling back to another organization, so a skill is never added to an organization the URL didn't name. The new `viewAnyOrganizationSkills` permission adds Skills to the AI settings sidebar, shows the navbar's AI settings link, and lets the AI settings index redirect to the skills page. The sidebar's Skills link keeps the current `?org=` only when the user can read that organization's skills, using the page's own filter (`readableSkillsOrganizations`), so following the link never lands on the permission error. **Member section.** The agent settings page becomes `/agents/settings/skills` ("Skills" in the sidebar; the old `/agents/settings/personal-skills` URL redirects). Below personal skills, "From your organizations" shows one organization at a time, read-only. Its picker lists the organizations the user is a member of that have enabled skills for them (an organization still loading or whose list failed stays listed), starts on the default organization, and is hidden when only one organization qualifies. Export all downloads only the selected organization's skills. If the selected organization's list fails, a retry button (labeled "Retry loading _name_ skills") replaces the table. **Shared sharing helpers.** `ChatModelPrincipalAutocomplete` becomes `components/ACLPrincipalAutocomplete`, used by the model, MCP server and skill sharing dialogs; `MCPServerPrincipalAutocomplete` is deleted and the common ACL diff logic moves to `aclSharing.ts`. A new `ChatModelSharingDialog.test.tsx` guards the model dialog through the swap. Declined on purpose (Mike's call): extra toggle cache hardening, namely a pending-toggle map, reconciling a View that 404s, and guarding a stale refetch against a retried toggle. Invalidating on settle already refetches the list after every toggle. ## Commits This PR reworks the old site layers rather than reproducing them. The commits go in review order: shared helpers, renames, the shared table, the two pages, then review fixes. 1. `6463992b717` refactor: share the ACL principal autocomplete and sharing helpers. 2. `e18599f5049` refactor: move the personal skills files to shared skills paths (file moves plus import path updates only). 3. `3c71e6c2168` refactor: drop the personal prefix from shared skill symbols. 4. `3be35bcde68` refactor: key the skills table and queries by owner. 5. `d0c1dfcfdd1` fix: return focus to the opener when a skills dialog closes. 6. `92ef6351eb4` feat: add an Enabled toggle to the skills table. 7. `31c103e5396` refactor: move the skill edit and delete dialogs to their own file. 8. `cff0bef4576` feat: view a skill read-only when the user cannot edit it. 9. `a8b54a22981` feat: add the organization skills admin page with sharing. 10. `391401b62b7` feat: show organization skills on the personal skills page. 11. `2cf6e250a60` refactor: keep the skill dialogs in the table view and extract focus return. This undoes the file split from commit 7 so `SkillsTableView.tsx` still diffs as a rename (497 lines then, 507 at the head). 12. `0af93452c7d` test: keep only the role wiring test for the skills page. 13. `8032c6708a8` fix: refetch the skill list after a create when no list is cached (superseded by item 22). 14. `a52f9ba659c` refactor: tidy skills copy, editor prop naming, and story fixtures. 15. `f10bc316305` fix: refresh the organization skill list after saving its ACL. Saving an ACL bumps the skill's `updated_at`, so the save now also invalidates the organization skill list and the skill's detail; a new test in `api/queries/skills.test.ts` (now `organizationSkills.test.ts`) covers it. 16. `698a6d0b55f` Merge the updated sharing layer into settings-ui, bringing in #30610's PATCH fix. 17. `aaa356f0fd5` fix: drop the editing hint and field focus from the read-only skill view. From the remote UAT: read-only View no longer shows the Name field's naming hint, and the dialog opens with focus on Close instead of a field (`onOpenAutoFocus`, like `ChatSearchDialog`). The new `SkillEditor.test.tsx` case "starts focus on Close when read-only" fails without the fix. 18. `698dd83946d` fix: drop a deleted skill's ACL caches with its detail. The delete now removes every query under the skill's key, not only the exact detail; a new case in `api/queries/skills.test.ts` (now `organizationSkills.test.ts`) covers it. 19. `f82573cd1c2` fix: carry only a skill-readable organization on the Skills link, with a table-driven `AISettingsSidebar.test.tsx` case. 20. `0e55137de7b` fix: share the skill-readable organization filter (`SkillsPage/readableSkillsOrganizations.ts`, used by the sidebar and `SkillsPage`) and cover an organization that is listed but unreadable. 21. `97d42fd3cd8` refactor: split skill queries by owner. `queries/skills.ts` goes back to main's `userSkills.ts` (main's names and keys) plus a new `organizationSkills.ts` for the organization and ACL queries; the table takes a `SkillsScope` instead of an owner union. 22. `4ddfb80ac54` refactor: invalidate skill lists after writes, instead of patching the cached list; update still writes the detail from the PATCH response. 23. `14d5b755087` refactor: pass skill organizations to the sidebar as `Organization` objects instead of names. 24. `463df6a5e69` feat: pick one organization on the member skills page. 25. `9c802557b9c` refactor: tighten the skill query comments and tests. 26. `9002dc60382` test: cover the failed sharing save in Vitest (`OrganizationSkillSharingDialog.test.tsx`). 27. `a9df756b976` and `5ebf7b8984a` merge the updated sharing layer, which brings in main. `a9df756b976` moves the `toast` imports in `OrganizationSkillSharingDialog.tsx` and `SkillEnabledSwitch.tsx` to `#/components/Toaster/toast`, as main's Biome import rule now requires, and resolves conflicts with main in the model and MCP server sharing dialogs. 28. `ac3e62c83b7` test: assert the member page's organization switch through requests. After a switch, the test checks that the list and the viewed skill are requested from the selected organization (a Codex review finding). 29. `63bb42754e3` fix: show read-only skill status instead of a disabled switch (from jaaydenh's review). `SkillEnabledSwitch` loses its `disabled` prop; a new `ReadOnly` story in `SkillsTableView.stories.tsx` shows the icons, and the read-only case in `SkillsTable.test.tsx` asserted the status icon and no switch (removed again in item 30). 30. `82b2487ad17` test: address Codex review on skills tests. The `SkillsPage.test.tsx` update-only case now edits the description, saves, and asserts the `updateOrganizationSkill` call. The read-only status case in `SkillsTable.test.tsx` is removed because the `ReadOnly` story covers that visual state, and `renderTable` loses its `access` argument. `AISettingsSidebar.test.tsx`'s hoisted permissions mock drops its cast for an explicit return type. 31. `658be8b8df5` merges the updated sharing layer (main at `e7b76872098`); no changes of its own. ## Size Production +1465/-617, tests and stories +1104/-141 (tests +690 net, stories +273 net), generated +4/-0; total +2573/-758. The site changes in this PR and #30613 net +2263 lines, against +3058 for the same scope in the old stack. This is above the 500-line reviewer-friendly limit on purpose: Mike asked for 4 to 5 PRs instead of the 21 small ones this stack replaces. ## Replaces #30590 (site rename), #30595 (site API), #30596 (skills table), #30597 (Enabled toggle), #30598 (organization page), #30599 (ACL autocomplete), #30600 (permissions), #30603 (member page). ## Validation - `pnpm run lint` clean (tsc, oxlint, knip, React Compiler) and Biome clean; `pnpm run lint` reran clean on `13f03d095ff`, which contains this PR up to `0e55137de7b`. - Vitest: 9 files, 49 tests. Storybook Vitest: 7 files, 101 tests. These full runs were on `a52f9ba659c`; on `0e55137de7b`, the three test files the later fixes change (`api/queries/skills.test.ts`, `AISettingsSidebar.test.tsx`, `SkillEditor.test.tsx`) and the two `SkillsPage` tests that cover the changed `SkillsPage.tsx` pass, 20 tests. - On `a6f8f63ad13`, which contains this PR up to `5ebf7b8984a` and both main merges: 38 Vitest files (the skill queries, the sidebar, the skill pages and components, the slash menu, and `pages/AgentsPage/utils`) pass, 542 tests with 2 skipped. - On `63bb42754e3`: `pnpm check` and the full `pnpm run lint` pass; Vitest (`SkillsTable`, `SkillsPage`, `AgentSettingsSkillsPage`: 6 tests) and Storybook Vitest (the `SkillsTableView`, `AgentSettingsSkillsPage` and `SkillsPage` stories: 34 tests) pass; `coder-frontend-review` passed. - On `82b2487ad17`: `pnpm check`, the full `pnpm run lint`, and Vitest on the three changed test files (7 tests) pass. - On `658be8b8df5`: `go build`, `pnpm check` and the full `pnpm lint` pass; 6 Vitest files with 39 tests pass. - `frontend-review` (FE1 to FE10) and `coder-frontend-review` (8 reviewer roles); findings fixed or declined as noted above. - Remote UAT round 7 through Coder Agents passed on the whole stack, last on `13f03d095ff`, which contains this PR up to `0e55137de7b`; see #30613's Validation for scope and findings. - Remote UAT round 8 passed on `b03c14c86de` (one item graded on intent; see #30613), which contains this PR up to `9c802557b9c`: the member page organization picker, list refresh after writes on the personal and admin tables, no stale ACL after deleting and recreating a skill, and the sidebar Skills `?org=`. See #30613 for details. `9002dc60382`, `ac3e62c83b7`, `63bb42754e3`, `82b2487ad17` and the main merges have tests but no UAT run. > Xum opened this PR on behalf of @ibetitsmike. --- site/permissions.json | 4 + site/src/api/api.ts | 87 ++++++ .../api/queries/organizationSkills.test.ts | 88 ++++++ site/src/api/queries/organizationSkills.ts | 134 +++++++++ site/src/api/queries/userSkills.test.ts | 53 ++-- site/src/api/queries/userSkills.ts | 58 ++-- site/src/modules/dashboard/Navbar/Navbar.tsx | 1 + .../management/AISettingsSidebar.test.tsx | 59 +++- .../modules/management/AISettingsSidebar.tsx | 5 + .../AISettingsSidebarView.stories.tsx | 13 + .../management/AISettingsSidebarView.tsx | 23 ++ site/src/modules/permissions/organizations.ts | 35 +++ .../AISettingsIndexRedirect.test.tsx | 27 ++ .../AISettingsIndexRedirect.tsx | 4 + .../MCPServerPrincipalAutocomplete.tsx | 109 -------- .../components/MCPServerSharingDialog.tsx | 69 +---- .../ChatModelSharingDialog.test.tsx | 91 ++++++ .../components/ChatModelSharingDialog.tsx | 74 +---- ...OrganizationSkillSharingDialog.stories.tsx | 53 ++++ .../OrganizationSkillSharingDialog.test.tsx | 126 +++++++++ .../OrganizationSkillSharingDialog.tsx | 81 ++++++ .../SkillsPage/SkillsPage.stories.tsx | 104 +++++++ .../SkillsPage/SkillsPage.test.tsx | 89 ++++++ .../AISettingsPage/SkillsPage/SkillsPage.tsx | 169 ++++++++++++ .../SkillsPage/readableSkillsOrganizations.ts | 14 + .../ACLPrincipalAutocomplete.tsx} | 46 ++-- .../components/ResourceSharingDialog.tsx | 4 +- .../AISettingsPage/components/aclSharing.ts | 55 ++++ .../AgentSettingsSkillsPage.stories.tsx | 68 +++++ .../AgentSettingsSkillsPage.test.tsx | 107 ++++++++ .../AgentsPage/AgentSettingsSkillsPage.tsx | 134 +++++++++ .../ChatMessageInput/ChatMessageInput.tsx | 10 +- .../SkillsTriggerMenu.stories.tsx | 2 +- .../ChatMessageInput/SkillsTriggerPlugin.tsx | 4 +- .../dialogs/ChatSearchDialog.stories.tsx | 2 +- .../ChatsSidebar/settings/SettingsPanel.tsx | 6 +- .../AgentsPage/components/RetryButton.tsx | 18 ++ .../AgentsPage/components/SectionHeader.tsx | 8 +- ...llEditor.test.tsx => SkillEditor.test.tsx} | 24 +- ...ersonalSkillEditor.tsx => SkillEditor.tsx} | 252 +++++++++-------- .../components/SkillEnabledSwitch.tsx | 41 +++ .../components/SkillsTable.test.tsx | 78 ++++++ .../SkillsTable.tsx} | 160 ++++++----- .../SkillsTableView.stories.tsx} | 81 ++++-- .../SkillsTableView.tsx} | 258 ++++++++++++------ .../components/useDialogFocusReturn.ts | 45 +++ ...{personalSkills.test.ts => skills.test.ts} | 131 ++++----- .../utils/{personalSkills.ts => skills.ts} | 166 ++++++++--- site/src/router.test.tsx | 21 ++ site/src/router.tsx | 12 +- site/src/testHelpers/entities.ts | 20 ++ site/src/testHelpers/skills.ts | 8 + 52 files changed, 2573 insertions(+), 758 deletions(-) create mode 100644 site/src/api/queries/organizationSkills.test.ts create mode 100644 site/src/api/queries/organizationSkills.ts delete mode 100644 site/src/pages/AISettingsPage/MCPServersPage/components/MCPServerPrincipalAutocomplete.tsx create mode 100644 site/src/pages/AISettingsPage/ModelsPage/components/ChatModelSharingDialog.test.tsx create mode 100644 site/src/pages/AISettingsPage/SkillsPage/OrganizationSkillSharingDialog.stories.tsx create mode 100644 site/src/pages/AISettingsPage/SkillsPage/OrganizationSkillSharingDialog.test.tsx create mode 100644 site/src/pages/AISettingsPage/SkillsPage/OrganizationSkillSharingDialog.tsx create mode 100644 site/src/pages/AISettingsPage/SkillsPage/SkillsPage.stories.tsx create mode 100644 site/src/pages/AISettingsPage/SkillsPage/SkillsPage.test.tsx create mode 100644 site/src/pages/AISettingsPage/SkillsPage/SkillsPage.tsx create mode 100644 site/src/pages/AISettingsPage/SkillsPage/readableSkillsOrganizations.ts rename site/src/pages/AISettingsPage/{ModelsPage/components/ChatModelPrincipalAutocomplete.tsx => components/ACLPrincipalAutocomplete.tsx} (70%) create mode 100644 site/src/pages/AISettingsPage/components/aclSharing.ts create mode 100644 site/src/pages/AgentsPage/AgentSettingsSkillsPage.stories.tsx create mode 100644 site/src/pages/AgentsPage/AgentSettingsSkillsPage.test.tsx create mode 100644 site/src/pages/AgentsPage/AgentSettingsSkillsPage.tsx create mode 100644 site/src/pages/AgentsPage/components/RetryButton.tsx rename site/src/pages/AgentsPage/components/{PersonalSkillEditor.test.tsx => SkillEditor.test.tsx} (85%) rename site/src/pages/AgentsPage/components/{PersonalSkillEditor.tsx => SkillEditor.tsx} (64%) create mode 100644 site/src/pages/AgentsPage/components/SkillEnabledSwitch.tsx create mode 100644 site/src/pages/AgentsPage/components/SkillsTable.test.tsx rename site/src/pages/AgentsPage/{AgentSettingsPersonalSkillsPage.tsx => components/SkillsTable.tsx} (68%) rename site/src/pages/AgentsPage/{AgentSettingsPersonalSkillsPageView.stories.tsx => components/SkillsTableView.stories.tsx} (88%) rename site/src/pages/AgentsPage/{AgentSettingsPersonalSkillsPageView.tsx => components/SkillsTableView.tsx} (57%) create mode 100644 site/src/pages/AgentsPage/components/useDialogFocusReturn.ts rename site/src/pages/AgentsPage/utils/{personalSkills.test.ts => skills.test.ts} (68%) rename site/src/pages/AgentsPage/utils/{personalSkills.ts => skills.ts} (54%) create mode 100644 site/src/router.test.tsx diff --git a/site/permissions.json b/site/permissions.json index c9a0ad9438..5c19943408 100644 --- a/site/permissions.json +++ b/site/permissions.json @@ -99,6 +99,10 @@ "object": { "resource_type": "mcp_server_config", "any_org": true }, "action": "read" }, + "viewAnyOrganizationSkills": { + "object": { "resource_type": "organization_skill", "any_org": true }, + "action": "read" + }, "createAnyMCPServerConfig": { "object": { "resource_type": "mcp_server_config", "any_org": true }, "action": "create" diff --git a/site/src/api/api.ts b/site/src/api/api.ts index 5fdf20e62e..f9ce795bf6 100644 --- a/site/src/api/api.ts +++ b/site/src/api/api.ts @@ -362,6 +362,12 @@ const userSkillsPath = (user: string) => `/api/experimental/users/${encodeURIComponent(user)}/skills`; const userSkillPath = (user: string, name: string) => `${userSkillsPath(user)}/${encodeURIComponent(name)}`; +const organizationSkillsPath = (organizationId: string) => + `/api/experimental/organizations/${encodeURIComponent(organizationId)}/skills`; +const organizationSkillPath = (organizationId: string, name: string) => + `${organizationSkillsPath(organizationId)}/${encodeURIComponent(name)}`; +const organizationSkillACLPath = (organizationId: string, name: string) => + `${organizationSkillPath(organizationId, name)}/acl`; const userAIProviderKeysPath = (user = "me") => `/api/v2/users/${encodeURIComponent(user)}/ai-provider-keys`; const mcpServerConfigsPath = (organization: string) => @@ -3894,6 +3900,87 @@ class ExperimentalApiMethods { await this.axios.delete(userSkillPath(user, name)); }; + createOrganizationSkill = async ( + organizationId: string, + req: TypesGen.CreateSkillRequest, + ): Promise => { + const response = await this.axios.post( + organizationSkillsPath(organizationId), + req, + ); + return response.data; + }; + + getOrganizationSkills = async ( + organizationId: string, + ): Promise => { + const response = await this.axios.get( + organizationSkillsPath(organizationId), + ); + return response.data; + }; + + getOrganizationSkillByName = async ( + organizationId: string, + name: string, + ): Promise => { + const response = await this.axios.get( + organizationSkillPath(organizationId, name), + ); + return response.data; + }; + + updateOrganizationSkill = async ( + organizationId: string, + name: string, + req: TypesGen.UpdateSkillRequest, + ): Promise => { + const response = await this.axios.patch( + organizationSkillPath(organizationId, name), + req, + ); + return response.data; + }; + + deleteOrganizationSkill = async ( + organizationId: string, + name: string, + ): Promise => { + await this.axios.delete(organizationSkillPath(organizationId, name)); + }; + + getOrganizationSkillACL = async ( + organizationId: string, + name: string, + ): Promise => { + const response = await this.axios.get( + organizationSkillACLPath(organizationId, name), + ); + return response.data; + }; + + getOrganizationSkillACLAvailable = async ( + organizationId: string, + name: string, + options: TypesGen.UsersRequest, + ): Promise => { + const response = await this.axios.get( + getURLWithSearchParams( + `${organizationSkillACLPath(organizationId, name)}/available`, + options, + ), + ); + return response.data; + }; + + updateOrganizationSkillACL = async ( + organizationId: string, + name: string, + req: TypesGen.UpdateOrganizationSkillACLRequest, + ): Promise => { + await this.axios.patch(organizationSkillACLPath(organizationId, name), req); + }; + getUserChatCompactionThresholds = async (): Promise => { const response = diff --git a/site/src/api/queries/organizationSkills.test.ts b/site/src/api/queries/organizationSkills.test.ts new file mode 100644 index 0000000000..41c47bb13a --- /dev/null +++ b/site/src/api/queries/organizationSkills.test.ts @@ -0,0 +1,88 @@ +import { QueryClient } from "react-query"; +import { describe, expect, it } from "vitest"; +import type { Skill } from "#/api/typesGenerated"; +import { + deleteOrganizationSkill, + organizationSkill, + organizationSkillACL, + organizationSkillACLAvailable, + organizationSkills, + updateOrganizationSkillACL, +} from "./organizationSkills"; + +const createTestQueryClient = (): QueryClient => + new QueryClient({ + defaultOptions: { + queries: { + retry: false, + gcTime: Number.POSITIVE_INFINITY, + refetchOnWindowFocus: false, + networkMode: "offlineFirst", + }, + }, + }); + +const mockAlphaSkill: Skill = { + id: "alpha-id", + name: "alpha", + description: "alpha description", + enabled: true, + content: "---\nname: alpha\n---\nBody\n", + created_at: "2026-05-21T00:00:00Z", + updated_at: "2026-05-21T00:00:00Z", +}; + +const { content: _content, ...mockAlphaSkillMetadata } = mockAlphaSkill; + +describe("organization skill ACL queries", () => { + it("removes a deleted skill's ACL caches", async () => { + const queryClient = createTestQueryClient(); + const aclKey = organizationSkillACL("org", "alpha").queryKey; + const availableKey = organizationSkillACLAvailable("org", "alpha", { + q: "", + }).queryKey; + queryClient.setQueryData(organizationSkills("org").queryKey, [ + mockAlphaSkillMetadata, + ]); + queryClient.setQueryData(aclKey, { users: [], groups: [] }); + queryClient.setQueryData(availableKey, { users: [], groups: [] }); + + await deleteOrganizationSkill(queryClient, "org").onSuccess( + undefined, + "alpha", + ); + + expect(queryClient.getQueryData(aclKey)).toBeUndefined(); + expect(queryClient.getQueryData(availableKey)).toBeUndefined(); + expect( + queryClient.getQueryState(organizationSkills("org").queryKey) + ?.isInvalidated, + ).toBe(true); + }); + + it("invalidates the skill list and detail after saving the ACL", async () => { + const queryClient = createTestQueryClient(); + queryClient.setQueryData(organizationSkills("org").queryKey, [ + mockAlphaSkillMetadata, + ]); + queryClient.setQueryData( + organizationSkill("org", "alpha").queryKey, + mockAlphaSkill, + ); + + await updateOrganizationSkillACL(queryClient).onSuccess(undefined, { + organizationId: "org", + name: "alpha", + req: {}, + }); + + expect( + queryClient.getQueryState(organizationSkills("org").queryKey) + ?.isInvalidated, + ).toBe(true); + expect( + queryClient.getQueryState(organizationSkill("org", "alpha").queryKey) + ?.isInvalidated, + ).toBe(true); + }); +}); diff --git a/site/src/api/queries/organizationSkills.ts b/site/src/api/queries/organizationSkills.ts new file mode 100644 index 0000000000..2f2efbaf6c --- /dev/null +++ b/site/src/api/queries/organizationSkills.ts @@ -0,0 +1,134 @@ +import type { QueryClient } from "react-query"; +import { API } from "#/api/api"; +import type * as TypesGen from "#/api/typesGenerated"; + +const organizationSkillsKey = (organizationId: string) => + ["organization-skills", organizationId] as const; + +const organizationSkillKey = (organizationId: string, name: string) => + [...organizationSkillsKey(organizationId), name] as const; + +export const organizationSkills = (organizationId: string) => ({ + queryKey: organizationSkillsKey(organizationId), + queryFn: (): Promise => + API.experimental.getOrganizationSkills(organizationId), +}); + +export const organizationSkill = (organizationId: string, name: string) => ({ + queryKey: organizationSkillKey(organizationId, name), + queryFn: (): Promise => + API.experimental.getOrganizationSkillByName(organizationId, name), +}); + +const organizationSkillACLKey = (organizationId: string, name: string) => + [...organizationSkillKey(organizationId, name), "acl"] as const; + +export const organizationSkillACL = (organizationId: string, name: string) => ({ + queryKey: organizationSkillACLKey(organizationId, name), + queryFn: (): Promise => + API.experimental.getOrganizationSkillACL(organizationId, name), +}); + +export const organizationSkillACLAvailable = ( + organizationId: string, + name: string, + options: TypesGen.UsersRequest, +) => ({ + queryKey: [ + ...organizationSkillACLKey(organizationId, name), + "available", + options, + ] as const, + queryFn: (): Promise => + API.experimental.getOrganizationSkillACLAvailable( + organizationId, + name, + options, + ), +}); + +type UpdateOrganizationSkillACLArgs = { + organizationId: string; + name: string; + req: TypesGen.UpdateOrganizationSkillACLRequest; +}; + +export const updateOrganizationSkillACL = (queryClient: QueryClient) => ({ + mutationFn: ({ organizationId, name, req }: UpdateOrganizationSkillACLArgs) => + API.experimental.updateOrganizationSkillACL(organizationId, name, req), + onSuccess: async ( + _data: unknown, + { organizationId, name }: UpdateOrganizationSkillACLArgs, + ) => { + // Saving an ACL bumps the skill's updated_at, so its metadata is stale. + await Promise.all([ + queryClient.invalidateQueries({ + queryKey: organizationSkillACLKey(organizationId, name), + exact: true, + }), + queryClient.invalidateQueries({ + queryKey: organizationSkillsKey(organizationId), + exact: true, + }), + queryClient.invalidateQueries({ + queryKey: organizationSkillKey(organizationId, name), + exact: true, + }), + ]); + }, +}); + +export const createOrganizationSkill = ( + queryClient: QueryClient, + organizationId: string, +) => ({ + mutationFn: (req: TypesGen.CreateSkillRequest) => + API.experimental.createOrganizationSkill(organizationId, req), + onSuccess: () => + queryClient.invalidateQueries({ + queryKey: organizationSkillsKey(organizationId), + exact: true, + }), +}); + +type UpdateOrganizationSkillArgs = { + name: string; + req: TypesGen.UpdateSkillRequest; +}; + +export const updateOrganizationSkill = ( + queryClient: QueryClient, + organizationId: string, +) => ({ + mutationFn: ({ name, req }: UpdateOrganizationSkillArgs) => + API.experimental.updateOrganizationSkill(organizationId, name, req), + // Invalidation refetches only active queries, so write the PATCH response + // to keep a cached detail no editor observes from showing old content. + onSuccess: (skill: TypesGen.Skill, { name }: UpdateOrganizationSkillArgs) => { + queryClient.setQueryData(organizationSkillKey(organizationId, name), skill); + }, + onSettled: () => + queryClient.invalidateQueries({ + queryKey: organizationSkillsKey(organizationId), + exact: true, + }), +}); + +export const deleteOrganizationSkill = ( + queryClient: QueryClient, + organizationId: string, +) => ({ + mutationFn: (name: string) => + API.experimental.deleteOrganizationSkill(organizationId, name), + onSuccess: (_data: unknown, name: string) => { + // Not exact: the ACL queries are keyed under the skill, and a skill + // recreated with the same name must not inherit them. + queryClient.removeQueries({ + queryKey: organizationSkillKey(organizationId, name), + }); + return queryClient.invalidateQueries({ + queryKey: organizationSkillsKey(organizationId), + exact: true, + }); + }, +}); diff --git a/site/src/api/queries/userSkills.test.ts b/site/src/api/queries/userSkills.test.ts index d883f15d9b..b72fc9b23d 100644 --- a/site/src/api/queries/userSkills.test.ts +++ b/site/src/api/queries/userSkills.test.ts @@ -53,70 +53,57 @@ describe("user skill queries", () => { ]); }); - it("adds a created skill to the sorted list cache", () => { + it("invalidates the list after a create", async () => { const queryClient = createTestQueryClient(); - const alpha = makeSkill("alpha"); - const zeta = makeSkill("zeta"); - queryClient.setQueryData(userSkills().queryKey, [toMetadata(zeta)]); + queryClient.setQueryData(userSkills().queryKey, []); - createUserSkill(queryClient).onSuccess(alpha); + await createUserSkill(queryClient).onSuccess(); - expect(queryClient.getQueryData(userSkills().queryKey)).toEqual([ - toMetadata(alpha), - toMetadata(zeta), - ]); - expect(queryClient.getQueryData(userSkill("alpha").queryKey)).toEqual( - alpha, - ); + expect( + queryClient.getQueryState(userSkills().queryKey)?.isInvalidated, + ).toBe(true); }); - it("updates list and detail caches for an updated skill", () => { + it("writes the detail and invalidates the list after an update", async () => { const queryClient = createTestQueryClient(); const alpha = makeSkill("alpha"); - const beta = makeSkill("beta"); const updatedAlpha = makeSkill("alpha", { description: "updated description", content: "---\nname: alpha\ndescription: updated description\n---\nUpdated\n", updated_at: "2026-05-21T01:00:00Z", }); - queryClient.setQueryData(userSkills().queryKey, [ - toMetadata(alpha), - toMetadata(beta), - ]); + queryClient.setQueryData(userSkills().queryKey, [toMetadata(alpha)]); queryClient.setQueryData(userSkill("alpha").queryKey, alpha); - updateUserSkill(queryClient).onSuccess(updatedAlpha, { + const options = updateUserSkill(queryClient); + options.onSuccess(updatedAlpha, { name: "alpha", req: { content: updatedAlpha.content }, }); + await options.onSettled(); - expect(queryClient.getQueryData(userSkills().queryKey)).toEqual([ - toMetadata(updatedAlpha), - toMetadata(beta), - ]); expect(queryClient.getQueryData(userSkill("alpha").queryKey)).toEqual( updatedAlpha, ); + expect( + queryClient.getQueryState(userSkills().queryKey)?.isInvalidated, + ).toBe(true); }); - it("removes a deleted skill from list and detail caches", () => { + it("removes a deleted skill's detail and invalidates the list", async () => { const queryClient = createTestQueryClient(); const alpha = makeSkill("alpha"); - const beta = makeSkill("beta"); - queryClient.setQueryData(userSkills().queryKey, [ - toMetadata(alpha), - toMetadata(beta), - ]); + queryClient.setQueryData(userSkills().queryKey, [toMetadata(alpha)]); queryClient.setQueryData(userSkill("alpha").queryKey, alpha); - deleteUserSkill(queryClient).onSuccess(undefined, "alpha"); + await deleteUserSkill(queryClient).onSuccess(undefined, "alpha"); - expect(queryClient.getQueryData(userSkills().queryKey)).toEqual([ - toMetadata(beta), - ]); expect( queryClient.getQueryData(userSkill("alpha").queryKey), ).toBeUndefined(); + expect( + queryClient.getQueryState(userSkills().queryKey)?.isInvalidated, + ).toBe(true); }); }); diff --git a/site/src/api/queries/userSkills.ts b/site/src/api/queries/userSkills.ts index db23939018..927a0977c6 100644 --- a/site/src/api/queries/userSkills.ts +++ b/site/src/api/queries/userSkills.ts @@ -7,30 +7,6 @@ const userSkillsKey = (user = "me") => ["user-skills", user] as const; const userSkillKey = (name: string, user = "me") => [...userSkillsKey(user), name] as const; -const toUserSkillMetadata = ( - skill: TypesGen.Skill, -): TypesGen.SkillMetadata => ({ - id: skill.id, - name: skill.name, - description: skill.description, - enabled: skill.enabled, - created_at: skill.created_at, - updated_at: skill.updated_at, -}); - -const sortUserSkillMetadata = ( - skills: TypesGen.SkillMetadata[], -): TypesGen.SkillMetadata[] => - skills.toSorted((a, b) => a.name.localeCompare(b.name, "en-US")); - -const upsertUserSkillMetadata = ( - skills: TypesGen.SkillMetadata[] | undefined, - skill: TypesGen.SkillMetadata, -): TypesGen.SkillMetadata[] => { - const withoutSkill = skills?.filter(({ name }) => name !== skill.name) ?? []; - return sortUserSkillMetadata([...withoutSkill, skill]); -}; - export const userSkills = (user = "me") => ({ queryKey: userSkillsKey(user), queryFn: (): Promise => @@ -46,13 +22,11 @@ export const userSkill = (name: string, user = "me") => ({ export const createUserSkill = (queryClient: QueryClient, user = "me") => ({ mutationFn: (req: TypesGen.CreateSkillRequest) => API.experimental.createUserSkill(user, req), - onSuccess: (skill: TypesGen.Skill) => { - queryClient.setQueryData( - userSkillsKey(user), - (skills) => upsertUserSkillMetadata(skills, toUserSkillMetadata(skill)), - ); - queryClient.setQueryData(userSkillKey(skill.name, user), skill); - }, + onSuccess: () => + queryClient.invalidateQueries({ + queryKey: userSkillsKey(user), + exact: true, + }), }); type UpdateUserSkillArgs = { @@ -63,16 +37,16 @@ type UpdateUserSkillArgs = { export const updateUserSkill = (queryClient: QueryClient, user = "me") => ({ mutationFn: ({ name, req }: UpdateUserSkillArgs) => API.experimental.updateUserSkill(user, name, req), + // Invalidation refetches only active queries, so write the PATCH response + // to keep a cached detail no editor observes from showing old content. onSuccess: (skill: TypesGen.Skill, { name }: UpdateUserSkillArgs) => { queryClient.setQueryData(userSkillKey(name, user), skill); - queryClient.setQueryData( - userSkillsKey(user), - (skills) => - skills - ? upsertUserSkillMetadata(skills, toUserSkillMetadata(skill)) - : skills, - ); }, + onSettled: () => + queryClient.invalidateQueries({ + queryKey: userSkillsKey(user), + exact: true, + }), }); export const deleteUserSkill = (queryClient: QueryClient, user = "me") => ({ @@ -82,9 +56,9 @@ export const deleteUserSkill = (queryClient: QueryClient, user = "me") => ({ queryKey: userSkillKey(name, user), exact: true, }); - queryClient.setQueryData( - userSkillsKey(user), - (skills) => skills?.filter((skill) => skill.name !== name), - ); + return queryClient.invalidateQueries({ + queryKey: userSkillsKey(user), + exact: true, + }); }, }); diff --git a/site/src/modules/dashboard/Navbar/Navbar.tsx b/site/src/modules/dashboard/Navbar/Navbar.tsx index e900c6dbe7..9275838152 100644 --- a/site/src/modules/dashboard/Navbar/Navbar.tsx +++ b/site/src/modules/dashboard/Navbar/Navbar.tsx @@ -46,6 +46,7 @@ export const Navbar: React.FC = () => { permissions.createAnyMCPServerConfig || permissions.updateAnyMCPServerConfig || permissions.deleteAnyMCPServerConfig || + permissions.viewAnyOrganizationSkills || permissions.updateAnyTemplate || canAccessAnyModel; const organizationMCPSharing = useCanShareOrganizationMCPServers( diff --git a/site/src/modules/management/AISettingsSidebar.test.tsx b/site/src/modules/management/AISettingsSidebar.test.tsx index 1c0eb50e63..e08db3f1b2 100644 --- a/site/src/modules/management/AISettingsSidebar.test.tsx +++ b/site/src/modules/management/AISettingsSidebar.test.tsx @@ -1,12 +1,14 @@ -import { screen } from "@testing-library/react"; +import { screen, waitFor } from "@testing-library/react"; import userEvent from "@testing-library/user-event"; import { createMemoryRouter } from "react-router"; -import { expect, it, vi } from "vitest"; +import { beforeEach, expect, it, vi } from "vitest"; import { API, withDefaultFeatures } from "#/api/api"; +import type { Permissions } from "#/modules/permissions"; import { MockEntitlements, MockNoPermissions, MockOrganization, + MockOrganization2, MockUserMember, } from "#/testHelpers/entities"; import { renderWithRouter } from "#/testHelpers/renderHelpers"; @@ -19,19 +21,27 @@ const mockAIGatewayEntitlements = { }), }; +const mockPermissions = vi.hoisted((): { current: Partial } => ({ + current: {}, +})); + vi.mock("#/hooks/useAuthenticated", () => ({ useAuthenticated: () => ({ user: MockUserMember, - permissions: MockNoPermissions, + permissions: { ...MockNoPermissions, ...mockPermissions.current }, }), })); vi.mock("#/modules/dashboard/useDashboard", () => ({ useDashboard: () => ({ entitlements: mockAIGatewayEntitlements, - organizations: [MockOrganization], + organizations: [MockOrganization, MockOrganization2], }), })); +beforeEach(() => { + mockPermissions.current = {}; +}); + it("links organization group member readers to the Spend page", async () => { const user = userEvent.setup(); vi.spyOn(API, "getOrganizations").mockResolvedValue([MockOrganization]); @@ -79,3 +89,44 @@ it("links organization instruction readers to the Instructions page", async () = expect(router.state.location.pathname).toBe("/ai/settings/instructions"); expect(router.state.location.search).toBe("?org=second"); }); + +it.each([ + { requested: MockOrganization2.name, expectedSearch: "" }, + { + requested: MockOrganization.name, + expectedSearch: `?org=${MockOrganization.name}`, + }, +])( + "keeps ?org=$requested on the Skills link only when the user can read its skills", + async ({ requested, expectedSearch }) => { + const user = userEvent.setup(); + mockPermissions.current = { viewAnyOrganizationSkills: true }; + vi.spyOn(API, "getOrganizations").mockResolvedValue([MockOrganization]); + vi.spyOn(API, "checkAuthorization").mockResolvedValue({ + [`${MockOrganization.id}.viewOrganizationSkills`]: true, + }); + vi.spyOn(API.experimental, "getChatModels").mockResolvedValue({ + models: [], + providers: [], + unsupported_providers: [], + }); + const router = createMemoryRouter( + [ + { path: "/ai/settings", element: }, + { path: "/ai/settings/skills", element:
}, + ], + { initialEntries: [`/ai/settings?org=${requested}`] }, + ); + renderWithRouter(router); + const skillsLink = await screen.findByRole("link", { name: "Skills" }); + await waitFor(() => + expect(skillsLink).toHaveAttribute( + "href", + `/ai/settings/skills${expectedSearch}`, + ), + ); + await user.click(skillsLink); + expect(router.state.location.pathname).toBe("/ai/settings/skills"); + expect(router.state.location.search).toBe(expectedSearch); + }, +); diff --git a/site/src/modules/management/AISettingsSidebar.tsx b/site/src/modules/management/AISettingsSidebar.tsx index 97365e9af0..66f37995f1 100644 --- a/site/src/modules/management/AISettingsSidebar.tsx +++ b/site/src/modules/management/AISettingsSidebar.tsx @@ -6,6 +6,7 @@ import AISettingsSidebarView from "#/modules/management/AISettingsSidebarView"; import { readableInstructionsOrganizations } from "#/pages/AISettingsPage/InstructionsPage/readableInstructionsOrganizations"; import { useCanShareOrganizationMCPServers } from "#/pages/AISettingsPage/MCPServersPage/organizationSharing"; import { useAccessibleModelOrganizations } from "#/pages/AISettingsPage/ModelsPage/organizationModels"; +import { readableSkillsOrganizations } from "#/pages/AISettingsPage/SkillsPage/readableSkillsOrganizations"; import { canViewAISpend } from "#/pages/AISettingsPage/SpendPage/spendAccess"; /** @@ -41,6 +42,10 @@ export const AISettingsSidebar: React.FC = () => { accessibleOrgsQuery.permissionsByOrganization, ).length > 0 } + skillOrganizations={readableSkillsOrganizations( + organizations, + accessibleOrgsQuery.permissionsByOrganization, + )} /> ); }; diff --git a/site/src/modules/management/AISettingsSidebarView.stories.tsx b/site/src/modules/management/AISettingsSidebarView.stories.tsx index bdcc352a09..f76c3c0fb3 100644 --- a/site/src/modules/management/AISettingsSidebarView.stories.tsx +++ b/site/src/modules/management/AISettingsSidebarView.stories.tsx @@ -45,6 +45,7 @@ const aiSettingsRoutes: [RouterRoute, ...RouterRoute[]] = [ { path: "/ai/settings/mcp-servers/add", useStoryElement: true }, { path: "/ai/settings/templates", useStoryElement: true }, { path: "/ai/settings/instructions", useStoryElement: true }, + { path: "/ai/settings/skills", useStoryElement: true }, { path: "/ai/settings/lifecycle", useStoryElement: true }, ]; @@ -410,6 +411,18 @@ export const InstructionsForOrganizationAdmin: Story = { }, }; +export const SkillsForOrganizationAuditor: Story = { + args: { + permissions: { + ...MockNoPermissions, + viewAnyOrganizationSkills: true, + }, + }, + parameters: { + reactRouter: atLocation("/ai/settings/skills"), + }, +}; + export const NoPermissions: Story = { args: { permissions: MockNoPermissions, diff --git a/site/src/modules/management/AISettingsSidebarView.tsx b/site/src/modules/management/AISettingsSidebarView.tsx index 90688bc385..10f3a68cae 100644 --- a/site/src/modules/management/AISettingsSidebarView.tsx +++ b/site/src/modules/management/AISettingsSidebarView.tsx @@ -6,6 +6,7 @@ import { useMatch, useSearchParams, } from "react-router"; +import type { Organization } from "#/api/typesGenerated"; import { Sidebar as BaseSidebar, SettingsSidebarNavItem as SidebarNavItem, @@ -23,6 +24,8 @@ type AISettingsSidebarViewProps = { canAccessOrganizationModels?: boolean; canShareOrganizationMCPServers?: boolean; canViewOrganizationInstructions: boolean; + /** Organizations whose skills the user can read. */ + skillOrganizations?: readonly Organization[]; }; const SubNavItem: React.FC<{ href: To; children?: React.ReactNode }> = ({ @@ -89,6 +92,7 @@ const AISettingsSidebarView: React.FC = ({ canAccessOrganizationModels = false, canShareOrganizationMCPServers = false, canViewOrganizationInstructions, + skillOrganizations = [], }) => { const [searchParams] = useSearchParams(); const organizationName = searchParams.get(modelOrganizationSearchParam); @@ -116,6 +120,16 @@ const AISettingsSidebarView: React.FC = ({ "/ai/settings/instructions", organizationName, ); + // The Skills page denies an organization the user cannot read rather than + // replacing it, so only carry over an organization it will show. + const skillsPath = organizationScopedPath( + "/ai/settings/skills", + skillOrganizations.some( + (organization) => organization.name === organizationName, + ) + ? organizationName + : null, + ); return ( @@ -152,6 +166,9 @@ const AISettingsSidebarView: React.FC = ({ Templates )} Instructions + {permissions.viewAnyOrganizationSkills && ( + Skills + )} Lifecycle )} @@ -187,6 +204,12 @@ const AISettingsSidebarView: React.FC = ({ Instructions )} + {!permissions.editDeploymentConfig && + permissions.viewAnyOrganizationSkills && ( + + Skills + + )}
); diff --git a/site/src/modules/permissions/organizations.ts b/site/src/modules/permissions/organizations.ts index e70bf473a5..4e2b3ca8a7 100644 --- a/site/src/modules/permissions/organizations.ts +++ b/site/src/modules/permissions/organizations.ts @@ -157,6 +157,41 @@ export const organizationPermissionChecks = (organizationId: string) => }, action: "share", }, + viewOrganizationSkills: { + object: { + resource_type: "organization_skill", + organization_id: organizationId, + }, + action: "read", + }, + createOrganizationSkill: { + object: { + resource_type: "organization_skill", + organization_id: organizationId, + }, + action: "create", + }, + updateOrganizationSkill: { + object: { + resource_type: "organization_skill", + organization_id: organizationId, + }, + action: "update", + }, + deleteOrganizationSkill: { + object: { + resource_type: "organization_skill", + organization_id: organizationId, + }, + action: "delete", + }, + shareOrganizationSkill: { + object: { + resource_type: "organization_skill", + organization_id: organizationId, + }, + action: "share", + }, viewChatModelConfigs: { object: { resource_type: "chat_model_config", diff --git a/site/src/pages/AISettingsPage/AISettingsIndexRedirect.test.tsx b/site/src/pages/AISettingsPage/AISettingsIndexRedirect.test.tsx index 95f7b40bb9..8a7505ead5 100644 --- a/site/src/pages/AISettingsPage/AISettingsIndexRedirect.test.tsx +++ b/site/src/pages/AISettingsPage/AISettingsIndexRedirect.test.tsx @@ -101,3 +101,30 @@ it("redirects an organization group member reader to Spend", async () => { await screen.findByText("Spend"); expect(router.state.location.pathname).toBe("/ai/settings/spend"); }); + +it("redirects an organization skills reader to Skills", async () => { + permissions = { ...MockNoPermissions, viewAnyOrganizationSkills: true }; + vi.spyOn(API.experimental, "getChatModels").mockRejectedValue({ + isAxiosError: true, + response: { status: 403 }, + }); + const queryClient = new QueryClient({ + defaultOptions: { queries: { retry: false } }, + }); + const router = createMemoryRouter( + [ + { path: "/ai/settings", element: }, + { path: "/ai/settings/skills", element:
Skills
}, + ], + { initialEntries: ["/ai/settings"] }, + ); + + render( + + + , + ); + + await screen.findByText("Skills"); + expect(router.state.location.pathname).toBe("/ai/settings/skills"); +}); diff --git a/site/src/pages/AISettingsPage/AISettingsIndexRedirect.tsx b/site/src/pages/AISettingsPage/AISettingsIndexRedirect.tsx index acc53cf7ef..d38dbfb52d 100644 --- a/site/src/pages/AISettingsPage/AISettingsIndexRedirect.tsx +++ b/site/src/pages/AISettingsPage/AISettingsIndexRedirect.tsx @@ -53,6 +53,10 @@ export const AISettingsIndexRedirect = () => { return ; } + if (permissions.viewAnyOrganizationSkills) { + return ; + } + if (accessibleOrgsQuery.isLoading) { return ; } diff --git a/site/src/pages/AISettingsPage/MCPServersPage/components/MCPServerPrincipalAutocomplete.tsx b/site/src/pages/AISettingsPage/MCPServersPage/components/MCPServerPrincipalAutocomplete.tsx deleted file mode 100644 index 20c4ad6bba..0000000000 --- a/site/src/pages/AISettingsPage/MCPServersPage/components/MCPServerPrincipalAutocomplete.tsx +++ /dev/null @@ -1,109 +0,0 @@ -import { CheckIcon } from "lucide-react"; -import { useId, useState } from "react"; -import { keepPreviousData, useQuery } from "react-query"; -import { mcpServerConfigACLAvailable } from "#/api/queries/chats"; -import type { Group, ReducedUser } from "#/api/typesGenerated"; -import { ErrorAlert } from "#/components/Alert/ErrorAlert"; -import { Autocomplete } from "#/components/Autocomplete/Autocomplete"; -import { AvatarData } from "#/components/Avatar/AvatarData"; -import { getGroupSubtitle, isGroup } from "#/modules/groups"; -import { prepareQuery } from "#/utils/filters"; - -export type MCPServerPrincipalAutocompleteValue = ReducedUser | Group | null; -type AutocompleteOption = Exclude; - -type MCPServerPrincipalAutocompleteProps = { - value: MCPServerPrincipalAutocompleteValue; - onChange: (value: MCPServerPrincipalAutocompleteValue) => void; - organizationId: string; - serverId: string; - excludedPrincipalIds: readonly string[]; - className?: string; -}; - -export const MCPServerPrincipalAutocomplete: React.FC< - MCPServerPrincipalAutocompleteProps -> = ({ - value, - onChange, - organizationId, - serverId, - excludedPrincipalIds, - className, -}) => { - const [inputValue, setInputValue] = useState(""); - const [open, setOpen] = useState(false); - const autocompleteId = useId(); - - const handleOpenChange = (newOpen: boolean) => { - setOpen(newOpen); - if (!newOpen) { - setInputValue(""); - } - }; - - const aclAvailableQuery = useQuery({ - ...mcpServerConfigACLAvailable(organizationId, serverId, { - q: prepareQuery(inputValue), - limit: 25, - }), - enabled: open, - placeholderData: keepPreviousData, - }); - - const options: AutocompleteOption[] = aclAvailableQuery.data - ? [ - ...aclAvailableQuery.data.groups, - ...aclAvailableQuery.data.users, - ].filter((principal) => !excludedPrincipalIds.includes(principal.id)) - : []; - - return ( -
- option.id} - getOptionLabel={(option) => - isGroup(option) ? option.display_name || option.name : option.email - } - isOptionEqualToValue={(option, optionValue) => - option.id === optionValue.id - } - renderOption={(option, isSelected) => ( -
- - {isSelected && } -
- )} - open={open} - onOpenChange={handleOpenChange} - inputValue={inputValue} - onInputChange={setInputValue} - loading={aclAvailableQuery.isFetching} - placeholder="Search for user or group" - noOptionsText={ - aclAvailableQuery.error - ? "Unable to load users or groups" - : "No users or groups found" - } - className={className} - id={autocompleteId} - /> - {aclAvailableQuery.error && ( - - )} -
- ); -}; diff --git a/site/src/pages/AISettingsPage/MCPServersPage/components/MCPServerSharingDialog.tsx b/site/src/pages/AISettingsPage/MCPServersPage/components/MCPServerSharingDialog.tsx index 5c20ec1fdc..aa2ac94e26 100644 --- a/site/src/pages/AISettingsPage/MCPServersPage/components/MCPServerSharingDialog.tsx +++ b/site/src/pages/AISettingsPage/MCPServersPage/components/MCPServerSharingDialog.tsx @@ -1,21 +1,16 @@ import { useMutation, useQuery, useQueryClient } from "react-query"; import { mcpServerConfigACL, + mcpServerConfigACLAvailable, updateMCPServerConfigACL, } from "#/api/queries/chats"; -import type * as TypesGen from "#/api/typesGenerated"; import { toast } from "#/components/Toaster/toast"; -import { getGroupSubtitle, isGroup } from "#/modules/groups"; +import { ACLPrincipalAutocomplete } from "../../components/ACLPrincipalAutocomplete"; import { - ResourceSharingDialog, - type SharingDialogData, - type SharingPrincipal, - type SharingPrincipalSelection, -} from "../../components/ResourceSharingDialog"; -import { - MCPServerPrincipalAutocomplete, - type MCPServerPrincipalAutocompleteValue, -} from "./MCPServerPrincipalAutocomplete"; + selectedPrincipal, + sharingDialogData, +} from "../../components/aclSharing"; +import { ResourceSharingDialog } from "../../components/ResourceSharingDialog"; type MCPServerSharingDialogProps = { open: boolean; @@ -25,50 +20,6 @@ type MCPServerSharingDialogProps = { serverName: string; }; -type MCPServerPrincipal = Exclude; - -const groupPrincipal = (group: TypesGen.Group): SharingPrincipal => ({ - id: group.id, - name: group.display_name || group.name, - subtitle: getGroupSubtitle(group), - avatarUrl: group.avatar_url, -}); - -const userPrincipal = (user: TypesGen.MinimalUser): SharingPrincipal => ({ - id: user.id, - name: user.username, - subtitle: user.name || "User", - avatarUrl: user.avatar_url, -}); - -const sharingDialogData = ( - acl: TypesGen.MCPServerConfigACL, -): SharingDialogData => ({ - acl: { - user_roles: Object.fromEntries( - acl.users.map((user) => [user.id, user.role]), - ), - group_roles: Object.fromEntries( - acl.groups.map((group) => [group.id, group.role]), - ), - }, - principals: { - users: Object.fromEntries( - acl.users.map((user) => [user.id, userPrincipal(user)]), - ), - groups: Object.fromEntries( - acl.groups.map((group) => [group.id, groupPrincipal(group)]), - ), - }, -}); - -const selectedPrincipal = ( - option: MCPServerPrincipal, -): SharingPrincipalSelection => - isGroup(option) - ? { kind: "group", principal: groupPrincipal(option) } - : { kind: "user", principal: userPrincipal(option) }; - type OpenMCPServerSharingDialogProps = Omit< MCPServerSharingDialogProps, "open" @@ -110,13 +61,13 @@ const OpenMCPServerSharingDialog: React.FC = ({ readRole="read" deletedRole="" renderAutocomplete={({ value, onChange, excludedPrincipalIds }) => ( - + mcpServerConfigACLAvailable(organizationId, serverId, options) + } excludedPrincipalIds={excludedPrincipalIds} - className="w-full" /> )} getPrincipal={selectedPrincipal} diff --git a/site/src/pages/AISettingsPage/ModelsPage/components/ChatModelSharingDialog.test.tsx b/site/src/pages/AISettingsPage/ModelsPage/components/ChatModelSharingDialog.test.tsx new file mode 100644 index 0000000000..184aa16f86 --- /dev/null +++ b/site/src/pages/AISettingsPage/ModelsPage/components/ChatModelSharingDialog.test.tsx @@ -0,0 +1,91 @@ +import { screen, waitFor } from "@testing-library/react"; +import userEvent from "@testing-library/user-event"; +import { HttpResponse, http, type PathParams } from "msw"; +import { QueryClientProvider } from "react-query"; +import { describe, expect, it, vi } from "vitest"; +import type { + ChatModelACL, + UpdateChatModelACLRequest, +} from "#/api/typesGenerated"; +import { + MockChatModelACLAvailable, + MockGroup, + MockOrganization, + MockUserOwner, +} from "#/testHelpers/entities"; +import { + createTestQueryClient, + renderComponent, +} from "#/testHelpers/renderHelpers"; +import { server } from "#/testHelpers/server"; +import { ChatModelSharingDialog } from "./ChatModelSharingDialog"; + +const aclPath = "/api/v2/organizations/:organization/chats/models/:model/acl"; + +describe("ChatModelSharingDialog", () => { + it("saves only the changed entries", async () => { + const acl: ChatModelACL = { + users: [], + groups: [{ ...MockGroup, role: "read" }], + }; + const updates: unknown[] = []; + server.use( + http.get(aclPath, () => HttpResponse.json(acl)), + http.get(`${aclPath}/available`, () => + HttpResponse.json(MockChatModelACLAvailable), + ), + http.patch( + aclPath, + async ({ params, request }) => { + updates.push({ ...params, body: await request.json() }); + return new HttpResponse(null, { status: 204 }); + }, + ), + ); + const onOpenChange = vi.fn(); + const user = userEvent.setup(); + renderComponent( + + + , + ); + + await user.click( + await screen.findByRole("button", { + name: `Remove ${MockGroup.display_name}`, + }), + ); + await user.click( + screen.getByRole("button", { name: "Search for user or group" }), + ); + await user.type( + screen.getByPlaceholderText("Search for user or group"), + MockUserOwner.email, + ); + await user.click( + await screen.findByRole("option", { + name: new RegExp(MockUserOwner.email, "i"), + }), + ); + await user.click(screen.getByRole("button", { name: "Add member" })); + await user.click(screen.getByRole("button", { name: "Save permissions" })); + + await waitFor(() => expect(onOpenChange).toHaveBeenCalledWith(false)); + expect(updates).toStrictEqual([ + { + organization: MockOrganization.id, + model: "model-1", + body: { + user_roles: { [MockUserOwner.id]: "read" }, + group_roles: { [MockGroup.id]: "" }, + }, + }, + ]); + }); +}); diff --git a/site/src/pages/AISettingsPage/ModelsPage/components/ChatModelSharingDialog.tsx b/site/src/pages/AISettingsPage/ModelsPage/components/ChatModelSharingDialog.tsx index 57459e89d1..c60d705ed0 100644 --- a/site/src/pages/AISettingsPage/ModelsPage/components/ChatModelSharingDialog.tsx +++ b/site/src/pages/AISettingsPage/ModelsPage/components/ChatModelSharingDialog.tsx @@ -1,18 +1,16 @@ import { useMutation, useQuery, useQueryClient } from "react-query"; -import { chatModelACL, updateChatModelACL } from "#/api/queries/chats"; -import type * as TypesGen from "#/api/typesGenerated"; +import { + chatModelACL, + chatModelACLAvailable, + updateChatModelACL, +} from "#/api/queries/chats"; import { toast } from "#/components/Toaster/toast"; -import { getGroupSubtitle, isGroup } from "#/modules/groups"; +import { ACLPrincipalAutocomplete } from "../../components/ACLPrincipalAutocomplete"; import { - ResourceSharingDialog, - type SharingDialogData, - type SharingPrincipal, - type SharingPrincipalSelection, -} from "../../components/ResourceSharingDialog"; -import { - ChatModelPrincipalAutocomplete, - type ChatModelPrincipalAutocompleteValue, -} from "./ChatModelPrincipalAutocomplete"; + selectedPrincipal, + sharingDialogData, +} from "../../components/aclSharing"; +import { ResourceSharingDialog } from "../../components/ResourceSharingDialog"; type ChatModelSharingDialogProps = { open: boolean; @@ -22,50 +20,6 @@ type ChatModelSharingDialogProps = { modelName: string; }; -type ChatModelPrincipal = Exclude; - -const groupPrincipal = (group: TypesGen.Group): SharingPrincipal => ({ - id: group.id, - name: group.display_name || group.name, - subtitle: getGroupSubtitle(group), - avatarUrl: group.avatar_url, -}); - -const userPrincipal = (user: TypesGen.MinimalUser): SharingPrincipal => ({ - id: user.id, - name: user.username, - subtitle: user.name || "User", - avatarUrl: user.avatar_url, -}); - -const sharingDialogData = ( - acl: TypesGen.ChatModelACL, -): SharingDialogData => ({ - acl: { - user_roles: Object.fromEntries( - acl.users.map((user) => [user.id, user.role]), - ), - group_roles: Object.fromEntries( - acl.groups.map((group) => [group.id, group.role]), - ), - }, - principals: { - users: Object.fromEntries( - acl.users.map((user) => [user.id, userPrincipal(user)]), - ), - groups: Object.fromEntries( - acl.groups.map((group) => [group.id, groupPrincipal(group)]), - ), - }, -}); - -const selectedPrincipal = ( - option: ChatModelPrincipal, -): SharingPrincipalSelection => - isGroup(option) - ? { kind: "group", principal: groupPrincipal(option) } - : { kind: "user", principal: userPrincipal(option) }; - type OpenChatModelSharingDialogProps = Omit< ChatModelSharingDialogProps, "open" @@ -107,13 +61,13 @@ const OpenChatModelSharingDialog: React.FC = ({ readRole="read" deletedRole="" renderAutocomplete={({ value, onChange, excludedPrincipalIds }) => ( - + chatModelACLAvailable(organizationId, modelId, options) + } excludedPrincipalIds={excludedPrincipalIds} - className="w-full" /> )} getPrincipal={selectedPrincipal} diff --git a/site/src/pages/AISettingsPage/SkillsPage/OrganizationSkillSharingDialog.stories.tsx b/site/src/pages/AISettingsPage/SkillsPage/OrganizationSkillSharingDialog.stories.tsx new file mode 100644 index 0000000000..616ca81130 --- /dev/null +++ b/site/src/pages/AISettingsPage/SkillsPage/OrganizationSkillSharingDialog.stories.tsx @@ -0,0 +1,53 @@ +import type { Meta, StoryObj } from "@storybook/react-vite"; +import { fn, screen, spyOn, userEvent } from "storybook/test"; +import { API } from "#/api/api"; +import { + MockDefaultOrganization, + MockGroup, + MockOrganizationSkillACL, + mockApiError, +} from "#/testHelpers/entities"; +import { MockSkills } from "#/testHelpers/skills"; +import { OrganizationSkillSharingDialog } from "./OrganizationSkillSharingDialog"; + +const meta: Meta = { + title: "pages/AISettingsPage/SkillsPage/OrganizationSkillSharingDialog", + component: OrganizationSkillSharingDialog, + args: { + organizationId: MockDefaultOrganization.id, + skillName: MockSkills[0].name, + onClose: fn(), + }, +}; + +export default meta; +type Story = StoryObj; + +export const Populated: Story = { + beforeEach: () => { + spyOn(API.experimental, "getOrganizationSkillACL").mockResolvedValue( + MockOrganizationSkillACL, + ); + }, +}; + +export const SaveError: Story = { + beforeEach: () => { + spyOn(API.experimental, "getOrganizationSkillACL").mockResolvedValue( + MockOrganizationSkillACL, + ); + spyOn(API.experimental, "updateOrganizationSkillACL").mockRejectedValue( + mockApiError({ message: "Failed to save skill permissions." }), + ); + }, + play: async () => { + await userEvent.click( + await screen.findByRole("button", { + name: `Remove ${MockGroup.display_name}`, + }), + ); + await userEvent.click( + screen.getByRole("button", { name: "Save permissions" }), + ); + }, +}; diff --git a/site/src/pages/AISettingsPage/SkillsPage/OrganizationSkillSharingDialog.test.tsx b/site/src/pages/AISettingsPage/SkillsPage/OrganizationSkillSharingDialog.test.tsx new file mode 100644 index 0000000000..87c599e707 --- /dev/null +++ b/site/src/pages/AISettingsPage/SkillsPage/OrganizationSkillSharingDialog.test.tsx @@ -0,0 +1,126 @@ +import { screen, waitFor } from "@testing-library/react"; +import userEvent from "@testing-library/user-event"; +import { HttpResponse, http, type PathParams } from "msw"; +import { QueryClientProvider } from "react-query"; +import { describe, expect, it, vi } from "vitest"; +import { API } from "#/api/api"; +import type { UpdateOrganizationSkillACLRequest } from "#/api/typesGenerated"; +import { + MockGroup, + MockMCPServerConfigACLAvailable, + MockOrganization, + MockOrganizationSkillACL, + MockUserOwner, + mockApiError, +} from "#/testHelpers/entities"; +import { + createTestQueryClient, + renderComponent, +} from "#/testHelpers/renderHelpers"; +import { server } from "#/testHelpers/server"; +import { OrganizationSkillSharingDialog } from "./OrganizationSkillSharingDialog"; + +const aclPath = + "/api/experimental/organizations/:organization/skills/:skillName/acl"; + +describe("OrganizationSkillSharingDialog", () => { + it("saves only the changed entries", async () => { + const updates: unknown[] = []; + server.use( + http.get(aclPath, () => HttpResponse.json(MockOrganizationSkillACL)), + http.get(`${aclPath}/available`, () => + HttpResponse.json(MockMCPServerConfigACLAvailable), + ), + http.patch( + aclPath, + async ({ params, request }) => { + updates.push({ ...params, body: await request.json() }); + return new HttpResponse(null, { status: 204 }); + }, + ), + ); + const onClose = vi.fn(); + const user = userEvent.setup(); + renderComponent( + + + , + ); + + await user.click( + await screen.findByRole("button", { + name: `Remove ${MockGroup.display_name}`, + }), + ); + await user.click( + screen.getByRole("button", { name: "Search for user or group" }), + ); + await user.type( + screen.getByPlaceholderText("Search for user or group"), + MockUserOwner.email, + ); + await user.click( + await screen.findByRole("option", { + name: new RegExp(MockUserOwner.email, "i"), + }), + ); + await user.click(screen.getByRole("button", { name: "Add member" })); + await user.click(screen.getByRole("button", { name: "Save permissions" })); + + await waitFor(() => expect(onClose).toHaveBeenCalledTimes(1)); + expect(updates).toStrictEqual([ + { + organization: MockOrganization.id, + skillName: "review-sql", + body: { + user_roles: { [MockUserOwner.id]: "read" }, + group_roles: { [MockGroup.id]: "" }, + }, + }, + ]); + }); + + it("keeps the dialog open when saving fails", async () => { + server.use( + http.get(aclPath, () => HttpResponse.json(MockOrganizationSkillACL)), + ); + const updateACL = vi + .spyOn(API.experimental, "updateOrganizationSkillACL") + .mockRejectedValue( + mockApiError({ message: "Failed to save skill permissions." }), + ); + const queryClient = createTestQueryClient(); + const onClose = vi.fn(); + const user = userEvent.setup(); + renderComponent( + + + , + ); + + await user.click( + await screen.findByRole("button", { + name: `Remove ${MockGroup.display_name}`, + }), + ); + await user.click(screen.getByRole("button", { name: "Save permissions" })); + + await waitFor(() => + expect(queryClient.getMutationCache().getAll()[0]?.state.status).toBe( + "error", + ), + ); + expect(updateACL).toHaveBeenCalledWith(MockOrganization.id, "review-sql", { + group_roles: { [MockGroup.id]: "" }, + }); + expect(onClose).not.toHaveBeenCalled(); + }); +}); diff --git a/site/src/pages/AISettingsPage/SkillsPage/OrganizationSkillSharingDialog.tsx b/site/src/pages/AISettingsPage/SkillsPage/OrganizationSkillSharingDialog.tsx new file mode 100644 index 0000000000..3b5193a38e --- /dev/null +++ b/site/src/pages/AISettingsPage/SkillsPage/OrganizationSkillSharingDialog.tsx @@ -0,0 +1,81 @@ +import { useMutation, useQuery, useQueryClient } from "react-query"; +import { + organizationSkillACL, + organizationSkillACLAvailable, + updateOrganizationSkillACL, +} from "#/api/queries/organizationSkills"; +import { toast } from "#/components/Toaster/toast"; +import { ACLPrincipalAutocomplete } from "../components/ACLPrincipalAutocomplete"; +import { selectedPrincipal, sharingDialogData } from "../components/aclSharing"; +import { ResourceSharingDialog } from "../components/ResourceSharingDialog"; + +type OrganizationSkillSharingDialogProps = { + organizationId: string; + skillName: string; + onClose: () => void; + onCloseAutoFocus?: (event: Event) => void; +}; + +export const OrganizationSkillSharingDialog: React.FC< + OrganizationSkillSharingDialogProps +> = ({ organizationId, skillName, onClose, onCloseAutoFocus }) => { + const queryClient = useQueryClient(); + const aclOptions = organizationSkillACL(organizationId, skillName); + const aclQuery = useQuery({ ...aclOptions, refetchOnMount: "always" }); + const updateMutation = useMutation(updateOrganizationSkillACL(queryClient)); + const data = aclQuery.data ? sharingDialogData(aclQuery.data) : undefined; + + const close = () => { + onClose(); + queryClient.removeQueries({ queryKey: aclOptions.queryKey, exact: true }); + }; + + return ( + + Manage which organization members and groups can use {skillName}. Site + owners, organization admins, and auditors can use it whenever it is + enabled. + + } + loadingLabel="Loading skill permissions" + emptyTitle="No members or groups have permission yet" + tableLabel="Skill permissions for members and groups" + roleLabel="Use" + confirmText="Save permissions" + data={data} + loadError={data ? null : aclQuery.error} + refetchError={data ? aclQuery.error : null} + saveError={updateMutation.error} + isSaving={updateMutation.isPending} + readRole="read" + deletedRole="" + renderAutocomplete={({ value, onChange, excludedPrincipalIds }) => ( + + organizationSkillACLAvailable(organizationId, skillName, options) + } + excludedPrincipalIds={excludedPrincipalIds} + /> + )} + getPrincipal={selectedPrincipal} + onClose={close} + onCloseAutoFocus={onCloseAutoFocus} + onSave={(req) => + updateMutation.mutate( + { organizationId, name: skillName, req }, + { + onSuccess: () => { + toast.success(`Permissions for "${skillName}" updated.`); + close(); + }, + }, + ) + } + /> + ); +}; diff --git a/site/src/pages/AISettingsPage/SkillsPage/SkillsPage.stories.tsx b/site/src/pages/AISettingsPage/SkillsPage/SkillsPage.stories.tsx new file mode 100644 index 0000000000..40f4b8c657 --- /dev/null +++ b/site/src/pages/AISettingsPage/SkillsPage/SkillsPage.stories.tsx @@ -0,0 +1,104 @@ +import type { Meta, StoryObj } from "@storybook/react-vite"; +import { reactRouterParameters } from "storybook-addon-remix-react-router"; +import { organizationSkills } from "#/api/queries/organizationSkills"; +import { organizationsPermissions } from "#/api/queries/organizations"; +import { + MockDefaultOrganization, + MockNoOrganizationPermissions, + MockOrganization2, + MockOrganizationPermissions, + MockUserOwner, +} from "#/testHelpers/entities"; +import { MockDisabledSkill, MockSkills } from "#/testHelpers/skills"; +import { + withAuthProvider, + withDashboardProvider, + withToaster, +} from "#/testHelpers/storybook"; +import SkillsPage from "./SkillsPage"; + +const mockOrganizationSkills = [...MockSkills, MockDisabledSkill]; + +const secondOrganizationRoute = reactRouterParameters({ + location: { + path: "/ai/settings/skills", + searchParams: { org: MockOrganization2.name }, + }, + routing: { path: "/ai/settings/skills" }, +}); + +const meta = { + title: "pages/AISettingsPage/SkillsPage", + component: SkillsPage, + decorators: [withToaster, withAuthProvider, withDashboardProvider], + parameters: { + layout: "fullscreen", + user: MockUserOwner, + permissions: { viewAnyOrganizationSkills: true }, + organizations: [MockDefaultOrganization], + reactRouter: reactRouterParameters({ + location: { path: "/ai/settings/skills" }, + routing: { path: "/ai/settings/skills" }, + }), + }, +} satisfies Meta; + +export default meta; +type Story = StoryObj; + +export const OrganizationAdmin: Story = { + parameters: { + queries: [ + { + key: organizationsPermissions([MockDefaultOrganization.id]).queryKey, + data: { [MockDefaultOrganization.id]: MockOrganizationPermissions }, + }, + { + key: organizationSkills(MockDefaultOrganization.id).queryKey, + data: mockOrganizationSkills, + }, + ], + }, +}; + +export const MultipleOrganizations: Story = { + parameters: { + organizations: [MockDefaultOrganization, MockOrganization2], + reactRouter: secondOrganizationRoute, + queries: [ + { + key: organizationsPermissions([ + MockDefaultOrganization.id, + MockOrganization2.id, + ]).queryKey, + data: { + [MockDefaultOrganization.id]: MockOrganizationPermissions, + [MockOrganization2.id]: MockOrganizationPermissions, + }, + }, + { + key: organizationSkills(MockOrganization2.id).queryKey, + data: mockOrganizationSkills, + }, + ], + }, +}; + +export const DeniedOrganization: Story = { + parameters: { + organizations: [MockDefaultOrganization, MockOrganization2], + reactRouter: secondOrganizationRoute, + queries: [ + { + key: organizationsPermissions([ + MockDefaultOrganization.id, + MockOrganization2.id, + ]).queryKey, + data: { + [MockDefaultOrganization.id]: MockOrganizationPermissions, + [MockOrganization2.id]: MockNoOrganizationPermissions, + }, + }, + ], + }, +}; diff --git a/site/src/pages/AISettingsPage/SkillsPage/SkillsPage.test.tsx b/site/src/pages/AISettingsPage/SkillsPage/SkillsPage.test.tsx new file mode 100644 index 0000000000..9446d460ad --- /dev/null +++ b/site/src/pages/AISettingsPage/SkillsPage/SkillsPage.test.tsx @@ -0,0 +1,89 @@ +import { screen, waitFor } from "@testing-library/react"; +import userEvent from "@testing-library/user-event"; +import { createMemoryRouter } from "react-router"; +import { expect, it, vi } from "vitest"; +import { API } from "#/api/api"; +import { + MockNoPermissions, + MockOrganization, + MockOrganization2, + MockUserOwner, +} from "#/testHelpers/entities"; +import { renderWithRouter } from "#/testHelpers/renderHelpers"; +import { MockSkill } from "#/testHelpers/skills"; +import SkillsPage from "./SkillsPage"; + +vi.mock("#/hooks/useAuthenticated", () => ({ + useAuthenticated: () => ({ + user: MockUserOwner, + permissions: { ...MockNoPermissions, viewAnyOrganizationSkills: true }, + }), +})); +vi.mock("#/modules/dashboard/useDashboard", () => ({ + useDashboard: () => ({ + organizations: [MockOrganization, MockOrganization2], + }), +})); + +it("lets a role that can only update skills toggle and edit them", async () => { + const user = userEvent.setup(); + vi.spyOn(API, "checkAuthorization").mockImplementation(async ({ checks }) => + Object.fromEntries( + Object.keys(checks).map((key) => [ + key, + !key.endsWith(".createOrganizationSkill") && + !key.endsWith(".deleteOrganizationSkill"), + ]), + ), + ); + const mockReviewSkill = { ...MockSkill, name: "review-sql", enabled: true }; + vi.spyOn(API.experimental, "getOrganizationSkills").mockResolvedValue([ + mockReviewSkill, + ]); + vi.spyOn(API.experimental, "getOrganizationSkillByName").mockResolvedValue({ + ...mockReviewSkill, + content: "---\nname: review-sql\ndescription: Review SQL.\n---\n\nBody\n", + }); + const updateSkill = vi + .spyOn(API.experimental, "updateOrganizationSkill") + .mockResolvedValue({ ...mockReviewSkill, enabled: false, content: "" }); + const router = createMemoryRouter( + [{ path: "/ai/settings/skills", element: }], + { initialEntries: ["/ai/settings/skills"] }, + ); + renderWithRouter(router); + + await user.click( + await screen.findByRole("switch", { name: "Enable review-sql" }), + ); + await waitFor(() => + expect(updateSkill).toHaveBeenCalledWith( + MockOrganization.id, + "review-sql", + { + enabled: false, + }, + ), + ); + + await user.click(screen.getByRole("button", { name: "Open menu" })); + await user.click(screen.getByRole("menuitem", { name: "Edit" })); + await waitFor(() => + expect(screen.getByRole("textbox", { name: "Body" })).toHaveValue("Body"), + ); + await user.type( + screen.getByRole("textbox", { name: "Description" }), + "Review SQL changes.", + ); + await user.click(screen.getByRole("button", { name: "Save skill" })); + await waitFor(() => + expect(updateSkill).toHaveBeenLastCalledWith( + MockOrganization.id, + "review-sql", + { + content: + '---\nname: review-sql\ndescription: "Review SQL changes."\n---\nBody\n', + }, + ), + ); +}); diff --git a/site/src/pages/AISettingsPage/SkillsPage/SkillsPage.tsx b/site/src/pages/AISettingsPage/SkillsPage/SkillsPage.tsx new file mode 100644 index 0000000000..9c6d6d43fe --- /dev/null +++ b/site/src/pages/AISettingsPage/SkillsPage/SkillsPage.tsx @@ -0,0 +1,169 @@ +import { useState } from "react"; +import { useQuery } from "react-query"; +import { useSearchParams } from "react-router"; +import { organizationsPermissions } from "#/api/queries/organizations"; +import type { Organization } from "#/api/typesGenerated"; +import { ErrorAlert } from "#/components/Alert/ErrorAlert"; +import { Loader } from "#/components/Loader/Loader"; +import { useAuthenticated } from "#/hooks/useAuthenticated"; +import { useDashboard } from "#/modules/dashboard/useDashboard"; +import { RequirePermission } from "#/modules/permissions/RequirePermission"; +import { SkillsTable } from "#/pages/AgentsPage/components/SkillsTable"; +import { + organizationSkillsScope, + type SkillAccess, +} from "#/pages/AgentsPage/utils/skills"; +import { pageTitle } from "#/utils/page"; +import { OrganizationPicker } from "../MCPServersPage/components/OrganizationPicker"; +import { + orgSearchParam, + selectOrganization, +} from "../MCPServersPage/organizationParam"; +import { OrganizationSkillSharingDialog } from "./OrganizationSkillSharingDialog"; +import { readableSkillsOrganizations } from "./readableSkillsOrganizations"; + +const SkillsPage: React.FC = () => { + const { permissions } = useAuthenticated(); + const { organizations } = useDashboard(); + const [searchParams, setSearchParams] = useSearchParams(); + const organizationPermissionsQuery = useQuery({ + ...organizationsPermissions( + organizations.map((organization) => organization.id), + ), + enabled: permissions.viewAnyOrganizationSkills, + }); + const readableOrganizations = readableSkillsOrganizations( + organizations, + organizationPermissionsQuery.data, + ); + const requestedOrganizationName = searchParams.get(orgSearchParam); + const defaultOrganization = + readableOrganizations.length > 0 + ? selectOrganization(readableOrganizations, null) + : undefined; + // A requested organization the user cannot read is denied rather than + // replaced, so a skill is never added to an organization the URL did not + // name. + const organization = + requestedOrganizationName === null + ? defaultOrganization + : readableOrganizations.find( + (organization) => organization.name === requestedOrganizationName, + ); + const organizationPermissions = organization + ? organizationPermissionsQuery.data?.[organization.id] + : undefined; + + return ( + + {pageTitle("Skills", "AI Settings")} + {organizationPermissionsQuery.isLoadingError ? ( + + ) : !organizationPermissionsQuery.data ? ( + + ) : ( + + {organizationPermissionsQuery.isRefetchError && ( + + )} + {organization && ( + { + setSearchParams((params) => { + const next = new URLSearchParams(params); + next.set(orgSearchParam, org.name); + return next; + }); + }} + showLabel={false} + /> + } + /> + )} + + )} + + ); +}; + +type OrganizationSkillsProps = { + organization: Organization; + access: SkillAccess; + canShare: boolean; + toolbar: React.ReactNode; +}; + +const OrganizationSkills: React.FC = ({ + organization, + access, + canShare, + toolbar, +}) => { + const [sharingSkill, setSharingSkill] = useState<{ + name: string; + onCloseAutoFocus: (event: Event) => void; + }>(); + + return ( + <> + + setSharingSkill({ name: skill.name, onCloseAutoFocus }) + : undefined + } + toolbar={toolbar} + /> + {sharingSkill && ( + setSharingSkill(undefined)} + onCloseAutoFocus={sharingSkill.onCloseAutoFocus} + /> + )} + + ); +}; + +export default SkillsPage; diff --git a/site/src/pages/AISettingsPage/SkillsPage/readableSkillsOrganizations.ts b/site/src/pages/AISettingsPage/SkillsPage/readableSkillsOrganizations.ts new file mode 100644 index 0000000000..e25f2bd36e --- /dev/null +++ b/site/src/pages/AISettingsPage/SkillsPage/readableSkillsOrganizations.ts @@ -0,0 +1,14 @@ +import type { Organization } from "#/api/typesGenerated"; +import type { OrganizationPermissions } from "#/modules/permissions/organizations"; + +/** Returns the organizations whose skills the user can read. */ +export const readableSkillsOrganizations = ( + organizations: readonly Organization[], + permissionsByOrganization: + | Readonly> + | undefined, +): readonly Organization[] => + organizations.filter( + (organization) => + permissionsByOrganization?.[organization.id]?.viewOrganizationSkills, + ); diff --git a/site/src/pages/AISettingsPage/ModelsPage/components/ChatModelPrincipalAutocomplete.tsx b/site/src/pages/AISettingsPage/components/ACLPrincipalAutocomplete.tsx similarity index 70% rename from site/src/pages/AISettingsPage/ModelsPage/components/ChatModelPrincipalAutocomplete.tsx rename to site/src/pages/AISettingsPage/components/ACLPrincipalAutocomplete.tsx index 8e2b995ade..b49379e913 100644 --- a/site/src/pages/AISettingsPage/ModelsPage/components/ChatModelPrincipalAutocomplete.tsx +++ b/site/src/pages/AISettingsPage/components/ACLPrincipalAutocomplete.tsx @@ -1,36 +1,32 @@ import { CheckIcon } from "lucide-react"; import { useId, useState } from "react"; -import { keepPreviousData, useQuery } from "react-query"; -import { chatModelACLAvailable } from "#/api/queries/chats"; -import type { Group, ReducedUser } from "#/api/typesGenerated"; +import { keepPreviousData, type UseQueryOptions, useQuery } from "react-query"; +import type { + ACLAvailable, + Group, + ReducedUser, + UsersRequest, +} from "#/api/typesGenerated"; import { ErrorAlert } from "#/components/Alert/ErrorAlert"; import { Autocomplete } from "#/components/Autocomplete/Autocomplete"; import { AvatarData } from "#/components/Avatar/AvatarData"; import { getGroupSubtitle, isGroup } from "#/modules/groups"; import { prepareQuery } from "#/utils/filters"; -export type ChatModelPrincipalAutocompleteValue = ReducedUser | Group | null; -type AutocompleteOption = Exclude; +export type ACLPrincipal = ReducedUser | Group; -type ChatModelPrincipalAutocompleteProps = { - value: ChatModelPrincipalAutocompleteValue; - onChange: (value: ChatModelPrincipalAutocompleteValue) => void; - organizationId: string; - modelId: string; +type ACLPrincipalAutocompleteProps = { + value: ACLPrincipal | null; + onChange: (value: ACLPrincipal | null) => void; + availableQueryOptions: ( + options: UsersRequest, + ) => UseQueryOptions; excludedPrincipalIds: readonly string[]; - className?: string; }; -export const ChatModelPrincipalAutocomplete: React.FC< - ChatModelPrincipalAutocompleteProps -> = ({ - value, - onChange, - organizationId, - modelId, - excludedPrincipalIds, - className, -}) => { +export const ACLPrincipalAutocomplete: React.FC< + ACLPrincipalAutocompleteProps +> = ({ value, onChange, availableQueryOptions, excludedPrincipalIds }) => { const [inputValue, setInputValue] = useState(""); const [open, setOpen] = useState(false); const autocompleteId = useId(); @@ -43,15 +39,12 @@ export const ChatModelPrincipalAutocomplete: React.FC< }; const aclAvailableQuery = useQuery({ - ...chatModelACLAvailable(organizationId, modelId, { - q: prepareQuery(inputValue), - limit: 25, - }), + ...availableQueryOptions({ q: prepareQuery(inputValue), limit: 25 }), enabled: open, placeholderData: keepPreviousData, }); - const options: AutocompleteOption[] = aclAvailableQuery.data + const options: ACLPrincipal[] = aclAvailableQuery.data ? [ ...aclAvailableQuery.data.groups, ...aclAvailableQuery.data.users, @@ -98,7 +91,6 @@ export const ChatModelPrincipalAutocomplete: React.FC< ? "Unable to load users or groups" : "No users or groups found" } - className={className} id={autocompleteId} /> {aclAvailableQuery.error && ( diff --git a/site/src/pages/AISettingsPage/components/ResourceSharingDialog.tsx b/site/src/pages/AISettingsPage/components/ResourceSharingDialog.tsx index e767980f18..14723660ac 100644 --- a/site/src/pages/AISettingsPage/components/ResourceSharingDialog.tsx +++ b/site/src/pages/AISettingsPage/components/ResourceSharingDialog.tsx @@ -81,6 +81,7 @@ type ResourceSharingDialogProps = { ) => React.ReactNode; getPrincipal: (option: Option) => SharingPrincipalSelection; onClose: () => void; + onCloseAutoFocus?: (event: Event) => void; onSave: (update: SharingACLUpdate) => void; }; @@ -398,6 +399,7 @@ export const ResourceSharingDialog = ({ renderAutocomplete, getPrincipal, onClose, + onCloseAutoFocus, onSave, }: ResourceSharingDialogProps) => ( ({ } }} > - + {title} {description} diff --git a/site/src/pages/AISettingsPage/components/aclSharing.ts b/site/src/pages/AISettingsPage/components/aclSharing.ts new file mode 100644 index 0000000000..069db9541e --- /dev/null +++ b/site/src/pages/AISettingsPage/components/aclSharing.ts @@ -0,0 +1,55 @@ +import type { Group, MinimalUser } from "#/api/typesGenerated"; +import { getGroupSubtitle, isGroup } from "#/modules/groups"; +import type { ACLPrincipal } from "./ACLPrincipalAutocomplete"; +import type { + SharingDialogData, + SharingPrincipal, + SharingPrincipalSelection, +} from "./ResourceSharingDialog"; + +type ResourceACL = { + readonly users: readonly (MinimalUser & { readonly role: Role })[]; + readonly groups: readonly (Group & { readonly role: Role })[]; +}; + +const groupPrincipal = (group: Group): SharingPrincipal => ({ + id: group.id, + name: group.display_name || group.name, + subtitle: getGroupSubtitle(group), + avatarUrl: group.avatar_url, +}); + +const userPrincipal = (user: MinimalUser): SharingPrincipal => ({ + id: user.id, + name: user.username, + subtitle: user.name || "User", + avatarUrl: user.avatar_url, +}); + +export const sharingDialogData = ( + acl: ResourceACL, +): SharingDialogData => ({ + acl: { + user_roles: Object.fromEntries( + acl.users.map((user) => [user.id, user.role]), + ), + group_roles: Object.fromEntries( + acl.groups.map((group) => [group.id, group.role]), + ), + }, + principals: { + users: Object.fromEntries( + acl.users.map((user) => [user.id, userPrincipal(user)]), + ), + groups: Object.fromEntries( + acl.groups.map((group) => [group.id, groupPrincipal(group)]), + ), + }, +}); + +export const selectedPrincipal = ( + option: ACLPrincipal, +): SharingPrincipalSelection => + isGroup(option) + ? { kind: "group", principal: groupPrincipal(option) } + : { kind: "user", principal: userPrincipal(option) }; diff --git a/site/src/pages/AgentsPage/AgentSettingsSkillsPage.stories.tsx b/site/src/pages/AgentsPage/AgentSettingsSkillsPage.stories.tsx new file mode 100644 index 0000000000..cb7235282d --- /dev/null +++ b/site/src/pages/AgentsPage/AgentSettingsSkillsPage.stories.tsx @@ -0,0 +1,68 @@ +import type { Meta, StoryObj } from "@storybook/react-vite"; +import { reactRouterParameters } from "storybook-addon-remix-react-router"; +import { organizationSkills } from "#/api/queries/organizationSkills"; +import { userSkills } from "#/api/queries/userSkills"; +import { + MockDefaultOrganization, + MockOrganization2, + MockOrganization3, + MockUserMember, +} from "#/testHelpers/entities"; +import { MockDisabledSkill, MockSkills } from "#/testHelpers/skills"; +import { + withAuthProvider, + withDashboardProvider, + withToaster, +} from "#/testHelpers/storybook"; +import AgentSettingsSkillsPage from "./AgentSettingsSkillsPage"; + +const meta = { + title: "pages/AgentsPage/AgentSettingsSkillsPage", + component: AgentSettingsSkillsPage, + decorators: [withToaster, withAuthProvider, withDashboardProvider], + parameters: { + user: { + ...MockUserMember, + organization_ids: [ + MockDefaultOrganization.id, + MockOrganization2.id, + MockOrganization3.id, + ], + }, + organizations: [ + MockDefaultOrganization, + MockOrganization2, + MockOrganization3, + ], + reactRouter: reactRouterParameters({ + location: { path: "/agents/settings/skills" }, + routing: { path: "/agents/settings/skills" }, + }), + }, +} satisfies Meta; + +export default meta; +type Story = StoryObj; + +export const OrganizationSkills: Story = { + parameters: { + queries: [ + { + key: userSkills().queryKey, + data: MockSkills, + }, + { + key: organizationSkills(MockDefaultOrganization.id).queryKey, + data: [...MockSkills.slice(0, 2), MockDisabledSkill], + }, + { + key: organizationSkills(MockOrganization2.id).queryKey, + data: [MockSkills[2]], + }, + { + key: organizationSkills(MockOrganization3.id).queryKey, + data: [MockDisabledSkill], + }, + ], + }, +}; diff --git a/site/src/pages/AgentsPage/AgentSettingsSkillsPage.test.tsx b/site/src/pages/AgentsPage/AgentSettingsSkillsPage.test.tsx new file mode 100644 index 0000000000..e5376e045b --- /dev/null +++ b/site/src/pages/AgentsPage/AgentSettingsSkillsPage.test.tsx @@ -0,0 +1,107 @@ +import { screen, within } from "@testing-library/react"; +import userEvent from "@testing-library/user-event"; +import { describe, expect, it, vi } from "vitest"; +import { API } from "#/api/api"; +import { + MockDefaultOrganization, + MockOrganization, + MockOrganization2, + MockOrganization3, + MockUserMember, +} from "#/testHelpers/entities"; +import { renderWithAuth } from "#/testHelpers/renderHelpers"; +import { MockSkill } from "#/testHelpers/skills"; +import AgentSettingsSkillsPage from "./AgentSettingsSkillsPage"; + +describe("AgentSettingsSkillsPage", () => { + it("lists skills only from organizations the user belongs to", async () => { + vi.spyOn(API, "getAuthenticatedUser").mockResolvedValue({ + ...MockUserMember, + organization_ids: [MockOrganization.id], + }); + vi.spyOn(API, "getOrganizations").mockResolvedValue([ + MockOrganization, + MockOrganization2, + ]); + vi.spyOn(API.experimental, "getUserSkills").mockResolvedValue([]); + const getOrganizationSkills = vi + .spyOn(API.experimental, "getOrganizationSkills") + .mockResolvedValue([{ ...MockSkill, name: "review-sql" }]); + + renderWithAuth(); + + await within( + await screen.findByRole("table", { name: MockOrganization.display_name }), + ).findByText("review-sql"); + expect( + getOrganizationSkills.mock.calls.map( + ([organizationId]) => organizationId, + ), + ).not.toContain(MockOrganization2.id); + }); + + it("shows one organization at a time, starting with the default", async () => { + const user = userEvent.setup(); + vi.spyOn(API, "getAuthenticatedUser").mockResolvedValue({ + ...MockUserMember, + organization_ids: [MockOrganization2.id, MockDefaultOrganization.id], + }); + vi.spyOn(API, "getOrganizations").mockResolvedValue([ + MockOrganization2, + MockDefaultOrganization, + MockOrganization3, + ]); + vi.spyOn(API.experimental, "getUserSkills").mockResolvedValue([]); + const skillName = (organizationId: string) => + organizationId === MockOrganization2.id + ? "second-skill" + : "default-skill"; + const getOrganizationSkills = vi + .spyOn(API.experimental, "getOrganizationSkills") + .mockImplementation(async (organizationId) => [ + { ...MockSkill, name: skillName(organizationId) }, + ]); + const getOrganizationSkillByName = vi + .spyOn(API.experimental, "getOrganizationSkillByName") + .mockImplementation(async (_organizationId, name) => ({ + ...MockSkill, + name, + content: `---\nname: ${name}\n---\nBody\n`, + })); + const viewSkill = async (organizationName: string) => { + const table = await screen.findByRole("table", { + name: organizationName, + }); + await user.click( + await within(table).findByRole("button", { name: "Open menu" }), + ); + await user.click(await screen.findByRole("menuitem", { name: /View/ })); + await user.click(await screen.findByRole("button", { name: "Close" })); + }; + + renderWithAuth(); + + await viewSkill(MockDefaultOrganization.display_name); + expect(getOrganizationSkillByName).toHaveBeenLastCalledWith( + MockDefaultOrganization.id, + "default-skill", + ); + + await user.click( + screen.getByRole("combobox", { + name: `Organization ${MockDefaultOrganization.display_name}`, + }), + ); + await user.click(screen.getByRole("option", { name: /My Organization 2/ })); + await viewSkill(MockOrganization2.display_name); + expect(getOrganizationSkillByName).toHaveBeenLastCalledWith( + MockOrganization2.id, + "second-skill", + ); + expect( + getOrganizationSkills.mock.calls.map( + ([organizationId]) => organizationId, + ), + ).not.toContain(MockOrganization3.id); + }); +}); diff --git a/site/src/pages/AgentsPage/AgentSettingsSkillsPage.tsx b/site/src/pages/AgentsPage/AgentSettingsSkillsPage.tsx new file mode 100644 index 0000000000..2157403734 --- /dev/null +++ b/site/src/pages/AgentsPage/AgentSettingsSkillsPage.tsx @@ -0,0 +1,134 @@ +import { useState } from "react"; +import { useQueries } from "react-query"; +import { organizationSkills } from "#/api/queries/organizationSkills"; +import { ErrorAlert } from "#/components/Alert/ErrorAlert"; +import { OrganizationField } from "#/components/OrganizationAutocomplete/OrganizationAutocomplete"; +import { useAuthenticated } from "#/hooks/useAuthenticated"; +import { useDashboard } from "#/modules/dashboard/useDashboard"; +import { RetryButton } from "./components/RetryButton"; +import { SectionHeader } from "./components/SectionHeader"; +import { SkillsTable } from "./components/SkillsTable"; +import { + fullSkillAccess, + organizationSkillsScope, + readOnlySkillAccess, + userSkillsScope, +} from "./utils/skills"; + +const AgentSettingsSkillsPage: React.FC = () => { + const { user } = useAuthenticated(); + // The dashboard lists every organization the user can read, but chats + // only run in organizations the user is a member of. + const memberOrganizations = useDashboard().organizations.filter( + (organization) => user.organization_ids.includes(organization.id), + ); + const organizationSkillQueries = useQueries({ + queries: memberOrganizations.map((organization) => + organizationSkills(organization.id), + ), + }); + const visibleOrganizations = memberOrganizations.flatMap( + (organization, index) => { + const query = organizationSkillQueries[index]; + const hasEnabledSkills = Boolean( + query.data?.some((skill) => skill.enabled), + ); + return hasEnabledSkills || query.isError || query.isPending + ? [{ organization, query, hasEnabledSkills }] + : []; + }, + ); + const [selectedOrganizationId, setSelectedOrganizationId] = + useState(); + const selected = + visibleOrganizations.find( + ({ organization }) => organization.id === selectedOrganizationId, + ) ?? + visibleOrganizations.find(({ organization }) => organization.is_default) ?? + visibleOrganizations.at(0); + const organizationName = selected + ? selected.organization.display_name || selected.organization.name + : ""; + + return ( +
+ + {selected && ( +
+ organization, + )} + organization={selected.organization} + onChange={(organization) => + setSelectedOrganizationId(organization.id) + } + showLabel={false} + /> + } + /> + {selected.query.isError && !selected.hasEnabledSkills ? ( + // The table's own observer would refetch a failed list on + // mount and clear its error, so a failed list renders here. +
+ + { + void selected.query.refetch(); + }} + /> + } + /> +
+ ) : ( + + )} +
+ )} +
+ ); +}; + +export default AgentSettingsSkillsPage; diff --git a/site/src/pages/AgentsPage/components/ChatMessageInput/ChatMessageInput.tsx b/site/src/pages/AgentsPage/components/ChatMessageInput/ChatMessageInput.tsx index 292f84ccf9..a7b7d20254 100644 --- a/site/src/pages/AgentsPage/components/ChatMessageInput/ChatMessageInput.tsx +++ b/site/src/pages/AgentsPage/components/ChatMessageInput/ChatMessageInput.tsx @@ -36,10 +36,7 @@ import { useQuery } from "react-query"; import { userSkills } from "#/api/queries/userSkills"; import type * as TypesGen from "#/api/typesGenerated"; import { MODIFIER_AGENT_CHAT_SEND_SHORTCUT } from "../../utils/agentChatSendShortcut"; -import { - filterSkillsByQuery, - isPersonalSkillTriggerToken, -} from "../../utils/personalSkills"; +import { filterSkillsByQuery, isSkillTriggerToken } from "../../utils/skills"; import type { ChatSlashCommand } from "../../utils/slashCommands"; import { $createFileReferenceNode, @@ -799,10 +796,7 @@ const ChatMessageInput = ({ const token = node .getTextContent() .slice(trigger.slashOffset, caretOffset); - if ( - caretOffset < trigger.slashOffset || - !isPersonalSkillTriggerToken(token) - ) { + if (caretOffset < trigger.slashOffset || !isSkillTriggerToken(token)) { return; } diff --git a/site/src/pages/AgentsPage/components/ChatMessageInput/SkillsTriggerMenu.stories.tsx b/site/src/pages/AgentsPage/components/ChatMessageInput/SkillsTriggerMenu.stories.tsx index 9e2a4d7581..3165f6b527 100644 --- a/site/src/pages/AgentsPage/components/ChatMessageInput/SkillsTriggerMenu.stories.tsx +++ b/site/src/pages/AgentsPage/components/ChatMessageInput/SkillsTriggerMenu.stories.tsx @@ -2,7 +2,7 @@ import type { Meta, StoryObj } from "@storybook/react-vite"; import { useState } from "react"; import { expect, fn, userEvent } from "storybook/test"; import { MockSkills } from "#/testHelpers/skills"; -import { filterSkillsByQuery } from "../../utils/personalSkills"; +import { filterSkillsByQuery } from "../../utils/skills"; import { COMPACT_SLASH_COMMAND } from "../../utils/slashCommands"; import { createCommandMenuItem, diff --git a/site/src/pages/AgentsPage/components/ChatMessageInput/SkillsTriggerPlugin.tsx b/site/src/pages/AgentsPage/components/ChatMessageInput/SkillsTriggerPlugin.tsx index 1f796dea3c..dad81ee4bc 100644 --- a/site/src/pages/AgentsPage/components/ChatMessageInput/SkillsTriggerPlugin.tsx +++ b/site/src/pages/AgentsPage/components/ChatMessageInput/SkillsTriggerPlugin.tsx @@ -12,7 +12,7 @@ import { type NodeKey, } from "lexical"; import { useEffect, useEffectEvent, useLayoutEffect, useRef } from "react"; -import { parsePersonalSkillTrigger } from "../../utils/personalSkills"; +import { parseSkillTrigger } from "../../utils/skills"; import type { SkillMenuItem } from "./SkillsTriggerMenu"; export type ActiveSkillsTrigger = { @@ -64,7 +64,7 @@ const activeTriggerFromSelection = (): ActiveSkillsTrigger | null => { const textBeforeCaret = node.getTextContent().slice(0, anchor.offset); const lineStart = textBeforeCaret.lastIndexOf("\n") + 1; - const trigger = parsePersonalSkillTrigger(textBeforeCaret.slice(lineStart)); + const trigger = parseSkillTrigger(textBeforeCaret.slice(lineStart)); if (!trigger) { return null; } diff --git a/site/src/pages/AgentsPage/components/ChatsSidebar/dialogs/ChatSearchDialog.stories.tsx b/site/src/pages/AgentsPage/components/ChatsSidebar/dialogs/ChatSearchDialog.stories.tsx index 66043ff000..102d6b8f11 100644 --- a/site/src/pages/AgentsPage/components/ChatsSidebar/dialogs/ChatSearchDialog.stories.tsx +++ b/site/src/pages/AgentsPage/components/ChatsSidebar/dialogs/ChatSearchDialog.stories.tsx @@ -115,7 +115,7 @@ const meta: Meta = { { path: "/agents", useStoryElement: true }, { path: "/agents/:agentId", useStoryElement: true }, { path: "/agents/settings", useStoryElement: true }, - { path: "/agents/settings/personal-skills", useStoryElement: true }, + { path: "/agents/settings/skills", useStoryElement: true }, ], }), }, diff --git a/site/src/pages/AgentsPage/components/ChatsSidebar/settings/SettingsPanel.tsx b/site/src/pages/AgentsPage/components/ChatsSidebar/settings/SettingsPanel.tsx index 1d85a1cdcf..21c326bc77 100644 --- a/site/src/pages/AgentsPage/components/ChatsSidebar/settings/SettingsPanel.tsx +++ b/site/src/pages/AgentsPage/components/ChatsSidebar/settings/SettingsPanel.tsx @@ -91,9 +91,9 @@ export const SettingsPanel: React.FC = ({ )} & { + isRetrying: boolean; + onRetry: () => void; +}; + +export const RetryButton: React.FC = ({ + isRetrying, + onRetry, + ...buttonProps +}) => ( + +); diff --git a/site/src/pages/AgentsPage/components/SectionHeader.tsx b/site/src/pages/AgentsPage/components/SectionHeader.tsx index bd7e2afd96..2d500baa26 100644 --- a/site/src/pages/AgentsPage/components/SectionHeader.tsx +++ b/site/src/pages/AgentsPage/components/SectionHeader.tsx @@ -1,9 +1,11 @@ +export type SectionHeaderLevel = "page" | "section"; + type SectionHeaderProps = { label: string; description?: string; badge?: React.ReactNode; action?: React.ReactNode; - level?: "page" | "section"; + level?: SectionHeaderLevel; }; export const SectionHeader: React.FC = ({ @@ -24,8 +26,8 @@ export const SectionHeader: React.FC = ({ : "m-0 mt-0.5 text-sm text-content-secondary"; return ( -
-
+
+
{label} {badge} diff --git a/site/src/pages/AgentsPage/components/PersonalSkillEditor.test.tsx b/site/src/pages/AgentsPage/components/SkillEditor.test.tsx similarity index 85% rename from site/src/pages/AgentsPage/components/PersonalSkillEditor.test.tsx rename to site/src/pages/AgentsPage/components/SkillEditor.test.tsx index f09b995a10..5fa6949a7f 100644 --- a/site/src/pages/AgentsPage/components/PersonalSkillEditor.test.tsx +++ b/site/src/pages/AgentsPage/components/SkillEditor.test.tsx @@ -1,18 +1,21 @@ import { render, screen, waitFor } from "@testing-library/react"; import userEvent from "@testing-library/user-event"; import { describe, expect, it, vi } from "vitest"; -import { PERSONAL_SKILL_MAX_SIZE_BYTES } from "../utils/personalSkills"; -import { PersonalSkillEditor } from "./PersonalSkillEditor"; +import { SKILL_MAX_SIZE_BYTES } from "../utils/skills"; +import { SkillEditor } from "./SkillEditor"; const markdown = "---\nname: imported-skill\ndescription: Imported guidance.\n---\n\nUse imported instructions."; -const renderEditor = (mode: "create" | "edit" = "create") => { +const renderEditor = (mode: "create" | "edit" = "create", readOnly = false) => { const onSubmit = vi.fn(); render( - { return { user: userEvent.setup(), onSubmit }; }; -describe("PersonalSkillEditor file import", () => { +describe("SkillEditor", () => { + it("starts focus on Close when read-only", async () => { + renderEditor("edit", true); + await waitFor(() => + expect(screen.getByRole("button", { name: "Close" })).toHaveFocus(), + ); + }); +}); + +describe("SkillEditor file import", () => { it.each(["create", "edit"] as const)( "imports a file before submitting in %s mode", async (mode) => { @@ -69,7 +81,7 @@ describe("PersonalSkillEditor file import", () => { }, { content: " \n", error: "File is empty" }, { - content: "x".repeat(PERSONAL_SKILL_MAX_SIZE_BYTES + 1), + content: "x".repeat(SKILL_MAX_SIZE_BYTES + 1), error: "File is too large", }, ])("preserves edits when $error", async ({ content, error }) => { diff --git a/site/src/pages/AgentsPage/components/PersonalSkillEditor.tsx b/site/src/pages/AgentsPage/components/SkillEditor.tsx similarity index 64% rename from site/src/pages/AgentsPage/components/PersonalSkillEditor.tsx rename to site/src/pages/AgentsPage/components/SkillEditor.tsx index a3334700f4..1ffdf5dc7c 100644 --- a/site/src/pages/AgentsPage/components/PersonalSkillEditor.tsx +++ b/site/src/pages/AgentsPage/components/SkillEditor.tsx @@ -20,29 +20,33 @@ import { Spinner } from "#/components/Spinner/Spinner"; import { formatKiB } from "#/utils/fileSize"; import { readAgentAttachmentText } from "../utils/fileAttachmentLimits"; import { - buildPersonalSkillMarkdown, - getPersonalSkillContentSizeBytes, - isValidPersonalSkillDescription, - isValidPersonalSkillName, - PERSONAL_SKILL_MAX_SIZE_BYTES, - type PersonalSkillFormValues, - tryParsePersonalSkillMarkdown, -} from "../utils/personalSkills"; + buildSkillMarkdown, + getSkillContentSizeBytes, + isValidSkillDescription, + isValidSkillName, + SKILL_MAX_SIZE_BYTES, + type SkillFormValues, + tryParseSkillMarkdown, +} from "../utils/skills"; -export type PersonalSkillErrorDisplay = { +export type SkillErrorDisplay = { message: string; detail?: string; }; -type PersonalSkillEditorProps = { +type SkillEditorProps = { open: boolean; mode: "create" | "edit"; - initialValues: PersonalSkillFormValues; + readOnly?: boolean; + noun: string; + editorDescription: string; + initialValues: SkillFormValues; existingNames: readonly string[]; - submitError?: PersonalSkillErrorDisplay; + submitError?: SkillErrorDisplay; isSubmitting: boolean; onOpenChange: (open: boolean) => void; - onSubmit: (values: PersonalSkillFormValues, content: string) => void; + onCloseAutoFocus?: (event: Event) => void; + onSubmit: (values: SkillFormValues, content: string) => void; }; type ImportStatus = { @@ -57,14 +61,18 @@ const beginsWithFrontmatterDelimiter = (content: string): boolean => .split(/\r?\n/, 1)[0] ?.trim() === "---"; -export const PersonalSkillEditor: React.FC = ({ +export const SkillEditor: React.FC = ({ open, mode, + readOnly = false, + noun, + editorDescription, initialValues, existingNames, submitError, isSubmitting, onOpenChange, + onCloseAutoFocus, onSubmit, }) => { const isCreate = mode === "create"; @@ -82,7 +90,7 @@ export const PersonalSkillEditor: React.FC = ({ .test( "skill-name", "Use kebab-case with lowercase letters, numbers, and single hyphens, up to 256 bytes.", - (value) => Boolean(value && isValidPersonalSkillName(value.trim())), + (value) => Boolean(value && isValidSkillName(value.trim())), ) .test( "unique-name", @@ -96,28 +104,26 @@ export const PersonalSkillEditor: React.FC = ({ description: Yup.string().test( "description-size", "Description must be 4096 bytes or smaller.", - (value) => isValidPersonalSkillDescription(value ?? ""), + (value) => isValidSkillDescription(value ?? ""), ), body: Yup.string().test("body-required", "Body is required.", (value) => Boolean(value?.trim()), ), }); - const validate = ( - values: PersonalSkillFormValues, - ): FormikErrors => { + const validate = (values: SkillFormValues): FormikErrors => { if ( - getPersonalSkillContentSizeBytes(buildPersonalSkillMarkdown(values)) <= - PERSONAL_SKILL_MAX_SIZE_BYTES + getSkillContentSizeBytes(buildSkillMarkdown(values)) <= + SKILL_MAX_SIZE_BYTES ) { return {}; } return { - body: `Skill content must be ${formatKiB(PERSONAL_SKILL_MAX_SIZE_BYTES)} or smaller.`, + body: `Skill content must be ${formatKiB(SKILL_MAX_SIZE_BYTES)} or smaller.`, }; }; - const form = useFormik({ + const form = useFormik({ initialValues, enableReinitialize: true, validationSchema, @@ -128,11 +134,12 @@ export const PersonalSkillEditor: React.FC = ({ description: values.description.trim(), body: values.body.trim(), }; - onSubmit(normalizedValues, buildPersonalSkillMarkdown(normalizedValues)); + onSubmit(normalizedValues, buildSkillMarkdown(normalizedValues)); }, }); const fileInputRef = useRef(null); + const closeButtonRef = useRef(null); const [isReadingFile, setIsReadingFile] = useState(false); const isBusy = isSubmitting || isReadingFile; const [importContent, setImportContent] = useState(""); @@ -143,7 +150,7 @@ export const PersonalSkillEditor: React.FC = ({ return; } - const result = tryParsePersonalSkillMarkdown(contentToImport); + const result = tryParseSkillMarkdown(contentToImport); if (!result.ok) { setImportStatus({ kind: "error", @@ -191,11 +198,11 @@ export const PersonalSkillEditor: React.FC = ({ } setImportStatus(null); - if (file.size > PERSONAL_SKILL_MAX_SIZE_BYTES) { + if (file.size > SKILL_MAX_SIZE_BYTES) { setImportStatus({ kind: "error", title: "File is too large", - detail: `Choose a file that is ${formatKiB(PERSONAL_SKILL_MAX_SIZE_BYTES)} or smaller.`, + detail: `Choose a file that is ${formatKiB(SKILL_MAX_SIZE_BYTES)} or smaller.`, }); return; } @@ -244,33 +251,40 @@ export const PersonalSkillEditor: React.FC = ({ void importSkillMarkdown(pastedContent); }; - const content = buildPersonalSkillMarkdown(form.values); - const sizeBytes = getPersonalSkillContentSizeBytes(content); + const content = buildSkillMarkdown(form.values); + const sizeBytes = getSkillContentSizeBytes(content); const nameError = form.touched.name ? form.errors.name : undefined; const descriptionError = form.touched.description ? form.errors.description : undefined; const bodyError = form.touched.body ? form.errors.body : undefined; - const isTooLarge = sizeBytes > PERSONAL_SKILL_MAX_SIZE_BYTES; - const isNearLimit = sizeBytes > PERSONAL_SKILL_MAX_SIZE_BYTES * 0.9; - const title = isCreate ? "Create personal skill" : "Edit personal skill"; + const isTooLarge = sizeBytes > SKILL_MAX_SIZE_BYTES; + const isNearLimit = sizeBytes > SKILL_MAX_SIZE_BYTES * 0.9; + const lowerNoun = noun.toLocaleLowerCase("en-US"); + const title = readOnly + ? `View ${lowerNoun}` + : `${isCreate ? "Create" : "Edit"} ${lowerNoun}`; const submitLabel = isCreate ? "Create skill" : "Save skill"; return ( - + { + if (readOnly) { + event.preventDefault(); + closeButtonRef.current?.focus(); + } + }} + >
{title} - - Personal skills are available to your agents and stored as a - single SKILL.md file with frontmatter. For richer skills with - supporting files, add them to your repo under `.agents/skills/` or - load them from a workspace. - + {editorDescription}
@@ -283,79 +297,81 @@ export const PersonalSkillEditor: React.FC = ({ )} -
-
- -

- Upload or paste a full SKILL.md file with frontmatter to - auto-fill the fields below. -

-
- - {importStatus && ( - - {importStatus.title} - {importStatus.detail && ( - {importStatus.detail} - )} - - )} -
- +
+ +

+ Upload or paste a full SKILL.md file with frontmatter to + auto-fill the fields below. +

+
+ - - {importContent && ( + {importStatus && ( + + {importStatus.title} + {importStatus.detail && ( + {importStatus.detail} + )} + + )} +
+ + {importContent && ( + + )} + - )} - +
-
+ )}
= ({ {nameError}

) : ( -

- Use lowercase letters, numbers, and hyphens. Names cannot be - changed after creation. -

+ !readOnly && ( +

+ Use lowercase letters, numbers, and hyphens. Names cannot be + changed after creation. +

+ ) )}
@@ -395,6 +413,7 @@ export const PersonalSkillEditor: React.FC = ({ onChange={form.handleChange} onBlur={form.handleBlur} placeholder="When to use this skill" + readOnly={readOnly} disabled={isBusy} aria-invalid={Boolean(descriptionError)} aria-describedby={ @@ -420,6 +439,7 @@ export const PersonalSkillEditor: React.FC = ({ onChange={form.handleChange} onBlur={form.handleBlur} placeholder="Describe when and how agents should use this skill." + readOnly={readOnly} disabled={isBusy} minRows={8} aria-invalid={Boolean(bodyError)} @@ -444,8 +464,7 @@ export const PersonalSkillEditor: React.FC = ({ isTooLarge && "text-content-destructive", )} > - {formatKiB(sizeBytes)} of{" "} - {formatKiB(PERSONAL_SKILL_MAX_SIZE_BYTES)} + {formatKiB(sizeBytes)} of {formatKiB(SKILL_MAX_SIZE_BYTES)} used.

@@ -453,19 +472,22 @@ export const PersonalSkillEditor: React.FC = ({ - + {!readOnly && ( + + )}
diff --git a/site/src/pages/AgentsPage/components/SkillEnabledSwitch.tsx b/site/src/pages/AgentsPage/components/SkillEnabledSwitch.tsx new file mode 100644 index 0000000000..d3154d6ba0 --- /dev/null +++ b/site/src/pages/AgentsPage/components/SkillEnabledSwitch.tsx @@ -0,0 +1,41 @@ +import { useMutation, useQueryClient } from "react-query"; +import { getErrorDetail, getErrorMessage } from "#/api/errors"; +import type { SkillMetadata } from "#/api/typesGenerated"; +import { Switch } from "#/components/Switch/Switch"; +import { toast } from "#/components/Toaster/toast"; +import type { SkillsScope } from "../utils/skills"; + +type SkillEnabledSwitchProps = { + scope: SkillsScope; + skill: SkillMetadata; +}; + +export const SkillEnabledSwitch: React.FC = ({ + scope, + skill, +}) => { + const queryClient = useQueryClient(); + const toggleMutation = useMutation({ + ...scope.update(queryClient), + onError: (error) => { + toast.error(getErrorMessage(error, `Failed to update ${skill.name}.`), { + description: getErrorDetail(error), + }); + }, + }); + + return ( + + toggleMutation.mutate({ name: skill.name, req: { enabled } }) + } + /> + ); +}; diff --git a/site/src/pages/AgentsPage/components/SkillsTable.test.tsx b/site/src/pages/AgentsPage/components/SkillsTable.test.tsx new file mode 100644 index 0000000000..3ddf0de164 --- /dev/null +++ b/site/src/pages/AgentsPage/components/SkillsTable.test.tsx @@ -0,0 +1,78 @@ +import { screen, waitFor } from "@testing-library/react"; +import userEvent from "@testing-library/user-event"; +import { QueryClientProvider } from "react-query"; +import { describe, expect, it, vi } from "vitest"; +import { API } from "#/api/api"; +import { MockOrganization } from "#/testHelpers/entities"; +import { + createTestQueryClient, + renderComponent, +} from "#/testHelpers/renderHelpers"; +import { MockSkill } from "#/testHelpers/skills"; +import { + fullSkillAccess, + organizationSkillsScope, + type SkillsCopy, +} from "../utils/skills"; +import { SkillsTable } from "./SkillsTable"; + +const copy: SkillsCopy = { + noun: "Organization skill", + title: "Skills", + description: "Organization skills.", + emptyDescription: "Add a skill.", + editorDescription: "Organization skill guidance.", + archiveName: "organization-skills.zip", +}; + +const mockReviewSkill = { ...MockSkill, name: "review-sql", enabled: true }; + +const renderTable = () => { + vi.spyOn(API.experimental, "getOrganizationSkills").mockResolvedValue([ + mockReviewSkill, + ]); + const updateSkill = vi + .spyOn(API.experimental, "updateOrganizationSkill") + .mockResolvedValue({ ...mockReviewSkill, enabled: false, content: "" }); + renderComponent( + + + , + ); + return { user: userEvent.setup(), updateSkill }; +}; + +describe("SkillsTable enabled toggle", () => { + it("sends only the enabled flag", async () => { + const { user, updateSkill } = renderTable(); + + await user.click( + await screen.findByRole("switch", { name: "Enable review-sql" }), + ); + + await waitFor(() => expect(updateSkill).toHaveBeenCalledTimes(1)); + expect(updateSkill.mock.calls[0]).toStrictEqual([ + MockOrganization.id, + "review-sql", + { enabled: false }, + ]); + }); +}); + +describe("SkillsTable row menu dialogs", () => { + it("returns focus to the row menu button when a dialog closes", async () => { + const { user } = renderTable(); + + const menuButton = await screen.findByRole("button", { name: "Open menu" }); + await user.click(menuButton); + await user.click(await screen.findByRole("menuitem", { name: /Delete/ })); + await screen.findByRole("dialog"); + await user.keyboard("{Escape}"); + + await waitFor(() => expect(menuButton).toHaveFocus()); + }); +}); diff --git a/site/src/pages/AgentsPage/AgentSettingsPersonalSkillsPage.tsx b/site/src/pages/AgentsPage/components/SkillsTable.tsx similarity index 68% rename from site/src/pages/AgentsPage/AgentSettingsPersonalSkillsPage.tsx rename to site/src/pages/AgentsPage/components/SkillsTable.tsx index 559f67cca4..7e97a66ad2 100644 --- a/site/src/pages/AgentsPage/AgentSettingsPersonalSkillsPage.tsx +++ b/site/src/pages/AgentsPage/components/SkillsTable.tsx @@ -4,28 +4,26 @@ import JSZip from "jszip"; import { useState } from "react"; import { useMutation, useQuery, useQueryClient } from "react-query"; import { getErrorDetail, getErrorMessage } from "#/api/errors"; -import { - createUserSkill, - deleteUserSkill, - updateUserSkill, - userSkill, - userSkills, -} from "#/api/queries/userSkills"; import type { SkillMetadata } from "#/api/typesGenerated"; import { toast } from "#/components/Toaster/toast"; import { - AgentSettingsPersonalSkillsPageView, - type PersonalSkillDeleteState, - type PersonalSkillEditorState, -} from "./AgentSettingsPersonalSkillsPageView"; -import type { PersonalSkillErrorDisplay } from "./components/PersonalSkillEditor"; + parseSkillMarkdown, + SKILLS_MAX_PER_OWNER, + type SkillAccess, + type SkillFormValues, + type SkillsCopy, + type SkillsScope, +} from "../utils/skills"; +import type { SectionHeaderLevel } from "./SectionHeader"; +import type { SkillErrorDisplay } from "./SkillEditor"; import { - PERSONAL_SKILLS_MAX_PER_USER, - type PersonalSkillFormValues, - parsePersonalSkillMarkdown, -} from "./utils/personalSkills"; + type SkillDeleteState, + type SkillEditorState, + SkillsTableView, + type SkillsTableViewProps, +} from "./SkillsTableView"; -const emptySkillFormValues: PersonalSkillFormValues = { +const emptySkillFormValues: SkillFormValues = { name: "", description: "", body: "", @@ -34,13 +32,15 @@ const emptySkillFormValues: PersonalSkillFormValues = { type DialogState = | { type: "create"; submittedContent?: string } | { type: "edit"; name: string; submittedContent?: string } + | { type: "view"; name: string } | { type: "delete"; skill: SkillMetadata; submittedName?: string } | null; -const personalSkillError = ( +const skillError = ( error: unknown, fallback: string, -): PersonalSkillErrorDisplay | undefined => { + lowerNoun: string, +): SkillErrorDisplay | undefined => { if (!error) { return undefined; } @@ -50,9 +50,9 @@ const personalSkillError = ( if (status === 400) { statusFallback = "Skill content is invalid."; } else if (status === 403) { - statusFallback = "You do not have permission to manage personal skills."; + statusFallback = `You do not have permission to manage ${lowerNoun}s.`; } else if (status === 404) { - statusFallback = "That personal skill was not found."; + statusFallback = `That ${lowerNoun} was not found.`; } else if (status === 409) { statusFallback = "A skill with that name already exists."; } @@ -63,7 +63,7 @@ const personalSkillError = ( }; }; -const downloadPersonalSkillFile = async ( +const downloadSkillFile = async ( name: string, fetchContent: (name: string) => Promise, ): Promise => { @@ -74,9 +74,10 @@ const downloadPersonalSkillFile = async ( ); }; -const exportPersonalSkillsArchive = async ( +const exportSkillsArchive = async ( skills: readonly SkillMetadata[], fetchContent: (name: string) => Promise, + archiveName: string, ): Promise => { const contents = await Promise.all( skills.map(async (skill) => ({ @@ -89,24 +90,48 @@ const exportPersonalSkillsArchive = async ( zip.file(`${name}/SKILL.md`, content); } const archive = await zip.generateAsync({ type: "blob" }); - saveAs(archive, "personal-skills.zip"); + saveAs(archive, archiveName); }; -const AgentSettingsPersonalSkillsPage: React.FC = () => { +type SkillsTableProps = { + scope: SkillsScope; + copy: SkillsCopy; + access: SkillAccess; + enabledOnly?: boolean; + headerLevel?: SectionHeaderLevel; + toolbar?: React.ReactNode; + onManagePermissions?: SkillsTableViewProps["onManagePermissions"]; +}; + +export const SkillsTable: React.FC = ({ + scope, + copy, + access, + enabledOnly = false, + headerLevel, + toolbar, + onManagePermissions, +}) => { + const lowerNoun = copy.noun.toLocaleLowerCase("en-US"); const queryClient = useQueryClient(); const [dialogState, setDialogState] = useState(null); - const skillsQuery = useQuery(userSkills()); - const skills = skillsQuery.data ?? []; + const skillsQuery = useQuery(scope.list); + const skills = (skillsQuery.data ?? []).filter( + (skill) => !enabledOnly || skill.enabled, + ); const existingNames = skills.map((skill) => skill.name.toLocaleLowerCase("en-US"), ); - const editName = dialogState?.type === "edit" ? dialogState.name : ""; + const editName = + dialogState?.type === "edit" || dialogState?.type === "view" + ? dialogState.name + : ""; const editSkillQuery = useQuery({ - ...userSkill(editName), + ...scope.detail(editName), enabled: Boolean(editName), }); - const createMutationOptions = createUserSkill(queryClient); + const createMutationOptions = scope.create(queryClient); const createMutation = useMutation({ ...createMutationOptions, onSuccess: async (_skill, variables) => { @@ -117,11 +142,11 @@ const AgentSettingsPersonalSkillsPage: React.FC = () => { ? null : current, ); - toast.success("Personal skill created."); + toast.success(`${copy.noun} created.`); }, }); - const updateMutationOptions = updateUserSkill(queryClient); + const updateMutationOptions = scope.update(queryClient); const updateMutation = useMutation({ ...updateMutationOptions, onSuccess: async (skill, variables) => { @@ -133,11 +158,11 @@ const AgentSettingsPersonalSkillsPage: React.FC = () => { ? null : current, ); - toast.success("Personal skill saved."); + toast.success(`${copy.noun} saved.`); }, onError: (error, variables) => { if (isAxiosError(error) && error.response?.status === 404) { - toast.info("That skill was deleted while you were editing it."); + toast.info(`That ${lowerNoun} was deleted while you were editing it.`); setDialogState((current) => current?.type === "edit" && current.name === variables.name && @@ -150,7 +175,7 @@ const AgentSettingsPersonalSkillsPage: React.FC = () => { }, }); - const deleteMutationOptions = deleteUserSkill(queryClient); + const deleteMutationOptions = scope.remove(queryClient); const deleteMutation = useMutation({ ...deleteMutationOptions, onSuccess: async (data, variables) => { @@ -162,7 +187,7 @@ const AgentSettingsPersonalSkillsPage: React.FC = () => { ? null : current, ); - toast.success("Personal skill deleted."); + toast.success(`${copy.noun} deleted.`); }, onError: (error, variables) => { if (isAxiosError(error) && error.response?.status === 404) { @@ -179,25 +204,22 @@ const AgentSettingsPersonalSkillsPage: React.FC = () => { }); const fetchSkillContent = (name: string): Promise => - queryClient.fetchQuery(userSkill(name)).then((skill) => skill.content); + queryClient.fetchQuery(scope.detail(name)).then((detail) => detail.content); const downloadMutation = useMutation({ - mutationFn: (name: string) => - downloadPersonalSkillFile(name, fetchSkillContent), + mutationFn: (name: string) => downloadSkillFile(name, fetchSkillContent), onError: (error) => { - toast.error( - getErrorMessage(error, "Failed to download personal skill."), - { - description: getErrorDetail(error), - }, - ); + toast.error(getErrorMessage(error, `Failed to download ${lowerNoun}.`), { + description: getErrorDetail(error), + }); }, }); const exportAllMutation = useMutation({ - mutationFn: () => exportPersonalSkillsArchive(skills, fetchSkillContent), + mutationFn: () => + exportSkillsArchive(skills, fetchSkillContent, copy.archiveName), onError: (error) => { - toast.error(getErrorMessage(error, "Failed to export personal skills."), { + toast.error(getErrorMessage(error, `Failed to export ${lowerNoun}s.`), { description: getErrorDetail(error), }); }, @@ -207,11 +229,11 @@ const AgentSettingsPersonalSkillsPage: React.FC = () => { ? downloadMutation.variables : undefined; - let editInitialValues: PersonalSkillFormValues | undefined; + let editInitialValues: SkillFormValues | undefined; let editLoadError: unknown = editSkillQuery.error; if (editSkillQuery.data) { try { - const parsed = parsePersonalSkillMarkdown(editSkillQuery.data.content); + const parsed = parseSkillMarkdown(editSkillQuery.data.content); editInitialValues = { name: editSkillQuery.data.name, description: editSkillQuery.data.description, @@ -222,7 +244,7 @@ const AgentSettingsPersonalSkillsPage: React.FC = () => { } } - let editorState: PersonalSkillEditorState | undefined; + let editorState: SkillEditorState | undefined; if (dialogState?.type === "create") { editorState = { mode: "create", @@ -230,9 +252,10 @@ const AgentSettingsPersonalSkillsPage: React.FC = () => { existingNames, submitError: createMutation.variables?.content === dialogState.submittedContent - ? personalSkillError( + ? skillError( createMutation.error, - "Failed to create personal skill.", + `Failed to create ${lowerNoun}.`, + lowerNoun, ) : undefined, isSubmitting: createMutation.isPending, @@ -246,9 +269,12 @@ const AgentSettingsPersonalSkillsPage: React.FC = () => { }, onClose: () => setDialogState(null), }; - } else if (dialogState?.type === "edit") { + } else if (dialogState?.type === "edit" || dialogState?.type === "view") { + const submittedContent = + dialogState.type === "edit" ? dialogState.submittedContent : undefined; editorState = { mode: "edit", + readOnly: dialogState.type === "view", initialValues: editInitialValues, existingNames, loadError: editLoadError, @@ -256,10 +282,11 @@ const AgentSettingsPersonalSkillsPage: React.FC = () => { isRetrying: editSkillQuery.isFetching, submitError: updateMutation.variables?.name === dialogState.name && - updateMutation.variables.req.content === dialogState.submittedContent - ? personalSkillError( + updateMutation.variables.req.content === submittedContent + ? skillError( updateMutation.error, - "Failed to save personal skill.", + `Failed to save ${lowerNoun}.`, + lowerNoun, ) : undefined, isSubmitting: updateMutation.isPending, @@ -281,16 +308,17 @@ const AgentSettingsPersonalSkillsPage: React.FC = () => { }; } - let deleteState: PersonalSkillDeleteState | undefined; + let deleteState: SkillDeleteState | undefined; if (dialogState?.type === "delete") { deleteState = { skill: dialogState.skill, error: deleteMutation.variables === dialogState.skill.name && dialogState.submittedName === dialogState.skill.name - ? personalSkillError( + ? skillError( deleteMutation.error, - "Failed to delete personal skill.", + `Failed to delete ${lowerNoun}.`, + lowerNoun, ) : undefined, isDeleting: @@ -310,8 +338,14 @@ const AgentSettingsPersonalSkillsPage: React.FC = () => { } return ( - { void skillsQuery.refetch(); }} onCreate={() => { - if (skills.length >= PERSONAL_SKILLS_MAX_PER_USER) { + if (skills.length >= SKILLS_MAX_PER_OWNER) { return; } createMutation.reset(); @@ -327,7 +361,7 @@ const AgentSettingsPersonalSkillsPage: React.FC = () => { }} onEdit={(name) => { updateMutation.reset(); - setDialogState({ type: "edit", name }); + setDialogState({ type: access.update ? "edit" : "view", name }); }} onDelete={(skill) => { deleteMutation.reset(); @@ -346,5 +380,3 @@ const AgentSettingsPersonalSkillsPage: React.FC = () => { /> ); }; - -export default AgentSettingsPersonalSkillsPage; diff --git a/site/src/pages/AgentsPage/AgentSettingsPersonalSkillsPageView.stories.tsx b/site/src/pages/AgentsPage/components/SkillsTableView.stories.tsx similarity index 88% rename from site/src/pages/AgentsPage/AgentSettingsPersonalSkillsPageView.stories.tsx rename to site/src/pages/AgentsPage/components/SkillsTableView.stories.tsx index 6f492ee894..66b731d546 100644 --- a/site/src/pages/AgentsPage/AgentSettingsPersonalSkillsPageView.stories.tsx +++ b/site/src/pages/AgentsPage/components/SkillsTableView.stories.tsx @@ -2,9 +2,15 @@ import type { Meta, StoryObj } from "@storybook/react-vite"; import { expect, fn, userEvent, waitFor, within } from "storybook/test"; import { MockSkill } from "#/testHelpers/skills"; import { - AgentSettingsPersonalSkillsPageView, - type AgentSettingsPersonalSkillsPageViewProps, -} from "./AgentSettingsPersonalSkillsPageView"; + fullSkillAccess, + readOnlySkillAccess, + userSkillsScope, +} from "../utils/skills"; +import { + type SkillEditorState, + SkillsTableView, + type SkillsTableViewProps, +} from "./SkillsTableView"; const MockReviewSQLSkill = { ...MockSkill, @@ -20,6 +26,7 @@ const MockReleaseNotesSkill = { id: "skill-write-release-notes", name: "write-release-notes", description: "Draft concise release notes from a change list.", + enabled: false, created_at: "2026-05-01T12:00:00.000Z", updated_at: "2026-05-04T09:15:00.000Z", }; @@ -49,8 +56,21 @@ const MockDebugHTTPSkill = { const MockPersonalSkills = [MockReviewSQLSkill, MockReleaseNotesSkill]; -const baseArgs: AgentSettingsPersonalSkillsPageViewProps = { +const baseArgs: SkillsTableViewProps = { + scope: userSkillsScope(), skills: MockPersonalSkills, + copy: { + noun: "Personal skill", + title: "Personal skills", + description: + "Reusable instructions your agents can pick when they need specialized guidance. Personal skills hold a single SKILL.md file. For richer skills with supporting files, add them to your repo under `.agents/skills/` or load them from a workspace.", + emptyDescription: + "Create a personal skill to save reusable agent guidance for your workflows.", + editorDescription: + "Personal skills are available to your agents and stored as a single SKILL.md file with frontmatter. For richer skills with supporting files, add them to your repo under `.agents/skills/` or load them from a workspace.", + archiveName: "personal-skills.zip", + }, + access: fullSkillAccess, error: undefined, isLoading: false, isRetrying: false, @@ -64,13 +84,13 @@ const baseArgs: AgentSettingsPersonalSkillsPageViewProps = { }; const meta = { - title: "pages/AgentsPage/AgentSettingsPersonalSkillsPageView", - component: AgentSettingsPersonalSkillsPageView, + title: "pages/AgentsPage/components/SkillsTableView", + component: SkillsTableView, args: baseArgs, -} satisfies Meta; +} satisfies Meta; export default meta; -type Story = StoryObj; +type Story = StoryObj; export const Populated: Story = {}; @@ -249,23 +269,38 @@ export const UploadEmptyFile: Story = { }, }; +const mockReviewSQLEditorState: SkillEditorState = { + mode: "edit", + initialValues: { + name: "review-sql", + description: "Review SQL changes for query and index risks.", + body: "Check query plans, missing indexes, and transaction boundaries.", + }, + existingNames: MockPersonalSkills.map((skill) => skill.name), + isLoading: false, + isRetrying: false, + isSubmitting: false, + onRetry: fn(), + onSubmit: fn(), + onClose: fn(), +}; + export const EditDialogOpen: Story = { args: { - editorState: { - mode: "edit", - initialValues: { - name: "review-sql", - description: "Review SQL changes for query and index risks.", - body: "Check query plans, missing indexes, and transaction boundaries.", - }, - existingNames: MockPersonalSkills.map((skill) => skill.name), - isLoading: false, - isRetrying: false, - isSubmitting: false, - onRetry: fn(), - onSubmit: fn(), - onClose: fn(), - }, + editorState: mockReviewSQLEditorState, + }, +}; + +export const ReadOnly: Story = { + args: { + access: readOnlySkillAccess, + }, +}; + +export const ViewDialogOpen: Story = { + args: { + access: readOnlySkillAccess, + editorState: { ...mockReviewSQLEditorState, readOnly: true }, }, }; diff --git a/site/src/pages/AgentsPage/AgentSettingsPersonalSkillsPageView.tsx b/site/src/pages/AgentsPage/components/SkillsTableView.tsx similarity index 57% rename from site/src/pages/AgentsPage/AgentSettingsPersonalSkillsPageView.tsx rename to site/src/pages/AgentsPage/components/SkillsTableView.tsx index a0386b4bbd..3d33523682 100644 --- a/site/src/pages/AgentsPage/AgentSettingsPersonalSkillsPageView.tsx +++ b/site/src/pages/AgentsPage/components/SkillsTableView.tsx @@ -1,4 +1,4 @@ -import { EllipsisVerticalIcon, PlusIcon } from "lucide-react"; +import { CheckIcon, EllipsisVerticalIcon, PlusIcon, XIcon } from "lucide-react"; import type { SkillMetadata } from "#/api/typesGenerated"; import { Alert, AlertDescription } from "#/components/Alert/Alert"; import { ErrorAlert } from "#/components/Alert/ErrorAlert"; @@ -36,61 +36,78 @@ import { TableRowSkeleton, } from "#/components/TableLoader/TableLoader"; import { formatDate } from "#/utils/time"; -import type { PersonalSkillErrorDisplay } from "./components/PersonalSkillEditor"; -import { PersonalSkillEditor } from "./components/PersonalSkillEditor"; -import { SectionHeader } from "./components/SectionHeader"; import { - PERSONAL_SKILLS_MAX_PER_USER, - type PersonalSkillFormValues, -} from "./utils/personalSkills"; + SKILLS_MAX_PER_OWNER, + type SkillAccess, + type SkillFormValues, + type SkillsCopy, + type SkillsScope, +} from "../utils/skills"; +import { RetryButton } from "./RetryButton"; +import { SectionHeader, type SectionHeaderLevel } from "./SectionHeader"; +import type { SkillErrorDisplay } from "./SkillEditor"; +import { SkillEditor } from "./SkillEditor"; +import { SkillEnabledSwitch } from "./SkillEnabledSwitch"; +import { useDialogFocusReturn } from "./useDialogFocusReturn"; -export type PersonalSkillEditorState = +export type SkillEditorState = | { mode: "create"; - initialValues: PersonalSkillFormValues; + initialValues: SkillFormValues; existingNames: readonly string[]; - submitError?: PersonalSkillErrorDisplay; + submitError?: SkillErrorDisplay; isSubmitting: boolean; - onSubmit: (values: PersonalSkillFormValues, content: string) => void; + onSubmit: (values: SkillFormValues, content: string) => void; onClose: () => void; } | { mode: "edit"; - initialValues?: PersonalSkillFormValues; + readOnly?: boolean; + initialValues?: SkillFormValues; existingNames: readonly string[]; loadError?: unknown; isLoading: boolean; isRetrying: boolean; - submitError?: PersonalSkillErrorDisplay; + submitError?: SkillErrorDisplay; isSubmitting: boolean; onRetry: () => void; - onSubmit: (values: PersonalSkillFormValues, content: string) => void; + onSubmit: (values: SkillFormValues, content: string) => void; onClose: () => void; }; -export type PersonalSkillDeleteState = { +export type SkillDeleteState = { skill: SkillMetadata; - error?: PersonalSkillErrorDisplay; + error?: SkillErrorDisplay; isDeleting: boolean; onConfirm: () => void; onClose: () => void; }; -export type AgentSettingsPersonalSkillsPageViewProps = { +export type SkillsTableViewProps = { + scope: SkillsScope; skills: readonly SkillMetadata[]; + copy: SkillsCopy; + access: SkillAccess; + headerLevel?: SectionHeaderLevel; + toolbar?: React.ReactNode; error: unknown; isLoading: boolean; isRetrying: boolean; onRetry: () => void; onCreate: () => void; + /** Also handles View: the editor opens read-only without update access. */ onEdit: (name: string) => void; onDelete: (skill: SkillMetadata) => void; onDownload: (skill: SkillMetadata) => void; + onManagePermissions?: ( + skill: SkillMetadata, + onCloseAutoFocus: (event: Event) => void, + ) => void; onExportAll: () => void; downloadingSkillName?: string; isExportingAll: boolean; - editorState?: PersonalSkillEditorState; - deleteState?: PersonalSkillDeleteState; + editorState?: SkillEditorState; + deleteState?: SkillDeleteState; }; const formatUpdatedAt = (value: string) => { @@ -109,9 +126,17 @@ const formatUpdatedAt = (value: string) => { }); }; -const EditSkillDialog: React.FC<{ - state: Extract; -}> = ({ state }) => { +type DialogProps = { + state: State; + onCloseAutoFocus: (event: Event) => void; +}; + +const EditSkillDialog: React.FC< + DialogProps> & { + copy: SkillsCopy; + } +> = ({ copy, state, onCloseAutoFocus }) => { + const lowerNoun = copy.noun.toLocaleLowerCase("en-US"); const handleOpenChange = (open: boolean) => { if (!open) { state.onClose(); @@ -121,9 +146,9 @@ const EditSkillDialog: React.FC<{ if (state.isLoading) { return ( - + - Loading personal skill + Loading {lowerNoun} Fetching the latest SKILL.md content. @@ -137,11 +162,12 @@ const EditSkillDialog: React.FC<{ if (state.loadError || !state.initialValues) { return ( - + - Unable to load personal skill + Unable to load {lowerNoun} - The skill could not be loaded for editing. + The skill could not be loaded for{" "} + {state.readOnly ? "viewing" : "editing"}. {state.loadError ? ( @@ -157,10 +183,10 @@ const EditSkillDialog: React.FC<{ - + @@ -168,27 +194,33 @@ const EditSkillDialog: React.FC<{ } return ( - ); }; -const DeleteSkillDialog: React.FC<{ state: PersonalSkillDeleteState }> = ({ +const DeleteSkillDialog: React.FC> = ({ state, + onCloseAutoFocus, }) => { return ( = ({ ); }; -export const AgentSettingsPersonalSkillsPageView: React.FC< - AgentSettingsPersonalSkillsPageViewProps -> = ({ +export const SkillsTableView: React.FC = ({ + scope, skills, + copy, + access, + headerLevel, + toolbar, error, isLoading, isRetrying, @@ -225,26 +260,40 @@ export const AgentSettingsPersonalSkillsPageView: React.FC< onEdit, onDelete, onDownload, + onManagePermissions, onExportAll, downloadingSkillName, isExportingAll, editorState, deleteState, }) => { - const isAtLimit = skills.length >= PERSONAL_SKILLS_MAX_PER_USER; - const addSkillAction = ( - - ); + const { + setPrimaryFallback, + setSecondaryFallback, + rememberTrigger, + restoreFocus, + } = useDialogFocusReturn(); + const pluralNoun = `${copy.noun.toLocaleLowerCase("en-US")}s`; + const isAtLimit = skills.length >= SKILLS_MAX_PER_OWNER; + const addSkillAction = (ref?: React.Ref) => + access.create && ( + + ); const headerActions = (
- {addSkillAction} + {addSkillAction(setPrimaryFallback)}
); return (
+ {toolbar} - {isAtLimit && ( + {access.create && isAtLimit && ( - You have reached the limit of {PERSONAL_SKILLS_MAX_PER_USER}{" "} - personal skills. Delete a skill before creating another one. + You have reached the limit of {SKILLS_MAX_PER_OWNER} {pluralNoun}. + Delete a skill before creating another one. )} {Boolean(error) && } - +
Name Description - Updated + Enabled + + Updated + Actions @@ -289,7 +343,7 @@ export const AgentSettingsPersonalSkillsPageView: React.FC< {isLoading ? ( - + @@ -297,6 +351,9 @@ export const AgentSettingsPersonalSkillsPageView: React.FC< + + + @@ -306,23 +363,20 @@ export const AgentSettingsPersonalSkillsPageView: React.FC< ) : skills.length === 0 && error ? ( - {isRetrying && } - Retry - + isRetrying={isRetrying} + onRetry={onRetry} + /> } /> ) : skills.length === 0 ? ( ) : ( skills.map((skill) => ( @@ -340,7 +394,24 @@ export const AgentSettingsPersonalSkillsPageView: React.FC< )} - + + {access.update ? ( + + ) : skill.enabled ? ( + + ) : ( + + )} + + {formatUpdatedAt(skill.updated_at)} @@ -350,6 +421,8 @@ export const AgentSettingsPersonalSkillsPageView: React.FC< size="icon" variant="subtle" aria-label="Open menu" + onPointerDown={rememberTrigger} + onKeyDown={rememberTrigger} > {downloadingSkillName === skill.name ? ( @@ -365,16 +438,29 @@ export const AgentSettingsPersonalSkillsPageView: React.FC< > Download + {onManagePermissions && ( + + onManagePermissions(skill, restoreFocus) + } + > + Manage permissions + + )} onEdit(skill.name)}> - Edit - - - onDelete(skill)} - > - Delete… + {access.update ? "Edit" : "View"} + {access.delete && ( + <> + + onDelete(skill)} + > + Delete… + + + )} @@ -385,9 +471,11 @@ export const AgentSettingsPersonalSkillsPageView: React.FC<
{editorState?.mode === "create" && ( - + )} + {editorState?.mode === "edit" && ( + + )} + {deleteState && ( + )} - {editorState?.mode === "edit" && } - {deleteState && }
); }; diff --git a/site/src/pages/AgentsPage/components/useDialogFocusReturn.ts b/site/src/pages/AgentsPage/components/useDialogFocusReturn.ts new file mode 100644 index 0000000000..47abaa7b47 --- /dev/null +++ b/site/src/pages/AgentsPage/components/useDialogFocusReturn.ts @@ -0,0 +1,45 @@ +import { useRef } from "react"; + +const canFocus = ( + button: HTMLButtonElement | null, +): button is HTMLButtonElement => + Boolean(button?.isConnected && !button.disabled); + +/** + * Returns focus to the button that opened a dialog. Dialogs opened without a + * Radix DialogTrigger would otherwise return focus to the document body. An + * unmounted or disabled opener falls back to the primary, then the secondary + * fallback button, which callers register as callback refs. + */ +export const useDialogFocusReturn = () => { + const triggerRef = useRef(null); + const primaryFallbackRef = useRef(null); + const secondaryFallbackRef = useRef(null); + const rememberTrigger = (event: React.SyntheticEvent) => { + triggerRef.current = event.currentTarget; + }; + const restoreFocus = (event: Event) => { + // An array find here would stop the React Compiler memoizing this closure. + let target = triggerRef.current; + if (!canFocus(target)) { + target = primaryFallbackRef.current; + } + if (!canFocus(target)) { + target = secondaryFallbackRef.current; + } + if (canFocus(target)) { + event.preventDefault(); + target.focus(); + } + }; + return { + setPrimaryFallback: (button: HTMLButtonElement | null) => { + primaryFallbackRef.current = button; + }, + setSecondaryFallback: (button: HTMLButtonElement | null) => { + secondaryFallbackRef.current = button; + }, + rememberTrigger, + restoreFocus, + }; +}; diff --git a/site/src/pages/AgentsPage/utils/personalSkills.test.ts b/site/src/pages/AgentsPage/utils/skills.test.ts similarity index 68% rename from site/src/pages/AgentsPage/utils/personalSkills.test.ts rename to site/src/pages/AgentsPage/utils/skills.test.ts index c649e9bd8b..14a57e034f 100644 --- a/site/src/pages/AgentsPage/utils/personalSkills.test.ts +++ b/site/src/pages/AgentsPage/utils/skills.test.ts @@ -1,18 +1,17 @@ import { describe, expect, it } from "vitest"; import type * as TypesGen from "#/api/typesGenerated"; import { - buildPersonalSkillMarkdown, + buildSkillMarkdown, filterSkillsByQuery, - getPersonalSkillContentSizeBytes, - isPersonalSkillTriggerToken, - isValidPersonalSkillDescription, - isValidPersonalSkillName, - PERSONAL_SKILL_MAX_SIZE_BYTES, - parsePersonalSkillMarkdown, - parsePersonalSkillTrigger, - personalSkillTriggerText, - tryParsePersonalSkillMarkdown, -} from "./personalSkills"; + getSkillContentSizeBytes, + isSkillTriggerToken, + isValidSkillDescription, + isValidSkillName, + parseSkillMarkdown, + parseSkillTrigger, + SKILL_MAX_SIZE_BYTES, + tryParseSkillMarkdown, +} from "./skills"; const now = "2026-05-08T00:00:00Z"; @@ -105,38 +104,32 @@ describe("filterSkillsByQuery", () => { }); }); -describe("personal skill slash triggers", () => { - it("formats skill trigger text", () => { - expect(personalSkillTriggerText(skill("reviewer", "", 0))).toBe( - "/reviewer", - ); - }); - +describe("skill slash triggers", () => { it("parses trigger text at line start or after whitespace", () => { - expect(parsePersonalSkillTrigger("/rev")).toEqual({ + expect(parseSkillTrigger("/rev")).toEqual({ slashOffset: 0, query: "rev", }); - expect(parsePersonalSkillTrigger("ask /docs")).toEqual({ + expect(parseSkillTrigger("ask /docs")).toEqual({ slashOffset: 4, query: "docs", }); }); it("rejects mid-token slash triggers", () => { - expect(parsePersonalSkillTrigger("https://")).toBeNull(); + expect(parseSkillTrigger("https://")).toBeNull(); }); it("validates replacement trigger tokens", () => { - expect(isPersonalSkillTriggerToken("/rev")).toBe(true); - expect(isPersonalSkillTriggerToken("/bad token")).toBe(false); + expect(isSkillTriggerToken("/rev")).toBe(true); + expect(isSkillTriggerToken("/bad token")).toBe(false); }); }); -describe("parsePersonalSkillMarkdown", () => { +describe("parseSkillMarkdown", () => { it("parses SKILL.md frontmatter and body", () => { expect( - parsePersonalSkillMarkdown( + parseSkillMarkdown( '---\nname: "test-skill"\ndescription: "Does a thing"\n---\n\nUse this skill.', ), ).toEqual({ @@ -148,7 +141,7 @@ describe("parsePersonalSkillMarkdown", () => { it("parses folded YAML description values", () => { expect( - parsePersonalSkillMarkdown( + parseSkillMarkdown( [ "---", "name: brainstorming", @@ -175,7 +168,7 @@ describe("parsePersonalSkillMarkdown", () => { it("uses YAML comment semantics in frontmatter", () => { expect( - parsePersonalSkillMarkdown( + parseSkillMarkdown( "---\nname: test-skill\ndescription: Build # test\n---\nBody", ), ).toEqual({ @@ -186,19 +179,17 @@ describe("parsePersonalSkillMarkdown", () => { }); it("rejects non-string frontmatter fields", () => { + expect(() => parseSkillMarkdown("---\nname: null\n---\nBody")).toThrow( + "Skill name must be a string.", + ); expect(() => - parsePersonalSkillMarkdown("---\nname: null\n---\nBody"), - ).toThrow("Skill name must be a string."); - expect(() => - parsePersonalSkillMarkdown( - "---\nname: test-skill\ndescription: null\n---\nBody", - ), + parseSkillMarkdown("---\nname: test-skill\ndescription: null\n---\nBody"), ).toThrow("Skill description must be a string."); }); it("allows whitespace around frontmatter delimiters", () => { expect( - parsePersonalSkillMarkdown(" --- \nname: test-skill\n --- \nBody"), + parseSkillMarkdown(" --- \nname: test-skill\n --- \nBody"), ).toEqual({ name: "test-skill", description: "", @@ -207,14 +198,14 @@ describe("parsePersonalSkillMarkdown", () => { }); it("requires a non-empty body", () => { - expect(() => - parsePersonalSkillMarkdown("---\nname: test-skill\n---\n\n"), - ).toThrow("Skill body is required."); + expect(() => parseSkillMarkdown("---\nname: test-skill\n---\n\n")).toThrow( + "Skill body is required.", + ); }); it("preserves HTML comments in the body", () => { expect( - parsePersonalSkillMarkdown( + parseSkillMarkdown( "---\nname: test-skill\n---\n\nKeep notes.", ), ).toMatchObject({ @@ -223,10 +214,10 @@ describe("parsePersonalSkillMarkdown", () => { }); }); -describe("tryParsePersonalSkillMarkdown", () => { +describe("tryParseSkillMarkdown", () => { it("returns parsed values for valid SKILL.md content", () => { expect( - tryParsePersonalSkillMarkdown( + tryParseSkillMarkdown( "---\nname: test-skill\ndescription: Does a thing\n---\nBody", ), ).toEqual({ @@ -241,9 +232,7 @@ describe("tryParsePersonalSkillMarkdown", () => { it("returns an error message for invalid SKILL.md content", () => { expect( - tryParsePersonalSkillMarkdown( - "---\ndescription: Missing name\n---\nBody", - ), + tryParseSkillMarkdown("---\ndescription: Missing name\n---\nBody"), ).toEqual({ ok: false, error: "Skill name is required.", @@ -251,24 +240,20 @@ describe("tryParsePersonalSkillMarkdown", () => { }); it("keeps delimiter errors distinct", () => { - expect( - tryParsePersonalSkillMarkdown("name: test-skill\n---\nBody"), - ).toEqual({ + expect(tryParseSkillMarkdown("name: test-skill\n---\nBody")).toEqual({ ok: false, error: "Missing opening frontmatter delimiter.", }); - expect( - tryParsePersonalSkillMarkdown("---\nname: test-skill\nBody"), - ).toEqual({ + expect(tryParseSkillMarkdown("---\nname: test-skill\nBody")).toEqual({ ok: false, error: "Missing closing frontmatter delimiter.", }); }); }); -describe("buildPersonalSkillMarkdown", () => { +describe("buildSkillMarkdown", () => { it("builds backend-compatible skill markdown", () => { - const content = buildPersonalSkillMarkdown({ + const content = buildSkillMarkdown({ name: "test-skill", description: "Does a thing", body: "Use this skill.", @@ -277,7 +262,7 @@ describe("buildPersonalSkillMarkdown", () => { expect(content).toBe( '---\nname: test-skill\ndescription: "Does a thing"\n---\nUse this skill.\n', ); - expect(parsePersonalSkillMarkdown(content)).toEqual({ + expect(parseSkillMarkdown(content)).toEqual({ name: "test-skill", description: "Does a thing", body: "Use this skill.", @@ -286,7 +271,7 @@ describe("buildPersonalSkillMarkdown", () => { it("omits the description line when description is empty", () => { expect( - buildPersonalSkillMarkdown({ + buildSkillMarkdown({ name: "test-skill", description: "", body: "Use this skill.", @@ -295,30 +280,30 @@ describe("buildPersonalSkillMarkdown", () => { }); it("quotes skill names that YAML would otherwise coerce", () => { - const content = buildPersonalSkillMarkdown({ + const content = buildSkillMarkdown({ name: "true", description: "", body: "Use this skill.", }); expect(content).toContain('name: "true"'); - expect(parsePersonalSkillMarkdown(content)).toMatchObject({ + expect(parseSkillMarkdown(content)).toMatchObject({ name: "true", }); - const numericNameContent = buildPersonalSkillMarkdown({ + const numericNameContent = buildSkillMarkdown({ name: "123", description: "", body: "Use this skill.", }); expect(numericNameContent).toContain('name: "123"'); - expect(parsePersonalSkillMarkdown(numericNameContent)).toMatchObject({ + expect(parseSkillMarkdown(numericNameContent)).toMatchObject({ name: "123", }); }); it("escapes quoted description values", () => { - const content = buildPersonalSkillMarkdown({ + const content = buildSkillMarkdown({ name: "test-skill", description: 'Review "critical" C:\\paths.', body: "Use this skill.", @@ -327,43 +312,43 @@ describe("buildPersonalSkillMarkdown", () => { expect(content).toContain( 'description: "Review \\"critical\\" C:\\\\paths."', ); - expect(parsePersonalSkillMarkdown(content)).toMatchObject({ + expect(parseSkillMarkdown(content)).toMatchObject({ description: 'Review "critical" C:\\paths.', }); }); }); -describe("isValidPersonalSkillName", () => { +describe("isValidSkillName", () => { it("accepts kebab-case skill names", () => { - expect(isValidPersonalSkillName("test-skill-1")).toBe(true); + expect(isValidSkillName("test-skill-1")).toBe(true); }); it("rejects names outside the backend pattern", () => { - expect(isValidPersonalSkillName("Test Skill")).toBe(false); - expect(isValidPersonalSkillName("test--skill")).toBe(false); - expect(isValidPersonalSkillName("-test-skill")).toBe(false); + expect(isValidSkillName("Test Skill")).toBe(false); + expect(isValidSkillName("test--skill")).toBe(false); + expect(isValidSkillName("-test-skill")).toBe(false); }); it("rejects names over the backend byte limit", () => { - expect(isValidPersonalSkillName("a".repeat(256))).toBe(true); - expect(isValidPersonalSkillName("a".repeat(257))).toBe(false); + expect(isValidSkillName("a".repeat(256))).toBe(true); + expect(isValidSkillName("a".repeat(257))).toBe(false); }); }); -describe("isValidPersonalSkillDescription", () => { +describe("isValidSkillDescription", () => { it("rejects descriptions over the backend byte limit", () => { - expect(isValidPersonalSkillDescription("a".repeat(4096))).toBe(true); - expect(isValidPersonalSkillDescription("a".repeat(4097))).toBe(false); + expect(isValidSkillDescription("a".repeat(4096))).toBe(true); + expect(isValidSkillDescription("a".repeat(4097))).toBe(false); }); }); -describe("getPersonalSkillContentSizeBytes", () => { +describe("getSkillContentSizeBytes", () => { it("counts UTF-8 bytes", () => { - expect(getPersonalSkillContentSizeBytes("a")).toBe(1); - expect(getPersonalSkillContentSizeBytes("🧪")).toBe(4); + expect(getSkillContentSizeBytes("a")).toBe(1); + expect(getSkillContentSizeBytes("🧪")).toBe(4); }); it("exposes the backend size limit", () => { - expect(PERSONAL_SKILL_MAX_SIZE_BYTES).toBe(65_536); + expect(SKILL_MAX_SIZE_BYTES).toBe(65_536); }); }); diff --git a/site/src/pages/AgentsPage/utils/personalSkills.ts b/site/src/pages/AgentsPage/utils/skills.ts similarity index 54% rename from site/src/pages/AgentsPage/utils/personalSkills.ts rename to site/src/pages/AgentsPage/utils/skills.ts index 435951bc85..d622e23a79 100644 --- a/site/src/pages/AgentsPage/utils/personalSkills.ts +++ b/site/src/pages/AgentsPage/utils/skills.ts @@ -1,14 +1,35 @@ import frontMatter from "front-matter"; +import type { QueryClient } from "react-query"; +import { + createOrganizationSkill, + deleteOrganizationSkill, + organizationSkill, + organizationSkills, + updateOrganizationSkill, +} from "#/api/queries/organizationSkills"; +import { + createUserSkill, + deleteUserSkill, + updateUserSkill, + userSkill, + userSkills, +} from "#/api/queries/userSkills"; +import type { + CreateSkillRequest, + Skill, + SkillMetadata, + UpdateSkillRequest, +} from "#/api/typesGenerated"; -export const PERSONAL_SKILL_MAX_SIZE_BYTES = 64 * 1024; -const PERSONAL_SKILL_MAX_NAME_BYTES = 256; -const PERSONAL_SKILL_MAX_DESCRIPTION_BYTES = 4096; -export const PERSONAL_SKILLS_MAX_PER_USER = 100; +export const SKILL_MAX_SIZE_BYTES = 64 * 1024; +const SKILL_MAX_NAME_BYTES = 256; +const SKILL_MAX_DESCRIPTION_BYTES = 4096; +export const SKILLS_MAX_PER_OWNER = 100; -const personalSkillNamePattern = /^[a-z0-9]+(?:-[a-z0-9]+)*$/; +const skillNamePattern = /^[a-z0-9]+(?:-[a-z0-9]+)*$/; const textEncoder = new TextEncoder(); -export type PersonalSkillFormValues = { +export type SkillFormValues = { name: string; description: string; body: string; @@ -29,17 +50,14 @@ type RankedSkill = { index: number; }; -export const personalSkillTriggerText = (skill: { name: string }): string => - `/${skill.name}`; - -type PersonalSkillTriggerMatch = { +type SkillTriggerMatch = { slashOffset: number; query: string; }; -export const parsePersonalSkillTrigger = ( +export const parseSkillTrigger = ( linePrefix: string, -): PersonalSkillTriggerMatch | null => { +): SkillTriggerMatch | null => { const match = /(?:^|\s)\/(\S*)$/.exec(linePrefix); if (!match) { return null; @@ -51,7 +69,7 @@ export const parsePersonalSkillTrigger = ( }; }; -export const isPersonalSkillTriggerToken = (token: string): boolean => +export const isSkillTriggerToken = (token: string): boolean => /^\/\S*$/.test(token); /** @@ -110,7 +128,7 @@ export const filterSkillsByQuery = ( .map(({ skill }) => skill); }; -class PersonalSkillMarkdownError extends Error {} +class SkillMarkdownError extends Error {} const frontmatterStringField = ( attributes: Record, @@ -121,30 +139,24 @@ const frontmatterStringField = ( return ""; } if (typeof value !== "string") { - throw new PersonalSkillMarkdownError(`Skill ${key} must be a string.`); + throw new SkillMarkdownError(`Skill ${key} must be a string.`); } return value.replace(/[\r\n]+$/, ""); }; // The API re-validates on submit; this only projects content into form fields. -export const parsePersonalSkillMarkdown = ( - content: string, -): PersonalSkillFormValues => { +export const parseSkillMarkdown = (content: string): SkillFormValues => { const normalizedContent = content.replace(/^\uFEFF/, ""); const lines = normalizedContent.split("\n"); if (lines[0]?.trim() !== "---") { - throw new PersonalSkillMarkdownError( - "Missing opening frontmatter delimiter.", - ); + throw new SkillMarkdownError("Missing opening frontmatter delimiter."); } const closingIndex = lines.findIndex( (line, index) => index > 0 && line.trim() === "---", ); if (closingIndex < 0) { - throw new PersonalSkillMarkdownError( - "Missing closing frontmatter delimiter.", - ); + throw new SkillMarkdownError("Missing closing frontmatter delimiter."); } const parseableContent = [ @@ -158,7 +170,7 @@ export const parsePersonalSkillMarkdown = ( return frontMatter>(parseableContent); } catch (error) { const message = error instanceof Error ? error.message : "unknown error"; - throw new PersonalSkillMarkdownError(`Invalid frontmatter: ${message}`); + throw new SkillMarkdownError(`Invalid frontmatter: ${message}`); } })(); @@ -167,22 +179,20 @@ export const parsePersonalSkillMarkdown = ( const body = parsed.body.trim(); if (!name) { - throw new PersonalSkillMarkdownError("Skill name is required."); + throw new SkillMarkdownError("Skill name is required."); } if (!body) { - throw new PersonalSkillMarkdownError("Skill body is required."); + throw new SkillMarkdownError("Skill body is required."); } return { name, description, body }; }; -export const tryParsePersonalSkillMarkdown = ( +export const tryParseSkillMarkdown = ( content: string, -): - | { ok: true; values: PersonalSkillFormValues } - | { ok: false; error: string } => { +): { ok: true; values: SkillFormValues } | { ok: false; error: string } => { try { - return { ok: true, values: parsePersonalSkillMarkdown(content) }; + return { ok: true, values: parseSkillMarkdown(content) }; } catch (error) { return { ok: false, @@ -206,13 +216,10 @@ const frontmatterNameValue = (value: string): string => { return lineValue; }; -export const isValidPersonalSkillDescription = (description: string): boolean => - getPersonalSkillContentSizeBytes(description) <= - PERSONAL_SKILL_MAX_DESCRIPTION_BYTES; +export const isValidSkillDescription = (description: string): boolean => + getSkillContentSizeBytes(description) <= SKILL_MAX_DESCRIPTION_BYTES; -export const buildPersonalSkillMarkdown = ( - values: PersonalSkillFormValues, -): string => { +export const buildSkillMarkdown = (values: SkillFormValues): string => { const name = frontmatterNameValue(values.name); const description = frontmatterLineValue(values.description); const body = values.body.trim(); @@ -225,9 +232,84 @@ export const buildPersonalSkillMarkdown = ( return `${frontmatter.join("\n")}\n${body}\n`; }; -export const getPersonalSkillContentSizeBytes = (content: string): number => +export const getSkillContentSizeBytes = (content: string): number => textEncoder.encode(content).length; -export const isValidPersonalSkillName = (name: string): boolean => - personalSkillNamePattern.test(name) && - getPersonalSkillContentSizeBytes(name) <= PERSONAL_SKILL_MAX_NAME_BYTES; +export const isValidSkillName = (name: string): boolean => + skillNamePattern.test(name) && + getSkillContentSizeBytes(name) <= SKILL_MAX_NAME_BYTES; + +export type SkillsCopy = { + /** Singular noun in sentence case, for example "Personal skill". */ + noun: string; + title: string; + description: string; + emptyDescription?: string; + editorDescription: string; + archiveName: string; +}; + +export type SkillAccess = { + create: boolean; + update: boolean; + delete: boolean; +}; + +export const fullSkillAccess: SkillAccess = { + create: true, + update: true, + delete: true, +}; + +export const readOnlySkillAccess: SkillAccess = { + create: false, + update: false, + delete: false, +}; + +type SkillsQuery = { + queryKey: readonly unknown[]; + queryFn: () => Promise; +}; + +export type SkillsScope = { + list: SkillsQuery; + detail: (name: string) => SkillsQuery; + create: (queryClient: QueryClient) => { + mutationFn: (req: CreateSkillRequest) => Promise; + onSuccess: (skill: Skill) => unknown; + }; + update: (queryClient: QueryClient) => { + mutationFn: (args: { + name: string; + req: UpdateSkillRequest; + }) => Promise; + onSuccess: ( + skill: Skill, + args: { name: string; req: UpdateSkillRequest }, + ) => unknown; + onSettled: () => Promise; + }; + remove: (queryClient: QueryClient) => { + mutationFn: (name: string) => Promise; + onSuccess: (data: unknown, name: string) => unknown; + }; +}; + +export const userSkillsScope = (user = "me"): SkillsScope => ({ + list: userSkills(user), + detail: (name) => userSkill(name, user), + create: (queryClient) => createUserSkill(queryClient, user), + update: (queryClient) => updateUserSkill(queryClient, user), + remove: (queryClient) => deleteUserSkill(queryClient, user), +}); + +export const organizationSkillsScope = ( + organizationId: string, +): SkillsScope => ({ + list: organizationSkills(organizationId), + detail: (name) => organizationSkill(organizationId, name), + create: (queryClient) => createOrganizationSkill(queryClient, organizationId), + update: (queryClient) => updateOrganizationSkill(queryClient, organizationId), + remove: (queryClient) => deleteOrganizationSkill(queryClient, organizationId), +}); diff --git a/site/src/router.test.tsx b/site/src/router.test.tsx new file mode 100644 index 0000000000..2546368b59 --- /dev/null +++ b/site/src/router.test.tsx @@ -0,0 +1,21 @@ +import { render, screen } from "@testing-library/react"; +import { createMemoryRouter, matchRoutes, RouterProvider } from "react-router"; +import { expect, it } from "vitest"; +import { router } from "./router"; + +it("redirects the old personal skills settings path to the skills page", async () => { + const oldPath = "/agents/settings/personal-skills"; + const match = matchRoutes(router.routes, oldPath)?.at(-1); + const memoryRouter = createMemoryRouter( + [ + { path: oldPath, element: match?.route.element }, + { path: "/agents/settings/skills", element:
Skills
}, + ], + { initialEntries: [oldPath] }, + ); + + render(); + + await screen.findByText("Skills"); + expect(memoryRouter.state.location.pathname).toBe("/agents/settings/skills"); +}); diff --git a/site/src/router.tsx b/site/src/router.tsx index 59db87eba1..c67cc4f522 100644 --- a/site/src/router.tsx +++ b/site/src/router.tsx @@ -389,8 +389,8 @@ const CoderAgentsPage = lazy( const AgentSettingsUserAgentsPage = lazy( () => import("./pages/AgentsPage/AgentSettingsUserAgentsPage"), ); -const AgentSettingsPersonalSkillsPage = lazy( - () => import("./pages/AgentsPage/AgentSettingsPersonalSkillsPage"), +const AgentSettingsSkillsPage = lazy( + () => import("./pages/AgentsPage/AgentSettingsSkillsPage"), ); const AgentSettingsAPIKeysPage = lazy( () => import("./pages/AgentsPage/AgentSettingsAPIKeysPage"), @@ -475,6 +475,10 @@ const AISettingsUpdateMCPServerPage = lazy( ), ); +const AISettingsSkillsPage = lazy( + () => import("./pages/AISettingsPage/SkillsPage/SkillsPage"), +); + const GlobalLayout = () => { return ( }> @@ -772,6 +776,7 @@ export const router = createBrowserRouter( path="mcp-servers/:serverId" element={} /> + } /> } /> } /> + } /> } + element={} />