npmDependencies in the manifest, installed once for every plugin

replaces the per-plugin package.json from earlier tonight. the owner's call,
and the reason is one i had not weighed: sharp is ~50 MB of platform binaries
and the obvious dependency for anything touching images, so a second and third
plugin wanting it would have meant a second and third copy with no version they
could converge on.

so every plugin's `npmDependencies` merge into ONE generated plugins/package.json,
installed into plugins/node_modules/. bun resolves node_modules up the tree, so
that directory serves every plugin and none of the platform's own code.

ranges intersect rather than alternate. two plugins asking for ^0.33.5 and
^0.33.9 produce "^0.33.5 ^0.33.9", which npm and bun read as AND — the resolved
version must satisfy both. `||` would have meant "either", which is the silent
version of this failure: one plugin quietly getting a version the other cannot
import. incompatible requirements now fail the install with bun's own message,
and both askers are named in the steps either way.

generated from the plugins ON DISK, like generatePluginSchemas, and never
edited — so a package cannot appear there without a plugin having declared it.

what this gives up, knowingly: a committed lockfile. ranges are the whole point
and a range resolves per machine, so plugin packages are resolved at install
time like osDependencies are, and unlike the platform's own. CLAUDE.md is
emphatic about frozen lockfiles and 2026-08-04, so it is named in the manifest
type rather than left to be discovered.

the generated plugins/package.json and lockfile are gitignored — /plugins/*/
only ignores directories, so files at that level needed naming explicitly.
This commit is contained in:
2026-08-20 18:22:58 +00:00
parent 3450f5eb57
commit 21bedf1418
6 changed files with 242 additions and 166 deletions
+7
View File
@@ -105,6 +105,13 @@ src/databases/officer_db/migrations/
# to the platform; the plugins do not. # to the platform; the plugins do not.
/plugins/*/ /plugins/*/
# Generated by src/servers/plugins/npm-deps.ts from every plugin's `npmDependencies`, and installed into
# plugins/node_modules/. It describes whichever plugins are on THIS machine, so committing one would
# claim a dependency set nobody chose. The rule above only ignores directories, so these need naming.
/plugins/package.json
/plugins/bun.lock
/plugins/bun.lockb
# Visual baselines for the theme extraction — large, machine-specific, regenerate with scripts/theme-snapshot.ts # Visual baselines for the theme extraction — large, machine-specific, regenerate with scripts/theme-snapshot.ts
.snapshots/ .snapshots/
+27 -23
View File
@@ -24,7 +24,7 @@ import { generatePluginSchemas, pushPluginSchema } from './schema';
import { declaredTables, dropTables, existingTables } from './tables'; import { declaredTables, dropTables, existingTables } from './tables';
import type { OnLog } from './run-logged'; import type { OnLog } from './run-logged';
import { installDependencies, installRelease, manualInstallHint, reportDependencies, stillMissing } from './os-deps'; import { installDependencies, installRelease, manualInstallHint, reportDependencies, stillMissing } from './os-deps';
import { installNpmDependencies, npmManifestFor, npmWouldBlockUpdates, verifyNpmDependencies } from './npm-deps'; import { anyPluginWantsPackages, installNpmDependencies, unresolvableDependencies } from './npm-deps';
import { mountPrefix, type DiscoveredPlugin } from './manifest'; import { mountPrefix, type DiscoveredPlugin } from './manifest';
// The install runner: the four verbs, each as a short ordered list of effects. // The install runner: the four verbs, each as a short ordered list of effects.
@@ -269,23 +269,39 @@ export async function installPlugin(
await step(steps, onStep, `dependencies: ${deps.dependencies.map((d) => d.binary).join(', ')} already present`); await step(steps, onStep, `dependencies: ${deps.dependencies.map((d) => d.binary).join(', ')} already present`);
} }
// ── npm packages, in the plugin's own directory ── // ── npm packages, shared across every plugin ──
// //
// Beside the OS dependencies above and verified the same way, because the same failure is available: // Beside the OS dependencies above and verified the same way, because the same failure is available:
// an install that exits 0 and still leaves nothing the sidecar can import. Atelier hit exactly that // an install that exits 0 and still leaves nothing the sidecar can import. Atelier hit exactly that on
// on 2026-08-20 — `sharp` in the PLATFORM's package.json, alpha never told to `bun install`, and the // 2026-08-20 — `sharp` in the PLATFORM's package.json, alpha never told to `bun install`, and the
// sidecar crash-looped on a plugin that had reported a clean install. // sidecar crash-looped on a plugin that had reported a clean install.
if (npmManifestFor(plugin)) { //
await step(steps, onStep, "packages: installing from the plugin's own package.json"); // Generated from every plugin ON DISK rather than from this one, so a shared `sharp` is installed once
const npm = await installNpmDependencies(plugin); // and two plugins wanting it converge instead of carrying a copy each.
{
const { plugins: onDisk } = await discoverPlugins();
if (anyPluginWantsPackages(onDisk)) {
await step(steps, onStep, 'packages: resolving npm dependencies declared by plugins');
const npm = await installNpmDependencies(onDisk);
for (const [name, askers] of Object.entries(npm.conflicts)) {
// Named, not silently intersected. Two plugins disagreeing about a version is a thing their
// authors need to know, and the install either resolves it or fails on the next line.
await step(
steps,
onStep,
`packages: ${name} requested as ${askers.map((a) => `${a.range} (${a.appName})`).join(', ')} — intersected`,
);
}
if (!npm.ok) { if (!npm.ok) {
return { ok: false, appName, steps, error: `package install failed: ${npm.output || 'no output'}` }; return { ok: false, appName, steps, error: `package install failed: ${npm.output || 'no output'}` };
} }
// Resolved, not assumed. `sharp` resolves to a platform-specific binary package, so a lockfile that // Resolved, not assumed. `sharp` resolves to a platform-specific binary package, so an install
// installs cleanly on one machine can still leave nothing importable on another — which is the // that succeeds on one machine can still leave nothing importable on another — which is the shape
// shape of the failure this whole block exists to stop shipping. // of the failure this whole block exists to stop shipping.
const unresolvable = await verifyNpmDependencies(plugin); const unresolvable = unresolvableDependencies(plugin);
if (unresolvable.length) { if (unresolvable.length) {
return { return {
ok: false, ok: false,
@@ -295,18 +311,6 @@ export async function installPlugin(
}; };
} }
await step(steps, onStep, 'packages: installed and resolvable'); await step(steps, onStep, 'packages: installed and resolvable');
// Said now, while there is something to be done about it. `pullPlugin` refuses on any uncommitted
// change, so an unignored `node_modules/` turns the Update button into a permanent "N uncommitted
// changes" about a directory the platform created — and the person seeing that message has no
// reason to connect it to a package.json shipped three versions ago.
if (await npmWouldBlockUpdates(plugin)) {
await step(
steps,
onStep,
'packages: WARNING — this plugin does not ignore node_modules/, so Update will refuse as dirty. ' +
'Add `node_modules/` to its .gitignore.',
);
} }
} }
+19
View File
@@ -244,6 +244,25 @@ export type PluginManifest = {
* that runs and quietly produces nothing. * that runs and quietly produces nothing.
*/ */
osDependencies?: OsDependency[]; osDependencies?: OsDependency[];
/**
* npm packages this plugin imports. Absent means it needs none, which is every plugin but Atelier.
*
* Ranges, not pins, and deliberately: every plugin's declarations are merged into ONE shared install at
* `plugins/node_modules/`, so two plugins asking for `^0.33.5` and `^0.33.9` share one copy rather than
* carrying 50 MB of the same platform binaries twice. Where ranges differ they are intersected — both
* plugins have to work — so incompatible requirements fail the install loudly instead of handing one
* plugin a version the other cannot use.
*
* This is the one place in the codebase where a dependency is resolved at install time rather than from
* a committed lockfile. That is a real cost against the rule `CLAUDE.md` states, taken knowingly: the
* alternative is a lockfile per plugin and no sharing at all. What it buys back is that every package a
* plugin can pull is DECLARED here, visible before install, and the merged file is regenerated from
* these manifests rather than edited — so nothing can appear in it without a plugin having asked.
*
* See `npm-deps.ts`.
*/
npmDependencies?: Record<string, string>;
}; };
/** What the platform knows about a plugin on disk: its manifest, plus everything the tree said. */ /** What the platform knows about a plugin on disk: its manifest, plus everything the tree said. */
+48
View File
@@ -0,0 +1,48 @@
import { describe, expect, test } from 'bun:test';
import { mergeDependencies } from './npm-deps';
import type { DiscoveredPlugin } from './manifest';
const plugin = (appName: string, npmDependencies?: Record<string, string>) =>
({ appName, dir: `/tmp/${appName}`, manifest: { npmDependencies } }) as unknown as DiscoveredPlugin;
describe('merging what plugins ask for', () => {
test('one plugin, one range', () => {
expect(mergeDependencies([plugin('atelier', { sharp: '^0.33.5' })]).merged).toEqual({ sharp: '^0.33.5' });
});
test('two plugins asking for the same range share one entry, and it is not a conflict', () => {
const { merged, conflicts } = mergeDependencies([
plugin('atelier', { sharp: '^0.33.5' }),
plugin('photos', { sharp: '^0.33.5' }),
]);
expect(merged).toEqual({ sharp: '^0.33.5' });
expect(conflicts).toEqual({});
});
// A SPACE, not `||`. npm and bun read space-separated ranges as an intersection: the resolved version
// must satisfy both. `||` would mean "either", which would let one plugin get a version the other
// cannot import — the silent version of this failure, and the one worth designing against.
test('different ranges intersect, and are reported', () => {
const { merged, conflicts } = mergeDependencies([
plugin('atelier', { sharp: '^0.33.5' }),
plugin('photos', { sharp: '^0.33.9' }),
]);
expect(merged.sharp).toBe('^0.33.5 ^0.33.9');
expect(conflicts.sharp).toEqual([
{ appName: 'atelier', range: '^0.33.5' },
{ appName: 'photos', range: '^0.33.9' },
]);
});
test('plugins declaring nothing contribute nothing', () => {
expect(mergeDependencies([plugin('offscale'), plugin('music', {})]).merged).toEqual({});
});
// Sorted, so the generated file does not churn between installs and a diff means something changed.
test('output is stable regardless of plugin order', () => {
const a = mergeDependencies([plugin('zed', { b: '1' }), plugin('alpha', { a: '1' })]);
const b = mergeDependencies([plugin('alpha', { a: '1' }), plugin('zed', { b: '1' })]);
expect(Object.keys(a.merged)).toEqual(Object.keys(b.merged));
expect(Object.keys(a.merged)).toEqual(['a', 'b']);
});
});
+111 -95
View File
@@ -1,137 +1,153 @@
import { existsSync } from 'node:fs'; import { existsSync } from 'node:fs';
import { join } from 'node:path'; import { join } from 'node:path';
import { PLATFORM_DIR } from '../data-path';
import type { DiscoveredPlugin } from './manifest'; import type { DiscoveredPlugin } from './manifest';
// A plugin's own npm dependencies, installed into its own directory. // npm packages a plugin needs, declared in its manifest and installed ONCE for all plugins.
// //
// ── The gap this closes ── // ── The gap this closes ──
// //
// `osDependencies` covers binaries a plugin needs on PATH — `ffmpeg`, `radicale`, `tea`. There was // `osDependencies` covers binaries on PATH — `ffmpeg`, `radicale`, `tea`. There was nothing for a
// nothing for a JavaScript package, so a plugin needing one had to add it to the PLATFORM's // JavaScript package, so a plugin needing one had to add it to the PLATFORM's `package.json`. Atelier did
// `package.json`. Atelier did exactly that with `sharp` on 2026-08-20, carefully and with the frozen // that with `sharp`, and it broke on the first machine that was not the one it was written on: alpha
// lockfile dance documented, and it still went wrong the first time it met a second machine: alpha // pulled the `package.json`, nobody ran `bun install`, and the sidecar crash-looped on
// pulled the `package.json`, nobody ran `bun install`, and `officer-atelier` crash-looped on // `Cannot find package 'sharp'` — after the install flow had reported success.
// `Cannot find package 'sharp'`. The plugin reported a successful install and then could not start.
// //
// Three things were wrong with that shape, and none of them are the agent's fault: // ── Why one shared install, not one per plugin ──
// //
// - **The platform carried a dependency only one plugin needs.** Uninstall the plugin and `sharp` is // The first version gave each plugin its own `package.json` and `node_modules/`. That works and it is
// still in the platform's lockfile, with nothing recording why. // what a git repository normally does, but `sharp` is ~50 MB of platform binaries and it is the obvious
// - **Installing a plugin could not install it.** The install flow ran, said "installed", and left a // dependency for anything touching images — a second and third plugin wanting it would mean a second and
// process that could not boot. Nothing in the pipeline was positioned to notice. // third copy, and no version they could converge on.
// - **It forced the platform's lockfile open.** `frozenLockfile` exists so an unexplained lockfile
// change is a signal; a plugin needing a package makes that signal routine.
// //
// ── The shape ── // So: every plugin's `npmDependencies` are merged into ONE generated `plugins/package.json`, installed
// into `plugins/node_modules/`. Bun resolves node_modules up the directory tree, so that one directory
// serves every plugin and none of the platform's own code.
// //
// A plugin that needs packages ships its own `package.json` and its own lockfile, and they are committed // ── What is given up, deliberately ──
// in the plugin's OWN repository — which is the thing a plugin already is. Bun resolves `node_modules`
// up the directory tree, so `plugins/<name>/node_modules/` serves that plugin's code and nothing else.
// Uninstalling removes the directory and the packages with it.
// //
// The presence of the file is the declaration. No manifest field, for the same reason `sidecar/index.ts` // A committed lockfile. Ranges are the point — two plugins asking for `^0.33.5` and `^0.33.9` should
// has none: a second place to say the same thing is a second place for it to be wrong. // share one install — and a range resolves per machine. `CLAUDE.md` is emphatic about frozen lockfiles
// and the 2026-08-04 supply-chain compromise, so this is a real cost and it is worth naming: plugin
// packages are resolved at install time, like `osDependencies` are, and unlike the platform's own.
//
// The mitigation is that the surface is small and declared. Every package a plugin can pull is written in
// its manifest, visible before install, and the merged file is regenerated from those manifests rather
// than edited — so a package appearing there without a plugin asking for it is not something that can
// happen quietly.
/** `plugins/<name>/package.json`, or null when the plugin needs no packages — the ordinary case. */ /** The generated file. Regenerated from manifests on every install — never edited, never committed. */
export const npmManifestFor = (plugin: DiscoveredPlugin): string | null => { export const PLUGIN_PACKAGE_JSON = join(PLATFORM_DIR, 'plugins', 'package.json');
const path = join(plugin.dir, 'package.json'); const PLUGINS_DIR = join(PLATFORM_DIR, 'plugins');
return existsSync(path) ? path : null;
export type MergedDependencies = {
/** package → the range actually written. */
merged: Record<string, string>;
/** package → which plugins asked, and for what. Only entries where they disagreed. */
conflicts: Record<string, { appName: string; range: string }[]>;
}; };
export type NpmInstallResult = { ok: boolean; ran: boolean; output: string };
/** /**
* Will the packages this is about to install make the plugin un-updatable? * Merge every plugin's declared packages into one set of ranges.
* *
* `pullPlugin` refuses on ANY uncommitted change — `git status --porcelain` non-empty — because it is * Where two plugins ask for the same package with DIFFERENT ranges, the ranges are joined with a space —
* about to `merge --ff-only` over the checkout and will not overwrite someone's work. `node_modules/` * which npm and bun read as an intersection, not an alternative. That is the honest semantics: both
* that nothing ignores is an uncommitted change, so a plugin shipping `package.json` without ignoring it * plugins have to work, so the resolved version must satisfy both. Compatible ranges (`^0.33.5` and
* installs perfectly once and can then never be updated again. The Update button just says "N uncommitted * `^0.33.9`) resolve to one shared install; genuinely incompatible ones fail the install with bun's own
* changes" forever, about a directory the platform itself created. * message rather than silently giving one plugin a version the other cannot use.
* *
* Asked with `git check-ignore`, not by reading `.gitignore`: the rule may be nested, global, or in * Sorted throughout, so the generated file does not churn between installs.
* `.git/info/exclude`, and every one of those counts.
*
* Not fixed automatically. Writing a line into the plugin's own `.gitignore` would be the platform
* editing a repository it does not own — and would itself dirty the checkout, which is the very thing
* being avoided.
*/ */
export async function npmWouldBlockUpdates(plugin: DiscoveredPlugin): Promise<boolean> { export function mergeDependencies(plugins: DiscoveredPlugin[]): MergedDependencies {
if (!npmManifestFor(plugin)) return false; const asked: Record<string, { appName: string; range: string }[]> = {};
if (!existsSync(join(plugin.dir, '.git'))) return false; // not a checkout — nothing to update from
try { for (const plugin of [...plugins].sort((a, b) => a.appName.localeCompare(b.appName))) {
const proc = Bun.spawn(['git', 'check-ignore', '-q', 'node_modules'], { const deps = plugin.manifest.npmDependencies ?? {};
cwd: plugin.dir, for (const [name, range] of Object.entries(deps)) {
stdout: 'ignore', (asked[name] ??= []).push({ appName: plugin.appName, range });
stderr: 'ignore',
});
// 0 = ignored, 1 = not ignored, anything else = git could not answer, which is not a finding.
return (await proc.exited) === 1;
} catch {
return false;
} }
} }
const merged: Record<string, string> = {};
const conflicts: MergedDependencies['conflicts'] = {};
for (const name of Object.keys(asked).sort()) {
const ranges = [...new Set(asked[name]!.map((a) => a.range))];
merged[name] = ranges.join(' ');
if (ranges.length > 1) conflicts[name] = asked[name]!;
}
return { merged, conflicts };
}
export type NpmInstallResult = {
ok: boolean;
ran: boolean;
output: string;
conflicts: MergedDependencies['conflicts'];
};
/** /**
* Install a plugin's packages, in its own directory. * Write `plugins/package.json` from every plugin's manifest and install it.
* *
* **Frozen.** The plugin ships a lockfile beside its `package.json`, so this resolves from that and * Generated from the plugins ON DISK, like `generatePluginSchemas` — not from what is installed. A plugin
* nothing else — the same discipline the platform holds itself to, and for the same reason: a package * whose directory is present but which nobody has enabled still gets its packages, which is the same
* that changed without anyone choosing it is exactly what a supply-chain compromise looks like. A plugin * safety property that file documents: the thing that decides is what is there to import.
* whose lockfile does not satisfy its `package.json` fails here rather than quietly resolving something
* newer.
* *
* Never throws. A plugin whose packages fail to install is broken, but so is refusing the whole install * Never throws.
* over it — the caller decides, and it has more context than this does.
*/ */
export async function installNpmDependencies(plugin: DiscoveredPlugin): Promise<NpmInstallResult> { export async function installNpmDependencies(plugins: DiscoveredPlugin[]): Promise<NpmInstallResult> {
if (!npmManifestFor(plugin)) return { ok: true, ran: false, output: '' }; const { merged, conflicts } = mergeDependencies(plugins);
if (Object.keys(merged).length === 0) {
// Nothing wants packages. Leave any previous file alone rather than writing an empty one — an empty
// `package.json` beside a populated `node_modules` reads as "these were installed by nobody".
return { ok: true, ran: false, output: '', conflicts };
}
const body = {
name: '@officer/plugin-dependencies',
private: true,
'//': 'GENERATED from every plugin manifest by src/servers/plugins/npm-deps.ts. Not edited, not committed.',
dependencies: merged,
};
try { try {
const proc = Bun.spawn(['bun', 'install', '--frozen-lockfile'], { await Bun.write(PLUGIN_PACKAGE_JSON, `${JSON.stringify(body, null, 2)}\n`);
cwd: plugin.dir,
stdout: 'pipe', const proc = Bun.spawn(['bun', 'install'], { cwd: PLUGINS_DIR, stdout: 'pipe', stderr: 'pipe' });
stderr: 'pipe',
// The plugin's own directory is the root of this install. `--frozen-lockfile` is passed explicitly
// rather than relied upon from a bunfig the plugin may not ship.
});
const [out, err] = await Promise.all([new Response(proc.stdout).text(), new Response(proc.stderr).text()]); const [out, err] = await Promise.all([new Response(proc.stdout).text(), new Response(proc.stderr).text()]);
const code = await proc.exited; const code = await proc.exited;
return { ok: code === 0, ran: true, output: `${out}${err}`.trim() }; return { ok: code === 0, ran: true, output: `${out}${err}`.trim(), conflicts };
} catch (error) { } catch (error) {
return { ok: false, ran: true, output: error instanceof Error ? error.message : String(error) }; return { ok: false, ran: true, output: error instanceof Error ? error.message : String(error), conflicts };
} }
} }
/** /**
* Can this plugin's code actually resolve what it declared? * Can this plugin's code actually resolve what its manifest declared?
* *
* Asked AFTER installing, because "the install command exited 0" and "the sidecar can import it" are * Asked AFTER installing, because "the install exited 0" and "the sidecar can import it" are different
* different claims, and it was the second one that failed on alpha. `sharp` in particular resolves to a * claims and it was the second one that failed on alpha. `sharp` resolves to a platform-specific binary
* platform-specific binary package, so a lockfile that installs cleanly on one machine can still leave * package, so a lockfile that installs cleanly on one machine can leave nothing importable on another.
* nothing importable on another. *
* Resolved FROM THE PLUGIN'S OWN DIRECTORY, which is the question its sidecar will ask.
*/ */
export async function verifyNpmDependencies(plugin: DiscoveredPlugin): Promise<string[]> { export function unresolvableDependencies(plugin: DiscoveredPlugin): string[] {
const manifestPath = npmManifestFor(plugin); const names = Object.keys(plugin.manifest.npmDependencies ?? {});
if (!manifestPath) return []; return names.filter((name) => {
let names: string[];
try {
const parsed = (await Bun.file(manifestPath).json()) as { dependencies?: Record<string, string> };
names = Object.keys(parsed.dependencies ?? {});
} catch {
return [];
}
const missing: string[] = [];
for (const name of names) {
// `bun.resolveSync` from the plugin's directory answers the question the sidecar will ask, rather
// than the question "is there a folder with the right name".
try { try {
Bun.resolveSync(name, plugin.dir); Bun.resolveSync(name, plugin.dir);
return false;
} catch { } catch {
missing.push(name); return true;
} }
});
} }
return missing;
} /** Whether anything on this machine declares packages at all. */
export const anyPluginWantsPackages = (plugins: DiscoveredPlugin[]): boolean =>
plugins.some((p) => Object.keys(p.manifest.npmDependencies ?? {}).length > 0);
/** For the uninstall path: the generated file exists and can be regenerated without the removed plugin. */
export const pluginPackageJsonExists = (): boolean => existsSync(PLUGIN_PACKAGE_JSON);
-18
View File
@@ -3,8 +3,6 @@ import { mkdirSync, rmSync, writeFileSync } from 'node:fs';
import { join } from 'node:path'; import { join } from 'node:path';
import { PLUGINS_DIR } from './discover'; import { PLUGINS_DIR } from './discover';
import { pullPlugin } from './update'; import { pullPlugin } from './update';
import { discoverPlugins } from './discover';
import { npmManifestFor, npmWouldBlockUpdates } from './npm-deps';
// The REFUSALS are what this test is for. A pull that works is easy to believe; a pull that quietly // The REFUSALS are what this test is for. A pull that works is easy to believe; a pull that quietly
// discarded somebody's uncommitted work is the failure that matters, and every plugin directory on a // discarded somebody's uncommitted work is the failure that matters, and every plugin directory on a
@@ -55,19 +53,3 @@ describe('pullPlugin refuses rather than guesses', () => {
} }
}); });
}); });
describe('npm packages and the Update button', () => {
// The trap: `pullPlugin` refuses on ANY uncommitted change, and `node_modules/` that nothing ignores
// is one. So a plugin shipping `package.json` without ignoring it installs perfectly once and can then
// never be updated — the button says "N uncommitted changes" forever, about a directory the platform
// itself created. Every plugin that exists today ignores it, which is luck rather than a rule.
test('atelier — the only plugin with packages — ignores node_modules', async () => {
const { plugins } = await discoverPlugins();
const withPackages = plugins.filter((p) => npmManifestFor(p));
// Not asserting that atelier specifically is installed — a checkout without it is legitimate.
for (const plugin of withPackages) {
expect(await npmWouldBlockUpdates(plugin), `${plugin.appName} would block its own updates`).toBe(false);
}
});
});