diff --git a/src/servers/api/tasks/publish.test.ts b/src/servers/api/tasks/publish.test.ts new file mode 100644 index 00000000..b8bddc76 --- /dev/null +++ b/src/servers/api/tasks/publish.test.ts @@ -0,0 +1,97 @@ +import { afterEach, describe, expect, test } from 'bun:test'; +import { mkdirSync, mkdtempSync, rmSync, writeFileSync, existsSync, readFileSync } from 'node:fs'; +import { tmpdir } from 'node:os'; +import { join } from 'node:path'; +import { itemsDir } from '@@/data-path'; +import { publishTasks, taskSourceFor, unpublishTasks } from './publish'; +import type { DiscoveredPlugin } from '../../plugins/manifest'; + +/** + * A plugin shipping a task is how it reaches the file browser's context menu, so these are the two + * properties that matter: it arrives on install, and uninstalling takes back only what the plugin brought. + * + * The second is the one worth a test. `tasks/` is the OWNER'S directory — their own work lives beside + * whatever a plugin published, with nothing on disk marking which is which. A `rm -rf` of the wrong scope + * here destroys work no plugin ever created, which is not a failure a type checker can catch. + * + * Everything is prefixed `zz-publish-test-` and removed afterwards, because these write into the real + * items store rather than a fixture — `itemsDir` is derived from the install layout and cannot be pointed + * elsewhere for a test. + */ + +const PREFIX = 'zz-publish-test-'; +const made: string[] = []; + +const plugin = (dir: string): DiscoveredPlugin => ({ appName: `${PREFIX}plugin`, dir }) as DiscoveredPlugin; + +const seedPluginWithTasks = (names: string[]): string => { + const dir = mkdtempSync(join(tmpdir(), 'zz-plugin-')); + made.push(dir); + for (const name of names) { + const t = join(dir, 'tasks', name); + mkdirSync(t, { recursive: true }); + writeFileSync(join(t, 'TASK.md'), `---\nname: ${name}\n---\n`); + } + return dir; +}; + +afterEach(() => { + for (const d of made.splice(0)) rmSync(d, { recursive: true, force: true }); + // Anything this file could have written into the real store, by name. + for (const n of [`${PREFIX}one`, `${PREFIX}two`, `${PREFIX}owners-own`]) { + rmSync(join(itemsDir('tasks'), n), { recursive: true, force: true }); + } +}); + +describe('publishTasks', () => { + test('a plugin with no tasks directory publishes nothing and does not fail', async () => { + const dir = mkdtempSync(join(tmpdir(), 'zz-plugin-')); + made.push(dir); + expect(await publishTasks(plugin(dir))).toBe(0); + }); + + test('tasks arrive in the items store, contents and all', async () => { + const dir = seedPluginWithTasks([`${PREFIX}one`, `${PREFIX}two`]); + expect(await publishTasks(plugin(dir))).toBe(2); + + const landed = join(itemsDir('tasks'), `${PREFIX}one`, 'TASK.md'); + expect(existsSync(landed)).toBe(true); + expect(readFileSync(landed, 'utf8')).toContain(`name: ${PREFIX}one`); + }); + + test('publishing twice overwrites rather than failing', async () => { + const dir = seedPluginWithTasks([`${PREFIX}one`]); + await publishTasks(plugin(dir)); + expect(await publishTasks(plugin(dir))).toBe(1); + }); +}); + +describe('unpublishTasks leaves the owner alone', () => { + // The whole reason this file exists. + test("removes only what the plugin shipped, never the owner's own task", async () => { + const ownersOwn = join(itemsDir('tasks'), `${PREFIX}owners-own`); + mkdirSync(ownersOwn, { recursive: true }); + writeFileSync(join(ownersOwn, 'TASK.md'), '---\nname: hand written\n---\n'); + + const dir = seedPluginWithTasks([`${PREFIX}one`]); + await publishTasks(plugin(dir)); + await unpublishTasks(plugin(dir)); + + expect(existsSync(join(itemsDir('tasks'), `${PREFIX}one`))).toBe(false); + expect(existsSync(join(ownersOwn, 'TASK.md'))).toBe(true); + }); + + test('a plugin whose source is already gone removes nothing', async () => { + const ownersOwn = join(itemsDir('tasks'), `${PREFIX}owners-own`); + mkdirSync(ownersOwn, { recursive: true }); + + // The directory the plugin was cloned into no longer exists — deleted before uninstall ran. + await unpublishTasks(plugin(join(tmpdir(), 'zz-plugin-does-not-exist'))); + + expect(existsSync(ownersOwn)).toBe(true); + }); + + test('the source path is the plugin directory, not the store', () => { + expect(taskSourceFor(plugin('/somewhere/plugins/x'))).toBe('/somewhere/plugins/x/tasks'); + }); +}); diff --git a/src/servers/api/tasks/publish.ts b/src/servers/api/tasks/publish.ts new file mode 100644 index 00000000..bd9c7a18 --- /dev/null +++ b/src/servers/api/tasks/publish.ts @@ -0,0 +1,73 @@ +import { cp, readdir, rm } from 'node:fs/promises'; +import { existsSync } from 'node:fs'; +import { join } from 'node:path'; +import { itemsDir } from '@@/data-path'; +import type { DiscoveredPlugin } from '../../plugins/manifest'; + +// Publishing a plugin's tasks, the fourth of these and the same shape as assets, affordances, +// notification catalogues and skills. +// +// ── Why a plugin needs to ship a task at all ── +// +// A task is how something reaches the FILE BROWSER'S CONTEXT MENU. `TriggerConfig` is +// `{ type: 'file'; extensions: string[] } | { type: 'directory' }`, and `matchesTrigger` in +// `officerdev/src/apps/FileBrowser/useTasks.ts` is what decides which entries appear on a right-click. So +// a plugin that wants to offer "transcribe this" on an mp3 does not need a platform change, a menu +// registry or a new frontend hook — it needs a task with the right trigger, and that already works. +// +// What was missing was only the delivery: every other publishable thing had a publisher and tasks did +// not, so the capability had to be installed by hand alongside the plugin and removed by hand after. +// +// ── Flat, not namespaced by publisher ── +// +// Same trade as skills, for the same reason and with the same limit. A task's directory name is its +// identity everywhere — `taskDirName` in a job row, the path a script runner materialises, the thing the +// owner sees in a run history. Wrapping that in `officerdev/whisper/` would put a publisher in every one +// of those places for the benefit of a collision nobody has had. Collisions resolve first-come, which is +// the rule the items store has always used. +// +// That is acceptable because a task claims no authority — it is an entry in a menu, gated by the same +// permission the tasks surface already carries. It is NOT acceptable for affordances, which is why those +// are namespaced: an affordance that could claim `officer/core:` would be claiming the platform's voice. + +const tasksDir = () => itemsDir('tasks'); + +/** What the plugin ships, if anything. */ +export const taskSourceFor = (plugin: DiscoveredPlugin): string => join(plugin.dir, 'tasks'); + +/** Publish a plugin's tasks. Returns the number of directories, or 0 when it ships none. */ +export async function publishTasks(plugin: DiscoveredPlugin): Promise { + const source = taskSourceFor(plugin); + if (!existsSync(source)) return 0; + + await cp(source, tasksDir(), { recursive: true, force: true }); + + try { + return (await readdir(source, { withFileTypes: true })).filter((e) => e.isDirectory()).length; + } catch { + return 0; + } +} + +/** + * Drop a plugin's tasks. + * + * Removes only the directories THIS plugin shipped, read from its own source tree — never everything + * under `tasks/`, which holds the owner's own work. A task the owner wrote by hand has no marker saying + * so, so the plugin's source tree is the only honest record of what came from the plugin. + * + * The consequence is worth stating: if the owner EDITS a task a plugin shipped, uninstalling still + * removes it, because it is still one of the names the plugin published. That matches skills and is the + * behaviour to keep — the alternative is orphans nobody can attribute. + */ +export async function unpublishTasks(plugin: DiscoveredPlugin): Promise { + const source = taskSourceFor(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(tasksDir(), dir), { recursive: true, force: true }); +} diff --git a/src/servers/plugins/install.ts b/src/servers/plugins/install.ts index 63f4d7cd..5e297454 100644 --- a/src/servers/plugins/install.ts +++ b/src/servers/plugins/install.ts @@ -20,6 +20,7 @@ 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 { publishTasks, unpublishTasks } from '../api/tasks/publish'; import { refreshPluginMounts, snapshotPlugins } from './mount'; import { publishExtension, unpublishExtension, extensionFileName } from './zip'; import { generatePluginSchemas, pushPluginSchema } from './schema'; @@ -438,6 +439,17 @@ export async function installPlugin( await step(steps, onStep, `skills: NOT published — ${err instanceof Error ? err.message : err}`); } + // And the same for tasks, which is how a plugin reaches the FILE BROWSER'S CONTEXT MENU. A task with a + // file trigger appears on a right-click with no platform change at all — `matchesTrigger` in + // officerdev/src/apps/FileBrowser/useTasks.ts already does that work. What was missing was only the + // delivery, so the capability had to be installed by hand beside the plugin and removed by hand after. + try { + const published = await publishTasks(plugin); + if (published) await step(steps, onStep, `tasks: ${published} published to capabilities/tasks/`); + } catch (err) { + await step(steps, onStep, `tasks: 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) { @@ -540,7 +552,8 @@ export async function uninstallPlugin(appName: string, onStep?: OnStep, onLog?: // 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)'); + if (plugin) await unpublishTasks(plugin).catch(() => {}); + await step(steps, onStep, 'tables and data: untouched (published assets, affordances, notifications, skills and tasks removed)'); return { ok: true, appName, steps }; }