diff --git a/TODO.md b/TODO.md index 81befc74..b5a41bab 100644 --- a/TODO.md +++ b/TODO.md @@ -731,6 +731,23 @@ outside the example's new README: - The same trigger would serve `COMMS/command-hq/watcher-loop-proposal.md`, which wants infrastructure to own a loop rather than a model holding a protocol across turns. +- [ ] **Revisit the frozen-lockfile rule in `CLAUDE.md`, now that plugins resolve at install time.** + Raised by the owner 2026-08-20, to be discussed rather than decided alone. + + The rule as written is absolute: `bunfig.toml` sets `frozenLockfile = true`, `bun install` resolves + from `bun.lock` and nothing else, and an unexplained lockfile change in a diff is the signal the + whole arrangement exists to produce. The reasoning is the 2026-08-04 compromise, and it is good. + + `npmDependencies` does not follow it. Plugin packages are declared as RANGES and installed with a + plain `bun install` into `plugins/node_modules/`, so they resolve per machine and per install. That + was chosen deliberately — ranges are what let two plugins share one copy of `sharp` — but it means + the codebase now holds two different postures on the same risk, and only one of them is written down. + + Worth settling: whether the rule should say "the platform's own dependencies" rather than reading as + universal; whether the merged `plugins/bun.lock` should be committed after all (it would pin the + tree, at the cost of a per-machine file in git); and whether `bun install --frozen-lockfile` in + `plugins/` is achievable once that lockfile exists. + ## Backburner - [ ] **Four foreign keys are over Postgres's 63-character limit and re-create on every `db:push`.** diff --git a/src/servers/plugins/npm-deps.ts b/src/servers/plugins/npm-deps.ts index 0cf43431..165c51cb 100644 --- a/src/servers/plugins/npm-deps.ts +++ b/src/servers/plugins/npm-deps.ts @@ -1,4 +1,3 @@ -import { existsSync } from 'node:fs'; import { join } from 'node:path'; import { PLATFORM_DIR } from '../data-path'; import type { DiscoveredPlugin } from './manifest'; @@ -149,5 +148,14 @@ export function unresolvableDependencies(plugin: DiscoveredPlugin): string[] { 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); +// ── Uninstall deliberately does NOT shrink this ── +// +// There was an exported `pluginPackageJsonExists` here, written for an uninstall hook that was never +// wired. Removed rather than left dangling — an export with no caller is an unfinished thought pretending +// to be a feature. +// +// The behaviour it would have served is right anyway, for a reason worth stating. Uninstall keeps the +// plugin's DIRECTORY (see the runbook), so `discoverPlugins` still finds it and its packages are still +// declared. Only "Delete … and all its data" removes the directory, and after that the next install of +// any plugin regenerates the merged file without it. So the packages linger until the next install and +// then go, with nothing needing to remember them.