plugins: a plugin's tasks are read where they live
the last piece of docs/plugin-capabilities.md. skills, affordances and notification catalogues already loaded from each installed and enabled plugin's own directory; tasks did not, so the transcribe plugin shipped a task nothing could see — right-clicking an .m4a offered no Run Task at all. it turned out to be one function rather than several. every path that runs a task resolves it through getTaskByDirName and then reads implementation off the record, so the sources go in there and in listTasks and a plugin's task is runnable everywhere at once, identity unchanged: still a directory name, still what pipeline_jobs stores. the record carries `plugin` and that one field does the rest. updateTask and deleteTask throw on it, DELETE /api/tasks/:name answers 409 naming the plugin, and the capability screen shows whose it is instead of an edit and a delete that cannot work. without the delete guard it was quietly a no-op: rm -rf on a path in the owner's store that does not exist, then ok: true, and the task still in the menu. owner's store first, so a plugin cannot take over a name already in use. getTaskByDirName now refuses a name containing a separator. it always joined a caller-supplied string onto a path — from a websocket message and a job row — and searching several roots is the moment that stops being one directory nobody could escape. capabilityRootsFrom is gone; it was dead. capabilitySources replaces it and carries the provenance, with capabilityRoots the thin map over it that the loaders that only want paths still use. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Officer-Session: 656f5c85-e49a-4f1f-b9f3-042a04b4d043
This commit is contained in:
Claude Opus 5
@@ -0,0 +1,100 @@
|
||||
import { describe, expect, test } from 'bun:test';
|
||||
import { mkdirSync, mkdtempSync, rmSync, writeFileSync } from 'node:fs';
|
||||
import { tmpdir } from 'node:os';
|
||||
import { join } from 'node:path';
|
||||
import { listTasks, getTaskByDirName } from './task-files';
|
||||
|
||||
/**
|
||||
* A plugin's tasks are read where they live (docs/plugin-capabilities.md), and the whole reason it lands
|
||||
* in `task-files.ts` is that every runner resolves a task through these two functions. So these pin the
|
||||
* properties the file browser's context menu depends on — a plugin's task is listed, is resolvable by the
|
||||
* same directory name a menu click sends back, and carries its implementation.
|
||||
*
|
||||
* Sources are injected: resolving them for real reads the database and the plugin directory, which would
|
||||
* make this assert whatever this machine happens to have installed.
|
||||
*/
|
||||
|
||||
const seed = (files: Record<string, string>): string => {
|
||||
const dir = mkdtempSync(join(tmpdir(), 'zz-tasks-'));
|
||||
for (const [rel, body] of Object.entries(files)) {
|
||||
const abs = join(dir, rel);
|
||||
mkdirSync(join(abs, '..'), { recursive: true });
|
||||
writeFileSync(abs, body);
|
||||
}
|
||||
return dir;
|
||||
};
|
||||
|
||||
const taskMd = (name: string, extra = '') =>
|
||||
`---\nname: ${name}\nmode: script\nlanguage: typescript\n${extra}---\n\nbody\n`;
|
||||
|
||||
describe('tasks across sources', () => {
|
||||
test("a plugin's task is listed and carries its provenance", async () => {
|
||||
const owner = seed({ 'mine/TASK.md': taskMd('Mine') });
|
||||
const plugin = seed({ 'transcribe-audio-file/TASK.md': taskMd('Transcribe Audio File') });
|
||||
try {
|
||||
const found = await listTasks([
|
||||
{ dir: owner, plugin: null },
|
||||
{ dir: plugin, plugin: 'transcribe' },
|
||||
]);
|
||||
expect(found.map((t) => [t.dirName, t.plugin])).toEqual([
|
||||
['mine', null],
|
||||
['transcribe-audio-file', 'transcribe'],
|
||||
]);
|
||||
} finally {
|
||||
rmSync(owner, { recursive: true, force: true });
|
||||
rmSync(plugin, { recursive: true, force: true });
|
||||
}
|
||||
});
|
||||
|
||||
// The context menu sends back a directory name and nothing else, so resolution has to find the plugin's
|
||||
// copy by that name alone — otherwise the task is listed and then "not found" the moment it is clicked.
|
||||
test("a plugin's task resolves by directory name, with its implementation", async () => {
|
||||
const owner = seed({ 'mine/TASK.md': taskMd('Mine') });
|
||||
const plugin = seed({
|
||||
'transcribe-audio-file/TASK.md': taskMd('Transcribe Audio File'),
|
||||
'transcribe-audio-file/index.ts': 'export const ok = true;\n',
|
||||
});
|
||||
try {
|
||||
const sources = [
|
||||
{ dir: owner, plugin: null },
|
||||
{ dir: plugin, plugin: 'transcribe' },
|
||||
];
|
||||
const task = await getTaskByDirName('transcribe-audio-file', sources);
|
||||
expect(task?.plugin).toBe('transcribe');
|
||||
expect(task?.implementation).toBe('export const ok = true;\n');
|
||||
} finally {
|
||||
rmSync(owner, { recursive: true, force: true });
|
||||
rmSync(plugin, { recursive: true, force: true });
|
||||
}
|
||||
});
|
||||
|
||||
// Same precedence as skills: a plugin cannot take over a name its owner is already using.
|
||||
test("the owner's own wins a name collision", async () => {
|
||||
const owner = seed({ 'deploy/TASK.md': taskMd('Deploy (owner)') });
|
||||
const plugin = seed({ 'deploy/TASK.md': taskMd('Deploy (plugin)') });
|
||||
try {
|
||||
const sources = [
|
||||
{ dir: owner, plugin: null },
|
||||
{ dir: plugin, plugin: 'somebody' },
|
||||
];
|
||||
expect(await listTasks(sources)).toHaveLength(1);
|
||||
const task = await getTaskByDirName('deploy', sources);
|
||||
expect(task?.name).toBe('Deploy (owner)');
|
||||
expect(task?.plugin).toBeNull();
|
||||
} finally {
|
||||
rmSync(owner, { recursive: true, force: true });
|
||||
rmSync(plugin, { recursive: true, force: true });
|
||||
}
|
||||
});
|
||||
|
||||
// A directory name arrives from a websocket message and a job row before it is joined onto a path.
|
||||
test('a traversing directory name resolves to nothing', async () => {
|
||||
const owner = seed({ 'mine/TASK.md': taskMd('Mine') });
|
||||
try {
|
||||
const sources = [{ dir: join(owner, 'nested'), plugin: null }];
|
||||
expect(await getTaskByDirName('../mine', sources)).toBeNull();
|
||||
} finally {
|
||||
rmSync(owner, { recursive: true, force: true });
|
||||
}
|
||||
});
|
||||
});
|
||||
@@ -1,10 +1,28 @@
|
||||
import { readdir, mkdir, rm } from 'node:fs/promises';
|
||||
import { join, dirname } from 'node:path';
|
||||
import { itemsDir, OFFICER_ITEMS_DIR } from '../../data-path';
|
||||
import { capabilitySources, type CapabilitySource } from '../../capability-sources';
|
||||
|
||||
// File-backed task store. Every task is a directory under $OFFICER_ITEMS_DIR/tasks/<dirName>/ with a
|
||||
// TASK.md (metadata + prose body) and, for script-mode tasks, a sibling implementation file. There are
|
||||
// no scope tiers and no database — the directory name is the task's identity.
|
||||
//
|
||||
// ── Plus every enabled plugin's own tasks/ directory ──
|
||||
//
|
||||
// Read where they live, never copied in — the same rule as skills and affordances, and the same reasons
|
||||
// (`capability-sources.ts`, `docs/plugin-capabilities.md`). It lands here rather than in each caller
|
||||
// because EVERY execution path goes through `getTaskByDirName`: the context menu, the runner sockets,
|
||||
// the job queue and the pipeline executor all resolve a task by its directory name and then read
|
||||
// `implementation` off the record. Teaching this one function about plugin sources makes a plugin's task
|
||||
// runnable everywhere at once, and leaves the identity — the directory name — exactly as it was.
|
||||
//
|
||||
// The owner's own store is searched FIRST, so a name the owner is already using cannot be taken over by
|
||||
// a plugin that happened to pick the same directory name.
|
||||
//
|
||||
// Writing is a different matter: create, update and delete only ever touch the owner's store, and refuse
|
||||
// a task that belongs to a plugin. A plugin's task lives in a git checkout that `plugins:update` pulls —
|
||||
// editing it in place would be overwritten by the next update, and deleting it would make the plugin's
|
||||
// own files disappear from a screen that says nothing about plugins.
|
||||
|
||||
const YAML = (Bun as unknown as { YAML: { parse(input: string): unknown } }).YAML;
|
||||
|
||||
@@ -34,6 +52,8 @@ export type TaskRecord = TaskFrontmatter & {
|
||||
body: string;
|
||||
implementation: string | null;
|
||||
filePath: string;
|
||||
/** The plugin that ships it, or null when it is the owner's own. Read-only when set. */
|
||||
plugin: string | null;
|
||||
};
|
||||
|
||||
export type TaskSummary = {
|
||||
@@ -45,6 +65,7 @@ export type TaskSummary = {
|
||||
inline: boolean | 'ask';
|
||||
version: number;
|
||||
trigger: unknown;
|
||||
plugin: string | null;
|
||||
};
|
||||
|
||||
// Frontmatter keys, in write order. These mirror the old `tasks` table columns 1:1.
|
||||
@@ -145,9 +166,8 @@ function buildTaskMd(fm: Partial<TaskFrontmatter>, body: string): string {
|
||||
return lines.join('\n');
|
||||
}
|
||||
|
||||
async function readImplementation(dirName: string, language: string | null): Promise<string | null> {
|
||||
const implPath = join(taskDir(dirName), implFileName(language));
|
||||
const file = Bun.file(implPath);
|
||||
async function readImplementation(dir: string, language: string | null): Promise<string | null> {
|
||||
const file = Bun.file(join(dir, implFileName(language)));
|
||||
return (await file.exists()) ? file.text() : null;
|
||||
}
|
||||
|
||||
@@ -170,42 +190,63 @@ export async function readCategoryOrder(): Promise<string[]> {
|
||||
}
|
||||
}
|
||||
|
||||
export async function listTasks(): Promise<TaskSummary[]> {
|
||||
let entries;
|
||||
try {
|
||||
entries = await readdir(tasksRoot(), { withFileTypes: true });
|
||||
} catch {
|
||||
return [];
|
||||
/**
|
||||
* Every task on offer, the owner's own first.
|
||||
*
|
||||
* `sources` is injectable so this can be tested against temporary directories — resolving them for real
|
||||
* reads the database and the plugin directory, which would make a test assert whatever this machine
|
||||
* happens to have installed.
|
||||
*/
|
||||
export async function listTasks(sources?: CapabilitySource[]): Promise<TaskSummary[]> {
|
||||
const roots = sources ?? (await capabilitySources('tasks'));
|
||||
|
||||
const byDirName = new Map<string, TaskSummary>();
|
||||
for (const source of roots) {
|
||||
let entries;
|
||||
try {
|
||||
entries = await readdir(source.dir, { withFileTypes: true });
|
||||
} catch {
|
||||
continue; // most plugins ship no tasks at all — the ordinary case, not a fault
|
||||
}
|
||||
|
||||
for (const entry of entries) {
|
||||
if (!entry.isDirectory() || byDirName.has(entry.name)) continue;
|
||||
const file = Bun.file(join(source.dir, entry.name, 'TASK.md'));
|
||||
if (!(await file.exists())) continue;
|
||||
const { fm } = parseTaskMd(await file.text());
|
||||
byDirName.set(entry.name, {
|
||||
dirName: entry.name,
|
||||
name: fm.name || entry.name,
|
||||
description: fm.description,
|
||||
category: fm.category,
|
||||
mode: fm.mode,
|
||||
inline: fm.inline,
|
||||
version: fm.version,
|
||||
trigger: fm.trigger ?? [],
|
||||
plugin: source.plugin,
|
||||
});
|
||||
}
|
||||
}
|
||||
|
||||
const summaries: TaskSummary[] = [];
|
||||
for (const entry of entries) {
|
||||
if (!entry.isDirectory()) continue;
|
||||
const file = Bun.file(taskFile(entry.name));
|
||||
if (!(await file.exists())) continue;
|
||||
const { fm } = parseTaskMd(await file.text());
|
||||
summaries.push({
|
||||
dirName: entry.name,
|
||||
name: fm.name || entry.name,
|
||||
description: fm.description,
|
||||
category: fm.category,
|
||||
mode: fm.mode,
|
||||
inline: fm.inline,
|
||||
version: fm.version,
|
||||
trigger: fm.trigger ?? [],
|
||||
});
|
||||
}
|
||||
|
||||
summaries.sort((a, b) => a.name.localeCompare(b.name));
|
||||
return summaries;
|
||||
return [...byDirName.values()].sort((a, b) => a.name.localeCompare(b.name));
|
||||
}
|
||||
|
||||
export async function getTaskByDirName(dirName: string): Promise<TaskRecord | null> {
|
||||
const file = Bun.file(taskFile(dirName));
|
||||
if (!(await file.exists())) return null;
|
||||
const { fm, body } = parseTaskMd(await file.text());
|
||||
const implementation = await readImplementation(dirName, fm.language);
|
||||
return { ...fm, dirName, body, implementation, filePath: taskFile(dirName) };
|
||||
export async function getTaskByDirName(dirName: string, sources?: CapabilitySource[]): Promise<TaskRecord | null> {
|
||||
// A directory name reaches this from a websocket message and a job row, so it is caller-supplied and
|
||||
// gets joined onto a path. Traversal would otherwise read any TASK.md on the disk.
|
||||
if (!dirName || dirName.includes('/') || dirName.includes('\\') || dirName === '.' || dirName === '..') return null;
|
||||
|
||||
const roots = sources ?? (await capabilitySources('tasks'));
|
||||
for (const source of roots) {
|
||||
const dir = join(source.dir, dirName);
|
||||
const filePath = join(dir, 'TASK.md');
|
||||
const file = Bun.file(filePath);
|
||||
if (!(await file.exists())) continue;
|
||||
const { fm, body } = parseTaskMd(await file.text());
|
||||
const implementation = await readImplementation(dir, fm.language);
|
||||
return { ...fm, dirName, body, implementation, filePath, plugin: source.plugin };
|
||||
}
|
||||
return null;
|
||||
}
|
||||
|
||||
type CreateTaskInput = {
|
||||
@@ -240,6 +281,7 @@ export async function createTask(input: CreateTaskInput): Promise<TaskSummary &
|
||||
inline: false,
|
||||
version: 1,
|
||||
trigger: [],
|
||||
plugin: null,
|
||||
filePath: taskFile(dirName),
|
||||
};
|
||||
}
|
||||
@@ -250,6 +292,7 @@ export async function updateTask(
|
||||
): Promise<TaskRecord | null> {
|
||||
const existing = await getTaskByDirName(dirName);
|
||||
if (!existing) return null;
|
||||
if (existing.plugin) throw new Error(`${dirName} belongs to the ${existing.plugin} plugin and is read-only`);
|
||||
|
||||
const merged: TaskFrontmatter = { ...existing, ...patch };
|
||||
const body = patch.body ?? existing.body;
|
||||
@@ -262,7 +305,15 @@ export async function updateTask(
|
||||
return getTaskByDirName(dirName);
|
||||
}
|
||||
|
||||
/**
|
||||
* Only ever the owner's own copy.
|
||||
*
|
||||
* Without the guard this was quietly a no-op on a plugin's task — `rm -rf` on a path in the owner's store
|
||||
* that does not exist, followed by a cheerful `{ ok: true }` and a task still in the menu.
|
||||
*/
|
||||
export async function deleteTask(dirName: string): Promise<void> {
|
||||
const existing = await getTaskByDirName(dirName);
|
||||
if (existing?.plugin) throw new Error(`${dirName} belongs to the ${existing.plugin} plugin and cannot be deleted`);
|
||||
await rm(taskDir(dirName), { recursive: true, force: true });
|
||||
}
|
||||
|
||||
|
||||
@@ -15,6 +15,7 @@ tasksRouter.get('/', async (ctx) => {
|
||||
category: row.category,
|
||||
triggers: (row.trigger as TriggerConfig[]) ?? [],
|
||||
mode: row.mode ?? 'agentic',
|
||||
plugin: row.plugin,
|
||||
}));
|
||||
|
||||
return ctx.json(tasks);
|
||||
@@ -47,6 +48,7 @@ tasksRouter.get('/:name', async (ctx) => {
|
||||
config: task.config,
|
||||
version: task.version,
|
||||
filePath: task.filePath,
|
||||
plugin: task.plugin,
|
||||
});
|
||||
});
|
||||
|
||||
@@ -74,6 +76,8 @@ tasksRouter.delete('/:name', async (ctx) => {
|
||||
|
||||
const task = await getTaskByDirName(name);
|
||||
if (!task) return ctx.text('Not found', 404);
|
||||
// A plugin's task is its own file in its own checkout. Uninstalling the plugin is how it goes away.
|
||||
if (task.plugin) return ctx.text(`${name} belongs to the ${task.plugin} plugin — uninstall it instead`, 409);
|
||||
|
||||
await deleteTask(task.dirName);
|
||||
return ctx.json({ ok: true });
|
||||
|
||||
Reference in New Issue
Block a user