diff --git a/src/servers/api/items/rescan.ts b/src/servers/api/items/rescan.ts index 55b3ae71..fe08b35f 100644 --- a/src/servers/api/items/rescan.ts +++ b/src/servers/api/items/rescan.ts @@ -4,6 +4,7 @@ import { createRouter } from '../../create-router'; import { ITEM_TYPES, itemsDir, ensureItemDirs, type ItemType } from '../../data-path'; import { ensureToolLoader } from '../../ensure-tool-loader'; import { AFFORDANCES_DIR, loadAffordances } from '../../affordances/load'; +import { loadSkills } from '../../skills/load'; // Re-applies the boot-time item setup so changes under OFFICER_ITEMS_DIR take effect without a // server restart. Item lists are already read fresh from disk on every request; this ensures the @@ -54,6 +55,9 @@ rescanRouter.post('/', async (ctx) => { // rather than "markdown files present" — one with no body is skipped by the loader and should not be // reported as though it were working. const affordances = loadAffordances().length; + // Loaded, not file-counted, for the same reason: a SKILL.md with no `description` is skipped by the + // loader and would otherwise be reported as working. + const skillsOffered = loadSkills().length; - return ctx.json({ ok: true, counts: { ...counts, affordances } }); + return ctx.json({ ok: true, counts: { ...counts, affordances, skillsOffered } }); }); diff --git a/src/servers/plugins/install.ts b/src/servers/plugins/install.ts index d031f5a2..13803c5c 100644 --- a/src/servers/plugins/install.ts +++ b/src/servers/plugins/install.ts @@ -18,6 +18,7 @@ import { revokeGrantsForPermissions } from 'officerdb'; import { publishAssets, unpublishAssets } from '../app-store/assets'; import { publishAffordances, unpublishAffordances } from '../affordances/publish'; import { publishNotifications, unpublishNotifications } from '../notifications/publish'; +import { publishSkills, unpublishSkills } from '../skills/publish'; import { refreshPluginMounts, snapshotPlugins } from './mount'; import { generatePluginSchemas, pushPluginSchema } from './schema'; import { declaredTables, dropTables, existingTables } from './tables'; @@ -373,6 +374,15 @@ export async function installPlugin( await step(steps, onStep, `notifications: NOT published — ${err instanceof Error ? err.message : err}`); } + // Same bargain again: a plugin that cannot publish its skills still installs. The agent simply does + // not learn the reference exists, which is the state every plugin was in before this. + try { + const published = await publishSkills(plugin); + if (published) await step(steps, onStep, `skills: ${published} published to capabilities/skills/`); + } catch (err) { + await step(steps, onStep, `skills: NOT published — ${err instanceof Error ? err.message : err}`); + } + // The tile, for the person who just installed it. `plugin.web` because `pluginDockManifests` only // offers a tile for a plugin that has a frontend — a backend-only plugin has no screen to pin. if (plugin.web && forUserId !== undefined) { @@ -465,6 +475,7 @@ export async function uninstallPlugin(appName: string, onStep?: OnStep, onLog?: // And its notification codes, which stop resolving rather than resolving to something stale. A device // still holding a queued push renders the unknown-code fallback, which is a defined state. if (plugin) await unpublishNotifications(plugin).catch(() => {}); + if (plugin) await unpublishSkills(plugin).catch(() => {}); await step(steps, onStep, 'tables and data: untouched (published assets, affordances and notifications removed)'); return { ok: true, appName, steps }; } diff --git a/src/servers/sidecar/claude/claude-manager.ts b/src/servers/sidecar/claude/claude-manager.ts index 391fab58..38a170b7 100644 --- a/src/servers/sidecar/claude/claude-manager.ts +++ b/src/servers/sidecar/claude/claude-manager.ts @@ -5,6 +5,7 @@ import { join } from 'node:path'; import { query, type Query, type SpawnOptions, type SpawnedProcess } from '@anthropic-ai/claude-agent-sdk'; import type { ChatEvent, PromptImage } from '../../api/chat/types'; import { affordanceBlock } from '../../affordances/load'; +import { skillsBlock } from '../../skills/load'; import type { ClaudeSpawnParams, ClaudeSpawnStreamingParams, ClaudeCodeResult, LiveClaudeSession } from '../protocol'; import { setClaudeSession, clearClaudeSession, getClaudeSession } from './state'; import { countsAsOutput, diagnoseExit, stderrReason } from './exit-diagnosis'; @@ -579,6 +580,11 @@ function createSession( // Read here rather than at module load: a plugin installed an hour ago should be live on the next // message, and this costs a few kilobytes once per session. const affordances = affordanceBlock(); + // The skills INDEX — names, descriptions and paths, never bodies. See skills/load.ts for why this is + // the inverse of an affordance. Read here rather than at module load for the same reason: a skill + // written an hour ago should be offered on the next session. + const skills = skillsBlock(); + const preamble = [affordances, skills].filter(Boolean).join('\n\n') || null; const q = query({ prompt: input.gen as AsyncIterable, @@ -711,8 +717,8 @@ function createSession( * holds on one harness is the drift this exists to remove. Deliberate as of 2026-08-19: opencode * development is paused pending its v2 migration, so the work would be thrown away. See TODO.md. */ - ...(affordances - ? { systemPrompt: { type: 'preset' as const, preset: 'claude_code' as const, append: affordances } } + ...(preamble + ? { systemPrompt: { type: 'preset' as const, preset: 'claude_code' as const, append: preamble } } : {}), env: cleanEnv as Record, stderr: (d: string) => { diff --git a/src/servers/skills/load.test.ts b/src/servers/skills/load.test.ts new file mode 100644 index 00000000..de93bb71 --- /dev/null +++ b/src/servers/skills/load.test.ts @@ -0,0 +1,75 @@ +import { describe, expect, test } from 'bun:test'; +import { mkdtempSync, mkdirSync, writeFileSync, rmSync } from 'node:fs'; +import { tmpdir } from 'node:os'; +import { join } from 'node:path'; +import { loadSkills, compileSkills } from './load'; + +function store(files: Record): string { + const root = mkdtempSync(join(tmpdir(), 'skills-')); + for (const [rel, body] of Object.entries(files)) { + const abs = join(root, rel); + mkdirSync(join(abs, '..'), { recursive: true }); + writeFileSync(abs, body); + } + return root; +} + +const fm = (name: string, description: string, body = 'the whole reference') => + `---\nname: ${name}\ndescription: ${description}\n---\n\n${body}`; + +describe('skills', () => { + test('only the description is compiled — never the body', () => { + // The entire point: a skill is reference that must NOT be charged to a conversation about something + // else. If the body ever reaches the prompt this is just an expensive affordance. + const root = store({ 'email-db/SKILL.md': fm('Email database', 'Use when querying email.', 'SECRET_BODY_TEXT') }); + try { + const block = compileSkills(loadSkills(root))!; + expect(block).toContain('Use when querying email.'); + expect(block).not.toContain('SECRET_BODY_TEXT'); + // The path, because the agent cannot guess it. + expect(block).toContain(join(root, 'email-db', 'SKILL.md')); + } finally { + rmSync(root, { recursive: true, force: true }); + } + }); + + test('a skill with no description is skipped, not listed blank', () => { + // A line that does not say WHEN to reach for something costs context in every conversation and earns + // nothing — worse than an absent entry. + const root = store({ + 'good/SKILL.md': fm('Good', 'Use when X.'), + 'nameless/SKILL.md': '---\nname: Nameless\n---\n\nbody', + }); + try { + expect(loadSkills(root).map((s) => s.id)).toEqual(['good']); + } finally { + rmSync(root, { recursive: true, force: true }); + } + }); + + test('a directory without SKILL.md is not a skill', () => { + const root = store({ 'notaskill/README.md': 'hello' }); + try { + expect(loadSkills(root)).toEqual([]); + } finally { + rmSync(root, { recursive: true, force: true }); + } + }); + + test('nothing to offer compiles to null rather than an empty heading', () => { + expect(compileSkills([])).toBeNull(); + expect(loadSkills(join(tmpdir(), 'definitely-no-skills-here'))).toEqual([]); + }); + + test('order is stable, so the prompt does not churn between sessions', () => { + const root = store({ + 'b/SKILL.md': fm('B', 'Use for B.'), + 'a/SKILL.md': fm('A', 'Use for A.'), + }); + try { + expect(loadSkills(root).map((s) => s.id)).toEqual(['a', 'b']); + } finally { + rmSync(root, { recursive: true, force: true }); + } + }); +}); diff --git a/src/servers/skills/load.ts b/src/servers/skills/load.ts new file mode 100644 index 00000000..5fae2725 --- /dev/null +++ b/src/servers/skills/load.ts @@ -0,0 +1,109 @@ +import { readdirSync, readFileSync, existsSync } from 'node:fs'; +import { join } from 'node:path'; +import { OFFICER_ITEMS_DIR } from '../data-path'; + +// Skills: reference the agent reads WHEN IT NEEDS IT, not on every turn. +// +// ── The inverse of an affordance, on purpose ── +// +// affordance body always compiled in, `description` never sent +// skill `description` always compiled in, body read on demand +// +// An affordance is a fact about the surface, it is short, and every conversation needs it. A skill is +// reference material — a database schema, a tool's conventions — that is dead weight in a conversation +// about something else. So what gets compiled into the prompt is the INDEX: name, one line of when to +// reach for it, and the path. The body costs nothing until the agent decides it is worth opening. +// +// ── Why a path and not a protocol ── +// +// Progressive disclosure is normally a harness feature: Claude Code discovers `.claude/skills/`, opencode +// has its own conventions, the next one will have a third. Pointing at a file needs none of that — every +// harness worth running can already read a file, and the index is just text in the prompt. +// +// The cost is that the agent has to CHOOSE to read it, where a tool definition would be pushed into its +// face. That is what the description is for, and it is why a vague one makes a skill useless rather than +// merely verbose. + +const YAML = (Bun as unknown as { YAML: { parse(input: string): unknown } }).YAML; + +export const SKILLS_DIR = join(OFFICER_ITEMS_DIR, 'skills'); + +export type Skill = { + /** Directory name — `email-db`. Unique within the store. */ + id: string; + name: string; + /** WHEN to reach for this. The only thing the agent sees before deciding to open the body. */ + description: string; + /** Absolute path to SKILL.md. The agent cannot guess it, so the platform supplies it. */ + path: string; +}; + +function frontmatter(raw: string): Record { + const match = raw.match(/^---\n([\s\S]*?)\n---/); + if (!match) return {}; + try { + return (YAML.parse(match[1]!) as Record) ?? {}; + } catch { + return {}; + } +} + +/** + * Every skill in the store, sorted so the compiled block is stable. + * + * A skill with no `description` is SKIPPED rather than listed with a placeholder. The description is the + * entire mechanism — a line that does not say when to reach for something is worse than an absent entry, + * because it costs context in every conversation and earns nothing. + * + * Synchronous, like `loadAffordances`, and for the same reason: this runs once per session. + */ +export function loadSkills(root: string = SKILLS_DIR): Skill[] { + let entries: string[]; + try { + entries = readdirSync(root, { withFileTypes: true }) + .filter((e) => e.isDirectory()) + .map((e) => e.name); + } catch { + return []; // no directory yet is the ordinary state of a fresh install + } + + const found = entries.sort().map((dir): Skill | null => { + const path = join(root, dir, 'SKILL.md'); + if (!existsSync(path)) return null; + try { + const fm = frontmatter(readFileSync(path, 'utf8')); + const description = fm.description == null ? '' : String(fm.description).trim(); + if (!description) return null; + return { id: dir, name: fm.name == null ? dir : String(fm.name), description, path }; + } catch { + return null; + } + }); + + return found.filter((s): s is Skill => s !== null); +} + +/** + * The index, for the system prompt. Null when there is nothing to offer. + * + * Says READ THIS FILE explicitly. An agent that is merely told a skill exists tends to answer from + * memory about a schema it has never seen, which is worse than not knowing — the whole point is to stop + * it guessing at a database it could simply look at. + */ +export function compileSkills(skills: Skill[]): string | null { + if (!skills.length) return null; + return [ + '# Skills on this machine', + '', + 'Reference written for this machine, kept out of your context until you need it. When one of these', + 'covers what you are about to do, READ THE FILE first — it will be more current than anything you', + 'remember, and it exists so you do not have to rediscover what is already written down.', + '', + ...skills.map((s) => `- **${s.name}** — ${s.description}\n Read: \`${s.path}\``), + ].join('\n'); +} + +/** Read and compile in one step, for callers that only want the string. */ +export function skillsBlock(root: string = SKILLS_DIR): string | null { + return compileSkills(loadSkills(root)); +} diff --git a/src/servers/skills/publish.ts b/src/servers/skills/publish.ts new file mode 100644 index 00000000..31c1d5ca --- /dev/null +++ b/src/servers/skills/publish.ts @@ -0,0 +1,52 @@ +import { cp, readdir, rm } from 'node:fs/promises'; +import { existsSync } from 'node:fs'; +import { join } from 'node:path'; +import { SKILLS_DIR } from './load'; +import type { DiscoveredPlugin } from '../plugins/manifest'; + +// Publishing a plugin's skills, the same shape as affordances and notification catalogues. +// +// Deliberately NOT namespaced by publisher, unlike those two. A skill's directory name is what appears in +// the index and what the agent reads a path to, and `officerdev/gitea/SKILL.md` in a prompt is noise for +// a human and for a model. Collisions are handled by the loader taking whichever sorts first, which is +// the same rule the items store has always used — one flat namespace, first come. +// +// That trade is only acceptable because a skill is REFERENCE. An affordance's namespace matters because a +// plugin must not be able to claim `officer/core:`; a skill has no authority to usurp. + +export const skillDirFor = (plugin: DiscoveredPlugin): string => join(SKILLS_DIR, plugin.appName); + +/** What the plugin ships, if anything. */ +export const skillSourceFor = (plugin: DiscoveredPlugin): string => join(plugin.dir, 'skills'); + +/** Publish a plugin's skills. Returns the number of directories, or 0 when it ships none. */ +export async function publishSkills(plugin: DiscoveredPlugin): Promise { + const source = skillSourceFor(plugin); + if (!existsSync(source)) return 0; + + await cp(source, SKILLS_DIR, { recursive: true, force: true }); + + try { + return (await readdir(source, { withFileTypes: true })).filter((e) => e.isDirectory()).length; + } catch { + return 0; + } +} + +/** + * Drop a plugin's skills. + * + * Removes only the directories THIS plugin shipped, read from its own source tree — not everything under + * `skills/`, which holds the platform's own and every other plugin's. + */ +export async function unpublishSkills(plugin: DiscoveredPlugin): Promise { + const source = skillSourceFor(plugin); + if (!existsSync(source)) return; + let dirs: string[]; + try { + dirs = (await readdir(source, { withFileTypes: true })).filter((e) => e.isDirectory()).map((e) => e.name); + } catch { + return; + } + for (const dir of dirs) await rm(join(SKILLS_DIR, dir), { recursive: true, force: true }); +}