plugins: enabling pulls first, so a plugin off for a week comes back current

A disabled plugin keeps its directory, and the directory is what the next start runs. The longer it
stays off the more certainly it is stale, and enabling would quietly run whatever was current on the
day it was switched off — with every step reporting success, which is exactly how the jellyfin
install on alpha re-applied a two-day-old checkout.

Deleting no longer has that problem, since it takes the directory with it and installing is always a
fresh clone. That left disable/enable as the one remaining way to run old code without being told.

The pull is best-effort and the enable is not. `updatePlugin` returns ok:false when a pull is
refused, and it is refused for reasons that say nothing about whether the plugin can run: a dirty
working tree — every plugin directory on this machine is one — an unreachable forge, or a directory
copied in rather than cloned. Enable is the reversible half of a pair with disable, and a switch that
will not turn back on because a network is down is worse than one running last week's code. So the
refresh reports which of those happened and the enable proceeds regardless; a re-apply that fails
falls back to the code already on disk, which was running before it was disabled.

Tested by the refusal path rather than the happy one: a directory that is not a checkout comes back
having reported "not refreshed", and the error that surfaces is about not being installed rather than
about git — which is only possible if the refusal was stepped past instead of returned.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

src/servers/api/plugins/router.ts

@@ -14,7 +14,7 @@ import {
   purgePlugin,
   type PluginActionResult,
 } from '../../plugins/install';
-import { updatePlugin, syncCapabilities } from '../../plugins/update';
+import { updatePlugin, enablePlugin, syncCapabilities } from '../../plugins/update';
 
 // /api/plugins — what is on this machine, what is installed, and the four verbs that change it.
 //
@@ -233,8 +233,10 @@ pluginsRouter.post('/:appName/purge', async (ctx) => {
   return ctx.json(await purgePlugin(ctx.req.param('appName'), { dropData: true }));
 });
 
+// Enabling refreshes the checkout first — see `enablePlugin`. A disabled plugin's directory is what the
+// next start runs, so turning one back on used to run whatever was current the day it was switched off.
 pluginsRouter.post('/:appName/enable', async (ctx) => {
-  const result = await setPluginRunning(ctx.req.param('appName'), true);
+  const result = await enablePlugin(ctx.req.param('appName'), undefined, ctx.get('user').id);
   return ctx.json(result, result.ok ? 200 : 400);
 });
 
@@ -265,7 +267,7 @@ type VerbFn = (
 
 const VERBS = {
   install: (appName, onStep, userId, onLog) => installPlugin(appName, onStep, userId, onLog),
-  enable: (appName, onStep) => setPluginRunning(appName, true, onStep),
+  enable: (appName, onStep, userId, onLog) => enablePlugin(appName, onStep, userId, onLog),
   disable: (appName, onStep) => setPluginRunning(appName, false, onStep),
   // Same function, and the verb IS the flag — see the two routes above.
   delete: (appName, onStep, _userId, onLog) => purgePlugin(appName, { dropData: false }, onStep, onLog),

src/servers/plugins/update.test.ts

@@ -2,7 +2,7 @@ import { describe, expect, test, afterAll } from 'bun:test';
 import { mkdirSync, rmSync, writeFileSync } from 'node:fs';
 import { join } from 'node:path';
 import { PLUGINS_DIR } from './discover';
-import { pullPlugin } from './update';
+import { pullPlugin, enablePlugin } from './update';
 
 // 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 +53,27 @@ describe('pullPlugin refuses rather than guesses', () => {
     }
   });
 });
+
+// ── Enabling must survive a refusal to pull ──
+//
+// `enablePlugin` refreshes the checkout before turning a plugin on, so that a plugin which sat disabled
+// for a week does not come back running the code it was switched off with. The risk in that is obvious:
+// the pull is refused for reasons that say nothing about whether the plugin can run — a dirty tree, an
+// unreachable forge, a directory that was copied in rather than cloned — and enabling is meant to be the
+// reversible half of a pair with disable.
+//
+// So this asserts the pull refusal is REPORTED and then stepped past. The enable itself still fails here,
+// because a scratch directory has no install row — and that is the point: the failure that comes back is
+// about being installed, not about git, which is only possible if the refusal did not short-circuit.
+describe('enablePlugin steps past a pull it cannot do', () => {
+  test('a directory that is not a git checkout still reaches the enable', async () => {
+    // Named without the word "git" in it, so the last assertion below is about the error and not about
+    // the fixture's own name.
+    scratch('test-enable-plain');
+    const r = await enablePlugin('test-enable-plain');
+
+    expect(r.steps.some((s) => s.includes('not refreshed') && s.includes('not a git checkout'))).toBe(true);
+    expect(r.error).toContain('not installed');
+    expect(r.error).not.toContain('git');
+  });
+});

src/servers/plugins/update.ts

@@ -2,7 +2,7 @@ import { existsSync, readdirSync } from 'node:fs';
 import { join } from 'node:path';
 import { PLUGINS_DIR } from './discover';
 import { OFFICER_ITEMS_DIR, PLATFORM_DIR } from '../data-path';
-import { installPlugin } from './install';
+import { installPlugin, setPluginRunning } from './install';
 import type { OnStep, PluginActionResult } from './install';
 import type { OnLog } from './run-logged';
 
@@ -124,6 +124,65 @@ export async function updatePlugin(
   return { ...result, steps: [...steps, ...result.steps] };
 }
 
+/**
+ * Enable a plugin, refreshing it first.
+ *
+ * ── Why enabling pulls ──
+ *
+ * A disabled plugin keeps its directory, and the directory is what the next start actually runs. So the
+ * longer something stays disabled the more certainly it is stale, and turning it back on would quietly
+ * run whatever was current on the day it was switched off. Deleting no longer has that problem — it takes
+ * the directory with it, so installing is always a fresh clone — which leaves disable/enable as the one
+ * remaining way to run old code without being told.
+ *
+ * That is not hypothetical either. It is the same shape as the jellyfin install on alpha, which re-applied
+ * a two-day-old checkout and reported success at every step.
+ *
+ * ── The pull cannot be allowed to fail the enable ──
+ *
+ * `updatePlugin` returns `ok: false` when the pull is refused, and it is refused for reasons that have
+ * nothing to do with whether the plugin can run: a dirty working tree (every plugin directory on a
+ * developer's machine is one), a forge that cannot be reached, a checkout that is not a git repository at
+ * all. Enabling is the reversible half of a pair with disable, and a switch that will not turn back on
+ * because a network is down is worse than a plugin running last week's code.
+ *
+ * So the refresh is best-effort and SAYS which of the three happened, and the enable runs either way. The
+ * re-apply is likewise allowed to fail without taking the enable with it — the code already on disk was
+ * running before it was disabled, so falling back to it is a known-good state rather than a guess.
+ */
+export async function enablePlugin(
+  appName: string,
+  onStep?: OnStep,
+  userId?: number,
+  onLog?: OnLog,
+): Promise<PluginActionResult> {
+  const steps: string[] = [];
+  const say = async (line: string) => {
+    steps.push(line);
+    await onStep?.(line);
+  };
+
+  const pulled = await pullPlugin(appName, onLog);
+  const short = (sha: string | null) => sha?.slice(0, 7) ?? '?';
+
+  if (!pulled.ok) {
+    // Not an error here, only a fact worth stating: this is running the code that is on disk.
+    await say(`not refreshed — ${pulled.error}`);
+  } else if (pulled.updated) {
+    await say(`pulled ${short(pulled.before)} → ${short(pulled.after)}`);
+    const applied = await installPlugin(appName, onStep, userId, onLog);
+    steps.push(...applied.steps);
+    if (!applied.ok) await say(`re-apply failed (${applied.error}) — enabling what is on disk instead`);
+  } else {
+    await say('already at the latest commit');
+  }
+
+  // Always. `recordPluginInstall` deliberately never touches `enabled`, so the re-apply above cannot have
+  // switched it on as a side effect — which is correct, and means this line is the one that enables.
+  const enabled = await setPluginRunning(appName, true, onStep);
+  return { ...enabled, steps: [...steps, ...enabled.steps] };
+}
+
 // ── The capabilities repository ──
 //
 // `capabilities/` is the file-based item store, and most of what is in it belongs to the owner. A small

src/workspaces/officerdev/src/apps/Plugins/PluginDetail.tsx

@@ -540,7 +540,7 @@ export const PluginDetail = () => {
           rather than leaving someone to infer it from two buttons sitting next to each other. */}
       <p className="mt-4 text-xs text-duck-dark/40">
         {plugin.installed
-          ? 'Disabling unmounts its routes and stops its sidecar, and changes nothing else — it is reversible in one click. Deleting needs it disabled first.'
+          ? 'Disabling unmounts its routes and stops its sidecar, and changes nothing else. Enabling pulls the latest version first, so a plugin that was off for a while does not come back running old code. Deleting needs it disabled first.'
           : 'Not installed, but the code is still on this machine — which is why the marketplace does not offer it. Installing again reuses this directory as it stands; deleting gets you a fresh clone.'}
       </p>
     </div>