capabilities: read a plugin's where they live, stop copying them in
Four publishers copied a plugin's affordances, notifications and skills into capabilities/ at install. That directory is meant to be forked — core's work plus the owner's — so a fork accumulated files it did not author with nothing on disk saying which, and there was no good answer: commit them and carry another plugin's work, leave them and git status stays dirty. Copying was also wrong in two ways unrelated to git. A copy goes stale, because plugins:update pulls the plugin's directory and the copy keeps describing the old version. And a copy survived plugins:disable, so a disabled plugin's skill stayed in the agent's index — off, and still being announced. Now the loaders take a list of roots: the owner's store, plus each installed AND enabled plugin's own directory. capability-sources.ts resolves that list. No namespace is needed to keep them apart, because separate directories do it structurally rather than by convention. The owner's own win a skill name collision — their root is first and the merge keeps the first of each id — so a plugin cannot take over a name already in use. Affordances are deliberately NOT de-duplicated: several can be true at once. Notifications needed more than a root list, and it would have failed silently. Their codes were derived from the COPY'S PATH, <publisher>/<appName>/file.yaml. Read in place the path is just notifications/catalogue.yaml and carries no identity, so every plugin code would have vanished with nothing raised. loadPluginEntries takes the identity from the manifest instead, which is better provenance anyway — publisher and appName are validated at discovery, where a path was only ever a directory somebody chose. A yaml key still cannot claim another namespace, and that is pinned by test. createSession is async now, only because resolving enabled plugins reads the database. Once per session; its single caller already awaited. Deletes all four publishers including api/tasks/publish.ts from earlier today, superseded before it shipped. app-store/assets stays — images served over HTTP, where the copy is what makes them reachable. NOT done: tasks. That store is read-write, so reads must span sources while writes stay in the owner's directory, and a plugin's task has to be read-only in the UI. Next. Two things the design predicted as orphans turned out not to be, and both were checked before deleting rather than after. capabilities/extensions/tool-loader is written by the platform's own bootstrap every boot, not published by a plugin. And skills/email-db has no source to fall back on — the email plugin ships no skills directory, so that file exists in exactly one place and was hand-written. Neither was touched. 1422 pass, tsgo clean, officer restarts and serves. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Officer-Session: 656f5c85-e49a-4f1f-b9f3-042a04b4d043
This commit is contained in:
@@ -1,6 +1,7 @@
|
||||
import { readdirSync, readFileSync, existsSync } from 'node:fs';
|
||||
import { join } from 'node:path';
|
||||
import { OFFICER_ITEMS_DIR } from '../data-path';
|
||||
import { capabilityRoots } from '../capability-sources';
|
||||
|
||||
// Skills: reference the agent reads WHEN IT NEEDS IT, not on every turn.
|
||||
//
|
||||
@@ -104,6 +105,31 @@ export function compileSkills(skills: Skill[]): string | null {
|
||||
}
|
||||
|
||||
/** 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));
|
||||
/**
|
||||
* Every skill on offer: the owner's own, plus each installed and enabled plugin's own directory.
|
||||
*
|
||||
* A plugin's skills are read WHERE THEY LIVE rather than copied into `capabilities/` — see
|
||||
* `capability-sources.ts` for why, and `docs/plugin-capabilities.md` for the whole argument.
|
||||
*
|
||||
* **The owner's own win a name collision**, because `capabilityRoots` puts their directory first and this
|
||||
* keeps the first of each id. That is the right precedence: a skill somebody wrote deliberately should
|
||||
* not be displaced by a plugin that happened to pick the same directory name, and a plugin cannot
|
||||
* silently take over the meaning of a name already in use.
|
||||
*/
|
||||
export function loadSkillsAcross(roots: string[]): Skill[] {
|
||||
const byId = new Map<string, Skill>();
|
||||
for (const root of roots) {
|
||||
for (const skill of loadSkills(root)) if (!byId.has(skill.id)) byId.set(skill.id, skill);
|
||||
}
|
||||
return [...byId.values()].sort((a, b) => a.id.localeCompare(b.id));
|
||||
}
|
||||
|
||||
/**
|
||||
* Read and compile in one step, for callers that only want the string.
|
||||
*
|
||||
* Async because resolving which plugins are enabled reads the database. Called once per session, which is
|
||||
* what makes that affordable — see the note in `capability-sources.ts`.
|
||||
*/
|
||||
export async function skillsBlock(): Promise<string | null> {
|
||||
return compileSkills(loadSkillsAcross(await capabilityRoots('skills')));
|
||||
}
|
||||
|
||||
@@ -1,52 +0,0 @@
|
||||
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<number> {
|
||||
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<void> {
|
||||
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 });
|
||||
}
|
||||
Reference in New Issue
Block a user