mirror of
https://github.com/coder/coder.git
synced 2026-10-16 14:48:56 +08:00
Michael Suchacz
adcce66826
## Stack Closes [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 5. **#30613** chats: chatd loading, `/` menu, docs (this PR) ## What changes This PR makes chats use organization skills: chatd adds the skills shared with the chat owner in the chat's organization to the skill index, `read_skill` loads them, the `/` menu offers them, and the docs describe the feature. **chatd** (`coderd/x/skills`, `coderd/x/chatd`): - New source `skills.SourceOrganization` (`"org"`). `MergeSkills(personal, organization, workspace)` sorts by name, then source. A name in one source keeps its bare alias; a name in more than one source gets a qualified alias in every source that has it (`personal/<name>`, `org/<name>`, `workspace/<name>`), so no source overrides another. `Lookup` returns `ErrSkillAmbiguous` for a bare name that matches several sources. - **Authorization (please review closely):** each turn, chatd reads organization skill metadata and bodies as the chat owner's real RBAC subject with their roles and groups (`chatOwnerSubject`), never as the chatd system actor, so per-skill ACLs apply. Disabled skills are skipped. A skill that is deleted, disabled, or no longer shared by the time `read_skill` runs is reported as not found. If the owner subject or the metadata can't be loaded, turn preparation fails with the error, as main now does for personal skills (#30492), instead of running the turn without organization skills. - `read_skill` loads personal and organization bodies through one `LoadStoredSkillBody` callback. An index description longer than 1,024 runes, the Agent Skills specification limit (`maxSkillIndexDescriptionRunes`), is cut to 1,023 runes plus an ellipsis so it stays within the limit; `read_skill` still returns the full skill. - Parse and lookup errors keep `%w` last on purpose: with `xerrors`, a `%w` that isn't last repeats the cause, so main's detail read "skill is too large: got N bytes, maximum is 65536 bytes: skill is too large". This stack returns the cause once (ThomasK33 asked about the order in review). **`/` menu** (`site/src/pages/AgentsPage`): the chat page fetches the chat organization's enabled skills (`organizationSkills(...)` from #30612's queries) next to personal ones. `utils/skillAliases.ts` (`resolveSkillTriggers`) follows `MergeSkills`: a personal or organization trigger is bare unless its name is in more than one source, then it becomes `/personal/<name>` or `/org/<name>`. While any list is unknown every trigger is qualified, because a qualified alias always resolves; workspace triggers are always qualified. Built-in `/clear` and `/compact` stay pending until every skill list is known, and a list with no data counts as unknown even after an error, so a failed fetch can't make a built-in run in place of a same-named skill. Disabled skills never shadow a built-in. **Docs:** `docs/ai-coder/agents/extending-agents.md` gains two sections, Organization skills (manage them, control who can use one, find the ones you can use) and How chats load stored skills (turning a skill on or off, skills with the same name). The admin page's organization picker shows only for a user who can view more than one organization's skills. The member "From your organizations" section shows one organization at a time with its own picker, starts on the default organization, and has Export all for the selected organization. The Enabled column is a switch for editors and a check or X otherwise. The settings table in `platform-controls/organizations.md` gains rows for organization and personal skills. ## Commits The first commit reproduces the reviewed chatd code from the old stack; the site and docs commits reapply the old layers on #30612's queries, and the rest are labeled trims or fixes. - `aa634eec94a` feat(coderd/x): load organization skills in chats. Reproduction of the old skill sources and chatd layers (old head `a1047f4986`). - `97945db4e86` test(coderd/x/skills): make `TestMergeSkills` (6 cases) and `TestLookup` (8 cases) table-driven; coverage is 100% before and after, with the same blocks. - `dde61cc5fe2` test(coderd/x/chatd): drop skill tests duplicated by the organization skills integration test `TestOrganizationSkillsFollowChatOwnerAccess`; the covered block set of the touched code is identical (348 of 506 blocks). - `aad14b2dd05` feat(site/src/pages/AgentsPage): show organization skills in the slash menu. The old alias helper and slash menu layers, applied cleanly on #30612's queries. - `712e272e140` test(site/src/pages/AgentsPage): trim duplicated slash menu tests and stories. Deletes `AgentChatPage.test.tsx` (177 lines; its assertions live in `submitChatTurn.test.ts` and `ChatMessageInput.test.tsx`), two duplicate `ChatMessageInput` cases, six of 17 `SkillsTriggerMenu` stories, and one disabled-skill fixture row from a `ChatMessageInput` story. - `232535e1011` docs(docs/ai-coder/agents): document organization skills. From the old docs layer, with the View line updated for #30612's read-only dialog and Download. - `62dbbd2dfd3` refactor(site/src/pages/AgentsPage): address frontend review on the slash menu. - `7349eac4389` Merge the updated settings-ui layer into chats. Brings in the #30610 and #30612 fixes; the merge has no changes of its own. - `0f84a30ec69` Merge the updated settings-ui layer into chats again, for #30612's read-only View fix; no changes of its own. - `fb37822f985` fix(coderd/x/chatd/chattool): keep truncated skill index descriptions within 1,024 runes. The cut now happens at 1,023 runes so the ellipsis fits; the existing `skill_test.go` assertion now expects 1,023 runes plus the ellipsis. - `1543666b584` docs(docs/ai-coder/agents): state the 1,023-character cut before the skill index ellipsis. - `96f1da47c9a` and `13f03d095ff` merge the updated settings-ui layer for #30612's cache and Skills link fixes; no changes of their own. - `b03c14c86de` and `41cc078b85a` merge the updated settings-ui layer; `b03c14c86de` moves the `AgentChatPage.tsx` and `ChatMessageInput.tsx` call sites, and their tests and stories, to #30612's split `organizationSkills(...)` query. - `1a32d05c9bc` test(site/src/pages/AgentsPage/components/ChatMessageInput): restore per-source loading stories for the slash menu. - `830b0466982` fix(coderd/x/chatd): fail turn preparation when organization skills cannot load. Main now returns database errors from turn preparation (#30492); organization skills still logged and dropped the failure, so a failed read silently hid them from the model. The new `TestFetchOrganizationSkillMetadataListFailure` covers the returned error. - `84c135f8801` and `a6f8f63ad13` merge the updated settings-ui layer, which brings in main; they resolve `generation_preparer.go` against main's turn preparation error handling. - `19b7880840f` merges #30612's member page test change; no changes of its own. - `c74d597f3c5` merges #30612's read-only status change; no changes of its own. - `f89c232c081` merges #30612's skills test changes; no changes of its own. - `17d5b9bfbc5` docs(docs/ai-coder/agents): describe the member skills organization picker and read-only Enabled status. - `f97b1710f71` merges the updated settings-ui layer (main at `e7b76872098`); no changes of its own. ## Size Production +558/-213 (docs +116/-6), tests +895/-262; total +1453/-475. 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 #30593 (skill sources), #30594 (chatd), #30601 (alias helper), #30602 (slash menu), #30604 (docs). ## Validation - Go: all 21 packages under `coderd/x/skills` and `coderd/x/chatd` pass on `62dbbd2dfd3`, plus the coverage comparisons above; `coderd/x/chatd/chattool` passes again on `13f03d095ff` after the truncation fix. - Site: `pnpm run lint` and Biome clean; Vitest 102 tests; Storybook Vitest 49 + 4 tests. `pnpm run lint` reran clean on `13f03d095ff`. - On `a6f8f63ad13`, this PR's head before the test-only `19b7880840f` merge: `go test` passes for `coderd/x/skills`, `coderd/x/chatd/chattool`, and the `coderd/x/chatd` skill tests (`-run 'OrganizationSkill|Skill'`); 38 Vitest files covering skills and the slash menu pass, 542 tests with 2 skipped. - Docs: markdownlint 0 findings, Vale 0 findings. - On `17d5b9bfbc5`: Vale clean, markdownlint and `make pre-commit` pass. On `f97b1710f71`: `go build`, the `coderd/x/skills` and `chattool` tests, `pnpm check` and `tsc` pass. - Remote UAT round 7 through Coder Agents: PASS ([chat](https://dogfood.cdr.dev/agents/3426d4d3-2139-4ae1-8e0c-72ddeff413a8)). - Full run on `62dbbd2dfd3`, including an in-place upgrade from main `50c785cc6d7`: personal skills kept their IDs, names, content hashes and timestamps, came up enabled, and worked in chats (`read_skill`) and the `/` menu; create, edit, download and delete worked after the upgrade. The admin page, sharing, the Enabled toggle, the member section, the `/` menu, chats reading organization skills as the owner, limits, audit, authorization and scoped keys, and the docs all passed. - Delta run on `7349eac4389`: an update-only key gets 404 on PATCH with no content in the response; saving permissions refreshes Updated without a reload; smoke of the admin page, auditor View and Download, the `/` menu and `read_skill`. - Delta run on `0f84a30ec69` for `aaa356f0fd5`: read-only View, as an org auditor and as a member, shows no Name hint, focuses no field and starts on Close, and Escape or Close returns focus to the row menu button; Add skill and Edit still show the hint and focus the import field, and a saved edit persisted; smoke of the admin page, the `/` menu organization group and `read_skill`. Screen-reader behavior was not tested. - Delta run on `13f03d095ff` for `698dd83946d`, `f82573cd1c2`, `0e55137de7b` and `fb37822f985`: for an organization admin of org B who is a plain member of org A, the sidebar Skills link drops `?org=` for org A on seven AI settings pages and opens org B's skills, keeps `?org=` for org B, and a typed `/ai/settings/skills?org=` for org A still shows the permission error. A skill deleted and recreated with the same name loads a fresh sharing list showing only Everyone, with no stale grant. A 1,500-character description appears in the chat's skill index as 1,023 characters plus an ellipsis, while `read_skill` returns all of it. Smoke of the admin toggle, the member section, the `/` menu organization group and `read_skill`. - Low findings: the read-only View hint and field focus, fixed in `aaa356f0fd5` and verified by the delta run above. The member section's always-on disabled Enabled switch (also in the old stack) is replaced by a read-only status icon in #30612's `63bb42754e3`, which has no UAT run. Not fixed: `/` shows the `/personal/` prefix while organization skills fail to load (the unknown-list rule above); the shared Switch component's disabled styling (unchanged from main). A user who can read skills in only one organization sees no organization name on the Skills page, because the picker is hidden for a single organization, as on the MCP servers page on main. - Not tested: the workspace skills group (no running agent on the test deployment), the 100-per-user personal cap, and behavior without a Premium license. - Remote UAT round 8 through Coder Agents: PASS on `b03c14c86de` (same chat). The remote grader marked the unknown `read_skill` item FAIL only because the prompt asked for the literal phrase "skill not found"; the result names the skill once and says "not found" once, the same format as main, so the runner graded it PASS on intent. - Member page: starts on the default organization, shows one organization's table with the read-only note once, Export all downloads only the selected organization's skills, the picker is hidden with one organization, and an organization without enabled skills isn't listed. - Personal and admin tables: create, edit, toggle and delete each refetch the list; reopening the editor shows the new content even with the detail request delayed; a toggle changes state once, with no flicker. A skill deleted and recreated with the same name shows only Everyone in sharing. - The sidebar Skills link keeps `?org=` only for an organization whose skills the user can read. A create over 64 KiB returns 400 with the detail "got 70000 bytes, maximum is 65536 bytes: skill is too large", the cause once. The `/` menu shows Commands, Personal skills and Organization skills, and invoking an organization skill loads its body through `read_skill`. - New low findings, not fixed: the member page picker's listbox is labeled "Suggestions" (likely from the shared `OrganizationAutocomplete`); a typed body over 64 KiB shows only the size counter and a disabled Create until Import; `/ai/settings?org=` redirects to the models page without `?org=`. The first and third were not checked against main. - `9002dc60382`, `1a32d05c9bc`, `830b0466982`, `ac3e62c83b7`, `63bb42754e3`, `82b2487ad17`, `17d5b9bfbc5` and the main merges came after `b03c14c86de`; they have tests but no UAT run. - Every UAT round ran the migration as `000615_skills`. The heads ship the same up and down SQL as `000621_skills`, so the upgrade path from main's 000620 to 000621 has no UAT run. > Xum opened this PR on behalf of @ibetitsmike.