diff --git a/src/servers/api/plugins/router.ts b/src/servers/api/plugins/router.ts index 0afda51b..c01660f3 100644 --- a/src/servers/api/plugins/router.ts +++ b/src/servers/api/plugins/router.ts @@ -129,7 +129,7 @@ pluginsRouter.get('/', async (ctx) => { }; })(), })), - plugins: states.map(({ plugin, install, outdated }, i) => ({ + plugins: states.map(({ plugin, install, outdated, release }, i) => ({ appName: plugin.appName, prefix: mountPrefix(plugin), label: plugin.manifest.label, @@ -165,6 +165,11 @@ pluginsRouter.get('/', async (ctx) => { enabled: install?.enabled ?? false, installedVersion: install?.version ?? null, outdated, + // The tag rule — whether the REMOTE has something this checkout does not. See plugins/release.ts. + updateable: release.updateable, + releaseReason: release.reason, + tagAtHead: release.tagAtHead, + newestTag: release.newestTag, processStatus: statuses[i] ?? null, // Probed live, against this machine's PATH, on every read. // diff --git a/src/servers/plugins/mount.ts b/src/servers/plugins/mount.ts index 033058a9..36eec46c 100644 --- a/src/servers/plugins/mount.ts +++ b/src/servers/plugins/mount.ts @@ -12,6 +12,7 @@ import { type RootClaim, } from './public-paths'; import { discoverPlugins } from './discover'; +import { releaseState, type ReleaseState } from './release'; import { mountPrefix, type DiscoveredPlugin } from './manifest'; import { generatePluginsModule, rebuildFrontend } from './generate'; import { iconUrl, screenArtUrl } from '../app-store/assets'; @@ -35,6 +36,14 @@ export type PluginState = { install: PluginInstall | null; /** The manifest on disk moved after it was installed — normal while developing, worth being able to see. */ outdated: boolean; + /** + * Whether there is anything to update TO, by the tag rule — see `release.ts`. + * + * Distinct from `outdated`, which compares the installed version against the manifest on disk and so + * answers "this checkout moved since you installed it". This answers "the REMOTE has something you do + * not have", which is the question the Update button is really asking. + */ + release: ReleaseState; }; export type PluginsSnapshot = { @@ -54,10 +63,19 @@ export async function snapshotPlugins(): Promise { const [{ plugins, broken }, installs] = await Promise.all([discoverPlugins(), listPluginInstalls()]); const byName = new Map(installs.map((row) => [row.appName, row])); - const states = plugins.map((plugin) => { - const install = byName.get(plugin.appName) ?? null; - return { plugin, install, outdated: !!install && install.version !== plugin.manifest.version }; - }); + // Resolved in parallel: each is a few local git calls, and twenty-one in series is a visible pause on + // a screen that renders a list. Nothing here touches the network — see `release.ts`. + const states = await Promise.all( + plugins.map(async (plugin) => { + const install = byName.get(plugin.appName) ?? null; + return { + plugin, + install, + outdated: !!install && install.version !== plugin.manifest.version, + release: await releaseState(plugin.dir), + }; + }), + ); return { states, broken }; } diff --git a/src/servers/plugins/release.test.ts b/src/servers/plugins/release.test.ts new file mode 100644 index 00000000..0452b965 --- /dev/null +++ b/src/servers/plugins/release.test.ts @@ -0,0 +1,104 @@ +import { describe, expect, test, afterAll } from 'bun:test'; +import { mkdtempSync, rmSync, writeFileSync, mkdirSync } from 'node:fs'; +import { tmpdir } from 'node:os'; +import { join } from 'node:path'; +import { releaseState } from './release'; + +/** + * Against real repositories, because every assertion here is about what git reports. A mocked `git` would + * test the mock, and the two cases that matter — a tag pushed after the checkout was made, and a tag name + * that does not sort — are exactly the ones a mock would get wrong. + */ + +const root = mkdtempSync(join(tmpdir(), 'plugin-release-')); +afterAll(() => rmSync(root, { recursive: true, force: true })); + +const git = (dir: string, ...args: string[]) => { + const p = Bun.spawnSync(['git', '-C', dir, ...args], { stdout: 'pipe', stderr: 'pipe' }); + if (p.exitCode !== 0) throw new Error(`git ${args.join(' ')}: ${p.stderr.toString()}`); + return p.stdout.toString().trim(); +}; + +/** An origin and a clone of it, the shape every installed plugin actually has. */ +function pair(name: string) { + const origin = join(root, `${name}.git`); + const work = join(root, `${name}-origin`); + const clone = join(root, name); + mkdirSync(work, { recursive: true }); + git(work, 'init', '-q', '-b', 'master'); + git(work, 'config', 'user.email', 't@t'); + git(work, 'config', 'user.name', 'T'); + writeFileSync(join(work, 'f.txt'), 'one\n'); + git(work, 'add', '-A'); + git(work, 'commit', '-q', '-m', 'one'); + Bun.spawnSync(['git', 'clone', '-q', '--bare', work, origin]); + Bun.spawnSync(['git', 'clone', '-q', origin, clone]); + return { work, origin, clone }; +} + +const publish = (work: string, origin: string, msg: string, tag?: string) => { + writeFileSync(join(work, 'f.txt'), msg); + git(work, 'add', '-A'); + git(work, 'commit', '-q', '-m', msg); + if (tag) git(work, 'tag', '-a', tag, '-m', tag); + Bun.spawnSync(['git', '-C', work, 'push', '-q', origin, 'master', '--tags']); +}; + +describe('releaseState', () => { + test('a checkout NOT at a tag is always updateable', async () => { + const { clone } = pair('untagged'); + const s = await releaseState(clone); + expect(s.reason).toBe('untagged'); + expect(s.updateable).toBe(true); + }); + + test('a checkout at the newest tag is NOT updateable', async () => { + const { work, origin, clone } = pair('current'); + publish(work, origin, 'two', 'v1'); + git(clone, 'fetch', '-q', 'origin', '--tags'); + git(clone, 'merge', '-q', '--ff-only', 'origin/master'); + const s = await releaseState(clone); + expect(s.tagAtHead).toBe('v1'); + expect(s.reason).toBe('at-latest'); + expect(s.updateable).toBe(false); + }); + + test('a NEWER tag on the remote makes it updateable again', async () => { + const { work, origin, clone } = pair('moved'); + publish(work, origin, 'two', 'v1'); + git(clone, 'fetch', '-q', 'origin', '--tags'); + git(clone, 'merge', '-q', '--ff-only', 'origin/master'); + expect((await releaseState(clone)).updateable).toBe(false); + + publish(work, origin, 'three', 'v2'); + git(clone, 'fetch', '-q', 'origin', '--tags'); + const s = await releaseState(clone); + // HEAD has NOT moved — the checkout is still at v1. What changed is the remote. + expect(s.tagAtHead).toBe('v1'); + expect(s.newestTag).toBe('v2'); + expect(s.reason).toBe('new-release'); + expect(s.updateable).toBe(true); + }); + + test("newness is git's ordering, not a version comparison", async () => { + // `beta` sorts BEFORE `v1` as a string. Anything parsing version numbers calls this "not newer" and + // silently never offers the update. + const { work, origin, clone } = pair('unsortable'); + publish(work, origin, 'two', 'v1'); + git(clone, 'fetch', '-q', 'origin', '--tags'); + git(clone, 'merge', '-q', '--ff-only', 'origin/master'); + publish(work, origin, 'three', 'beta'); + git(clone, 'fetch', '-q', 'origin', '--tags'); + const s = await releaseState(clone); + expect(s.newestTag).toBe('beta'); + expect(s.updateable).toBe(true); + }); + + test('a directory that is not a checkout is not updateable', async () => { + const d = join(root, 'plain'); + mkdirSync(d, { recursive: true }); + const s = await releaseState(d); + expect(s.reason).toBe('not-a-checkout'); + expect(s.updateable).toBe(false); + }); +}); diff --git a/src/servers/plugins/release.ts b/src/servers/plugins/release.ts new file mode 100644 index 00000000..c78ad674 --- /dev/null +++ b/src/servers/plugins/release.ts @@ -0,0 +1,83 @@ +import { existsSync } from 'node:fs'; +import { join } from 'node:path'; + +/** + * Is there anything to update to? + * + * ── The rule, and why it is about TAGS rather than commits ── + * + * Until 2026-08-27 "update" meant `git pull` and re-install, unconditionally, for every plugin. Nothing + * could say whether there was anything to pull, so the button was always offered and usually did nothing + * — and "Update all" was a row of no-ops with a spinner on it. + * + * Tags fix that, and the owner's rule has two halves: + * + * · **A checkout NOT at a tag is always updateable.** It is somewhere between releases — mid-development, + * or on a commit somebody pushed without tagging. There is no claim being made about it, so pulling is + * always reasonable. + * · **A checkout AT a tag is updateable only when a NEWER tag exists.** It is at a release, and a release + * is a claim that this is a finished thing. Offering an update that lands on the same release is how + * you teach someone to ignore the button. + * + * ── Why "newer" is not a version comparison ── + * + * The obvious implementation parses `2026.08.27.01` and compares. It is wrong the first time a plugin is + * tagged `v2` or `1.0.0-rc1`, and wrong SILENTLY — a plugin that never offers an update looks like a + * plugin nobody has released. + * + * So newness is asked of git instead: the newest tag REACHABLE FROM THE REMOTE BRANCH. If the tag at HEAD + * is that tag, this checkout is at the latest release. If it is a different one, the remote has moved on. + * That holds for any tag names anyone ever chooses, including ones that do not sort. + * + * ── What this does NOT do ── + * + * It never fetches. Every answer comes from refs already on disk, because this is called once per plugin + * on a request that renders a list — twenty-one network round trips would make the plugins screen feel + * broken. Tags arrive at clone and at the fetch `pullPlugin` already does, so "check for updates" is a + * consequence of updating rather than a separate poll. + */ + +export type ReleaseState = { + /** The tag this checkout is exactly at, or null when it is between releases. */ + tagAtHead: string | null; + /** The newest tag reachable from the tracked remote branch, as git orders them. */ + newestTag: string | null; + updateable: boolean; + /** Why, in words a UI can show without restating the rule. */ + reason: 'untagged' | 'new-release' | 'at-latest' | 'not-a-checkout'; +}; + +const run = async (dir: string, args: string[]): Promise => { + const proc = Bun.spawn(['git', '-C', dir, ...args], { stdout: 'pipe', stderr: 'ignore' }); + const out = await new Response(proc.stdout).text(); + return (await proc.exited) === 0 ? out.trim() || null : null; +}; + +export async function releaseState(dir: string): Promise { + const none = { tagAtHead: null, newestTag: null }; + if (!existsSync(join(dir, '.git'))) { + // Copied in by hand or bundled. Nothing to compare, and nothing to pull either — `pullPlugin` says + // the same thing when asked. + return { ...none, updateable: false, reason: 'not-a-checkout' }; + } + + // `--exact-match`, so this is "the tag AT this commit", not "the last tag before it". Fails, and + // therefore returns null, when HEAD is between releases — which is the first half of the rule. + const tagAtHead = await run(dir, ['describe', '--exact-match', '--tags', 'HEAD']); + if (!tagAtHead) return { ...none, updateable: true, reason: 'untagged' }; + + // Asked of the REMOTE branch, not of HEAD: a tag pushed after this checkout was made is not reachable + // from HEAD and would be invisible if we described HEAD instead. + const branch = (await run(dir, ['rev-parse', '--abbrev-ref', 'HEAD'])) ?? 'master'; + const newestTag = + (await run(dir, ['describe', '--tags', '--abbrev=0', `origin/${branch}`])) ?? + (await run(dir, ['describe', '--tags', '--abbrev=0', 'origin/HEAD'])); + + // No remote tags known yet — a checkout whose origin has never been fetched for tags. Not updateable + // rather than "always updateable": claiming an update exists on no evidence is the worse error. + if (!newestTag) return { tagAtHead, newestTag: null, updateable: false, reason: 'at-latest' }; + + return newestTag === tagAtHead + ? { tagAtHead, newestTag, updateable: false, reason: 'at-latest' } + : { tagAtHead, newestTag, updateable: true, reason: 'new-release' }; +} diff --git a/src/servers/plugins/update.ts b/src/servers/plugins/update.ts index a2862f2f..b5712547 100644 --- a/src/servers/plugins/update.ts +++ b/src/servers/plugins/update.ts @@ -1,4 +1,5 @@ import { existsSync, readdirSync } from 'node:fs'; +import { releaseState } from './release'; import { join } from 'node:path'; import { PLUGINS_DIR } from './discover'; import { OFFICER_ITEMS_DIR, PLATFORM_DIR } from '../data-path'; @@ -111,6 +112,24 @@ export async function updatePlugin( onLog?: OnLog, ): Promise { const steps: string[] = []; + + /* + Refuse when there is nothing to update to. + + The UI already hides the button, and that is not enough: a plugins list is fetched once and a tag can + be pushed a second later, so the copy someone clicks is routinely older than the truth. More to the + point, `updateAll` iterates a list rather than asking per plugin — without this it would re-install + twenty-one plugins to land on the versions they were already on. + + Checked BEFORE the pull, because a pull is what would make the answer stale. + */ + const state = await releaseState(join(PLUGINS_DIR, appName)); + if (!state.updateable && state.reason === 'at-latest') { + steps.push(`already at ${state.tagAtHead}, the newest release`); + onStep?.(steps[steps.length - 1]!); + return { ok: true, appName, steps }; + } + const pulled = await pullPlugin(appName, onLog); if (!pulled.ok) return { ok: false, appName, steps, error: pulled.error }; diff --git a/src/workspaces/officerdev/src/apps/Plugins/PluginDetail.tsx b/src/workspaces/officerdev/src/apps/Plugins/PluginDetail.tsx index f46fc819..9eadd045 100644 --- a/src/workspaces/officerdev/src/apps/Plugins/PluginDetail.tsx +++ b/src/workspaces/officerdev/src/apps/Plugins/PluginDetail.tsx @@ -523,14 +523,25 @@ export const PluginDetail = () => { {t('plugins.enable')} )} - {/* Always offered, not gated on `outdated`. That flag compares the manifest's version to the - recorded one, and nothing bumps a version today — so an artwork or code change lands with - the number unmoved and the button would never appear. It also used to run `install`, - which cannot fetch: the directory exists, so the clone is skipped and only the apply - steps re-ran. `update` pulls first. */} - + {/* Offered only when there is something to update TO — the tag rule, see plugins/release.ts. + A checkout between releases always is; one sitting on a tag only when a newer tag exists. + + This was unconditional, with a comment explaining that nothing bumped versions so gating + it on `outdated` would hide it for ever. Versions are dated and tagged now, so the button + can finally mean something — and an Update that lands on the version you already have is + how you teach someone to stop pressing it. */} + {plugin.updateable && ( + + )} + {/* Said out loud rather than shown as an absent button. "No Update button" and "up to date" + look identical, and only one of them is information. */} + {!plugin.updateable && plugin.releaseReason === 'at-latest' && plugin.tagAtHead && ( + Up to date · {plugin.tagAtHead} + )} {/* Absent while enabled — disable first. `DeletePlugin` decides that itself rather than being conditioned here, so the rule lives next to the confirmation that explains it. */}