From 986b9489b040b41022ebcf2f5c3db5b3e9f5819c Mon Sep 17 00:00:00 2001 From: lab 1207 Date: Sun, 6 Sep 2026 12:33:07 +0530 Subject: [PATCH 1/7] Add missing test/setup-scm-loader.ts preload for .scm imports MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit cli/bunfig.toml lists test/setup-scm-loader.ts among its preloads, but the file was never exported to the public mirror. Any test reaching the SDK barrel (which re-exports code-map, which imports .scm tree-sitter query files) threw "Unknown file type" at import time, which bun surfaces as an unhandled error between tests — a fresh clone showed a wall of dead test files with no obvious cause. The plugin registers a bun loader that imports .scm files as a default-exported string, matching what the bundled build does. Verified against the CLI suite: 1,576 pass, with only the pre-existing Windows-path failures in export-conversation.test.ts remaining (unrelated). 🤖 Generated with Codebuff Co-Authored-By: Codebuff --- test/setup-scm-loader.ts | 29 +++++++++++++++++++++++++++++ 1 file changed, 29 insertions(+) create mode 100644 test/setup-scm-loader.ts diff --git a/test/setup-scm-loader.ts b/test/setup-scm-loader.ts new file mode 100644 index 0000000000..20fa413834 --- /dev/null +++ b/test/setup-scm-loader.ts @@ -0,0 +1,29 @@ +/** + * Bun preload: teach bun to import .scm (tree-sitter query) files as text. + * + * packages/code-map/src/languages.ts imports .scm files directly, and the SDK + * barrel re-exports code-map — so any test importing @codebuff/sdk (or the CLI + * modules that reach it) needs this registered before those imports evaluate. + * + * cli/bunfig.toml lists this preload, but the file itself was not present in + * the public mirror, so sdk-touching tests died at import time ("Unknown file + * type" for the first .scm import), which bun reports as an unhandled error + * between tests rather than a test failure (see docs/testing.md). + * + * The bundled build handles .scm imports the same way: loader 'text', default + * export is the file's contents as a string. + */ +import { plugin } from 'bun' +import { readFileSync } from 'fs' + +plugin({ + name: 'scm-text-loader', + setup(build) { + build.onLoad({ filter: /\.scm$/ }, (args) => ({ + // Wrap in a JS module: bun's plugin API only accepts code loaders + // (js/json/...), and the import sites expect a default-exported string. + contents: `export default ${JSON.stringify(readFileSync(args.path, 'utf8'))}`, + loader: 'js' as const, + })) + }, +}) From 6a0323e3707008de5b22479625be7bbd12d1b675 Mon Sep 17 00:00:00 2001 From: lab 1207 Date: Sun, 6 Sep 2026 12:54:00 +0530 Subject: [PATCH 2/7] Add /skills panel: browse, invoke, open, and delete loaded skills MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Skills already load from ~/.agents/skills/ and .agents/skills/ and run as /skill:, but the CLI had no surface answering "what do I have?". Project skills silently shadow global ones, and removing a skill meant hunting down files by hand. /skills (alias /skill) opens a panel cloning the queue-panel pattern: - rows show each skill's source (project vs global) and description - Enter invokes via the existing skill input mode (same path as /skill:, shared through enterSkillMode, so entries can't drift) - o opens the SKILL.md in $EDITOR; d deletes with confirmation - empty registry prints install guidance (npx skills add ...) instead of opening an empty panel The keymap lives in utils/skills-panel-actions.ts, testable without a renderer like queue-panel-actions. Panel participates in the same keyboard/dock arbitration as the queue panel (review, ask-user, sponsored menu). One deviation from queue-panel: the confirm state swallows all other keys, so a held d can't chain-delete rows. Includes test/setup-scm-loader.ts (cherry-picked) — the preload bunfig.toml references but the mirror lacked; without it, tests touching the SDK barrel die at import. Verified: new tests pass (9), CLI suite 1,586 pass / 12 fail, all 12 pre-existing Windows-path/locking failures present on main. 🤖 Generated with Codebuff Co-Authored-By: Codebuff --- cli/src/chat.tsx | 55 ++++ .../commands/__tests__/skills-command.test.ts | 98 +++++++ cli/src/commands/command-registry.ts | 24 +- cli/src/components/skills-panel.tsx | 253 ++++++++++++++++++ cli/src/data/slash-commands.ts | 6 + cli/src/state/skills-panel-store.ts | 27 ++ .../__tests__/skills-panel-actions.test.ts | 110 ++++++++ cli/src/utils/skills-panel-actions.ts | 58 ++++ 8 files changed, 630 insertions(+), 1 deletion(-) create mode 100644 cli/src/commands/__tests__/skills-command.test.ts create mode 100644 cli/src/components/skills-panel.tsx create mode 100644 cli/src/state/skills-panel-store.ts create mode 100644 cli/src/utils/__tests__/skills-panel-actions.test.ts create mode 100644 cli/src/utils/skills-panel-actions.ts diff --git a/cli/src/chat.tsx b/cli/src/chat.tsx index 09139d92c7..1f8c5e8ae9 100644 --- a/cli/src/chat.tsx +++ b/cli/src/chat.tsx @@ -32,6 +32,7 @@ import { ChatHeader } from './components/chat-header' import { FreebuffActiveSessionSummary } from './components/freebuff-active-session-summary' import { LoadPreviousButton } from './components/load-previous-button' import { QueuePanel } from './components/queue-panel' +import { SkillsPanel } from './components/skills-panel' import { ReviewScreen } from './components/review-screen' import { MessageWithAgents } from './components/message-with-agents' import { areCreditsRestored } from './components/out-of-credits-banner' @@ -69,6 +70,7 @@ import { getProjectRoot } from './project-files' import { useChatHistoryStore } from './state/chat-history-store' import { useChatStore } from './state/chat-store' import { useQueuePanelStore } from './state/queue-panel-store' +import { useSkillsPanelStore } from './state/skills-panel-store' import { useReviewStore } from './state/review-store' import { useFeedbackStore } from './state/feedback-store' import { useMessageBlockStore } from './state/message-block-store' @@ -995,6 +997,13 @@ export const Chat = ({ })), ) + const { skillsPanelOpen, closeSkillsPanel } = useSkillsPanelStore( + useShallow((state) => ({ + skillsPanelOpen: state.skillsPanelOpen, + closeSkillsPanel: state.closeSkillsPanel, + })), + ) + // Review and ask_user take the composer's place too. Leaving the panel // flagged open behind them would keep chat's keyboard disabled with nothing // rendered to handle keys, so hand the surface back for real. @@ -1004,11 +1013,24 @@ export const Chat = ({ } }, [queuePanelOpen, reviewMode, askUserState, closeQueuePanel]) + // Same arbitration as the queue panel: review/ask-user own the surface and + // the keyboard, so the skills panel hands them back rather than linger + // invisibly under them. + useEffect(() => { + if (skillsPanelOpen && (reviewMode || askUserState !== null)) { + closeSkillsPanel() + } + }, [skillsPanelOpen, reviewMode, askUserState, closeSkillsPanel]) + // The panel store outlives this component and a Freebuff session can end on // its own, unmounting chat mid-edit. Without this, the next session would // open onto a panel for a queue that no longer exists. useEffect(() => () => useQueuePanelStore.getState().closeQueuePanel(), []) + // A Freebuff session can end on its own, unmounting chat mid-panel; without + // this the next session would open onto a stale skills panel. + useEffect(() => () => useSkillsPanelStore.getState().closeSkillsPanel(), []) + const publishMutation = usePublishMutation() const handleCommandResult = useCallback( @@ -1051,6 +1073,10 @@ export const Chat = ({ if (queuedCount > 0) useQueuePanelStore.getState().openQueuePanel() else setMessages((prev) => [...prev, getSystemMessage('Nothing queued.')]) } + + if (result.openSkillsPanel) { + useSkillsPanelStore.getState().openSkillsPanel() + } }, [ saveCurrentInput, @@ -1220,6 +1246,25 @@ export const Chat = ({ inputRef.current?.focus() }, [closeQueuePanel, setInputFocused, inputRef]) + const handleCloseSkillsPanel = useCallback(() => { + closeSkillsPanel() + setInputFocused(true) + inputRef.current?.focus() + }, [closeSkillsPanel, setInputFocused, inputRef]) + + // Invoking from the panel closes it and drops into the existing skill input + // mode — the exact path /skill: takes, so the two entries cannot + // drift. Focus returns first so the composer receives what the user types. + const handleSkillsPanelInvoke = useCallback( + (name: string) => { + closeSkillsPanel() + setInputFocused(true) + inputRef.current?.focus() + useChatStore.getState().enterSkillMode(name) + }, + [closeSkillsPanel, setInputFocused, inputRef], + ) + const handleReviewCustom = useCallback(() => { closeReviewScreen() setInputMode('review') @@ -1594,6 +1639,7 @@ export const Chat = ({ askUserState !== null || reviewMode || queuePanelOpen || + skillsPanelOpen || sponsoredProposalMenuOpen, }) @@ -1771,6 +1817,7 @@ export const Chat = ({ askUserState !== null || reviewMode || queuePanelOpen || + skillsPanelOpen || sponsoredProposalMenuOpen || isFreebuffSessionOver useEffect(() => { @@ -1955,6 +2002,14 @@ export const Chat = ({ width={separatorWidth} maxVisibleRows={isCompactHeight ? 4 : 8} /> + ) : skillsPanelOpen && !askUserState ? ( + ) : isFreebuffSessionOver && !askUserState ? ( = {}): RouterParams => + ({ + agentMode: 'DEFAULT', + inputRef: { current: null }, + inputValue: '/skills', + isChainInProgressRef: { current: false }, + isStreaming: false, + logoutMutation: {} as RouterParams['logoutMutation'], + streamMessageIdRef: { current: null }, + addToQueue: mock(() => {}), + clearMessages: mock(() => {}), + saveToHistory: mock(() => {}), + scrollToLatest: mock(() => {}), + sendMessage: mock(async () => {}), + setCanProcessQueue: mock(() => {}), + setInputFocused: mock(() => {}), + setInputValue: mock(() => {}), + setIsAuthenticated: mock(() => {}), + setMessages: mock(() => {}), + setUser: mock(() => {}), + ...overrides, + }) as RouterParams + +const resetChatStore = () => { + useChatStore.getState().setInputMode('default') + useChatStore.getState().setPendingSkillName(null) +} + +beforeEach(() => { + __setSkillsForTests({ + [PROJECT_SKILL.name]: PROJECT_SKILL, + [GLOBAL_SKILL.name]: GLOBAL_SKILL, + }) +}) + +afterEach(() => { + __resetSkillRegistryForTests() +}) + +describe('/skills command', () => { + test('opens the panel when skills are loaded', async () => { + const command = findCommand('skills') + expect(command).toBeDefined() + + const params = createMockParams() + const result = await command!.handler(params, '') + + expect(result).toMatchObject({ openSkillsPanel: true }) + expect(params.sendMessage).not.toHaveBeenCalled() + }) + + test('reports install guidance instead of opening an empty panel', async () => { + __resetSkillRegistryForTests() + + const command = findCommand('skills') + const params = createMockParams() + const result = await command!.handler(params, '') + + expect(result).toBeUndefined() + expect(params.setMessages).toHaveBeenCalledTimes(1) + const [updater] = (params.setMessages as ReturnType).mock + .calls[0] as [(prev: unknown[]) => unknown[]] + const messages = updater([]) as { role: string; content: string }[] + const last = messages[messages.length - 1] + expect(last.content).toContain('No skills loaded') + expect(last.content).toContain('npx skills add') + }) + + test('is reachable through the skill alias', () => { + expect(findCommand('skill')).toBeDefined() + }) +}) diff --git a/cli/src/commands/command-registry.ts b/cli/src/commands/command-registry.ts index 8609655129..d2a8d63a1b 100644 --- a/cli/src/commands/command-registry.ts +++ b/cli/src/commands/command-registry.ts @@ -37,7 +37,7 @@ import { AGENT_MODES, END_SESSION_MESSAGE, IS_FREEBUFF } from '../utils/constant import { exitCliCleanly } from '../utils/exit-cleanly' import { getSystemMessage, getUserMessage } from '../utils/message-history' import { capturePendingAttachments } from '../utils/pending-attachments' -import { getSkillByName } from '../utils/skill-registry' +import { getSkillByName, getSkillCount } from '../utils/skill-registry' import type { MultilineInputHandle } from '../components/multiline-input' import type { InputValue, PendingAttachment } from '../types/store' @@ -81,6 +81,7 @@ export type CommandResult = { openChatHistory?: boolean openReviewScreen?: boolean openQueuePanel?: boolean + openSkillsPanel?: boolean preSelectAgents?: string[] } | void @@ -719,6 +720,27 @@ const ALL_COMMANDS: CommandDefinition[] = [ return { openQueuePanel: true } }, }), + defineCommand({ + name: 'skills', + aliases: ['skill'], + handler: (params) => { + if (getSkillCount() === 0) { + params.setMessages((prev) => [ + ...prev, + getUserMessage(params.inputValue.trim()), + getSystemMessage( + 'No skills loaded.\n\nSkills load from:\n - ~/.agents/skills/ (global)\n - .agents/skills/ (project)\n\nInstall some with: npx skills add ', + ), + ]) + params.saveToHistory(params.inputValue.trim()) + clearInput(params) + return + } + params.saveToHistory(params.inputValue.trim()) + clearInput(params) + return { openSkillsPanel: true } + }, + }), defineCommand({ name: 'theme:toggle', handler: (params) => { diff --git a/cli/src/components/skills-panel.tsx b/cli/src/components/skills-panel.tsx new file mode 100644 index 0000000000..4f5b42397c --- /dev/null +++ b/cli/src/components/skills-panel.tsx @@ -0,0 +1,253 @@ +import { spawnSync } from 'child_process' +import { existsSync } from 'fs' +import { unlink } from 'fs/promises' + +import { useKeyboard } from '@opentui/react' +import React, { useCallback, useEffect, useMemo, useState } from 'react' + +import { Button } from './button' +import { ClickableTitleBox } from './clickable-title-box' +import { useTheme } from '../hooks/use-theme' +import { truncateToSingleLinePreview } from '../utils/agent-display' +import { clamp } from '../utils/math' +import { + resolveSkillsPanelAction, +} from '../utils/skills-panel-actions' +import { BORDER_CHARS } from '../utils/ui-constants' + +import type { SkillDefinition } from '@codebuff/common/types/skill' +import type { KeyEvent } from '@opentui/core' + +interface SkillsPanelProps { + skills: SkillDefinition[] + /** Invoke the named skill: enters skill input mode, like /skill:. */ + onInvoke: (name: string) => void + onClose: () => void + /** Width of the surrounding chat chrome, so rows truncate on the same + * column the composer wraps on. */ + width: number + /** Rows to show before the list starts scrolling around the selection. */ + maxVisibleRows?: number +} + +const DEFAULT_MAX_VISIBLE_ROWS = 8 + +/** Keep the selected row inside the window even when the list scrolls past it. */ +function windowStart( + selectedIndex: number, + total: number, + visible: number, +): number { + if (total <= visible) return 0 + return clamp(selectedIndex - Math.floor(visible / 2), 0, total - visible) +} + +/** Which skills directory a skill loaded from, for the row badge. */ +function sourceOf(skill: SkillDefinition): 'project' | 'global' { + return skill.filePath.includes('.agents') && !skill.filePath.split('.agents')[0].startsWith(process.env.HOME ?? '~') + ? 'project' + : 'global' +} + +export const SkillsPanel: React.FC = ({ + skills, + onInvoke, + onClose, + width, + maxVisibleRows = DEFAULT_MAX_VISIBLE_ROWS, +}) => { + const theme = useTheme() + + // Selection tracks the skill name, so it stays put when the list re-sorts + // or a sibling is deleted. + const [selectedName, setSelectedName] = useState( + skills[0]?.name ?? null, + ) + const [confirmingDelete, setConfirmingDelete] = useState(false) + const [notice, setNotice] = useState(null) + + const selectedIndex = Math.max( + 0, + skills.findIndex((skill) => skill.name === selectedName), + ) + const selected = skills[selectedIndex] + + // Nothing left to manage: hand the composer back rather than leave an empty + // box for the user to dismiss. + useEffect(() => { + if (skills.length === 0) onClose() + }, [skills.length, onClose]) + + // A pending confirmation for a skill that just vanished (deleted out from + // under us in another terminal) can no longer land on anything. + useEffect(() => { + if (confirmingDelete && !selected) setConfirmingDelete(false) + }, [confirmingDelete, selected]) + + const deleteSelected = useCallback(async () => { + if (!selected) return + // The path came from the skill registry, but double-check existence so a + // stale entry reports honestly instead of throwing raw ENOENT. + if (!existsSync(selected.filePath)) { + setNotice(`File not found: ${selected.filePath}`) + return + } + try { + await unlink(selected.filePath) + // Dropping the row moves the cursor to whatever fills the vacancy; the + // parent re-renders with the refreshed list. + const successor = skills[selectedIndex + 1] ?? skills[selectedIndex - 1] + setSelectedName(successor?.name ?? null) + setNotice(`Deleted ${selected.filePath}`) + } catch (error) { + setNotice( + `Could not delete: ${error instanceof Error ? error.message : String(error)}`, + ) + } + }, [selected, skills, selectedIndex]) + + const openInEditor = useCallback(() => { + if (!selected) return + const editor = + process.env.VISUAL ?? process.env.EDITOR ?? (process.platform === 'win32' ? 'notepad' : 'vi') + // Inherit stdio so the editor owns the terminal, matching how /bash runs + // interactive commands. + const result = spawnSync(editor, [selected.filePath], { stdio: 'inherit' }) + if (result.error || result.status !== 0) { + setNotice(`Could not open $EDITOR (${editor}). Set EDITOR and try again.`) + } + }, [selected]) + + const handleKey = useCallback( + (key: KeyEvent) => { + const action = resolveSkillsPanelAction(key, { confirmingDelete }) + if (action.type === 'none') return + // Any deliberate action supersedes the last complaint. + if (action.type !== 'confirm') setNotice(null) + + switch (action.type) { + case 'close': + onClose() + return + case 'cancel': + setConfirmingDelete(false) + return + case 'select': { + const to = clamp(selectedIndex + action.delta, 0, skills.length - 1) + setSelectedName(skills[to]?.name ?? null) + return + } + case 'invoke': + if (selected) onInvoke(selected.name) + return + case 'open': + openInEditor() + return + case 'delete': + if (selected) setConfirmingDelete(true) + return + case 'confirm': + setConfirmingDelete(false) + void deleteSelected() + return + } + }, + [ + confirmingDelete, + deleteSelected, + onClose, + onInvoke, + openInEditor, + selected, + skills, + selectedIndex, + ], + ) + + useKeyboard(handleKey) + + // A row must fit one line or it wraps and the list stops being scannable. + // Budget: two border columns, two padding columns, then "❯ " + badge + gap. + const promptWidth = Math.max(10, width - 16) + const rowLabel = useCallback( + (skill: SkillDefinition) => { + const badge = sourceOf(skill) === 'project' ? 'project' : 'global' + const body = + truncateToSingleLinePreview(skill.description, promptWidth) ?? '' + return `${badge.padEnd(7)} ${body}` + }, + [promptWidth], + ) + + const projectCount = useMemo( + () => skills.filter((skill) => sourceOf(skill) === 'project').length, + [skills], + ) + const globalCount = skills.length - projectCount + + const start = windowStart(selectedIndex, skills.length, maxVisibleRows) + const visible = skills.slice(start, start + maxVisibleRows) + const hiddenBelow = skills.length - (start + visible.length) + + return ( + + {start > 0 && {` ↑ ${start} more`}} + + {visible.map((skill, offset) => { + const index = start + offset + const isSelected = index === selectedIndex + + return ( + + ) + })} + + {hiddenBelow > 0 && ( + {` ↓ ${hiddenBelow} more`} + )} + + {notice && {notice}} + + {confirmingDelete ? ( + + {`Delete ${selected?.filePath}? Enter confirm · Esc cancel`} + + ) : ( + + {'Enter run · o open · d delete · esc close'} + + )} + + ) +} diff --git a/cli/src/data/slash-commands.ts b/cli/src/data/slash-commands.ts index c9a1321c11..326456d5e0 100644 --- a/cli/src/data/slash-commands.ts +++ b/cli/src/data/slash-commands.ts @@ -121,6 +121,12 @@ const ALL_SLASH_COMMANDS: SlashCommand[] = [ description: 'Edit, reorder, or delete the messages waiting to be sent', aliases: ['queued'], }, + { + id: 'skills', + label: 'skills', + description: 'Browse, run, open, or delete your loaded skills', + aliases: ['skill'], + }, { id: 'new', label: 'new', diff --git a/cli/src/state/skills-panel-store.ts b/cli/src/state/skills-panel-store.ts new file mode 100644 index 0000000000..da9a3e78b6 --- /dev/null +++ b/cli/src/state/skills-panel-store.ts @@ -0,0 +1,27 @@ +import { create } from 'zustand' +import { immer } from 'zustand/middleware/immer' + +interface SkillsPanelState { + /** The skills panel takes the composer's place while open, the way the + * queue panel and review screen do — the skill list is what the user is + * "typing about". */ + skillsPanelOpen: boolean + openSkillsPanel: () => void + closeSkillsPanel: () => void +} + +export const useSkillsPanelStore = create()( + immer((set) => ({ + skillsPanelOpen: false, + openSkillsPanel: () => { + set((state) => { + state.skillsPanelOpen = true + }) + }, + closeSkillsPanel: () => { + set((state) => { + state.skillsPanelOpen = false + }) + }, + })), +) diff --git a/cli/src/utils/__tests__/skills-panel-actions.test.ts b/cli/src/utils/__tests__/skills-panel-actions.test.ts new file mode 100644 index 0000000000..f4f9739392 --- /dev/null +++ b/cli/src/utils/__tests__/skills-panel-actions.test.ts @@ -0,0 +1,110 @@ +import { describe, test, expect } from 'bun:test' + +import { resolveSkillsPanelAction } from '../skills-panel-actions' + +import type { KeyEvent } from '@opentui/core' + +const createKey = (overrides: Partial = {}): KeyEvent => + ({ + name: '', + sequence: '', + ctrl: false, + meta: false, + shift: false, + option: false, + ...overrides, + }) as KeyEvent + +const browsing = { confirmingDelete: false } +const confirming = { confirmingDelete: true } + +describe('resolveSkillsPanelAction', () => { + test('arrows and j/k move the selection', () => { + expect( + resolveSkillsPanelAction(createKey({ name: 'up' }), browsing), + ).toEqual({ type: 'select', delta: -1 }) + expect( + resolveSkillsPanelAction(createKey({ name: 'down' }), browsing), + ).toEqual({ type: 'select', delta: 1 }) + expect(resolveSkillsPanelAction(createKey({ name: 'k' }), browsing)).toEqual( + { type: 'select', delta: -1 }, + ) + expect(resolveSkillsPanelAction(createKey({ name: 'j' }), browsing)).toEqual( + { type: 'select', delta: 1 }, + ) + }) + + test('enter invokes, o opens, d deletes', () => { + expect( + resolveSkillsPanelAction(createKey({ name: 'return' }), browsing), + ).toEqual({ type: 'invoke' }) + expect( + resolveSkillsPanelAction(createKey({ name: 'enter' }), browsing), + ).toEqual({ type: 'invoke' }) + expect(resolveSkillsPanelAction(createKey({ name: 'o' }), browsing)).toEqual( + { type: 'open' }, + ) + expect(resolveSkillsPanelAction(createKey({ name: 'd' }), browsing)).toEqual( + { type: 'delete' }, + ) + expect( + resolveSkillsPanelAction(createKey({ name: 'delete' }), browsing), + ).toEqual({ type: 'delete' }) + }) + + test('escape, ctrl+c, and q close while browsing', () => { + expect( + resolveSkillsPanelAction(createKey({ name: 'escape' }), browsing), + ).toEqual({ type: 'close' }) + expect( + resolveSkillsPanelAction( + createKey({ name: 'c', ctrl: true }), + browsing, + ), + ).toEqual({ type: 'close' }) + expect(resolveSkillsPanelAction(createKey({ name: 'q' }), browsing)).toEqual( + { type: 'close' }, + ) + }) + + test('a pending delete swallows stray keys so held keys cannot chain-delete', () => { + // The next row's `d`, a printable key, even enter-adjacent modifiers: + // nothing but confirm/cancel may act while the prompt is up. + expect( + resolveSkillsPanelAction(createKey({ name: 'd' }), confirming), + ).toEqual({ type: 'none' }) + expect( + resolveSkillsPanelAction(createKey({ name: 'j' }), confirming), + ).toEqual({ type: 'none' }) + expect( + resolveSkillsPanelAction(createKey({ name: 'o' }), confirming), + ).toEqual({ type: 'none' }) + }) + + test('a pending delete confirms on enter and cancels on escape/n/q', () => { + expect( + resolveSkillsPanelAction(createKey({ name: 'return' }), confirming), + ).toEqual({ type: 'confirm' }) + expect( + resolveSkillsPanelAction(createKey({ name: 'enter' }), confirming), + ).toEqual({ type: 'confirm' }) + expect( + resolveSkillsPanelAction(createKey({ name: 'escape' }), confirming), + ).toEqual({ type: 'cancel' }) + expect( + resolveSkillsPanelAction(createKey({ name: 'n' }), confirming), + ).toEqual({ type: 'cancel' }) + expect( + resolveSkillsPanelAction(createKey({ name: 'q' }), confirming), + ).toEqual({ type: 'cancel' }) + }) + + test('unbound keys do nothing', () => { + expect(resolveSkillsPanelAction(createKey({ name: 'x' }), browsing)).toEqual( + { type: 'none' }, + ) + expect( + resolveSkillsPanelAction(createKey({ name: 'tab' }), browsing), + ).toEqual({ type: 'none' }) + }) +}) diff --git a/cli/src/utils/skills-panel-actions.ts b/cli/src/utils/skills-panel-actions.ts new file mode 100644 index 0000000000..71e1c97f2f --- /dev/null +++ b/cli/src/utils/skills-panel-actions.ts @@ -0,0 +1,58 @@ +import type { KeyEvent } from '@opentui/core' + +/** + * What a keypress means inside the skills panel. Kept separate from the + * component so the shortcut table is testable without a renderer, matching + * how the queue panel's shortcuts are resolved. + */ +export type SkillsPanelAction = + | { type: 'close' } + /** Move the cursor by `delta` rows. */ + | { type: 'select'; delta: number } + /** Invoke the selected skill (enters skill input mode). */ + | { type: 'invoke' } + /** Open the selected skill's SKILL.md in $EDITOR. */ + | { type: 'open' } + /** Delete the selected skill's SKILL.md (with confirmation). */ + | { type: 'delete' } + | { type: 'confirm' } + | { type: 'cancel' } + | { type: 'none' } + +export type SkillsPanelKeyboardState = { + /** While a delete is pending confirmation the panel only listens for + * confirm/cancel — a stray `d` must not chain-delete the next row. */ + confirmingDelete: boolean +} + +export function resolveSkillsPanelAction( + key: KeyEvent, + state: SkillsPanelKeyboardState, +): SkillsPanelAction { + const isEscape = key.name === 'escape' + const isCtrlC = key.ctrl && key.name === 'c' + + if (state.confirmingDelete) { + // Enter confirms; anything escape-shaped cancels; everything else is + // swallowed so a held key can't confirm or chain. + if (key.name === 'return' || key.name === 'enter') return { type: 'confirm' } + if (isEscape || isCtrlC || key.name === 'n' || key.name === 'q') + return { type: 'cancel' } + return { type: 'none' } + } + + // `q` closes, mirroring the queue panel. + if (isEscape || isCtrlC || key.name === 'q') return { type: 'close' } + + if (key.name === 'up' || key.name === 'k') return { type: 'select', delta: -1 } + if (key.name === 'down' || key.name === 'j') + return { type: 'select', delta: 1 } + + // Enter invokes; `o` opens the file in $EDITOR. + if (key.name === 'return' || key.name === 'enter') return { type: 'invoke' } + if (key.name === 'o') return { type: 'open' } + + if (key.name === 'd' || key.name === 'delete') return { type: 'delete' } + + return { type: 'none' } +} From bce93d768f47222c441215a26b3bd17df89ba830 Mon Sep 17 00:00:00 2001 From: lab 1207 Date: Sun, 6 Sep 2026 13:51:24 +0530 Subject: [PATCH 3/7] Reload skills live and accept Claude Code frontmatter MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Skills were frozen at startup: a skill installed mid-session was invisible until restart, and deleting from the /skills panel never removed the row because the component read a one-time useMemo snapshot. The registry now refreshes with content diffing and a version number that React subscribes to, skill directories are watched so changes land within the session, and the panel refreshes on open and after a delete. Frontmatter parity with Claude Code: user-invocable: false hides a skill from the / menu, when_to_use is appended to the agent's skill listing, and argument-hint is tolerated (coerced from the YAML list its unquoted docs form parses into). Deleting a skill now removes its directory rather than only SKILL.md, so supporting files are not orphaned, and the project/global badge derives from resolveSkillsDirs (now exported from the SDK) instead of a HOME string check that misclassified on Windows. 🤖 Generated with Codebuff Co-Authored-By: Codebuff --- cli/src/chat.tsx | 29 +++- .../commands/__tests__/skills-command.test.ts | 2 +- cli/src/commands/command-registry.ts | 2 +- cli/src/components/skills-panel.tsx | 53 ++++-- cli/src/data/slash-commands.ts | 18 ++- cli/src/index.tsx | 10 +- .../utils/__tests__/skill-registry.test.ts | 153 ++++++++++++++++++ cli/src/utils/skill-registry.ts | 149 ++++++++++++++++- common/src/types/skill.ts | 25 +++ common/src/util/parse-skill.test.ts | 85 ++++++++++ common/src/util/skills.test.ts | 34 ++++ common/src/util/skills.ts | 24 ++- sdk/src/index.ts | 1 + 13 files changed, 555 insertions(+), 30 deletions(-) create mode 100644 cli/src/utils/__tests__/skill-registry.test.ts create mode 100644 common/src/util/parse-skill.test.ts diff --git a/cli/src/chat.tsx b/cli/src/chat.tsx index 1f8c5e8ae9..2fbd2067e7 100644 --- a/cli/src/chat.tsx +++ b/cli/src/chat.tsx @@ -14,6 +14,7 @@ import { useMemo, useRef, useState, + useSyncExternalStore, } from 'react' import { useShallow } from 'zustand/react/shallow' @@ -47,6 +48,11 @@ import { import { TopBanner } from './components/top-banner' import { useChatRuntime } from './contexts/chat-runtime-context' import { getSlashCommandsWithSkills } from './data/slash-commands' +import { + getSkillsVersion, + refreshSkillRegistry, + subscribeToSkillsVersion, +} from './utils/skill-registry' import { useAskUserBridge } from './hooks/use-ask-user-bridge' import { useChatInput } from './hooks/use-chat-input' import { @@ -550,8 +556,19 @@ export const Chat = ({ const setInputMode = useChatStore((state) => state.setInputMode) const askUserState = useChatStore((state) => state.askUserState) - // Get loaded skills for slash commands - const loadedSkills = useMemo(() => getLoadedSkills(), []) + // Get loaded skills for slash commands. Keyed on the registry version so a + // live-reload (watcher or /skills refresh) swaps the list without a + // restart — the registry object itself is mutated in place, which zustand + // and useMemo would never notice. + const skillsVersion = useSyncExternalStore( + subscribeToSkillsVersion, + getSkillsVersion, + getSkillsVersion, + ) + const loadedSkills = useMemo(() => { + void skillsVersion + return getLoadedSkills() + }, [skillsVersion]) // Filter slash commands based on current ads state - only show the option that changes state // Hide both ads commands entirely for subscribers @@ -1252,6 +1269,14 @@ export const Chat = ({ inputRef.current?.focus() }, [closeSkillsPanel, setInputFocused, inputRef]) + // Refresh the registry when the /skills panel opens, so a skill installed + // moments ago shows up even if the watcher missed it (e.g. directory + // created and populated before the watcher could arm on it). + useEffect(() => { + if (!skillsPanelOpen) return + void refreshSkillRegistry() + }, [skillsPanelOpen]) + // Invoking from the panel closes it and drops into the existing skill input // mode — the exact path /skill: takes, so the two entries cannot // drift. Focus returns first so the composer receives what the user types. diff --git a/cli/src/commands/__tests__/skills-command.test.ts b/cli/src/commands/__tests__/skills-command.test.ts index 8061c6af41..7a7207f999 100644 --- a/cli/src/commands/__tests__/skills-command.test.ts +++ b/cli/src/commands/__tests__/skills-command.test.ts @@ -7,7 +7,7 @@ import { } from '../../utils/skill-registry' import { findCommand } from '../command-registry' -import type { RouterParams } from '../../command-registry' +import type { RouterParams } from '../command-registry' import type { SkillDefinition } from '@codebuff/common/types/skill' const PROJECT_SKILL: SkillDefinition = { diff --git a/cli/src/commands/command-registry.ts b/cli/src/commands/command-registry.ts index d2a8d63a1b..e0e07cf950 100644 --- a/cli/src/commands/command-registry.ts +++ b/cli/src/commands/command-registry.ts @@ -729,7 +729,7 @@ const ALL_COMMANDS: CommandDefinition[] = [ ...prev, getUserMessage(params.inputValue.trim()), getSystemMessage( - 'No skills loaded.\n\nSkills load from:\n - ~/.agents/skills/ (global)\n - .agents/skills/ (project)\n\nInstall some with: npx skills add ', + 'No skills loaded.\n\nSkills load from:\n - ~/.claude/skills/ (global, Claude Code compatible)\n - ~/.agents/skills/ (global)\n - .claude/skills/ (project, Claude Code compatible)\n - .agents/skills/ (project, overrides global)\n\nInstall some with: npx skills add \nNew and changed skills are picked up live — no restart needed.', ), ]) params.saveToHistory(params.inputValue.trim()) diff --git a/cli/src/components/skills-panel.tsx b/cli/src/components/skills-panel.tsx index 4f5b42397c..8a2f8e7a1a 100644 --- a/cli/src/components/skills-panel.tsx +++ b/cli/src/components/skills-panel.tsx @@ -1,13 +1,17 @@ import { spawnSync } from 'child_process' import { existsSync } from 'fs' -import { unlink } from 'fs/promises' +import { rm } from 'fs/promises' +import path from 'path' +import { resolveSkillsDirs } from '@codebuff/sdk' import { useKeyboard } from '@opentui/react' import React, { useCallback, useEffect, useMemo, useState } from 'react' import { Button } from './button' import { ClickableTitleBox } from './clickable-title-box' import { useTheme } from '../hooks/use-theme' +import { getProjectRoot, tryGetProjectRoot } from '../project-files' +import { refreshSkillRegistry } from '../utils/skill-registry' import { truncateToSingleLinePreview } from '../utils/agent-display' import { clamp } from '../utils/math' import { @@ -43,10 +47,28 @@ function windowStart( } /** Which skills directory a skill loaded from, for the row badge. */ +let projectSkillsDirs: Set | null = null + +function getProjectSkillsDirs(): Set { + // Lazily computed on first render: getProjectRoot() is only meaningful + // after the CLI finishes booting, and module-load order does not guarantee + // that. One Set, resolved once — the project root cannot change mid-session. + projectSkillsDirs ??= new Set( + resolveSkillsDirs({ cwd: tryGetProjectRoot() || process.cwd() }).map( + (dir) => path.resolve(dir), + ), + ) + return projectSkillsDirs +} + function sourceOf(skill: SkillDefinition): 'project' | 'global' { - return skill.filePath.includes('.agents') && !skill.filePath.split('.agents')[0].startsWith(process.env.HOME ?? '~') - ? 'project' - : 'global' + // //SKILL.md — the grandparent of the file is the + // skills directory it was discovered in. Resolved on both sides so mixed + // separators cannot break the comparison on Windows. + const skillsDir = path.resolve( + path.dirname(path.dirname(skill.filePath)), + ) + return getProjectSkillsDirs().has(skillsDir) ? 'project' : 'global' } export const SkillsPanel: React.FC = ({ @@ -86,19 +108,24 @@ export const SkillsPanel: React.FC = ({ const deleteSelected = useCallback(async () => { if (!selected) return - // The path came from the skill registry, but double-check existence so a - // stale entry reports honestly instead of throwing raw ENOENT. - if (!existsSync(selected.filePath)) { - setNotice(`File not found: ${selected.filePath}`) + // Claude Code semantics: removing a skill removes its DIRECTORY, not just + // SKILL.md — a skill can carry supporting files (reference docs, scripts) + // that would otherwise be orphaned. + const skillDir = path.dirname(selected.filePath) + if (!existsSync(skillDir)) { + setNotice(`Directory not found: ${skillDir}`) return } try { - await unlink(selected.filePath) - // Dropping the row moves the cursor to whatever fills the vacancy; the - // parent re-renders with the refreshed list. + await rm(skillDir, { recursive: true }) + // Refresh the registry right away: the version bump re-renders the + // panel with the refreshed list (the watcher would also catch it, but + // a whole-skills-directory delete deserves instant feedback). + void refreshSkillRegistry() + // Dropping the row moves the cursor to whatever fills the vacancy. const successor = skills[selectedIndex + 1] ?? skills[selectedIndex - 1] setSelectedName(successor?.name ?? null) - setNotice(`Deleted ${selected.filePath}`) + setNotice(`Deleted ${skillDir}`) } catch (error) { setNotice( `Could not delete: ${error instanceof Error ? error.message : String(error)}`, @@ -241,7 +268,7 @@ export const SkillsPanel: React.FC = ({ {confirmingDelete ? ( - {`Delete ${selected?.filePath}? Enter confirm · Esc cancel`} + {`Delete ${selected && path.dirname(selected.filePath)}/? Enter confirm · Esc cancel`} ) : ( diff --git a/cli/src/data/slash-commands.ts b/cli/src/data/slash-commands.ts index 326456d5e0..2470b962db 100644 --- a/cli/src/data/slash-commands.ts +++ b/cli/src/data/slash-commands.ts @@ -253,14 +253,20 @@ function truncateDescription(description: string): string { /** * Returns SLASH_COMMANDS merged with skill commands. * Skills become slash commands that users can invoke directly. + * + * Skills marked `user-invocable: false` (Claude Code frontmatter parity) are + * model-only knowledge: they are hidden from the / menu, the same way Claude + * Code hides them from its / menu. */ export function getSlashCommandsWithSkills(skills: SkillsMap): SlashCommand[] { - const skillCommands: SlashCommand[] = Object.values(skills).map((skill) => ({ - id: `skill:${skill.name}`, - label: `skill:${skill.name}`, - description: truncateDescription(skill.description), - insertText: `/skill:${skill.name} `, - })) + const skillCommands: SlashCommand[] = Object.values(skills) + .filter((skill) => skill.userInvocable !== false) + .map((skill) => ({ + id: `skill:${skill.name}`, + label: `skill:${skill.name}`, + description: truncateDescription(skill.description), + insertText: `/skill:${skill.name} `, + })) const commands = [...SLASH_COMMANDS, ...skillCommands] diff --git a/cli/src/index.tsx b/cli/src/index.tsx index cae4e380eb..a63b6a24e7 100644 --- a/cli/src/index.tsx +++ b/cli/src/index.tsx @@ -47,7 +47,10 @@ import { } from './utils/renderer-cleanup' import { startTerminalWatchdog } from './utils/terminal-watchdog' import { installTerminalProtocolController } from './utils/terminal-protocol-controller' -import { initializeSkillRegistry } from './utils/skill-registry' +import { + initializeSkillRegistry, + startSkillDirWatcher, +} from './utils/skill-registry' import { detectTerminalTheme } from './utils/terminal-color-detection' import { setOscDetectedTheme } from './utils/theme-system' @@ -263,6 +266,11 @@ async function main(): Promise { // Initialize skill registry (loads skills from .agents/skills) await initializeSkillRegistry() + // Claude Code parity: watch skill directories so skills installed, edited, + // or deleted mid-session appear without a restart. A no-op when none of the + // directories exist yet. + startSkillDirWatcher() + // Handle publish command before rendering the app if (isPublishCommand) { const publishIndex = process.argv.indexOf('publish') diff --git a/cli/src/utils/__tests__/skill-registry.test.ts b/cli/src/utils/__tests__/skill-registry.test.ts new file mode 100644 index 0000000000..94608d4371 --- /dev/null +++ b/cli/src/utils/__tests__/skill-registry.test.ts @@ -0,0 +1,153 @@ +import { afterEach, beforeEach, describe, expect, mock, test } from 'bun:test' +import fs from 'fs' +import os from 'os' +import path from 'path' + +import { setProjectRoot } from '../../project-files' +import { + __resetSkillRegistryForTests, + getLoadedSkills, + getSkillsVersion, + initializeSkillRegistry, + refreshSkillRegistry, + startSkillDirWatcher, + subscribeToSkillsVersion, +} from '../skill-registry' + +let tmpRoot: string + +const writeSkill = ( + kind: 'project' | 'global', + dirName: string, + frontmatter: Record, +) => { + const base = + kind === 'project' ? path.join(tmpRoot, '.agents', 'skills') : path.join(os.homedir(), '.claude', 'skills') + const skillDir = path.join(base, dirName) + fs.mkdirSync(skillDir, { recursive: true }) + const lines = Object.entries(frontmatter).map( + ([key, value]) => `${key}: ${value}`, + ) + fs.writeFileSync( + path.join(skillDir, 'SKILL.md'), + `---\n${lines.join('\n')}\n---\nBody of ${dirName}.\n`, + ) + return skillDir +} + +const wait = (ms: number) => new Promise((resolve) => setTimeout(resolve, ms)) + +beforeEach(() => { + tmpRoot = fs.mkdtempSync(path.join(os.tmpdir(), 'skill-registry-')) + setProjectRoot(tmpRoot) +}) + +afterEach(() => { + __resetSkillRegistryForTests() + fs.rmSync(tmpRoot, { recursive: true, force: true }) +}) + +describe('skill-registry refresh', () => { + test('initializeSkillRegistry loads project skills', async () => { + writeSkill('project', 'deploy', { + name: 'deploy', + description: 'Deploy the app', + }) + + await initializeSkillRegistry() + + expect(getLoadedSkills()['deploy']).toMatchObject({ + name: 'deploy', + description: 'Deploy the app', + }) + }) + + test('refreshSkillRegistry detects new, edited, and deleted skills', async () => { + const skillDir = writeSkill('project', 'deploy', { + name: 'deploy', + description: 'Deploy the app', + }) + await initializeSkillRegistry() + expect(getSkillCountForTest()).toBe(1) + + // New skill appears. + writeSkill('project', 'review', { + name: 'review', + description: 'Review changes', + }) + await expect(refreshSkillRegistry()).resolves.toBe(true) + expect(getSkillCountForTest()).toBe(2) + + // Content edit is detected (same name, new description). + writeSkill('project', 'deploy', { + name: 'deploy', + description: 'Deploy the app to production', + }) + await expect(refreshSkillRegistry()).resolves.toBe(true) + expect(getLoadedSkills()['deploy'].description).toBe( + 'Deploy the app to production', + ) + + // Deleting the whole directory removes the skill. + fs.rmSync(skillDir, { recursive: true }) + await expect(refreshSkillRegistry()).resolves.toBe(true) + expect(getSkillCountForTest()).toBe(1) + expect(getLoadedSkills()['deploy']).toBeUndefined() + + // Nothing changed: no new version. + const before = getSkillsVersion() + await expect(refreshSkillRegistry()).resolves.toBe(false) + expect(getSkillsVersion()).toBe(before) + }) + + test('version subscribers are notified on change', async () => { + writeSkill('project', 'deploy', { + name: 'deploy', + description: 'Deploy the app', + }) + await initializeSkillRegistry() + + const onChange = mock(() => {}) + const unsubscribe = subscribeToSkillsVersion(onChange) + const before = getSkillsVersion() + + writeSkill('project', 'review', { + name: 'review', + description: 'Review changes', + }) + await refreshSkillRegistry() + + expect(getSkillsVersion()).toBe(before + 1) + expect(onChange).toHaveBeenCalled() + + unsubscribe() + await refreshSkillRegistry() + expect(onChange).toHaveBeenCalledTimes(1) + }) + + test('watcher picks up an install without restart', async () => { + // The skills directory must exist BEFORE the watcher arms on it — fs.watch + // cannot watch a directory that does not exist. (A directory created and + // populated later is caught by the refresh-on-panel-open instead.) + fs.mkdirSync(path.join(tmpRoot, '.agents', 'skills'), { recursive: true }) + startSkillDirWatcher() + + await initializeSkillRegistry() + expect(getSkillCountForTest()).toBe(0) + + writeSkill('project', 'deploy', { + name: 'deploy', + description: 'Deploy the app', + }) + + // Watcher debounce is 300ms; give it room on slow CI. + await wait(900) + + expect(getSkillsVersion()).toBeGreaterThan(0) + expect(getLoadedSkills()['deploy']).toBeDefined() + }) +}) + +function getSkillCountForTest(): number { + return Object.keys(getLoadedSkills()).length +} diff --git a/cli/src/utils/skill-registry.ts b/cli/src/utils/skill-registry.ts index 79942f4e99..e859ab5494 100644 --- a/cli/src/utils/skill-registry.ts +++ b/cli/src/utils/skill-registry.ts @@ -1,6 +1,9 @@ -import { loadSkills as sdkLoadSkills } from '@codebuff/sdk' +import os from 'os' +import { watch, type FSWatcher } from 'fs' -import { getProjectRoot } from '../project-files' +import { loadSkills as sdkLoadSkills, resolveSkillsDirs } from '@codebuff/sdk' + +import { getProjectRoot, tryGetProjectRoot } from '../project-files' import { logger } from './logger' import type { SkillDefinition, SkillsMap } from '@codebuff/common/types/skill' @@ -11,6 +14,142 @@ import type { SkillDefinition, SkillsMap } from '@codebuff/common/types/skill' let skillsCache: SkillsMap = {} +/** + * Bumped whenever a refresh changes the set of loaded skills. React surfaces + * (the /skills panel, the slash-command merge) subscribe to this number + * instead of to the mutable cache itself, which zustand would never see. + */ +let skillsVersion = 0 + +export function getSkillsVersion(): number { + return skillsVersion +} + +/** + * Subscribe to registry version changes. Written for React's + * `useSyncExternalStore`, which needs a stable function that returns an + * unsubscribe. + */ +export function subscribeToSkillsVersion(onChange: () => void): () => void { + versionSubscribers.add(onChange) + return () => { + versionSubscribers.delete(onChange) + } +} + +const versionSubscribers = new Set<() => void>() + +/** The working directory skills should be resolved against. */ +function skillsCwd(): string { + return tryGetProjectRoot() || process.cwd() +} + +/** + * True when the two maps describe the same skill set: same names, same + * invocability, same content. Metadata-only edits still count as a change so + * the /skills panel shows fresh descriptions after the user edits SKILL.md. + */ +function skillsEqual(a: SkillsMap, b: SkillsMap): boolean { + const aKeys = Object.keys(a) + const bKeys = Object.keys(b) + if (aKeys.length !== bKeys.length) return false + return aKeys.every((key) => { + const x = a[key] + const y = b[key] + if (!y) return false + return ( + x.content === y.content && + x.disableModelInvocation === y.disableModelInvocation && + x.userInvocable === y.userInvocable && + x.description === y.description + ) + }) +} + +/** + * Re-load skills from disk and swap the cache only when something actually + * changed. Every caller funnels through this function so there is exactly one + * notion of "the skills changed" — the version bump — for the UI to key on. + * + * Returns true when the skill set changed. + */ +export async function refreshSkillRegistry(): Promise { + const cwd = skillsCwd() + try { + const fresh = await sdkLoadSkills({ + cwd, + verbose: false, + includeHomeSkills: true, + }) + if (skillsEqual(fresh, skillsCache)) return false + skillsCache = fresh + skillsVersion += 1 + for (const notify of versionSubscribers) notify() + return true + } catch (error) { + logger.warn({ error }, 'Failed to refresh skills') + return false + } +} + +// ============================================================================ +// Live reload (Claude Code parity) +// ============================================================================ +// Claude Code watches skill directories and picks up add/edit/delete within +// the running session. Without this, a skill installed mid-session is +// invisible until restart — the "install every time" complaint. + +const SKILLS_WATCH_DEBOUNCE_MS = 300 + +let skillWatchers: FSWatcher[] = [] + +/** + * Start watching the resolved skill directories (global + project, Claude + * locations included). Idempotent. A directory that does not exist yet is + * skipped — installs that create it later are caught by the refresh when the + * /skills panel opens. + */ +export function startSkillDirWatcher(): void { + if (skillWatchers.length > 0) return + + const cwd = skillsCwd() + const homeDir = os.homedir() + const dirs = resolveSkillsDirs({ cwd, homeDir }) + + let debounceTimer: ReturnType | null = null + const scheduleRefresh = () => { + if (debounceTimer) clearTimeout(debounceTimer) + debounceTimer = setTimeout(() => { + debounceTimer = null + void refreshSkillRegistry() + }, SKILLS_WATCH_DEBOUNCE_MS) + } + + for (const dir of dirs) { + try { + const watcher = watch(dir, { persistent: false }, () => { + // Filter nothing: skill installs create directories AND write + // SKILL.md inside them, whole-skill deletes only touch the dir name, + // and the refresh itself is a debounced handful of stat+read calls. + // Simpler and correct beats a filename heuristic that misses cases. + scheduleRefresh() + }) + watcher.on('error', (error) => { + logger.warn({ error }, `Skill watcher error for ${dir}`) + }) + skillWatchers.push(watcher) + } catch { + // Directory does not exist (e.g. no ~/.agents/skills yet). Nothing to + // watch; installs create it fresh and a restart picks them up. + } + } +} + +export function stopSkillDirWatcher(): void { + for (const watcher of skillWatchers) watcher.close() + skillWatchers = [] +} + /** * Initialize the skill registry by loading skills via the SDK. * This must be called at CLI startup. @@ -97,11 +236,17 @@ export function getLoadedSkillsMessage(): string | null { */ export function __resetSkillRegistryForTests(): void { skillsCache = {} + skillsVersion = 0 + stopSkillDirWatcher() } /** * Seed the cache without touching the filesystem. Intended for test scenarios. + * Bumps the version and notifies subscribers exactly like a real refresh, so + * React surfaces keyed on the version see the seeded set. */ export function __setSkillsForTests(skills: SkillsMap): void { skillsCache = skills + skillsVersion += 1 + for (const notify of versionSubscribers) notify() } diff --git a/common/src/types/skill.ts b/common/src/types/skill.ts index 17c0082ea5..180f0f4137 100644 --- a/common/src/types/skill.ts +++ b/common/src/types/skill.ts @@ -36,6 +36,22 @@ export const SkillFrontmatterSchema = z.object({ .transform((d) => d.slice(0, SKILL_DESCRIPTION_MAX_LENGTH)), license: z.string().optional(), 'disable-model-invocation': z.boolean().optional(), + // Claude Code compatibility: `user-invocable: false` marks a skill the + // model may invoke but the user cannot (hidden from / menu). Absent = + // invocable by both, matching Claude Code's default. + 'user-invocable': z.boolean().optional(), + // Claude Code compatibility: hint shown during autocomplete, e.g. + // "[issue-number]". Freebuff does not substitute arguments yet, but the + // field is accepted so a Claude Code skill does not fail validation. + // Claude Code's own docs write the value unquoted, which YAML parses as a + // single-element array — coerce that back to the display string. + 'argument-hint': z + .union([z.string(), z.array(z.string())]) + .optional() + .transform((v) => (Array.isArray(v) ? v.join(' ') : v)), + // Claude Code compatibility: extra trigger context appended to the + // description in listings. Accepted and surfaced; never rejected. + 'when_to_use': z.string().optional(), metadata: SkillMetadataSchema.optional(), }) @@ -53,6 +69,12 @@ export const SkillDefinitionSchema = z.object({ license: z.string().optional(), /** Whether only the user may invoke this skill. */ disableModelInvocation: z.boolean().optional(), + /** Whether only the model may invoke this skill (hidden from the / menu). */ + userInvocable: z.boolean().optional(), + /** Autocomplete hint for expected arguments, e.g. "[issue-number]". */ + argumentHint: z.string().optional(), + /** Extra trigger context appended to the description in listings. */ + whenToUse: z.string().optional(), /** Optional key-value metadata */ metadata: SkillMetadataSchema.optional(), /** Full SKILL.md content (including frontmatter) */ @@ -74,6 +96,9 @@ export function createSkillDefinition(params: { description: frontmatter.description, license: frontmatter.license, disableModelInvocation: frontmatter['disable-model-invocation'], + userInvocable: frontmatter['user-invocable'], + argumentHint: frontmatter['argument-hint'], + whenToUse: frontmatter['when_to_use'], metadata: frontmatter.metadata, content, filePath, diff --git a/common/src/util/parse-skill.test.ts b/common/src/util/parse-skill.test.ts new file mode 100644 index 0000000000..ecc236f978 --- /dev/null +++ b/common/src/util/parse-skill.test.ts @@ -0,0 +1,85 @@ +import { describe, expect, test } from 'bun:test' + +import { parseSkillFileContent } from './parse-skill' + +describe('parseSkillFileContent', () => { + const baseOptions = { + directoryName: 'deploy', + filePath: '/skills/deploy/SKILL.md', + } + + test('parses name, description, and license', () => { + const skill = parseSkillFileContent( + [ + '---', + 'name: deploy', + 'description: Deploy the application', + 'license: MIT', + '---', + 'Run the deploy script.', + ].join('\n'), + baseOptions, + ) + + expect(skill).toMatchObject({ + name: 'deploy', + description: 'Deploy the application', + license: 'MIT', + filePath: '/skills/deploy/SKILL.md', + }) + expect(skill?.content).toContain('Run the deploy script.') + }) + + test('accepts Claude Code frontmatter fields', () => { + const skill = parseSkillFileContent( + [ + '---', + 'name: deploy', + 'description: Deploy the application', + 'user-invocable: false', + // Claude Code's docs write this unquoted: YAML parses it as a flow + // sequence, which we join back to the display string — brackets are + // lost to YAML either way, in Claude Code just as here. + 'argument-hint: [environment]', + 'when_to_use: When the user asks to ship or release', + 'disable-model-invocation: true', + '---', + 'Deploy steps.', + ].join('\n'), + baseOptions, + ) + + expect(skill).not.toBeNull() + expect(skill?.userInvocable).toBe(false) + expect(skill?.argumentHint).toBe('environment') + expect(skill?.whenToUse).toBe( + 'When the user asks to ship or release', + ) + expect(skill?.disableModelInvocation).toBe(true) + }) + + test('rejects a name that does not match the directory', () => { + const skill = parseSkillFileContent( + '---\nname: other\ndescription: Mismatched\n---\nbody', + baseOptions, + ) + + expect(skill).toBeNull() + }) + + test('returns null when there is no frontmatter', () => { + expect(parseSkillFileContent('Just some instructions.', baseOptions)).toBe( + null, + ) + }) + + test('returns null when frontmatter is invalid per the schema', () => { + // uppercase name violates SKILL_NAME_REGEX + const skill = parseSkillFileContent( + '---\nname: Deploy\ndescription: Bad name casing\n---\nbody', + baseOptions, + ) + + expect(skill).toBeNull() + }) +}) diff --git a/common/src/util/skills.test.ts b/common/src/util/skills.test.ts index deb31a3aeb..545cfca99e 100644 --- a/common/src/util/skills.test.ts +++ b/common/src/util/skills.test.ts @@ -28,6 +28,40 @@ describe('formatAvailableSkillsXml', () => { expect(xml).not.toContain('deploy') }) + test('keeps model-only skills in the model listing (user-invocable: false)', () => { + const skills: SkillsMap = { + legacy: { + name: 'legacy', + description: 'How the legacy system works', + content: 'legacy system context', + userInvocable: false, + filePath: '/skills/legacy/SKILL.md', + }, + } + + // Model-only means "hidden from the user", not "hidden from the model" — + // and this listing IS the model's, so the skill appears here. + const xml = formatAvailableSkillsXml(skills) + expect(xml).toContain('legacy') + }) + + test('appends when_to_use as trigger context', () => { + const skills: SkillsMap = { + deploy: { + name: 'deploy', + description: 'Deploy the application', + content: 'deployment instructions', + whenToUse: 'When the user asks to ship or release', + filePath: '/skills/deploy/SKILL.md', + }, + } + + const xml = formatAvailableSkillsXml(skills) + expect(xml).toContain( + 'When the user asks to ship or release', + ) + }) + test('returns an empty listing when every skill is user-only', () => { const skills: SkillsMap = { deploy: { diff --git a/common/src/util/skills.ts b/common/src/util/skills.ts index 7dc9de121c..a6b0ccdc0a 100644 --- a/common/src/util/skills.ts +++ b/common/src/util/skills.ts @@ -1,5 +1,11 @@ import type { SkillDefinition, SkillsMap } from '../types/skill' +/** + * Whether the agent may load this skill on its own. A skill is model-invocable + * unless it opted out via `disable-model-invocation: true` (user-only) or + * `user-invocable: false` (model-only skills ARE invocable by the model — the + * flag hides them from the user's / menu, not from the agent). + */ export function isSkillModelInvocable(skill: SkillDefinition): boolean { return skill.disableModelInvocation !== true } @@ -26,10 +32,20 @@ export function formatAvailableSkillsXml(skills: SkillsMap): string { } const skillsXml = skillEntries - .map( - (skill) => - ` \n ${skill.name}\n ${escapeXml(skill.description)}\n `, - ) + .map((skill) => { + const lines = [ + ' ', + ` ${skill.name}`, + ` ${escapeXml(skill.description)}`, + ] + // Claude Code parity: `when_to_use` carries extra trigger context that + // helps the model decide when the skill applies. + if (skill.whenToUse) { + lines.push(` ${escapeXml(skill.whenToUse)}`) + } + lines.push(' ') + return lines.join('\n') + }) .join('\n') return `\n${skillsXml}\n` diff --git a/sdk/src/index.ts b/sdk/src/index.ts index 7e0a151b08..6d3418cee0 100644 --- a/sdk/src/index.ts +++ b/sdk/src/index.ts @@ -46,6 +46,7 @@ export { loadSkills, loadSkillsSync, parseSkillFileContent, + resolveSkillsDirs, } from './skills/load-skills' export { formatAvailableSkillsXml } from '@codebuff/common/util/skills' export type { LoadSkillsOptions } from './skills/load-skills' From 1071f3503dff6e9763fa2d95eecc44e069ce7b73 Mon Sep 17 00:00:00 2001 From: lab 1207 Date: Sun, 6 Sep 2026 13:59:15 +0530 Subject: [PATCH 4/7] Verify real Claude Code skills load: ponytail package compat test MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The ponytail skills (built for Claude Code, Codex, and Copilot) are installed by the skills CLI into ~/.claude/skills and must load in Freebuff unmodified for the cross-platform promise to hold. The test runs all six through the SDK loader and asserts name, description, content, and source path survive. Skips where the package is not installed so CI stays green. 🤖 Generated with Codebuff Co-Authored-By: Codebuff --- .../utils/__tests__/ponytail-compat.test.ts | 72 +++++++++++++++++++ 1 file changed, 72 insertions(+) create mode 100644 cli/src/utils/__tests__/ponytail-compat.test.ts diff --git a/cli/src/utils/__tests__/ponytail-compat.test.ts b/cli/src/utils/__tests__/ponytail-compat.test.ts new file mode 100644 index 0000000000..38dda92f20 --- /dev/null +++ b/cli/src/utils/__tests__/ponytail-compat.test.ts @@ -0,0 +1,72 @@ +/** + * Real-world compatibility check: the ponytail skills + * (github.com/DietrichGebert/ponytail) are built for Claude Code / Codex / + * Copilot and installed by the `skills` CLI into ~/.claude/skills. This test + * runs them through Freebuff's actual SDK loader, which is the contract that + * matters: a skill built for Claude Code must load in Freebuff without + * modification. + * + * Skips locally when the skills are not installed, so CI stays green. + */ +import { describe, expect, test } from 'bun:test' +import fs from 'fs' +import os from 'os' +import path from 'path' + +import { loadSkillsSync, resolveSkillsDirs } from '@codebuff/sdk' + +const home = os.homedir() +const ponytailDir = path.join(home, '.claude', 'skills') +const installed = fs.existsSync(path.join(ponytailDir, 'ponytail', 'SKILL.md')) + +describe.skipIf(!installed)('ponytail skills load through the real loader', () => { + const PONYTAIL_SKILLS = [ + 'ponytail', + 'ponytail-audit', + 'ponytail-debt', + 'ponytail-gain', + 'ponytail-help', + 'ponytail-review', + ] + + const dirs = resolveSkillsDirs({ cwd: process.cwd(), homeDir: home }) + const skills = loadSkillsSync({ cwd: process.cwd(), includeHomeSkills: true }) + + test('all six ponytail skills load with no skill rejected', () => { + const loaded = PONYTAIL_SKILLS.filter((name) => skills[name]) + expect(loaded.length).toBe(PONYTAIL_SKILLS.length) + }) + + for (const name of PONYTAIL_SKILLS) { + test(`ponytail skill '${name}' has name/description/content`, () => { + const skill = skills[name] + expect(skill).toBeDefined() + expect(skill!.name).toBe(name) + expect(skill!.description.length).toBeGreaterThan(0) + expect(skill!.content.length).toBeGreaterThan(0) + expect(skill!.filePath.startsWith(home)).toBe(true) + expect(skill!.filePath.includes('.claude')).toBe(true) + }) + } + + test('frontmatter carried the fields Claude Code writes', () => { + const raw = fs.readFileSync( + path.join(ponytailDir, 'ponytail', 'SKILL.md'), + 'utf8', + ) + // The main ponytail skill targets the model with trigger phrases, which + // in Claude Code land in `description` (and/or `when_to_use`); make sure + // whatever is in the file survives parsing. + const skillsMap = loadSkillsSync({ + cwd: process.cwd(), + skillsPath: ponytailDir, + }) + expect(skillsMap['ponytail'].description).toBeTruthy() + expect(raw).toContain('name:') + expect(raw).toContain('description:') + }) + + test('resolveSkillsDirs covers the claude global location', () => { + expect(dirs).toContain(path.join(home, '.claude', 'skills')) + }) +}) From e1b5dd1a8b45c7a2d70c637302ccfd202f1953d8 Mon Sep 17 00:00:00 2001 From: lab 1207 Date: Sun, 6 Sep 2026 14:27:30 +0530 Subject: [PATCH 5/7] Add token estimates and / search filter to the skills panel MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Rows now show a right-aligned ~token estimate (chars/4, k-abbreviated past 999) so users can see what each skill costs their context, and `/` enters a search mode that filters by name/description while keeping arrows/enter live; letters type instead of firing shortcuts, esc exits, backspace on an empty query exits too. Helpers live in renderer-free skills-panel-format.ts. Also fixes the two skill-registry live-reload tests, which passed only on machines without Claude Code skills: includeHomeSkills loaded the real ~/.claude/skills into the counts (7 != 1 with ponytail installed). Tests now redirect HOME/USERPROFILE into the tmp dir, making them hermetic. 🤖 Generated with Codebuff Co-Authored-By: Codebuff --- cli/src/components/skills-panel.tsx | 86 +++++++++++++++---- .../utils/__tests__/skill-registry.test.ts | 14 +++ .../__tests__/skills-panel-actions.test.ts | 83 +++++++++++++++++- .../__tests__/skills-panel-format.test.ts | 70 +++++++++++++++ cli/src/utils/skills-panel-actions.ts | 47 +++++++++- cli/src/utils/skills-panel-format.ts | 35 ++++++++ 6 files changed, 312 insertions(+), 23 deletions(-) create mode 100644 cli/src/utils/__tests__/skills-panel-format.test.ts create mode 100644 cli/src/utils/skills-panel-format.ts diff --git a/cli/src/components/skills-panel.tsx b/cli/src/components/skills-panel.tsx index 8a2f8e7a1a..d9133df07c 100644 --- a/cli/src/components/skills-panel.tsx +++ b/cli/src/components/skills-panel.tsx @@ -17,6 +17,11 @@ import { clamp } from '../utils/math' import { resolveSkillsPanelAction, } from '../utils/skills-panel-actions' +import { + estimateTokens, + matchesSkillQuery, + renderTokens, +} from '../utils/skills-panel-format' import { BORDER_CHARS } from '../utils/ui-constants' import type { SkillDefinition } from '@codebuff/common/types/skill' @@ -87,12 +92,24 @@ export const SkillsPanel: React.FC = ({ ) const [confirmingDelete, setConfirmingDelete] = useState(false) const [notice, setNotice] = useState(null) + const [searching, setSearching] = useState(false) + const [query, setQuery] = useState('') + + const filtered = useMemo( + () => skills.filter((skill) => matchesSkillQuery(skill, query)), + [skills, query], + ) + // Selection rides through the filter by name; when the current selection + // (or the whole list) filters away, snap to the top so Enter is never dead. const selectedIndex = Math.max( 0, - skills.findIndex((skill) => skill.name === selectedName), + filtered.findIndex((skill) => skill.name === selectedName), ) - const selected = skills[selectedIndex] + const selected = filtered[selectedIndex] + useEffect(() => { + if (!selected) setSelectedName(filtered[0]?.name ?? null) + }, [selected, filtered]) // Nothing left to manage: hand the composer back rather than leave an empty // box for the user to dismiss. @@ -123,7 +140,7 @@ export const SkillsPanel: React.FC = ({ // a whole-skills-directory delete deserves instant feedback). void refreshSkillRegistry() // Dropping the row moves the cursor to whatever fills the vacancy. - const successor = skills[selectedIndex + 1] ?? skills[selectedIndex - 1] + const successor = filtered[selectedIndex + 1] ?? filtered[selectedIndex - 1] setSelectedName(successor?.name ?? null) setNotice(`Deleted ${skillDir}`) } catch (error) { @@ -131,7 +148,7 @@ export const SkillsPanel: React.FC = ({ `Could not delete: ${error instanceof Error ? error.message : String(error)}`, ) } - }, [selected, skills, selectedIndex]) + }, [selected, filtered, selectedIndex]) const openInEditor = useCallback(() => { if (!selected) return @@ -147,7 +164,7 @@ export const SkillsPanel: React.FC = ({ const handleKey = useCallback( (key: KeyEvent) => { - const action = resolveSkillsPanelAction(key, { confirmingDelete }) + const action = resolveSkillsPanelAction(key, { confirmingDelete, searching }) if (action.type === 'none') return // Any deliberate action supersedes the last complaint. if (action.type !== 'confirm') setNotice(null) @@ -159,9 +176,24 @@ export const SkillsPanel: React.FC = ({ case 'cancel': setConfirmingDelete(false) return + case 'search-start': + setSearching(true) + return + case 'search-exit': + setSearching(false) + return + case 'search-input': + setQuery((prev) => prev + action.char) + return + case 'search-backspace': + if (query.length > 0) setQuery(query.slice(0, -1)) + // Backspace on an empty query leaves search mode (common editor + // convention) instead of stranding the user in an empty search. + else setSearching(false) + return case 'select': { - const to = clamp(selectedIndex + action.delta, 0, skills.length - 1) - setSelectedName(skills[to]?.name ?? null) + const to = clamp(selectedIndex + action.delta, 0, filtered.length - 1) + setSelectedName(filtered[to]?.name ?? null) return } case 'invoke': @@ -182,26 +214,32 @@ export const SkillsPanel: React.FC = ({ [ confirmingDelete, deleteSelected, + filtered, onClose, onInvoke, openInEditor, + query, + searching, selected, - skills, selectedIndex, - ], + ], ) useKeyboard(handleKey) // A row must fit one line or it wraps and the list stops being scannable. - // Budget: two border columns, two padding columns, then "❯ " + badge + gap. - const promptWidth = Math.max(10, width - 16) + // Budget: two border columns, two padding columns, then "❯ " + badge + gap + // + right-aligned token estimate, so descriptions truncate where the + // composer would. + const promptWidth = Math.max(10, width - 22) const rowLabel = useCallback( (skill: SkillDefinition) => { const badge = sourceOf(skill) === 'project' ? 'project' : 'global' + const tokens = renderTokens(estimateTokens(skill)) + const suffix = ` ${tokens}` const body = - truncateToSingleLinePreview(skill.description, promptWidth) ?? '' - return `${badge.padEnd(7)} ${body}` + truncateToSingleLinePreview(skill.description, promptWidth - suffix.length) ?? '' + return `${badge.padEnd(7)} ${body.padEnd(Math.max(0, promptWidth - suffix.length))}${suffix}` }, [promptWidth], ) @@ -211,10 +249,12 @@ export const SkillsPanel: React.FC = ({ [skills], ) const globalCount = skills.length - projectCount + const hiddenBySearch = skills.length - filtered.length + const filterActive = query.trim().length > 0 - const start = windowStart(selectedIndex, skills.length, maxVisibleRows) - const visible = skills.slice(start, start + maxVisibleRows) - const hiddenBelow = skills.length - (start + visible.length) + const start = windowStart(selectedIndex, filtered.length, maxVisibleRows) + const visible = filtered.slice(start, start + maxVisibleRows) + const hiddenBelow = filtered.length - (start + visible.length) return ( = ({ {` ↓ ${hiddenBelow} more`} )} + {hiddenBySearch > 0 && ( + + {`${hiddenBySearch} hidden by filter · esc clears`} + + )} + {notice && {notice}} - {confirmingDelete ? ( + {searching ? ( + + {`/${query}▏ type to filter · esc done`} + + ) : confirmingDelete ? ( {`Delete ${selected && path.dirname(selected.filePath)}/? Enter confirm · Esc cancel`} ) : ( - {'Enter run · o open · d delete · esc close'} + {'/ filter · Enter run · o open · d delete · esc close'} )} diff --git a/cli/src/utils/__tests__/skill-registry.test.ts b/cli/src/utils/__tests__/skill-registry.test.ts index 94608d4371..4e8e79c289 100644 --- a/cli/src/utils/__tests__/skill-registry.test.ts +++ b/cli/src/utils/__tests__/skill-registry.test.ts @@ -15,6 +15,8 @@ import { } from '../skill-registry' let tmpRoot: string +let oldHome: string | undefined +let oldUserProfile: string | undefined const writeSkill = ( kind: 'project' | 'global', @@ -40,10 +42,22 @@ const wait = (ms: number) => new Promise((resolve) => setTimeout(resolve, ms)) beforeEach(() => { tmpRoot = fs.mkdtempSync(path.join(os.tmpdir(), 'skill-registry-')) setProjectRoot(tmpRoot) + // Hermetic home: includeHomeSkills makes the SDK load real ~/.claude/skills + // too, so on a machine with skills installed for Claude Code (any + // maintainer's, in practice) the counts below would include those. Redirect + // the home directory so tests only see what they write themselves. + oldHome = process.env.HOME + oldUserProfile = process.env.USERPROFILE + process.env.HOME = tmpRoot + process.env.USERPROFILE = tmpRoot }) afterEach(() => { __resetSkillRegistryForTests() + if (oldHome !== undefined) process.env.HOME = oldHome + else delete process.env.HOME + if (oldUserProfile !== undefined) process.env.USERPROFILE = oldUserProfile + else delete process.env.USERPROFILE fs.rmSync(tmpRoot, { recursive: true, force: true }) }) diff --git a/cli/src/utils/__tests__/skills-panel-actions.test.ts b/cli/src/utils/__tests__/skills-panel-actions.test.ts index f4f9739392..98b711568f 100644 --- a/cli/src/utils/__tests__/skills-panel-actions.test.ts +++ b/cli/src/utils/__tests__/skills-panel-actions.test.ts @@ -15,8 +15,9 @@ const createKey = (overrides: Partial = {}): KeyEvent => ...overrides, }) as KeyEvent -const browsing = { confirmingDelete: false } -const confirming = { confirmingDelete: true } +const browsing = { confirmingDelete: false, searching: false } +const confirming = { confirmingDelete: true, searching: false } +const searching = { confirmingDelete: false, searching: true } describe('resolveSkillsPanelAction', () => { test('arrows and j/k move the selection', () => { @@ -99,6 +100,84 @@ describe('resolveSkillsPanelAction', () => { ).toEqual({ type: 'cancel' }) }) + test('/ enters search mode', () => { + expect( + resolveSkillsPanelAction(createKey({ name: '/', sequence: '/' }), browsing), + ).toEqual({ type: 'search-start' }) + // Only a bare `/` — ctrl/meta variants stay unbound so a chord can never + // drop the user into a typing mode they did not ask for. + expect( + resolveSkillsPanelAction(createKey({ name: '/', sequence: '/', ctrl: true }), browsing), + ).toEqual({ type: 'none' }) + }) + + test('search mode: printable keys become query edits, not shortcuts', () => { + // `d` and `o` would delete/open while browsing; while searching they are + // just letters in the query. + expect( + resolveSkillsPanelAction(createKey({ name: 'd', sequence: 'd' }), searching), + ).toEqual({ type: 'search-input', char: 'd' }) + expect( + resolveSkillsPanelAction(createKey({ name: 'o', sequence: 'o' }), searching), + ).toEqual({ type: 'search-input', char: 'o' }) + expect( + resolveSkillsPanelAction(createKey({ name: 'q', sequence: 'q' }), searching), + ).toEqual({ type: 'search-input', char: 'q' }) + expect( + resolveSkillsPanelAction( + createKey({ name: 'space', sequence: ' ' }), + searching, + ), + ).toEqual({ type: 'search-input', char: ' ' }) + }) + + test('search mode: backspace deletes, escape exits, ctrl+c still closes', () => { + expect( + resolveSkillsPanelAction(createKey({ name: 'backspace' }), searching), + ).toEqual({ type: 'search-backspace' }) + expect( + resolveSkillsPanelAction(createKey({ name: 'escape' }), searching), + ).toEqual({ type: 'search-exit' }) + expect( + resolveSkillsPanelAction(createKey({ name: 'c', ctrl: true }), searching), + ).toEqual({ type: 'close' }) + }) + + test('search mode: navigation and invoke still work', () => { + // Named keys (arrows, enter) still navigate/invoke while typing. + expect( + resolveSkillsPanelAction(createKey({ name: 'up' }), searching), + ).toEqual({ type: 'select', delta: -1 }) + expect( + resolveSkillsPanelAction(createKey({ name: 'down' }), searching), + ).toEqual({ type: 'select', delta: 1 }) + expect( + resolveSkillsPanelAction(createKey({ name: 'return' }), searching), + ).toEqual({ type: 'invoke' }) + // But the letter forms of those shortcuts are query edits here — `j` + // types a j, it does not move the cursor. + expect( + resolveSkillsPanelAction(createKey({ name: 'j', sequence: 'j' }), searching), + ).toEqual({ type: 'search-input', char: 'j' }) + }) + + test('search mode: delete confirmation still outranks typing', () => { + // A stray `d` while a delete is pending must never edit a query — the + // confirm prompt only answers to enter/confirm or cancel. + expect( + resolveSkillsPanelAction( + createKey({ name: 'd', sequence: 'd' }), + { confirmingDelete: true, searching: true }, + ), + ).toEqual({ type: 'none' }) + expect( + resolveSkillsPanelAction( + createKey({ name: 'return' }), + { confirmingDelete: true, searching: true }, + ), + ).toEqual({ type: 'confirm' }) + }) + test('unbound keys do nothing', () => { expect(resolveSkillsPanelAction(createKey({ name: 'x' }), browsing)).toEqual( { type: 'none' }, diff --git a/cli/src/utils/__tests__/skills-panel-format.test.ts b/cli/src/utils/__tests__/skills-panel-format.test.ts new file mode 100644 index 0000000000..648dfd34b0 --- /dev/null +++ b/cli/src/utils/__tests__/skills-panel-format.test.ts @@ -0,0 +1,70 @@ +import { describe, expect, test } from 'bun:test' + +import { + estimateTokens, + matchesSkillQuery, + renderTokens, +} from '../skills-panel-format' + +import type { SkillDefinition } from '@codebuff/common/types/skill' + +const skill = (overrides: Partial = {}): SkillDefinition => ({ + name: 'release-notes', + description: 'Draft release notes from recent commits', + content: 'x'.repeat(400), + filePath: '/project/.agents/skills/release-notes/SKILL.md', + ...overrides, +}) + +describe('estimateTokens', () => { + test('scales with content length at ~4 chars per token', () => { + // Both cases pass an empty description so content dominates the math. + expect( + estimateTokens(skill({ content: 'x'.repeat(400), description: '' })), + ).toBe(100) + expect( + estimateTokens(skill({ content: 'x'.repeat(4000), description: '' })), + ).toBe(1000) + }) + + test('counts the description and never returns zero', () => { + // 12 chars of description + empty content still rounds to 3. + expect(estimateTokens(skill({ content: '', description: '123456789012' }))).toBe( + 3, + ) + expect(estimateTokens(skill({ content: '', description: '' }))).toBe(1) + }) +}) + +describe('renderTokens', () => { + test('plain numbers below 1000, k notation above', () => { + expect(renderTokens(70)).toBe('70 tok') + expect(renderTokens(999)).toBe('999 tok') + expect(renderTokens(1000)).toBe('1k tok') + expect(renderTokens(2800)).toBe('2.8k tok') + }) +}) + +describe('matchesSkillQuery', () => { + const ponytail = skill({ + name: 'ponytail', + description: 'Forces the laziest solution that actually works', + }) + + test('empty or whitespace queries match everything', () => { + expect(matchesSkillQuery(ponytail, '')).toBe(true) + expect(matchesSkillQuery(ponytail, ' ')).toBe(true) + }) + + test('matches by name or description, case-insensitively', () => { + expect(matchesSkillQuery(ponytail, 'pony')).toBe(true) + expect(matchesSkillQuery(ponytail, 'PONY')).toBe(true) + expect(matchesSkillQuery(ponytail, 'laziest')).toBe(true) + expect(matchesSkillQuery(ponytail, 'git-helper')).toBe(false) + }) + + test('trims the query before matching', () => { + expect(matchesSkillQuery(ponytail, ' pony ')).toBe(true) + expect(matchesSkillQuery(ponytail, ' zzz ')).toBe(false) + }) +}) diff --git a/cli/src/utils/skills-panel-actions.ts b/cli/src/utils/skills-panel-actions.ts index 71e1c97f2f..e4271ca9c9 100644 --- a/cli/src/utils/skills-panel-actions.ts +++ b/cli/src/utils/skills-panel-actions.ts @@ -17,12 +17,31 @@ export type SkillsPanelAction = | { type: 'delete' } | { type: 'confirm' } | { type: 'cancel' } + /** Enter search mode: keystrokes filter the list instead of triggering actions. */ + | { type: 'search-start' } + /** Append `char` to the search query (search mode only). */ + | { type: 'search-input'; char: string } + /** Remove the last character of the search query (search mode only). */ + | { type: 'search-backspace' } + /** Leave search mode, keeping the current filter (search mode only). */ + | { type: 'search-exit' } | { type: 'none' } export type SkillsPanelKeyboardState = { /** While a delete is pending confirmation the panel only listens for * confirm/cancel — a stray `d` must not chain-delete the next row. */ confirmingDelete: boolean + /** While search mode is active, printable keys edit the query instead of + * firing single-letter shortcuts like o/d/j/k. */ + searching: boolean +} + +/** A printable character keypress (no ctrl/meta modifiers). */ +function printableChar(key: KeyEvent): string | null { + if (key.ctrl || key.meta || key.option) return null + if (key.sequence && key.sequence.length === 1 && key.sequence >= ' ') + return key.sequence + return null } export function resolveSkillsPanelAction( @@ -34,15 +53,34 @@ export function resolveSkillsPanelAction( if (state.confirmingDelete) { // Enter confirms; anything escape-shaped cancels; everything else is - // swallowed so a held key can't confirm or chain. + // swallowed so a held key can't confirm or chain. Delete confirmation + // outranks search mode: it must never be dismissible by a query edit. if (key.name === 'return' || key.name === 'enter') return { type: 'confirm' } if (isEscape || isCtrlC || key.name === 'n' || key.name === 'q') return { type: 'cancel' } return { type: 'none' } } - // `q` closes, mirroring the queue panel. - if (isEscape || isCtrlC || key.name === 'q') return { type: 'close' } + // `q` closes, mirroring the queue panel. Ctrl+C closes even from search. + if (isEscape && !state.searching) return { type: 'close' } + if (isCtrlC) return { type: 'close' } + if (key.name === 'q' && !state.searching) return { type: 'close' } + + if (state.searching) { + if (key.name === 'backspace') + return { type: 'search-backspace' } + if (isEscape) return { type: 'search-exit' } + // Arrows and enter keep working while typing — but only their named + // forms: letters (j/k/q/d/o) are query edits in search mode. + if (key.name === 'up') return { type: 'select', delta: -1 } + if (key.name === 'down') return { type: 'select', delta: 1 } + if (key.name === 'return' || key.name === 'enter') return { type: 'invoke' } + const char = printableChar(key) + if (char) return { type: 'search-input', char } + // Anything else (modifiers, chords, unnamed keys) does nothing rather + // than firing a single-letter shortcut underneath the typing. + return { type: 'none' } + } if (key.name === 'up' || key.name === 'k') return { type: 'select', delta: -1 } if (key.name === 'down' || key.name === 'j') @@ -54,5 +92,8 @@ export function resolveSkillsPanelAction( if (key.name === 'd' || key.name === 'delete') return { type: 'delete' } + // `/` enters search mode (matches the queue panel's slash-filter affordance). + if (printableChar(key) === '/') return { type: 'search-start' } + return { type: 'none' } } diff --git a/cli/src/utils/skills-panel-format.ts b/cli/src/utils/skills-panel-format.ts new file mode 100644 index 0000000000..9291203b8c --- /dev/null +++ b/cli/src/utils/skills-panel-format.ts @@ -0,0 +1,35 @@ +import type { SkillDefinition } from '@codebuff/common/types/skill' + +/** + * Panel display helpers, kept renderer-free so they are unit-testable — + * the same split as skills-panel-actions.ts. + */ + +/** + * Rough context cost of a skill, in tokens (~4 chars/token). Matches the + * order of magnitude of Claude Code's per-row estimates; the point is the + * relative weight, not an exact count. + */ +export function estimateTokens(skill: SkillDefinition): number { + const chars = skill.content.length + skill.description.length + return Math.max(1, Math.round(chars / 4)) +} + +/** Compact right-aligned token readout: 1–3 digits, then `k` past 999. */ +export function renderTokens(tokens: number): string { + const short = tokens > 999 ? `${Math.round(tokens / 100) / 10}k` : `${tokens}` + return `${short} tok` +} + +/** + * Case-insensitive substring match against name + description. Shared by the + * panel and its tests so the filter semantics have exactly one definition. + */ +export function matchesSkillQuery(skill: SkillDefinition, query: string): boolean { + const q = query.trim().toLowerCase() + if (!q) return true + return ( + skill.name.toLowerCase().includes(q) || + skill.description.toLowerCase().includes(q) + ) +} From 9f030e0220adef2d8a04146ad21fcfbf9cf8e0f2 Mon Sep 17 00:00:00 2001 From: lab 1207 Date: Sun, 6 Sep 2026 14:48:48 +0530 Subject: [PATCH 6/7] Make escape unwind the skills panel one layer at a time MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The footer promised "esc clears" the filter, but while browsing escape closed the panel — the filter could only be cleared by backspacing in search mode. Escape now unwinds one layer at a time: search mode → active filter → close, with a footer hint that changes to "esc clears filter · esc again closes" while a filter is active. q and ctrl+c still close directly. 🤖 Generated with Codebuff Co-Authored-By: Codebuff --- cli/src/components/skills-panel.tsx | 20 ++++++-- .../__tests__/skills-panel-actions.test.ts | 49 +++++++++++++++++-- cli/src/utils/skills-panel-actions.ts | 14 ++++-- 3 files changed, 69 insertions(+), 14 deletions(-) diff --git a/cli/src/components/skills-panel.tsx b/cli/src/components/skills-panel.tsx index d9133df07c..15615221b4 100644 --- a/cli/src/components/skills-panel.tsx +++ b/cli/src/components/skills-panel.tsx @@ -94,6 +94,7 @@ export const SkillsPanel: React.FC = ({ const [notice, setNotice] = useState(null) const [searching, setSearching] = useState(false) const [query, setQuery] = useState('') + const filterActive = query.length > 0 const filtered = useMemo( () => skills.filter((skill) => matchesSkillQuery(skill, query)), @@ -164,7 +165,11 @@ export const SkillsPanel: React.FC = ({ const handleKey = useCallback( (key: KeyEvent) => { - const action = resolveSkillsPanelAction(key, { confirmingDelete, searching }) + const action = resolveSkillsPanelAction(key, { + confirmingDelete, + searching, + filterActive, + }) if (action.type === 'none') return // Any deliberate action supersedes the last complaint. if (action.type !== 'confirm') setNotice(null) @@ -182,6 +187,9 @@ export const SkillsPanel: React.FC = ({ case 'search-exit': setSearching(false) return + case 'search-clear': + setQuery('') + return case 'search-input': setQuery((prev) => prev + action.char) return @@ -215,6 +223,7 @@ export const SkillsPanel: React.FC = ({ confirmingDelete, deleteSelected, filtered, + filterActive, onClose, onInvoke, openInEditor, @@ -250,7 +259,6 @@ export const SkillsPanel: React.FC = ({ ) const globalCount = skills.length - projectCount const hiddenBySearch = skills.length - filtered.length - const filterActive = query.trim().length > 0 const start = windowStart(selectedIndex, filtered.length, maxVisibleRows) const visible = filtered.slice(start, start + maxVisibleRows) @@ -306,8 +314,8 @@ export const SkillsPanel: React.FC = ({ {hiddenBySearch > 0 && ( - {`${hiddenBySearch} hidden by filter · esc clears`} - + {`${hiddenBySearch} hidden by filter`} + )} {notice && {notice}} @@ -322,7 +330,9 @@ export const SkillsPanel: React.FC = ({ ) : ( - {'/ filter · Enter run · o open · d delete · esc close'} + {filterActive + ? '/ filter · esc clears filter · esc again closes' + : '/ filter · Enter run · o open · d delete · esc close'} )} diff --git a/cli/src/utils/__tests__/skills-panel-actions.test.ts b/cli/src/utils/__tests__/skills-panel-actions.test.ts index 98b711568f..87d12ad480 100644 --- a/cli/src/utils/__tests__/skills-panel-actions.test.ts +++ b/cli/src/utils/__tests__/skills-panel-actions.test.ts @@ -15,9 +15,26 @@ const createKey = (overrides: Partial = {}): KeyEvent => ...overrides, }) as KeyEvent -const browsing = { confirmingDelete: false, searching: false } -const confirming = { confirmingDelete: true, searching: false } -const searching = { confirmingDelete: false, searching: true } +const browsing = { + confirmingDelete: false, + searching: false, + filterActive: false, +} +const confirming = { + confirmingDelete: true, + searching: false, + filterActive: false, +} +const searching = { + confirmingDelete: false, + searching: true, + filterActive: false, +} +const filtered = { + confirmingDelete: false, + searching: false, + filterActive: true, +} describe('resolveSkillsPanelAction', () => { test('arrows and j/k move the selection', () => { @@ -167,17 +184,39 @@ describe('resolveSkillsPanelAction', () => { expect( resolveSkillsPanelAction( createKey({ name: 'd', sequence: 'd' }), - { confirmingDelete: true, searching: true }, + { confirmingDelete: true, searching: true, filterActive: true }, ), ).toEqual({ type: 'none' }) expect( resolveSkillsPanelAction( createKey({ name: 'return' }), - { confirmingDelete: true, searching: true }, + { confirmingDelete: true, searching: true, filterActive: true }, ), ).toEqual({ type: 'confirm' }) }) + test('escape unwinds one layer at a time: search, then filter, then close', () => { + // Browsing with an active filter: escape clears the filter, not the panel. + expect( + resolveSkillsPanelAction(createKey({ name: 'escape' }), filtered), + ).toEqual({ type: 'search-clear' }) + // With the filter gone, the same key closes. + expect( + resolveSkillsPanelAction(createKey({ name: 'escape' }), browsing), + ).toEqual({ type: 'close' }) + // And from inside search mode it only leaves the search. + expect( + resolveSkillsPanelAction(createKey({ name: 'escape' }), searching), + ).toEqual({ type: 'search-exit' }) + // q and ctrl+c still close even while a filter is active. + expect( + resolveSkillsPanelAction(createKey({ name: 'q' }), filtered), + ).toEqual({ type: 'close' }) + expect( + resolveSkillsPanelAction(createKey({ name: 'c', ctrl: true }), filtered), + ).toEqual({ type: 'close' }) + }) + test('unbound keys do nothing', () => { expect(resolveSkillsPanelAction(createKey({ name: 'x' }), browsing)).toEqual( { type: 'none' }, diff --git a/cli/src/utils/skills-panel-actions.ts b/cli/src/utils/skills-panel-actions.ts index e4271ca9c9..9ce59451c5 100644 --- a/cli/src/utils/skills-panel-actions.ts +++ b/cli/src/utils/skills-panel-actions.ts @@ -25,6 +25,8 @@ export type SkillsPanelAction = | { type: 'search-backspace' } /** Leave search mode, keeping the current filter (search mode only). */ | { type: 'search-exit' } + /** Clear the active filter (browsing with a non-empty query only). */ + | { type: 'search-clear' } | { type: 'none' } export type SkillsPanelKeyboardState = { @@ -34,6 +36,9 @@ export type SkillsPanelKeyboardState = { /** While search mode is active, printable keys edit the query instead of * firing single-letter shortcuts like o/d/j/k. */ searching: boolean + /** True while a non-empty query filters the list: escape then clears the + * filter instead of closing the panel (press it again to close). */ + filterActive: boolean } /** A printable character keypress (no ctrl/meta modifiers). */ @@ -61,15 +66,16 @@ export function resolveSkillsPanelAction( return { type: 'none' } } - // `q` closes, mirroring the queue panel. Ctrl+C closes even from search. - if (isEscape && !state.searching) return { type: 'close' } - if (isCtrlC) return { type: 'close' } + // Escape unwinds one layer at a time: search mode → filter → close. + // (Ctrl+C always closes; `q` closes, mirroring the queue panel.) + if (isEscape && state.searching) return { type: 'search-exit' } + if (isEscape && state.filterActive) return { type: 'search-clear' } + if (isEscape || isCtrlC) return { type: 'close' } if (key.name === 'q' && !state.searching) return { type: 'close' } if (state.searching) { if (key.name === 'backspace') return { type: 'search-backspace' } - if (isEscape) return { type: 'search-exit' } // Arrows and enter keep working while typing — but only their named // forms: letters (j/k/q/d/o) are query edits in search mode. if (key.name === 'up') return { type: 'select', delta: -1 } From fec794363f93808dea3717cb261bb707af0e031c Mon Sep 17 00:00:00 2001 From: lab 1207 Date: Sun, 6 Sep 2026 22:19:24 +0530 Subject: [PATCH 7/7] Remove live-reload watcher to keep the panel PR focused MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Per review on #1289: the filesystem watcher is an independently reviewable feature and moves to its own PR (recursive-watch semantics differ per platform). The panel keeps its refresh-on-open and refresh-after-delete behavior, which covers installs and edits without any watcher. Also drops ponytail-compat.test.ts (external dependency, no CI signal). 🤖 Generated with Codebuff Co-Authored-By: Codebuff --- cli/src/chat.tsx | 9 ++- cli/src/components/skills-panel.tsx | 4 +- cli/src/index.tsx | 10 +-- .../utils/__tests__/ponytail-compat.test.ts | 72 ------------------- .../utils/__tests__/skill-registry.test.ts | 25 ------- cli/src/utils/skill-registry.ts | 63 ++-------------- 6 files changed, 12 insertions(+), 171 deletions(-) delete mode 100644 cli/src/utils/__tests__/ponytail-compat.test.ts diff --git a/cli/src/chat.tsx b/cli/src/chat.tsx index 2fbd2067e7..bfd5f5726e 100644 --- a/cli/src/chat.tsx +++ b/cli/src/chat.tsx @@ -557,9 +557,9 @@ export const Chat = ({ const askUserState = useChatStore((state) => state.askUserState) // Get loaded skills for slash commands. Keyed on the registry version so a - // live-reload (watcher or /skills refresh) swaps the list without a - // restart — the registry object itself is mutated in place, which zustand - // and useMemo would never notice. + // mid-session change (delete via the panel, edit on disk + reopen) swaps the + // list without a restart — the registry object itself is mutated in place, + // which zustand and useMemo would never notice. const skillsVersion = useSyncExternalStore( subscribeToSkillsVersion, getSkillsVersion, @@ -1270,8 +1270,7 @@ export const Chat = ({ }, [closeSkillsPanel, setInputFocused, inputRef]) // Refresh the registry when the /skills panel opens, so a skill installed - // moments ago shows up even if the watcher missed it (e.g. directory - // created and populated before the watcher could arm on it). + // or edited moments ago shows up without restarting the CLI. useEffect(() => { if (!skillsPanelOpen) return void refreshSkillRegistry() diff --git a/cli/src/components/skills-panel.tsx b/cli/src/components/skills-panel.tsx index 15615221b4..f1c5deef94 100644 --- a/cli/src/components/skills-panel.tsx +++ b/cli/src/components/skills-panel.tsx @@ -137,8 +137,8 @@ export const SkillsPanel: React.FC = ({ try { await rm(skillDir, { recursive: true }) // Refresh the registry right away: the version bump re-renders the - // panel with the refreshed list (the watcher would also catch it, but - // a whole-skills-directory delete deserves instant feedback). + // panel with the refreshed list instead of waiting for the + // refresh-on-open. void refreshSkillRegistry() // Dropping the row moves the cursor to whatever fills the vacancy. const successor = filtered[selectedIndex + 1] ?? filtered[selectedIndex - 1] diff --git a/cli/src/index.tsx b/cli/src/index.tsx index a63b6a24e7..cae4e380eb 100644 --- a/cli/src/index.tsx +++ b/cli/src/index.tsx @@ -47,10 +47,7 @@ import { } from './utils/renderer-cleanup' import { startTerminalWatchdog } from './utils/terminal-watchdog' import { installTerminalProtocolController } from './utils/terminal-protocol-controller' -import { - initializeSkillRegistry, - startSkillDirWatcher, -} from './utils/skill-registry' +import { initializeSkillRegistry } from './utils/skill-registry' import { detectTerminalTheme } from './utils/terminal-color-detection' import { setOscDetectedTheme } from './utils/theme-system' @@ -266,11 +263,6 @@ async function main(): Promise { // Initialize skill registry (loads skills from .agents/skills) await initializeSkillRegistry() - // Claude Code parity: watch skill directories so skills installed, edited, - // or deleted mid-session appear without a restart. A no-op when none of the - // directories exist yet. - startSkillDirWatcher() - // Handle publish command before rendering the app if (isPublishCommand) { const publishIndex = process.argv.indexOf('publish') diff --git a/cli/src/utils/__tests__/ponytail-compat.test.ts b/cli/src/utils/__tests__/ponytail-compat.test.ts deleted file mode 100644 index 38dda92f20..0000000000 --- a/cli/src/utils/__tests__/ponytail-compat.test.ts +++ /dev/null @@ -1,72 +0,0 @@ -/** - * Real-world compatibility check: the ponytail skills - * (github.com/DietrichGebert/ponytail) are built for Claude Code / Codex / - * Copilot and installed by the `skills` CLI into ~/.claude/skills. This test - * runs them through Freebuff's actual SDK loader, which is the contract that - * matters: a skill built for Claude Code must load in Freebuff without - * modification. - * - * Skips locally when the skills are not installed, so CI stays green. - */ -import { describe, expect, test } from 'bun:test' -import fs from 'fs' -import os from 'os' -import path from 'path' - -import { loadSkillsSync, resolveSkillsDirs } from '@codebuff/sdk' - -const home = os.homedir() -const ponytailDir = path.join(home, '.claude', 'skills') -const installed = fs.existsSync(path.join(ponytailDir, 'ponytail', 'SKILL.md')) - -describe.skipIf(!installed)('ponytail skills load through the real loader', () => { - const PONYTAIL_SKILLS = [ - 'ponytail', - 'ponytail-audit', - 'ponytail-debt', - 'ponytail-gain', - 'ponytail-help', - 'ponytail-review', - ] - - const dirs = resolveSkillsDirs({ cwd: process.cwd(), homeDir: home }) - const skills = loadSkillsSync({ cwd: process.cwd(), includeHomeSkills: true }) - - test('all six ponytail skills load with no skill rejected', () => { - const loaded = PONYTAIL_SKILLS.filter((name) => skills[name]) - expect(loaded.length).toBe(PONYTAIL_SKILLS.length) - }) - - for (const name of PONYTAIL_SKILLS) { - test(`ponytail skill '${name}' has name/description/content`, () => { - const skill = skills[name] - expect(skill).toBeDefined() - expect(skill!.name).toBe(name) - expect(skill!.description.length).toBeGreaterThan(0) - expect(skill!.content.length).toBeGreaterThan(0) - expect(skill!.filePath.startsWith(home)).toBe(true) - expect(skill!.filePath.includes('.claude')).toBe(true) - }) - } - - test('frontmatter carried the fields Claude Code writes', () => { - const raw = fs.readFileSync( - path.join(ponytailDir, 'ponytail', 'SKILL.md'), - 'utf8', - ) - // The main ponytail skill targets the model with trigger phrases, which - // in Claude Code land in `description` (and/or `when_to_use`); make sure - // whatever is in the file survives parsing. - const skillsMap = loadSkillsSync({ - cwd: process.cwd(), - skillsPath: ponytailDir, - }) - expect(skillsMap['ponytail'].description).toBeTruthy() - expect(raw).toContain('name:') - expect(raw).toContain('description:') - }) - - test('resolveSkillsDirs covers the claude global location', () => { - expect(dirs).toContain(path.join(home, '.claude', 'skills')) - }) -}) diff --git a/cli/src/utils/__tests__/skill-registry.test.ts b/cli/src/utils/__tests__/skill-registry.test.ts index 4e8e79c289..4c438ec39a 100644 --- a/cli/src/utils/__tests__/skill-registry.test.ts +++ b/cli/src/utils/__tests__/skill-registry.test.ts @@ -10,7 +10,6 @@ import { getSkillsVersion, initializeSkillRegistry, refreshSkillRegistry, - startSkillDirWatcher, subscribeToSkillsVersion, } from '../skill-registry' @@ -37,8 +36,6 @@ const writeSkill = ( return skillDir } -const wait = (ms: number) => new Promise((resolve) => setTimeout(resolve, ms)) - beforeEach(() => { tmpRoot = fs.mkdtempSync(path.join(os.tmpdir(), 'skill-registry-')) setProjectRoot(tmpRoot) @@ -138,28 +135,6 @@ describe('skill-registry refresh', () => { await refreshSkillRegistry() expect(onChange).toHaveBeenCalledTimes(1) }) - - test('watcher picks up an install without restart', async () => { - // The skills directory must exist BEFORE the watcher arms on it — fs.watch - // cannot watch a directory that does not exist. (A directory created and - // populated later is caught by the refresh-on-panel-open instead.) - fs.mkdirSync(path.join(tmpRoot, '.agents', 'skills'), { recursive: true }) - startSkillDirWatcher() - - await initializeSkillRegistry() - expect(getSkillCountForTest()).toBe(0) - - writeSkill('project', 'deploy', { - name: 'deploy', - description: 'Deploy the app', - }) - - // Watcher debounce is 300ms; give it room on slow CI. - await wait(900) - - expect(getSkillsVersion()).toBeGreaterThan(0) - expect(getLoadedSkills()['deploy']).toBeDefined() - }) }) function getSkillCountForTest(): number { diff --git a/cli/src/utils/skill-registry.ts b/cli/src/utils/skill-registry.ts index e859ab5494..c2080e78ba 100644 --- a/cli/src/utils/skill-registry.ts +++ b/cli/src/utils/skill-registry.ts @@ -1,7 +1,6 @@ import os from 'os' -import { watch, type FSWatcher } from 'fs' -import { loadSkills as sdkLoadSkills, resolveSkillsDirs } from '@codebuff/sdk' +import { loadSkills as sdkLoadSkills } from '@codebuff/sdk' import { getProjectRoot, tryGetProjectRoot } from '../project-files' import { logger } from './logger' @@ -93,62 +92,11 @@ export async function refreshSkillRegistry(): Promise { } // ============================================================================ -// Live reload (Claude Code parity) +// Live reload // ============================================================================ -// Claude Code watches skill directories and picks up add/edit/delete within -// the running session. Without this, a skill installed mid-session is -// invisible until restart — the "install every time" complaint. - -const SKILLS_WATCH_DEBOUNCE_MS = 300 - -let skillWatchers: FSWatcher[] = [] - -/** - * Start watching the resolved skill directories (global + project, Claude - * locations included). Idempotent. A directory that does not exist yet is - * skipped — installs that create it later are caught by the refresh when the - * /skills panel opens. - */ -export function startSkillDirWatcher(): void { - if (skillWatchers.length > 0) return - - const cwd = skillsCwd() - const homeDir = os.homedir() - const dirs = resolveSkillsDirs({ cwd, homeDir }) - - let debounceTimer: ReturnType | null = null - const scheduleRefresh = () => { - if (debounceTimer) clearTimeout(debounceTimer) - debounceTimer = setTimeout(() => { - debounceTimer = null - void refreshSkillRegistry() - }, SKILLS_WATCH_DEBOUNCE_MS) - } - - for (const dir of dirs) { - try { - const watcher = watch(dir, { persistent: false }, () => { - // Filter nothing: skill installs create directories AND write - // SKILL.md inside them, whole-skill deletes only touch the dir name, - // and the refresh itself is a debounced handful of stat+read calls. - // Simpler and correct beats a filename heuristic that misses cases. - scheduleRefresh() - }) - watcher.on('error', (error) => { - logger.warn({ error }, `Skill watcher error for ${dir}`) - }) - skillWatchers.push(watcher) - } catch { - // Directory does not exist (e.g. no ~/.agents/skills yet). Nothing to - // watch; installs create it fresh and a restart picks them up. - } - } -} - -export function stopSkillDirWatcher(): void { - for (const watcher of skillWatchers) watcher.close() - skillWatchers = [] -} +// Implemented on feat/skills-reload — deliberately not here. Watching skill +// directories is an independently reviewable feature (recursive watch +// semantics differ per platform) and lives in its own PR. /** * Initialize the skill registry by loading skills via the SDK. @@ -237,7 +185,6 @@ export function getLoadedSkillsMessage(): string | null { export function __resetSkillRegistryForTests(): void { skillsCache = {} skillsVersion = 0 - stopSkillDirWatcher() } /**