From 3450f5eb57a03b8c00f1fc08533853639e7e50bd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andr=C3=A9=20Padez?= Date: Thu, 20 Aug 2026 18:13:53 +0000 Subject: [PATCH] warn when a plugin's packages would block its own updates MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit update is a git pull and then the ordinary install path, so the npm step is picked up for free — a plugin that adds a dependency in a later version gets it installed on update, with no second implementation to drift. but 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 again: the button reports "N uncommitted changes" forever, about a directory the platform itself created, and nobody seeing that message has a reason to connect it to a package.json from three versions ago. every plugin that exists today ignores node_modules, which is luck rather than a rule — this makes it a checked one. asked with `git check-ignore` rather than by reading .gitignore: the rule may be nested, global, or in .git/info/exclude, and all three count. warned, not fixed. writing 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 exact thing being avoided. --- src/servers/plugins/install.ts | 15 +++++++++++++- src/servers/plugins/npm-deps.ts | 32 ++++++++++++++++++++++++++++++ src/servers/plugins/update.test.ts | 18 +++++++++++++++++ 3 files changed, 64 insertions(+), 1 deletion(-) diff --git a/src/servers/plugins/install.ts b/src/servers/plugins/install.ts index 069dfb84..d327d1ce 100644 --- a/src/servers/plugins/install.ts +++ b/src/servers/plugins/install.ts @@ -24,7 +24,7 @@ import { generatePluginSchemas, pushPluginSchema } from './schema'; import { declaredTables, dropTables, existingTables } from './tables'; import type { OnLog } from './run-logged'; import { installDependencies, installRelease, manualInstallHint, reportDependencies, stillMissing } from './os-deps'; -import { installNpmDependencies, npmManifestFor, verifyNpmDependencies } from './npm-deps'; +import { installNpmDependencies, npmManifestFor, npmWouldBlockUpdates, verifyNpmDependencies } from './npm-deps'; import { mountPrefix, type DiscoveredPlugin } from './manifest'; // The install runner: the four verbs, each as a short ordered list of effects. @@ -295,6 +295,19 @@ export async function installPlugin( }; } 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.', + ); + } } if (plugin.schema) { diff --git a/src/servers/plugins/npm-deps.ts b/src/servers/plugins/npm-deps.ts index e930d519..7aef2ffb 100644 --- a/src/servers/plugins/npm-deps.ts +++ b/src/servers/plugins/npm-deps.ts @@ -40,6 +40,38 @@ export const npmManifestFor = (plugin: DiscoveredPlugin): string | null => { export type NpmInstallResult = { ok: boolean; ran: boolean; output: string }; +/** + * Will the packages this is about to install make the plugin un-updatable? + * + * `pullPlugin` refuses on ANY uncommitted change — `git status --porcelain` non-empty — because it is + * about to `merge --ff-only` over the checkout and will not overwrite someone's work. `node_modules/` + * that nothing ignores is an uncommitted change, so a plugin shipping `package.json` without ignoring it + * installs perfectly once and can then never be updated again. The Update button just says "N uncommitted + * changes" forever, about a directory the platform itself created. + * + * Asked with `git check-ignore`, not by reading `.gitignore`: the rule may be nested, global, or in + * `.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 { + if (!npmManifestFor(plugin)) return false; + if (!existsSync(join(plugin.dir, '.git'))) return false; // not a checkout — nothing to update from + try { + const proc = Bun.spawn(['git', 'check-ignore', '-q', 'node_modules'], { + cwd: plugin.dir, + stdout: 'ignore', + 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; + } +} + /** * Install a plugin's packages, in its own directory. * diff --git a/src/servers/plugins/update.test.ts b/src/servers/plugins/update.test.ts index 4bd9e265..ef795360 100644 --- a/src/servers/plugins/update.test.ts +++ b/src/servers/plugins/update.test.ts @@ -3,6 +3,8 @@ import { mkdirSync, rmSync, writeFileSync } from 'node:fs'; import { join } from 'node:path'; import { PLUGINS_DIR } from './discover'; 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 // discarded somebody's uncommitted work is the failure that matters, and every plugin directory on a @@ -53,3 +55,19 @@ 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); + } + }); +});