update means something now: it depends on tags
"update" was `git pull` and re-install, unconditionally, for every plugin.
nothing could say whether there was anything to pull, so the button was always
there and usually did nothing — and "update all" was twenty-one no-ops with a
spinner on it. the button's own comment admitted it: gating on `outdated` would
hide it for ever, because nothing bumped a version.
versions are dated and tagged now, so the rule the owner asked for is possible:
· a checkout NOT at a tag is always updateable — it is between releases, no
claim is being made about it, pulling is always reasonable
· a checkout AT a tag is updateable only when a NEWER tag exists
the part worth defending is how "newer" is decided. the obvious version-string
comparison 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 one nobody has
released. so newness is asked of git: the newest tag reachable from the REMOTE
branch, compared against the tag at HEAD. that holds for any names anyone
chooses, and there is a test with a tag called `beta` that sorts before `v1` to
keep it honest.
it never fetches. every answer comes from refs already on disk, because this
runs once per plugin on a request that renders a list and twenty-one network
round trips would make the screen feel broken. tags arrive at clone and at the
fetch `pullPlugin` already does.
the VERB refuses too, not just the UI. 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 — and `updateAll` iterates a list rather than asking per plugin.
"up to date · <tag>" is said out loud rather than shown as an absent button. no
button and up-to-date look identical, and only one of them is information.
verified on the real installs: example pulled to its tag and flipped to
up-to-date, caldav on an older commit stayed updateable, and updatePlugin on
example answered "already at 2026.08.27.01, the newest release" without pulling.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Officer-Session: 656f5c85-e49a-4f1f-b9f3-042a04b4d043
This commit is contained in:
@@ -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<PluginsSnapshot> {
|
||||
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 };
|
||||
}
|
||||
|
||||
@@ -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);
|
||||
});
|
||||
});
|
||||
@@ -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<string | null> => {
|
||||
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<ReleaseState> {
|
||||
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' };
|
||||
}
|
||||
@@ -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<PluginActionResult> {
|
||||
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 };
|
||||
|
||||
|
||||
Reference in New Issue
Block a user