From d8b9174235b97d0b4ef9f281e20e5704182d8dcb Mon Sep 17 00:00:00 2001 From: Guilhem Date: Fri, 11 Sep 2026 12:27:37 +0200 Subject: [PATCH] feat(ai-sessions): turn skills on by default, and group them by folder (#11058) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * feat(ai-sessions): turn skills on by default, and group them by folder A skill is instructions the workspace wrote for the assistant to use, so what carrying one costs is context rather than access. Selecting each one before it applied made publishing a skill a two-step affair, and left most of them unused. Skills now default to on. No storage is rewritten to get there: the preference keeps its key and holds a decision per path, so the older array of enabled paths still reads as "these were on" and only the paths nobody decided about move. MCP servers stay opt-in through the same factory — their tools reach an external system, which is a different question from context. The Skills settings list groups into a tree once skills span more than one folder, with a switch per folder acting on everything beneath it, and the list answers the keyboard: Up/Down walk it, Left/Right fold, Space flips the switch under the highlight, Enter opens the skill. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01JWUQ867ZJCZJmkWUxqHija * fix: give a modal the option to stand only as tall as the window `AIPromptsModal` asks for 1000px of height, which is taller than a laptop window: the dialog then scrolled inside the overlay while its list scrolled inside the dialog — two scrollbars, one of them moving the modal itself. The cap `Modal2` appeared to have, `max-h-screen-80`, is defined nowhere in the tailwind config, so it never applied to anything. `fixedHeight="viewport"` is a new value that stands as tall as the window allows. Deliberately a definite height rather than a max-height: bodies here size against the box with `h-full` / `grow min-h-0` and scroll inside it, and a max-height leaves them nothing to resolve against — they grow past the surface instead. Every existing size keeps the height it has today, so no other modal moves. The two classes that resolved to nothing are removed. The prompts modal and the assistant settings modal take the new value; both already scroll inside themselves. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01JWUQ867ZJCZJmkWUxqHija * fix: pin read_skill's gate in its test, and say who a delete affects The refusal `read_skill` gives for a path that is not a skill changed shape — it checks the workspace listing now, not just the off-switch — and its test was still asserting the old wording against an unmocked listing. The delete confirmation said everyone "who selected it" loses the skill, which stopped being true when skills started defaulting to on. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01JWUQ867ZJCZJmkWUxqHija * fix: keep the keyboard walk when the list scrolls under the pointer The mouse takes the skills list back on a real movement over it, not on `mouseenter`. The browser fires that one whenever rows arrive under a stationary pointer — every scroll the keyboard itself causes, and every folder collapse — so walking Down past the bottom of the list handed control back to a mouse nobody had touched, and the next press restarted at the top. Also from the review round: the "+" menu sorted skills on-first, a key that is constant now that they start on, and pushed the one row it did move — a skill just turned off there — out of the shortcut that turns it back on. It orders by path. The remaining "selection" wording follows the vocabulary the rest of this change moved to. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01JWUQ867ZJCZJmkWUxqHija * fix: carry the keyboard walk on from the row the mouse left it on Handing the list to the mouse dropped the highlight, so the next arrow press started again at the top. It moves to the row under the pointer instead — invisible while the mouse leads, since drawing and acting both wait on the keyboard being in charge, and exactly where someone would expect the walk to carry on from. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01JWUQ867ZJCZJmkWUxqHija * fix: cap the AI prompts modal from its own call site Reverts `Modal2` and the assistant settings modal to what they were. The prompts modal asks for `xxl`, 1000px, which is taller than a laptop window, so the dialog scrolled inside the overlay while its list scrolled inside the dialog. It now passes `max-h-[80vh]` through the `css.popup` the component already forwards. The height stays definite underneath, which is what lets the list bound its own scroller, and nothing outside this one modal changes. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01JWUQ867ZJCZJmkWUxqHija * refactor: drive the skills list highlight with useListHighlight The Tools section next door already had this: `useListHighlight` owns the highlighted index, wrapping, `scrollIntoView`, and the rule that a scroll under a resting pointer must not hand the list back to the mouse — the bug this section rediscovered the hard way. Reusing it drops the parallel implementation. What stays local is what is actually a tree: Left and Right fold a folder or step into it, Space flips the switch under the highlight, and Enter opens the lit skill. `restingIndex` is what keeps the highlight on a folder through a fold, where a search would instead send it back to its top hit. The keys are answered at the window rather than on the list: leaving the editor parks focus elsewhere, and a container-scoped handler goes silent when it does. `move` is now returned by the composable, for the step into a folder's children. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01JWUQ867ZJCZJmkWUxqHija * fix: stop the fold's sticky row resetting the keyboard walk `stickyKey` is read through `restingIndex`, which `useListHighlight` calls inside the effect that reacts to the row count. As `$state` it was also a dependency of that effect, so clearing it on the next arrow re-ran the effect and wrote the highlight back to nothing: after collapsing a folder, one Down lit nothing and the one after it started again at the top. It is a plain variable now, read when the effect runs and invalidating nothing. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01JWUQ867ZJCZJmkWUxqHija * fix: keep one lit row, and keep the fold's sticky row to its fold Three from the review of the `useListHighlight` swap: The sticky row a fold takes is now given up as soon as that fold has rendered. Held until the next arrow, it pulled the highlight back to that folder on any later change — another fold, a save, a delete, a workspace switch. Space and Enter on a focused control bring the highlight to that control's row before the control answers them. A switch keeps focus after a plain click, and the row drawn as highlighted was then a different one from the row that flipped. Up and Down carry on from a row reached with Tab. `useListHighlight` cannot see that by itself: `ListRow` puts the row's id on its outer div while focus sits on the button inside it. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01JWUQ867ZJCZJmkWUxqHija * fix: land the highlight on a named row rather than stepping to it `move` counts steps from wherever the highlight is, and from nothing lit it can only reach an end of the list — so the three places that meant "put it on this row" (a row reached with Tab, the row of a focused control, a folder's parent) sent it to the first row whenever nothing was lit yet. `useListHighlight` grows a `moveTo` for naming the row outright, and those three use it. The handler's own doc still said the keys are answered on the list; they went back to the window when the editor's page transition proved able to take focus away from it. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01JWUQ867ZJCZJmkWUxqHija * fix: fold from the header click the way every other fold does The header's own click wrote `collapsed` directly instead of going through `fold`, so the row count changed with no row named to keep: the highlight reset, and since the highlight is the header's only hover feedback, it went flat under a pointer that had not moved and stayed flat. Also from the round: a duplicated `svelte-ignore`, the missing one on the header wrapper that takes `onmouseenter`, and a trailing comma prettier wanted gone. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01JWUQ867ZJCZJmkWUxqHija --------- Co-authored-by: Claude Opus 5 (1M context) --- .../common/listRow/listHighlight.svelte.ts | 19 +- .../copilot/chat/AIChatManager.test.ts | 20 +- .../copilot/chat/AssistantMcpSection.svelte | 2 +- .../chat/AssistantSkillsSection.svelte | 464 +++++++++++++++--- .../copilot/chat/enabledPathsPreference.ts | 106 ++-- .../copilot/chat/global/core.test.ts | 10 +- .../components/copilot/chat/global/core.ts | 44 +- .../copilot/chat/skills/enabledSkills.ts | 11 +- .../copilot/chat/skills/skillResources.ts | 9 +- .../copilot/chat/skills/skillTree.ts | 119 +++++ .../copilot/chat/skills/skills.test.ts | 107 +++- .../copilot/chat/skills/skillsMenu.svelte.ts | 18 +- .../src/lib/components/mcp/enabledServers.ts | 7 +- .../components/settings/AIPromptsModal.svelte | 13 +- 14 files changed, 762 insertions(+), 187 deletions(-) create mode 100644 frontend/src/lib/components/copilot/chat/skills/skillTree.ts diff --git a/frontend/src/lib/components/common/listRow/listHighlight.svelte.ts b/frontend/src/lib/components/common/listRow/listHighlight.svelte.ts index 4da6f18f32..c7325698e3 100644 --- a/frontend/src/lib/components/common/listRow/listHighlight.svelte.ts +++ b/frontend/src/lib/components/common/listRow/listHighlight.svelte.ts @@ -35,9 +35,9 @@ export function useListHighlight(opts: { untrack(() => (index = resting)) }) - function move(delta: number) { + function land(next: number) { const count = opts.count() - if (count === 0) return + if (count === 0 || next < 0 || next >= count) return pointerOwns = false // Rows are tabbable, so focus can sit on one. Enter then activates whatever is // focused, which has to stay the highlighted row — so any row counts, not just @@ -47,16 +47,29 @@ export function useListHighlight(opts: { const focusedId = document.activeElement?.id const rowWasFocused = !!focusedId && Array.from({ length: count }, (_, i) => opts.rowId(i)).includes(focusedId) - index = index < 0 ? (delta > 0 ? 0 : count - 1) : (index + delta + count) % count + index = next const row = document.getElementById(opts.rowId(index)) row?.scrollIntoView({ block: 'nearest' }) if (rowWasFocused) row?.focus() } + function move(delta: number) { + const count = opts.count() + if (count === 0) return + land(index < 0 ? (delta > 0 ? 0 : count - 1) : (index + delta + count) % count) + } + return { get index() { return index }, + /** Step the highlight, for a list whose own keys move it beyond Up and Down — + * a tree stepping into the children a folder just revealed. */ + move, + /** Put the highlight on a row named outright, rather than a step from wherever + * it is — the row a caller's own key landed on, or the one that has focus. A + * step cannot say this: from nothing lit it can only reach an end of the list. */ + moveTo: land, /** Wire to each row's `onMouseEnter`. */ hovered(i: number) { if (pointerOwns) index = i diff --git a/frontend/src/lib/components/copilot/chat/AIChatManager.test.ts b/frontend/src/lib/components/copilot/chat/AIChatManager.test.ts index c0bc289002..7142646d90 100644 --- a/frontend/src/lib/components/copilot/chat/AIChatManager.test.ts +++ b/frontend/src/lib/components/copilot/chat/AIChatManager.test.ts @@ -580,11 +580,11 @@ describe('AIChatManager global skills', () => { mocks.tryGetCurrentModel.mockReturnValue(model) }) - // Only selected skills reach the prompt, and the selection is keyed by - // workspace and account (see skills/enabledSkills.ts). - function selectSkills(workspace: string, ...paths: string[]) { + // Every readable skill reaches the prompt; only the paths someone decided about + // are stored, keyed by workspace and account (see skills/enabledSkills.ts). + function turnOffSkills(workspace: string, ...paths: string[]) { const stored = JSON.parse(localStorage.getItem('wm_skills_enabled') ?? '{}') - stored[`${workspace}:${TEST_EMAIL}`] = paths + stored[`${workspace}:${TEST_EMAIL}`] = Object.fromEntries(paths.map((p) => [p, false])) localStorage.setItem('wm_skills_enabled', JSON.stringify(stored)) } @@ -594,8 +594,6 @@ describe('AIChatManager global skills', () => { resolveParentSkills = resolve }) mocks.workspace = 'parent' - selectSkills('parent', 'f/skills/parent-skill') - selectSkills('child', 'f/skills/child-skill') mocks.listResource.mockImplementation(({ workspace }: { workspace: string }) => { if (workspace === 'parent') { return parentSkills @@ -639,12 +637,12 @@ describe('AIChatManager global skills', () => { expect(manager.systemMessage.content).not.toContain('parent-skill') }) - it('leaves a readable but unselected skill out of the prompt', async () => { + it('leaves a skill turned off out of the prompt', async () => { mocks.listResource.mockResolvedValue([ - { path: 'f/skills/selected', description: 'the one turned on' }, - { path: 'f/skills/unselected', description: 'readable but never turned on' } + { path: 'f/skills/selected', description: 'left on, like every skill starts' }, + { path: 'f/skills/unselected', description: 'the one turned off' } ]) - selectSkills('test_workspace', 'f/skills/selected') + turnOffSkills('test_workspace', 'f/skills/unselected') const manager = new AIChatManager() manager.isSessionChat = true @@ -659,7 +657,6 @@ describe('AIChatManager global skills', () => { mocks.listResource.mockResolvedValue([ { path: 'u/admin/review-code', description: 'review code for bugs' } ]) - selectSkills('test_workspace', 'u/admin/review-code') mocks.runChatLoop.mockImplementation(async (config: any) => { const userMessage = config.messages[config.messages.length - 1] expect(userMessage.content).toContain('Use the skill at "u/admin/review-code". find bugs') @@ -686,7 +683,6 @@ describe('AIChatManager global skills', () => { { path: 'u/admin/deploy', description: 'personal deploy steps' }, { path: 'f/team/deploy', description: 'the team deploy steps' } ]) - selectSkills('test_workspace', 'u/admin/deploy', 'f/team/deploy') mocks.runChatLoop.mockImplementation(async (config: any) => { // Picking either one would silently apply instructions the user did not // choose, so the text is left alone for the model to ask about. diff --git a/frontend/src/lib/components/copilot/chat/AssistantMcpSection.svelte b/frontend/src/lib/components/copilot/chat/AssistantMcpSection.svelte index 9440aa8f58..1637165f87 100644 --- a/frontend/src/lib/components/copilot/chat/AssistantMcpSection.svelte +++ b/frontend/src/lib/components/copilot/chat/AssistantMcpSection.svelte @@ -482,7 +482,7 @@ switch that decides whether this chat carries its tools. {#snippet subtitle()}{server.description}{/snippet} {#snippet trailing()} await toggle(server.path, e.detail)} diff --git a/frontend/src/lib/components/copilot/chat/AssistantSkillsSection.svelte b/frontend/src/lib/components/copilot/chat/AssistantSkillsSection.svelte index b545eaea18..d55f8a9cd1 100644 --- a/frontend/src/lib/components/copilot/chat/AssistantSkillsSection.svelte +++ b/frontend/src/lib/components/copilot/chat/AssistantSkillsSection.svelte @@ -24,19 +24,25 @@ and the actions that create, edit, import and delete them. import { userStore } from '$lib/stores' import { sendUserToast } from '$lib/toast' import { logFeatureUsage } from '$lib/utils/featureUsage' - import { untrack } from 'svelte' + import { tick, untrack } from 'svelte' import { ArrowLeft, BookOpen, + ChevronDown, + ChevronRight, Eye, + Folder, + FolderTree, FolderUp, Pencil, Plus, RotateCcw, - Trash2 + Trash2, + User } from 'lucide-svelte' + import { useListHighlight } from '$lib/components/common/listRow/listHighlight.svelte' import { getAiChatManager } from './aiChatManagerContext' - import { isSkillEnabled, setSkillEnabled } from './skills/enabledSkills' + import { forgetSkill, isSkillEnabled, setSkillEnabled } from './skills/enabledSkills' import { ambiguousSkillNames, deleteSkillResource, @@ -52,6 +58,17 @@ and the actions that create, edit, import and delete them. parseSkillMd, type SkillUpload } from './skills/skillMd' + import { + buildSkillTree, + countSkills, + folderKey, + nodeSkills, + skillFolderPaths, + skillKey, + visibleEntries, + type SkillTreeEntry, + type SkillTreeNode + } from './skills/skillTree' let { ws, @@ -162,6 +179,53 @@ What the assistant should do when this skill applies. let overwriteChoices: Record = $state({}) let ambiguous = $derived(ambiguousSkillNames(skills)) + // Skills in one folder are a list; spread over several, the folder is what tells + // two of them apart, so they are grouped under it instead. + let grouped = $derived(skillFolderPaths(skills).size > 1) + let tree = $derived(buildSkillTree(skills)) + /** Folders the user closed, keyed by full prefix. Everything opens expanded: the + * point of this list is seeing what the assistant carries. */ + let collapsed = $state>({}) + /** The list top to bottom as it stands, for the keyboard to walk. Empty whenever + * the rows are not the thing on screen — a reload after a save or a delete shows + * the loading line, and a failed one shows the error, both over the rows this + * would otherwise still walk. Flat mode has no folders, so the same keys drive + * both layouts. */ + let entries: SkillTreeEntry[] = $derived( + loading || loadError !== undefined + ? [] + : grouped + ? visibleEntries(tree, (path) => collapsed[path] === true) + : skills.map((skill) => ({ kind: 'skill', key: skillKey(skill.path), skill })) + ) + /** Where each entry sits in the walk, so a row rendered deep in the tree can say + * whether it is the highlighted one. */ + let entryIndexByKey = $derived(new Map(entries.map((e, i) => [e.key, i]))) + /** The row the highlight goes back to when the list changes shape under it. + * Folding is the case: it changes the row count without reshuffling what the rows + * mean, and the folder just folded is where someone still is — unlike a search, + * which reranks everything and belongs back at its top hit. + * + * Deliberately not `$state`. `useListHighlight` reads this through `restingIndex` + * inside the effect that reacts to the row count, so a reactive write here would + * re-run that effect and reset the highlight — clearing it on the next arrow would + * undo the very move that cleared it. */ + let stickyKey: string | undefined = undefined + const highlight = useListHighlight({ + count: () => entries.length, + rowId: (index) => entryDomId(entries[index]?.key ?? ''), + // Otherwise nothing is lit until a key or the pointer picks a row: this list has + // no search ranking one to the top. + restingIndex: () => (stickyKey === undefined ? -1 : (entryIndexByKey.get(stickyKey) ?? -1)), + onActivate: (index) => { + const entry = entries[index] + if (entry === undefined) return + if (entry.kind === 'skill') openSkill(entry.skill) + else fold(entry.key, entry.node.path, !collapsed[entry.node.path]) + } + // No `activateEnterFrom`: Enter is answered by `onListKeydown`, which does not + // depend on where focus happens to be. + }) let parsed = $derived(parseSkillMd(content)) // The Path field is what names the skill, and Path validates it. The frontmatter // `name` only seeds that field and is never persisted, so validating it here @@ -219,21 +283,154 @@ What the assistant should do when this skill applies. if (!active && !contentChanged && !pathChanged) editorOpen = false }) + /** The list takes focus when this section is the one on screen. + * + * Without it the keyboard here is unreachable by the way people arrive: the click + * on "Skills" in the settings sidebar leaves that button focused, and the sidebar + * answers Up/Down itself (`arrowTabNav`), so the arrows would walk the sections + * rather than the skills. Taking focus is also what makes the keys unambiguous — + * a control keeps Space and Enter, and none holds them once the list has focus. */ + let listEl: HTMLDivElement | undefined = $state(undefined) + $effect(() => { + if (!active || editorOpen) return + const el = listEl + untrack(() => { + if (el && !el.contains(document.activeElement)) el.focus({ preventScroll: true }) + }) + }) + /** Escape leaves the editor rather than the whole modal: `blocksClose` stops the * modal's own handler, so this is the only thing left to answer the key. */ function onKeydown(event: KeyboardEvent) { // Every section stays mounted while the modal is open, and `stopPropagation` // does nothing between listeners on `window`: without this, a key aimed at the // section on screen is answered by the four behind it too. - if (!active || event.key !== 'Escape' || !editorOpen) return + if (!active) return + // A dialog of ours is up and owns the keyboard. if (toDelete !== undefined || pendingImport !== undefined) return - event.preventDefault() - event.stopPropagation() - closeEditor() + if (editorOpen) { + if (event.key !== 'Escape') return + event.preventDefault() + event.stopPropagation() + closeEditor() + return + } + onListKeydown(event) + } + + /** DOM id of a row, so the highlight can bring itself into view. */ + function entryDomId(key: string): string { + return `wm-skill-entry-${key}` + } + + /** Fold or unfold, keeping the highlight on the folder rather than losing it to + * the row count changing underneath. */ + function fold(key: string, path: string, shut: boolean) { + stickyKey = key + collapsed[path] = shut + // Held only across the re-render this fold causes. Left set, it would pull the + // highlight back to this folder on the next change of any kind — another fold, a + // save, a delete, a workspace switch. + tick().then(() => (stickyKey = undefined)) + } + + /** The tree keys this list adds to `useListHighlight`: Left and Right fold a folder + * or step into it, Space flips the switch under the highlight. Up, Down and Enter + * are the composable's, and so is everything about the mouse — a scroll under a + * resting pointer does not move the highlight, only a real movement does. + * + * Answered at the `window`, not on the list: focus moves around this modal — the + * page transition out of the editor parks it elsewhere — and a handler bound to the + * list would go silent whenever it did. Keys left unanswered keep their meaning: + * Left and Right with nothing lit still step between this list and the editor, + * which is `PagedContent` reading the same event. */ + function onListKeydown(event: KeyboardEvent) { + if (event.metaKey || event.ctrlKey || event.altKey || event.defaultPrevented) return + const target = event.target as HTMLElement | null + // A focused control answers its own activation keys — a row's switch, a folder + // header, the buttons above the list. `Toggle` hides a real checkbox behind its + // label (frontend/AGENTS.md), and that checkbox holds focus after a plain click. + const control = target?.closest?.('button, a, input, select, textarea, [role="button"]') as + | HTMLElement + | null + | undefined + if ((event.key === ' ' || event.key === 'Enter') && control) { + // The control answers the key, and the highlight follows it there: a switch + // holds focus after a plain click, and leaving another row lit would draw one + // row while flipping another. + const row = control.closest('[id^="wm-skill-entry-"]') + const index = row ? (entryIndexByKey.get(row.id.replace('wm-skill-entry-', '')) ?? -1) : -1 + if (index >= 0) highlight.moveTo(index) + return + } + if (event.key === 'ArrowDown' || event.key === 'ArrowUp') { + // A row reached with Tab is where the walk carries on from. `useListHighlight` + // cannot see that itself: `ListRow` puts the row's id on its outer div while + // focus sits on the button inside it. + const focusedRow = target?.closest?.('[id^="wm-skill-entry-"]') + const focusedIndex = focusedRow + ? (entryIndexByKey.get(focusedRow.id.replace('wm-skill-entry-', '')) ?? -1) + : -1 + if (focusedIndex >= 0) highlight.moveTo(focusedIndex) + // Taking the highlight takes the keyboard with it: a control left focused + // would keep Space and act on its own row while another one is lit. + if (control) { + control.blur() + listEl?.focus({ preventScroll: true }) + } + highlight.onKeydown(event) + return + } + const current = entries[highlight.index] + if (current === undefined) { + highlight.onKeydown(event) + return + } + const answer = () => { + event.preventDefault() + event.stopPropagation() + } + if (event.key === ' ') { + answer() + if (current.kind === 'skill') void toggle(current.skill.path, !current.skill.enabled) + else void toggleFolder(current.node, !folderEnabled(current.node)) + } else if (event.key === 'Enter') { + answer() + if (current.kind === 'skill') openSkill(current.skill) + else fold(current.key, current.node.path, !collapsed[current.node.path]) + } else if (event.key === 'ArrowRight') { + answer() + if (current.kind === 'skill') { + // Forward from a lit skill is that skill. Left unanswered the key reaches + // `PagedContent`, which steps to the editor page — showing whichever skill + // it was last parked on, not this one. + openSkill(current.skill) + } else if (collapsed[current.node.path]) { + fold(current.key, current.node.path, false) + } else { + // Into what opening it revealed: the next row down is its first child. + highlight.move(1) + } + } else if (event.key === 'ArrowLeft') { + if (current.kind === 'folder' && !collapsed[current.node.path]) { + answer() + fold(current.key, current.node.path, true) + } else if (current.parentKey !== undefined) { + answer() + const parent = entries.findIndex((e) => e.key === current.parentKey) + if (parent >= 0) highlight.moveTo(parent) + } + } else { + highlight.onKeydown(event) + } } function closeEditor() { editorOpen = false + // The list is what answers the keys, so it takes focus back as the editor gives + // way. Left to the effect above, the page transition lands focus somewhere else + // afterwards and the arrows do nothing until something is clicked. + tick().then(() => listEl?.focus({ preventScroll: true })) } /** Left and Right step between the two pages, which is `PagedContent` answering the @@ -254,6 +451,8 @@ What the assistant should do when this skill applies. untrack(() => { loadSeq++ skills = [] + // Keyed by path, and another workspace's folders are not these. + collapsed = {} listNotice = undefined toDelete = undefined pendingImport = undefined @@ -289,7 +488,7 @@ What the assistant should do when this skill applies. // A notice, not `loadError`: that one replaces the list, and a truncated // read still has skills worth showing. listNotice = truncated - ? `Showing the first ${found.length} skills; this workspace has more. Delete unused ones so the rest can be selected.` + ? `Showing the first ${found.length} skills; this workspace has more. Delete unused ones so the rest are listed here.` : undefined // Seeded rather than filled by the bindings: an unset entry would hand // DropdownV2 an `undefined` open state instead of a closed one. @@ -307,19 +506,41 @@ What the assistant should do when this skill applies. async function toggle(p: string, enabled: boolean) { if (blockedByPendingFork()) return if (!setSkillEnabled(ws, p, enabled)) { - sendUserToast('Could not save the selection for this account.', true) + sendUserToast('Could not save this choice for this account.', true) return } const skill = skills.find((s) => s.path === p) if (skill) skill.enabled = enabled - // Whether people select skills at all. Never the skill itself: a path is + // Whether people turn skills off at all. Never the skill itself: a path is // workspace-authored text. logFeatureUsage('ai_session', 'skill_toggle', { key: enabled ? 'on' : 'off', workspace: ws }) - // The prompt lists exactly the enabled skills, so it has to be rebuilt + // The prompt lists exactly the skills that are on, so it has to be rebuilt // before the next message rather than on the next mode change. await aiChatManager.refreshGlobalSkills(ws) } + /** A folder is on only when everything under it is, so its switch always offers + * the completing action: a folder with one skill off turns fully on first. */ + function folderEnabled(node: SkillTreeNode): boolean { + return nodeSkills(node).every((s) => s.enabled) + } + + async function toggleFolder(node: SkillTreeNode, enabled: boolean) { + if (blockedByPendingFork()) return + for (const skill of nodeSkills(node)) { + if (skill.enabled === enabled) continue + if (!setSkillEnabled(ws, skill.path, enabled)) { + sendUserToast('Could not save this choice for this account.', true) + return + } + skill.enabled = enabled + } + // Once for the folder, not once per skill: what this counts is the action a + // person took. Never the folder itself, which is workspace-authored text. + logFeatureUsage('ai_session', 'skill_toggle', { key: enabled ? 'on' : 'off', workspace: ws }) + await aiChatManager.refreshGlobalSkills(ws) + } + /** Personal folder the folder import writes into. The username is not always a * legal path segment — a superadmin who is not a member of the workspace gets * their email back from `whoami` — and `resource.path` is CHECK-constrained, so @@ -425,11 +646,13 @@ What the assistant should do when this skill applies. validated.skill.description, validated.skill.instructions ) - // A move leaves the old path selected but gone; carry the choice over - // so an edit that renames does not silently switch the skill off. - if (editing.path !== path && isSkillEnabled(target, editing.path)) { - setSkillEnabled(target, editing.path, false) - setSkillEnabled(target, path, true) + // A move leaves the old path's state behind on a path that no longer + // exists; carry it over so an edit that renames does not switch the + // skill back on, and does not inherit whatever the new path held. + if (editing.path !== path) { + const wasEnabled = isSkillEnabled(target, editing.path) + forgetSkill(target, editing.path) + setSkillEnabled(target, path, wasEnabled) } } else { await saveSkillResource( @@ -438,8 +661,9 @@ What the assistant should do when this skill applies. validated.skill.description, validated.skill.instructions ) - // Authoring a skill is the act of choosing it. - setSkillEnabled(target, path, true) + // A skill written here now is not the one someone turned off at this path + // earlier, so it starts from the default rather than inheriting that. + forgetSkill(target, path) } editorOpen = false await refresh(target) @@ -456,9 +680,9 @@ What the assistant should do when this skill applies. const target = ws try { await deleteSkillResource(target, skill.path) - // A later skill at this path is a different one; it must be turned on - // deliberately rather than inherit this one's selection. - setSkillEnabled(target, skill.path, false) + // A later skill at this path is a different one, and must not inherit a + // decision aimed at this one. + forgetSkill(target, skill.path) sendUserToast(`Deleted ${skill.path}`) await refresh(target) } catch (e) { @@ -589,7 +813,7 @@ What the assistant should do when this skill applies. await saveSkillResource(target, dest, skill.description, skill.instructions, { overwrite }) - setSkillEnabled(target, dest, true) + forgetSkill(target, dest) written++ } catch (e) { failed.push(`${skill.name} (${e.body ?? e.message})`) @@ -626,10 +850,22 @@ What the assistant should do when this skill applies. /> {#snippet listPage()} -
+ + + +
{#snippet action()}
@@ -668,8 +904,8 @@ What the assistant should do when this skill applies. {#if forkPending} - Skills are read-only until the first message creates this session's fork. Editing or - selecting one now would apply to the parent workspace and stop applying once the fork is + Skills are read-only until the first message creates this session's fork. Editing one or + turning it off now would apply to the parent workspace and stop applying once the fork is created. {/if} @@ -698,63 +934,143 @@ What the assistant should do when this skill applies. /> {:else}
- {#each skills as skill (skill.path)} - {#snippet icon()} - - {/snippet} - {#snippet title()} - - {ambiguous.has(skill.name) ? skill.path : skill.name} - - {/snippet} - {#snippet subtitle()}{skill.description}{/snippet} - {#snippet trailing()} - await toggle(skill.path, e.detail)} - /> - openSkill(skill) - }, - { - displayName: 'Delete', - icon: Trash2, - type: 'delete', - disabled: !skill.canWrite || forkPending, - action: () => (toDelete = skill) - } - ]} - /> - {/snippet} - openSkill(skill)} - /> - {/each} + {#if grouped} + {#each tree as node (node.path)} + {@render folder(node, true)} + {/each} + {:else} + {#each skills as skill (skill.path)} + {@render row(skill)} + {/each} + {/if}
{/if}
{/snippet} + +{#snippet row(skill: Row)} + {#snippet icon()} + + {/snippet} + {#snippet title()} + + + {!grouped && ambiguous.has(skill.name) ? skill.path : skill.name} + + {/snippet} + {#snippet subtitle()}{skill.description}{/snippet} + {#snippet trailing()} + await toggle(skill.path, e.detail)} + /> + openSkill(skill) + }, + { + displayName: 'Delete', + icon: Trash2, + type: 'delete', + disabled: !skill.canWrite || forkPending, + action: () => (toDelete = skill) + } + ]} + /> + {/snippet} + highlight.hovered(entryIndexByKey.get(skillKey(skill.path)) ?? -1)} + subtitle={skill.description ? subtitle : undefined} + onClick={() => openSkill(skill)} + /> +{/snippet} + + +{#snippet folder(node: SkillTreeNode, root: boolean)} +
+ + +
highlight.hovered(entryIndexByKey.get(folderKey(node.path)) ?? -1)} + > + + await toggleFolder(node, e.detail)} + /> +
+ {#if !collapsed[node.path]} +
+ {#each node.children as child (child.path)} + {@render folder(child, false)} + {/each} + {#each node.skills as skill (skill.path)} + {@render row(skill)} + {/each} +
+ {/if} +
+{/snippet} + {#snippet editorPage()} -
+ +