diff --git a/docs/marketplace-todo.md b/docs/marketplace-todo.md index ad54cc86..2281eaff 100644 --- a/docs/marketplace-todo.md +++ b/docs/marketplace-todo.md @@ -29,17 +29,21 @@ Ranked by what actually blocks something. ## Staleness -- [ ] **Nothing refreshes the catalogue automatically.** Publishing a plugin update changes nothing until - somebody runs `bun --env-file=.env scripts/marketplace-sync.ts`. A Gitea webhook or a cron is the - obvious next step. +- [x] **Nothing refreshes the catalogue automatically.** Done 2026-09-03: `scripts/marketplace-cron.sh` + every minute. A Gitea webhook would make it a second instead, at the price of a public write route + on the one process here with no login — not worth it for a minute. -- [ ] **The sync reads local clones, not the remotes.** The machine running it must have every plugin - checked out, so the catalogue describes that machine's copies. The version that clones each repo - into a temp directory is a small change. +- [x] **The sync reads local clones, not the remotes.** Done 2026-09-03. Bare mirrors, read at the newest + tag, driven by the registry rather than by a directory listing. The catalogue no longer describes + any machine's working copies. -- [ ] **`rawUrl()` assumes Gitea and a `main` branch.** A GitHub-hosted plugin, or one whose default branch - is `master`, gets a 404 for its icon. The forge belongs in the registry rather than inferred from a - URL shape. +- [ ] **`rawUrl()` assumes Gitea.** A GitHub-hosted plugin gets a 404 for its icon. The forge belongs in + the registry rather than inferred from a URL shape. (The `main`-vs-`master` half of this is gone — + URLs pin to the release tag now.) + +- [ ] **Four plugins have never been tagged**, so the store still shows rows the old sync took from a + working tree: av-club, kanban, openseo, fofos-admin. They are frozen until somebody tags them, and + the sync names them on every run somebody asks for. ## Scale diff --git a/docs/marketplace.md b/docs/marketplace.md index f3ec8d14..a3fafeba 100644 --- a/docs/marketplace.md +++ b/docs/marketplace.md @@ -10,8 +10,9 @@ from. **The website installs nothing** — installing means cloning a repository starting a process on someone else's machine, so it happens entirely inside the owner's own Officer. ``` -gitea.officer.dev/plugins/* the repositories (source of truth) - │ scripts/marketplace-sync.ts (manual, reads manifest.ts + README.md) +gitea.officer.dev/** the repositories (source of truth) + │ bare mirrors, read AT THE NEWEST TAG — src/servers/marketplace/sync.ts + │ every minute, scripts/marketplace-cron.sh ▼ officer_marketplace (postgres) plugins + plugin_metadata │ src/servers/marketplace/server.tsx :9100 @@ -20,6 +21,7 @@ marketplace.officer.dev GET /api/plugins, /api/plugins/:appName │ fetchCatalogue() live → cached → bundled ▼ an Officer install /marketplace → clone → push schema → mount + and the update badge — see "Publishing" below ``` ## The parts @@ -78,19 +80,87 @@ disappearing from the store. the clone, the plugin's own manifest is what the platform reads. A metadata row that disagrees with the repository is a marketplace bug, never a second source of truth. -## Sync +## Publishing + +**Push a tag. That is the whole procedure.** A timer on officer-dev picks it up within a minute and every +Officer install sees it on the next page load. ```bash -bun --env-file=.env scripts/marketplace-sync.ts +bun --env-file=.env scripts/marketplace-sync.ts # all of them, now +bun --env-file=.env scripts/marketplace-sync.ts photos # one +bun --env-file=.env scripts/marketplace-sync.ts --add https://gitea.officer.dev/plugins/finances.git +bun --env-file=.env scripts/marketplace-sync.ts --force # re-publish rows this file's own logic changed ``` -Reads every directory under `plugins/`, imports its `manifest.ts`, derives `has` from the directory -rather than from a declaration, reads `README.md`, and upserts. Re-running is always safe: it overwrites, -never merges. +### It reads releases, never a working tree -It rewrites SSH remotes to HTTPS. `example` and `offscale` had `git@gitea…` origins, which would have -handed every installer a URL that only works on the machine that published it — the install fails at -authentication, which reads as "the plugin is broken" rather than "the catalogue is wrong". +Every registered plugin is mirrored bare into `~/.cache/officer-marketplace/mirrors` and read at its +newest tag — manifest, README and file list all out of `git cat-file`. There is no working tree in the +path, so publishing something uncommitted is not a mistake that can be made. + +That is a change, and it was load-bearing. Until 2026-09-03 the sync imported `manifest.ts` off +`platform/plugins/` — the directories THIS machine has installed — so publishing meant "get officer-dev's +checkout to the release, then run the script", and nothing showed when that had not happened. On the day +it was rewritten the live store was advertising four plugins from untagged working copies: av-club +2026.08.29.01, kanban 2026.08.29.02, openseo 0.1.0 and fofos-admin 2026.08.29.26, none of which had ever +been tagged. It is also why the job could not go on a timer before: a cron over it published whatever was +in a working tree, on a schedule. + +### The registry decides which plugins, and where from + +The list comes from the `plugins` table — the half this document has always called authoritative, and +which was in fact a by-product of what happened to be checked out. Now it decides. A plugin nobody has +installed here can still be published, and a plugin lives wherever its row says: `fofos-admin` is under +`fofos/`, not `plugins/`, which an org listing would have missed. Registering one is `--add`. + +### A plugin with no tag is left alone + +It has never made a release, so the sync has nothing true to say about it: the existing row stays and the +run names it. Deleting it would drop somebody's plugin out of the store on the day the rule changed. +Tagging it is the fix, and the report is the reminder. + +### `release_tag`, and why it is not `version` + +`version` is the manifest's own claim; `release_tag` is what was pushed. They match every time somebody +tags correctly, and when they do not, the tag is the fact — a manifest whose version nobody bumped would +otherwise make a real release invisible to anything comparing versions. The sync warns on a mismatch. + +Icon URLs pin to the tag too. `/raw/HEAD/…` served whatever was on the default branch, via a 303, so a +plugin could show one icon in the store and ship another. + +### The timer + +`scripts/marketplace-cron.sh`, `* * * * *`, logging to `officer-dev/marketplace-sync.log`. One +timestamped summary line per tick — always, including the ticks that do nothing, because a fresh +timestamp is the only thing that distinguishes "nothing to publish" from "the timer died". A plugin-level +line appears only when one actually moved. + +A tick costs about 2.7s with nothing to do: one query, one fetch per repository, no writes. Nothing is +written unless the release commit differs from the one already published. + +A Gitea webhook would cut the delay from a minute to a second, at the price of a public write route and a +shared secret. A minute was never the problem anybody had — the complaint that started this was a release +that never appeared at all. + +### Remotes + +`--add` rewrites SSH to HTTPS. A `git@gitea…` origin in the catalogue hands every installer a URL that +only works on the machine that published it, and the install fails at authentication — which reads as +"the plugin is broken" rather than "the catalogue is wrong". + +## The update badge reads the catalogue + +`updateable` on `/api/plugins` compares the tag at the checkout's HEAD with the tag the catalogue says is +newest. Difference, not ordering: parsing `2026.08.27.01` works right up until somebody tags `v2`, and +then it fails silently. + +Before this, each host ran a rate-limited background `git fetch` against all twenty-one repositories to +re-derive a fact one place already knew. That is gone. `releaseState()` still holds the git rule as a +fallback for a plugin the store cannot speak for — an unregistered checkout, or a store that could not be +reached — and in that case its refs are whatever the last `pullPlugin` left, which is the accepted cost. + +`fetchCatalogue()`'s cache dropped from five minutes to one when the badge started depending on it: +otherwise the wait for a release to appear was the sync's minute plus the cache's five. ## The platform degrades, it does not break @@ -111,13 +181,8 @@ The full list, ranked, is `docs/marketplace-todo.md`. The ones that shape the de - **`[blocker]` Third-party submissions cannot use this sync.** It reads a manifest by importing it, which is code execution. Fine for our own repositories; not a mechanism a stranger's plugin can go through. Submissions need either a sandbox or a published `manifest.json` alongside the TypeScript one. -- **`[open]` Sync reads local clones, not the remotes.** The machine running it must have the plugins - checked out. The version that clones each repo into a temp directory is a small change and was left out - of the first pass. -- **`[open]` `rawUrl()` assumes Gitea and a `main` branch.** A GitHub-hosted plugin or one defaulting to - `master` gets a 404 for its icon. The forge should become part of the registry rather than inferred. +- **`[open]` `rawUrl()` assumes Gitea.** A GitHub-hosted plugin needs the other URL form. The forge should + become part of the registry rather than inferred from the URL. - **`[open]` No versions table.** `plugin_metadata` is one row per plugin — a snapshot, not a history. A store that can offer "install 1.2 instead of 1.3" needs a different shape. -- **`[open]` Nothing refreshes automatically.** Publishing a plugin update does not change the catalogue - until somebody runs the sync. A cron or a Gitea webhook is the obvious next step. - **`[open]` No search, no categories, no counts.** Three plugins do not need them. diff --git a/scripts/marketplace-cron.sh b/scripts/marketplace-cron.sh new file mode 100755 index 00000000..cca3e450 --- /dev/null +++ b/scripts/marketplace-cron.sh @@ -0,0 +1,46 @@ +#!/usr/bin/env bash +# Publish every plugin release to the marketplace catalogue. Meant for cron, on the ONE machine that +# publishes. +# +# ── Why a timer at all, when a webhook would be instant ── +# +# The sync reads bare mirrors at their newest tag, so running it on a schedule cannot publish anything +# that was not pushed — which is exactly what made a timer unsafe before, and is no longer true. A Gitea +# webhook would cut the delay from a minute to a second, and it would also mean a public write route, a +# shared secret, and a second path into the same code. A minute is not the problem anybody had: the +# complaint that started this was a release that never appeared at all. +# +# ── What it costs ── +# +# About 2.7s per tick with nothing to do — one query, one fetch per repository, no writes. Gitea is on +# this same machine, so none of that leaves the box. +# +# ── The log ── +# +# One timestamped summary line per tick, ALWAYS, including the ticks that do nothing. A fresh timestamp +# is the only thing that distinguishes "nothing to publish" from "the timer died". Plugin-level lines +# appear only for a plugin that actually moved, which is what `--quiet` buys. + +set -uo pipefail + +PLATFORM="$(cd "$(dirname "${BASH_SOURCE[0]}")/.." && pwd)" +LOG="${MARKETPLACE_SYNC_LOG:-$(dirname "$PLATFORM")/marketplace-sync.log}" +LOCK="/tmp/officer-marketplace-sync.lock" + +# Cron's PATH is three entries and bun is in none of them. +export PATH="$HOME/.bun/bin:/usr/local/bin:/usr/bin:/bin" + +cd "$PLATFORM" || exit 1 + +# Skip rather than queue. A tick that finds the previous one still running has nothing to add — the run +# in progress will publish whatever this one would have. +exec 9>"$LOCK" +flock -n 9 || exit 0 + +bun --env-file=.env scripts/marketplace-sync.ts --quiet >>"$LOG" 2>&1 + +# Bounded without logrotate, because this is one file on one machine and a config file for it would be +# more moving parts than the thing it manages. +if [ "$(wc -l <"$LOG")" -gt 20000 ]; then + tail -n 5000 "$LOG" >"$LOG.tmp" && mv "$LOG.tmp" "$LOG" +fi diff --git a/scripts/marketplace-sync.ts b/scripts/marketplace-sync.ts index 3ba40b6b..48d7694c 100644 --- a/scripts/marketplace-sync.ts +++ b/scripts/marketplace-sync.ts @@ -8,6 +8,8 @@ import { syncAll, syncPlugin } from '../src/servers/marketplace/sync'; * bun --env-file=.env scripts/marketplace-sync.ts every registered plugin * bun --env-file=.env scripts/marketplace-sync.ts photos just one * bun --env-file=.env scripts/marketplace-sync.ts --add register a new one, then sync it + * bun --env-file=.env scripts/marketplace-sync.ts --force re-publish releases already published + * bun --env-file=.env scripts/marketplace-sync.ts --quiet print only what changed * * All the reasoning is in `src/servers/marketplace/sync.ts` — this file is the command line around it. * It is a thin wrapper on purpose: the same functions are what the Gitea webhook calls, so a plugin @@ -16,7 +18,15 @@ import { syncAll, syncPlugin } from '../src/servers/marketplace/sync'; * Re-running is always safe. It overwrites and never merges. */ -const args = process.argv.slice(2); +const argv = process.argv.slice(2); +/* + `--quiet` is what the timer runs: one line per plugin that actually moved, and a summary. Not silence — + the summary is always printed, because a log whose last line is old is the only way to tell "nothing to + publish" from "the timer died", and that question does come up. +*/ +const quiet = argv.includes('--quiet'); +const force = argv.includes('--force'); +const args = argv.filter((a) => a !== '--quiet' && a !== '--force'); /** * A clone URL the whole internet can use. @@ -69,29 +79,36 @@ if (args[0] === '--add') { // unpublished, which is the correct state for one that has not made a release yet. await upsertPlugin({ appName, developer: 'unknown', repoUrl }); console.log(`[marketplace-sync] registered ${appName} → ${repoUrl}`); - const one = await syncPlugin(appName); + const one = await syncPlugin(appName, force); report([one]); process.exit(one.status === 'failed' ? 1 : 0); } -const results = args[0] ? [await syncPlugin(args[0])] : await syncAll(); +const results = args[0] ? [await syncPlugin(args[0], force)] : await syncAll(force); report(results); process.exit(results.some((r) => r.status === 'failed') ? 1 : 0); function report(results: Awaited>): void { + const stamp = new Date().toISOString(); for (const r of results) { if (r.status === 'synced') console.log(`[marketplace-sync] ${r.appName} ← ${r.tag}`); - else if (r.status === 'unreleased') console.warn(`[marketplace-sync] ${r.appName}: no tag — nothing published`); - else console.error(`[marketplace-sync] ${r.appName} failed: ${r.error}`); + else if (r.status === 'failed') console.error(`[marketplace-sync] ${r.appName} failed: ${r.error}`); + else if (quiet) continue; + else if (r.status === 'unchanged') console.log(`[marketplace-sync] ${r.appName} = ${r.tag}`); + else console.warn(`[marketplace-sync] ${r.appName}: no tag — nothing published`); } const synced = results.filter((r) => r.status === 'synced').length; + const unchanged = results.filter((r) => r.status === 'unchanged').length; const unreleased = results.filter((r) => r.status === 'unreleased').map((r) => r.appName); const failed = results.filter((r) => r.status === 'failed').map((r) => r.appName); - console.log(`[marketplace-sync] ${synced}/${results.length} published from a release`); - // Named rather than counted. "3 unreleased" is a number nobody acts on; the names are a to-do list, - // and every one of them is a plugin whose store entry is frozen until somebody tags it. - if (unreleased.length) console.warn(`[marketplace-sync] never tagged, so not updated: ${unreleased.join(', ')}`); + console.log(`[marketplace-sync] ${stamp} — ${synced} published, ${unchanged} unchanged, ${unreleased.length} untagged`); + // Named rather than counted, and not under `--quiet`: "4 untagged" is a number nobody acts on, but a + // timer that reprints the same four names every minute is a log nobody reads. The names belong in a + // run somebody asked for. + if (unreleased.length && !quiet) { + console.warn(`[marketplace-sync] never tagged, so not updated: ${unreleased.join(', ')}`); + } if (failed.length) console.error(`[marketplace-sync] failed: ${failed.join(', ')}`); } diff --git a/src/databases/marketplace_db/src/plugins/queries.ts b/src/databases/marketplace_db/src/plugins/queries.ts index 6f8083c1..dae90a04 100644 --- a/src/databases/marketplace_db/src/plugins/queries.ts +++ b/src/databases/marketplace_db/src/plugins/queries.ts @@ -115,6 +115,14 @@ export async function catalogueEntry(appName: string): Promise { +export async function listRegistry(): Promise { return marketplaceDb - .select({ id: plugins.id, appName: plugins.appName, repoUrl: plugins.repoUrl }) + .select({ + id: plugins.id, + appName: plugins.appName, + repoUrl: plugins.repoUrl, + sourceCommit: pluginMetadata.sourceCommit, + }) .from(plugins) + .leftJoin(pluginMetadata, eq(pluginMetadata.pluginId, plugins.id)) .orderBy(plugins.appName); } diff --git a/src/servers/api/plugins/router.ts b/src/servers/api/plugins/router.ts index 346ec85f..3cbda596 100644 --- a/src/servers/api/plugins/router.ts +++ b/src/servers/api/plugins/router.ts @@ -4,7 +4,7 @@ import { isSuperAdmin } from '../../super-admin'; import { mountPrefix } from '../../plugins/manifest'; import { snapshotPlugins } from '../../plugins/mount'; import { manualInstallHint, reportDependencies } from '../../plugins/os-deps'; -import { fetchCatalogue, fetchReadme } from '../../plugins/marketplace'; +import { fetchCatalogue, fetchReadme, publishedTags } from '../../plugins/marketplace'; import { findOrphanTables } from '../../plugins/orphans'; import { fetchPlugin } from '../../plugins/fetch'; import { discoverPlugins } from '../../plugins/discover'; @@ -57,7 +57,9 @@ pluginsRouter.get('/catalogue/:appName/readme', async (ctx) => { }); pluginsRouter.get('/', async (ctx) => { - const { states, broken } = await snapshotPlugins(); + // The catalogue, for the update badge — cached, never throws, and falls back to the git rule per + // plugin when the store has nothing to say. See `plugins/release.ts`. + const { states, broken } = await snapshotPlugins(await publishedTags()); // Asked per plugin rather than once, because `pm2 jlist` is a fork and most plugins have no sidecar to // ask about. A plugin that is installed and enabled but whose process is not online is the state worth // rendering differently — it is the difference between "off" and "broken". diff --git a/src/servers/marketplace/server.tsx b/src/servers/marketplace/server.tsx index a8f80efb..65b9bee6 100644 --- a/src/servers/marketplace/server.tsx +++ b/src/servers/marketplace/server.tsx @@ -18,9 +18,14 @@ import { catalogueEntry, listCatalogue } from '../../databases/marketplace_db/sr * * ── Read-only, and structurally so ── * - * There are two GET routes and no others. Registering a plugin is `scripts/marketplace-sync.ts`, run by - * hand against the database. That is not a temporary shortcut standing in for a write API: while - * submissions do not exist, a public write surface is all risk and no feature. + * There are two GET routes and no others. Everything that WRITES the catalogue — registering a plugin, + * publishing a release — is `scripts/marketplace-sync.ts` against the database, on the one machine that + * publishes. That is not a temporary shortcut standing in for a write API: while submissions do not + * exist, a public write surface is all risk and no feature. + * + * It stayed that way when publishing went onto a timer, which is the moment it would have been easy to + * add a `POST /api/sync` for a Gitea webhook. A minute of delay is cheaper than a write route on the one + * process here that has no login. * * ── CORS is wide open, deliberately ── * diff --git a/src/servers/marketplace/sync.ts b/src/servers/marketplace/sync.ts index 5508308d..0cd56d3b 100644 --- a/src/servers/marketplace/sync.ts +++ b/src/servers/marketplace/sync.ts @@ -3,6 +3,7 @@ import { homedir, tmpdir } from 'node:os'; import { join } from 'node:path'; import { listRegistry, + type RegistryRow, upsertMetadata, upsertPlugin, } from '../../databases/marketplace_db/src/plugins/queries'; @@ -61,6 +62,8 @@ import { export type SyncOutcome = | { appName: string; status: 'synced'; tag: string; version: string } + /** The release on the remote is the one already published. Nothing was written. */ + | { appName: string; status: 'unchanged'; tag: string } | { appName: string; status: 'unreleased' } | { appName: string; status: 'failed'; error: string }; @@ -187,7 +190,14 @@ function rawUrl(repoUrl: string, tag: string, path: string): string { return `${repoUrl.replace(/\.git$/, '')}/raw/tag/${encodeURIComponent(tag)}/${path}`; } -async function syncRegistered(row: { id: number; appName: string; repoUrl: string }): Promise { +/* + `force` re-publishes a release that is already published. + + Needed because "unchanged" is judged on the plugin's commit, and that is only half the input: the other + half is THIS file. Change how a field is derived — as the switch to tag-pinned icon URLs did — and every + row is stale while every commit still matches. Then the run to fix them all is `--force`. +*/ +async function syncRegistered(row: RegistryRow, force: boolean): Promise { const { appName, repoUrl } = row; const dir = await mirror(appName, repoUrl); @@ -198,6 +208,10 @@ async function syncRegistered(row: { id: number; appName: string; repoUrl: strin if (!commitAt.ok) throw new Error(`cannot resolve ${tag}`); const commit = commitAt.out; + // Before the manifest is read, not after: this is what makes a per-minute run affordable. Twenty-five + // repositories with nothing new cost one query, twenty-five fetches of nothing, and no writes at all. + if (!force && row.sourceCommit === commit) return { appName, status: 'unchanged', tag }; + const manifest = await manifestAt(dir, tag, appName, commit); // What the plugin SHIPS, read from the tree at the release rather than from a declaration — the same @@ -251,10 +265,10 @@ async function syncRegistered(row: { id: number; appName: string; repoUrl: strin } /** One plugin by app name. Unknown to the registry is a failure, not a silent no-op. */ -export async function syncPlugin(appName: string): Promise { +export async function syncPlugin(appName: string, force = false): Promise { const row = (await listRegistry()).find((r) => r.appName === appName); if (!row) return { appName, status: 'failed', error: 'not in the registry' }; - return syncRegistered(row).catch((err) => ({ + return syncRegistered(row, force).catch((err) => ({ appName, status: 'failed' as const, error: err instanceof Error ? err.message : String(err), @@ -271,12 +285,12 @@ export async function syncPlugin(appName: string): Promise { * and it has to be able to come back with current metadata rather than with whatever was true when it * was pulled. */ -export async function syncAll(): Promise { +export async function syncAll(force = false): Promise { const rows = await listRegistry(); const out: SyncOutcome[] = []; for (const row of rows) { out.push( - await syncRegistered(row).catch((err) => ({ + await syncRegistered(row, force).catch((err) => ({ appName: row.appName, status: 'failed' as const, error: err instanceof Error ? err.message : String(err), diff --git a/src/servers/plugins/marketplace.ts b/src/servers/plugins/marketplace.ts index 575759e2..2edeca0c 100644 --- a/src/servers/plugins/marketplace.ts +++ b/src/servers/plugins/marketplace.ts @@ -39,6 +39,15 @@ export type CatalogueEntry = { appName: string; publisher: string; version: string; + /** + * The git tag this entry was published from — the newest RELEASE, as opposed to whatever a manifest + * claims about itself. + * + * Optional, and absent means "cannot tell" rather than "no release": an older catalogue does not send + * the field at all, and a plugin that has never been tagged has nothing to put in it. Both send the + * badge back to the git rule. + */ + releaseTag?: string; platform: string; label: string; summary: string; @@ -556,7 +565,15 @@ const MARKETPLACE_URL = process.env.MARKETPLACE_URL ?? 'https://marketplace.offi * plugin published this morning shows up this morning. The cache is per PROCESS and dies with it — a * restart is always a fresh look. */ -const CACHE_TTL_MS = 5 * 60 * 1000; +/* + One minute, and the number changed when the badge started depending on it. + + Five was fine while this only drew the store's tiles; it is not fine now that "is there an update" is + answered from the catalogue, because the wait for a release to appear became the sync's minute PLUS + this one. The cost is one small request per process per minute, and only while somebody is looking at + a page that lists plugins. +*/ +const CACHE_TTL_MS = 60 * 1000; /** * How long to wait for the store before giving up on it. @@ -639,6 +656,22 @@ export async function fetchCatalogue(): Promise { } } +/** + * What the store says the newest release of each plugin is, keyed by app name. + * + * This is the fact the update badge is built on. It replaces every host in the fleet running `git fetch` + * against twenty-one repositories to re-derive something the catalogue already knows — see + * `plugins/release.ts` for the rule and for what happens to a plugin that has no entry here. + * + * A missing key and a null value mean the same thing to a caller and are kept apart for the reader: no + * entry at all (unregistered, or a store that could not be reached) versus an entry with no release yet. + * Both fall through to the git rule, which is the only honest answer when the store cannot speak. + */ +export async function publishedTags(): Promise> { + const entries = await fetchCatalogue().catch(() => []); + return new Map(entries.map((e) => [e.appName, e.releaseTag ?? null])); +} + /** One entry by app name, or null. Used by install to find where to clone from. */ export async function catalogueEntry(appName: string): Promise { const all = await fetchCatalogue().catch(() => BUNDLED); diff --git a/src/servers/plugins/mount.ts b/src/servers/plugins/mount.ts index 36eec46c..b04bd5c3 100644 --- a/src/servers/plugins/mount.ts +++ b/src/servers/plugins/mount.ts @@ -59,7 +59,13 @@ export type PluginsSnapshot = { * the tree while the row stayed, and there is nothing to mount, describe or offer. The row is left in the * database on purpose — deleting it here would turn "somebody moved the checkout" into silent data loss. */ -export async function snapshotPlugins(): Promise { +/** + * @param published Newest released tag per plugin, from the marketplace — `publishedTags()`. Passed by + * the one caller that renders update badges, and by nothing else on purpose: every other caller wants + * "what is on disk", and boot is among them. Making this function fetch the catalogue itself would put + * a network request with a timeout in front of mounting plugins on a machine with no internet. + */ +export async function snapshotPlugins(published?: Map): Promise { const [{ plugins, broken }, installs] = await Promise.all([discoverPlugins(), listPluginInstalls()]); const byName = new Map(installs.map((row) => [row.appName, row])); @@ -72,7 +78,7 @@ export async function snapshotPlugins(): Promise { plugin, install, outdated: !!install && install.version !== plugin.manifest.version, - release: await releaseState(plugin.dir), + release: await releaseState(plugin.dir, published?.get(plugin.appName) ?? null), }; }), ); diff --git a/src/servers/plugins/release.test.ts b/src/servers/plugins/release.test.ts index 0452b965..0d4a4947 100644 --- a/src/servers/plugins/release.test.ts +++ b/src/servers/plugins/release.test.ts @@ -94,6 +94,61 @@ describe('releaseState', () => { expect(s.updateable).toBe(true); }); + /* + The catalogue path. These are the ones that matter now: a host no longer fetches to find out what the + newest release is, so every assertion below is about the store's tag winning over local refs — and + about the fallback still being there when it has nothing to say. + */ + test('the published tag decides, without the checkout having ever fetched it', async () => { + const { work, origin, clone } = pair('published-newer'); + + // The clone is at v1 and has NEVER seen v2 — no fetch, no remote ref for it. Exactly the state a + // host is in a minute after somebody else tagged a release. + publish(work, origin, 'two', 'v1'); + git(clone, 'fetch', '-q', '--tags', 'origin'); + git(clone, 'checkout', '-q', 'v1'); + + const s = await releaseState(clone, 'v2'); + expect(s.tagAtHead).toBe('v1'); + expect(s.newestTag).toBe('v2'); + expect(s.updateable).toBe(true); + expect(s.reason).toBe('new-release'); + }); + + test('the published tag matching HEAD is up to date', async () => { + const { work, origin, clone } = pair('published-same'); + publish(work, origin, 'two', 'v1'); + git(clone, 'fetch', '-q', '--tags', 'origin'); + git(clone, 'checkout', '-q', 'v1'); + + const s = await releaseState(clone, 'v1'); + expect(s.updateable).toBe(false); + expect(s.reason).toBe('at-latest'); + }); + + test('an untagged checkout is updateable AND says what it would land on', async () => { + const { clone } = pair('published-untagged'); + const s = await releaseState(clone, 'v9'); + expect(s.tagAtHead).toBeNull(); + // The old code returned null here, so the button read "Update" with nothing after it. + expect(s.newestTag).toBe('v9'); + expect(s.updateable).toBe(true); + expect(s.reason).toBe('untagged'); + }); + + test('no published tag falls back to the git rule rather than to a guess', async () => { + const { work, origin, clone } = pair('no-entry'); + publish(work, origin, 'two', 'v1'); + git(clone, 'fetch', '-q', '--tags', 'origin'); + git(clone, 'checkout', '-q', 'v1'); + + // Null, as it is for a plugin nobody registered or a store that could not be reached. + const s = await releaseState(clone, null); + expect(s.tagAtHead).toBe('v1'); + expect(s.newestTag).toBe('v1'); + expect(s.updateable).toBe(false); + }); + test('a directory that is not a checkout is not updateable', async () => { const d = join(root, 'plain'); mkdirSync(d, { recursive: true }); diff --git a/src/servers/plugins/release.ts b/src/servers/plugins/release.ts index 3dc90259..0b414524 100644 --- a/src/servers/plugins/release.ts +++ b/src/servers/plugins/release.ts @@ -29,18 +29,32 @@ import { join } from 'node:path'; * 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. * - * ── The answer is never awaited on the network ── + * ── Where "the newest tag" comes from: the catalogue, not this machine's refs ── * - * Every answer comes from refs already on disk. This is called once per plugin on a request that renders - * a list, and twenty-one network round trips would make the plugins screen feel broken. + * Every answer here is local, and 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. That left one hard problem: + * refs on disk go stale. Tags arrived at clone and at the fetch `pullPlugin` does, so you learned an + * update existed BY UPDATING. A machine that had not touched a plugin since August compared this + * morning's release against a snapshot from before it existed and said, truthfully and uselessly, that + * it was at the latest. * - * But refs on disk go stale, and for a long time nothing refreshed them: tags arrived at clone and at - * the fetch `pullPlugin` does, so you only learned an update existed BY UPDATING. A machine that had not - * updated a plugin since August compared this morning's release against a snapshot from before it - * existed and said, truthfully and uselessly, that it was at the latest. + * That was patched with a rate-limited background `git fetch` per checkout, which worked and was the + * wrong shape: every host in the fleet polling twenty-one repositories to re-derive a fact one place + * already knows. The marketplace publishes releases now — see `servers/marketplace/sync.ts` — so the + * newest release is simply asked of the catalogue, which every host already fetches for the store. * - * So `refreshRemote` runs a fetch in the background and this request answers from what is there now. The - * NEXT read is current. Rate-limited per checkout and run one at a time — see the note on it. + * published tag (catalogue) vs tag at HEAD (this checkout) → differ, so there is an update + * + * DIFFERENCE, not ordering. `2026.08.27.01` parses fine until somebody tags `v2`, and then it fails + * silently. Two strings that are not equal is a comparison that cannot rot. + * + * ── The git rule stays, for a plugin the store cannot speak for ── + * + * A checkout of something unpublished — a plugin being written, one never registered — has no catalogue + * entry, and the fallback below is the old rule against local remote-tracking refs. Those refs are no + * longer refreshed here, so for that case this can miss a tag until something else fetches. That is the + * accepted cost: the alternative was every machine polling every repository forever to cover a case that + * only arises on a developer's own box, where `git fetch` is a thing they were about to run anyway. */ export type ReleaseState = { @@ -53,65 +67,18 @@ export type ReleaseState = { reason: 'untagged' | 'new-release' | 'at-latest' | 'not-a-checkout'; }; -/** - * A background `git fetch --tags`, at most one per checkout per REFRESH_MS. - * - * ── Not awaited, on purpose ── - * - * The caller is rendering a list. Waiting would put the network on the critical path of a page that has - * to feel instant, which is the reason this file never fetched in the first place. The request answers - * from the refs it has; the fetch it starts is for the next one. - * - * The cost of that is one stale view after a release, corrected by a refresh. The alternative — never - * fetching — is a badge that is wrong until somebody updates for an unrelated reason. - * - * ── One at a time ── - * - * Twenty-one plugins would otherwise spawn twenty-one gits at once, every ten minutes, on machines that - * are not all fast. Chained rather than pooled because there is no hurry: nothing is waiting on it. - * - * Failure is silence. A checkout with no remote, an unreachable host, a repository that has moved — none - * of those should surface as anything, because the list still has a true answer without them. - */ -/* - Two minutes, and the number is chosen for how releases are actually watched. - - It was ten. The way this gets used is: ship a plugin, open the page, look for the badge — and ten - minutes of that is indistinguishable from the feature not working. A release tagged at 12:42 was - invisible at 12:45 because the last fetch happened at 12:37, which is correct behaviour and no comfort. - - The cost is bounded by the page being open: nothing fetches when nobody is looking, and the fetches are - chained, so the worst case is one small round trip per plugin every two minutes while somebody watches - the plugins screen. -*/ -const REFRESH_MS = 2 * 60 * 1000; -const fetchedAt = new Map(); -let queue: Promise = Promise.resolve(); - -export function refreshRemote(dir: string): void { - const last = fetchedAt.get(dir) ?? 0; - if (Date.now() - last < REFRESH_MS) return; - // Stamped BEFORE the fetch runs, so a slow one cannot be started again by the next twenty requests. - fetchedAt.set(dir, Date.now()); - - queue = queue.then(async () => { - const proc = Bun.spawn(['git', '-C', dir, 'fetch', '--tags', '--quiet', 'origin'], { - stdout: 'ignore', - stderr: 'ignore', - // A host that hangs must not hold the queue: every plugin behind it would stop refreshing too. - signal: AbortSignal.timeout(20_000), - }); - await proc.exited.catch(() => {}); - }); -} - 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 { +/** + * @param publishedTag The tag the marketplace says is this plugin's newest release, or null when the + * catalogue has nothing to say — an unregistered plugin, one never tagged, or a store that could not + * be reached. Null falls through to the git rule rather than to a guess. + */ +export async function releaseState(dir: string, publishedTag: string | null = null): 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 @@ -119,12 +86,19 @@ export async function releaseState(dir: string): Promise { return { ...none, updateable: false, reason: 'not-a-checkout' }; } - // Started, not awaited: this answer comes from the refs on disk and the fetch is for the next one. - refreshRemote(dir); - // `--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 (publishedTag) { + // Reported even when HEAD is untagged, so the UI can say what pressing Update lands on. The old + // code returned a bare null here and the button read "Update" with nothing after it. + if (!tagAtHead) return { tagAtHead: null, newestTag: publishedTag, updateable: true, reason: 'untagged' }; + return publishedTag === tagAtHead + ? { tagAtHead, newestTag: publishedTag, updateable: false, reason: 'at-latest' } + : { tagAtHead, newestTag: publishedTag, updateable: true, reason: 'new-release' }; + } + 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 diff --git a/src/servers/plugins/update.ts b/src/servers/plugins/update.ts index dac87d47..ea10de3f 100644 --- a/src/servers/plugins/update.ts +++ b/src/servers/plugins/update.ts @@ -1,4 +1,5 @@ import { existsSync, readdirSync } from 'node:fs'; +import { publishedTags } from './marketplace'; import { releaseState } from './release'; import { join } from 'node:path'; import { PLUGINS_DIR } from './discover'; @@ -144,7 +145,10 @@ export async function updatePlugin( Checked BEFORE the pull, because a pull is what would make the answer stale. */ - const state = await releaseState(join(PLUGINS_DIR, appName)); + // Against the CATALOGUE, like the badge. Without it this asked local remote-tracking refs, which are + // no longer refreshed in the background — so a plugin at the latest tag it knows about would have + // short-circuited here and told the owner it was up to date without checking anything. + const state = await releaseState(join(PLUGINS_DIR, appName), (await publishedTags()).get(appName) ?? null); if (!state.updateable && state.reason === 'at-latest') { steps.push(`already at ${state.tagAtHead}, the newest release`); onStep?.(steps[steps.length - 1]!); diff --git a/src/workspaces/officerdev/src/apps/Plugins/PluginDetail.tsx b/src/workspaces/officerdev/src/apps/Plugins/PluginDetail.tsx index f8cb4c64..ee0dc577 100644 --- a/src/workspaces/officerdev/src/apps/Plugins/PluginDetail.tsx +++ b/src/workspaces/officerdev/src/apps/Plugins/PluginDetail.tsx @@ -534,9 +534,11 @@ export const PluginDetail = () => { 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"