fix(plugins): updatePlugins.sh actually updates installed plugins (closes #6670)

[JohnMcLear]

Summary

Closes #6670.

bin/updatePlugins.sh has been silently doing nothing for any non-trivial install: it detected outdated plugins via pnpm --filter ep_etherpad-lite outdated --depth=0, but installed plugins are not registered in src/package.json — bin/plugins.ts installs them via linkInstaller.installPlugin, which writes to src/plugin_packages/.versions/<name>@<version>/ and records the result in var/installed_plugins.json. pnpm has no view of them, so outdated returns empty and the script always reports All plugins are up-to-date, even with ep_markdown@11.0.5 installed against a registry latest of 11.0.18. PR #7468 fixed the npm→pnpm and install→update parts but kept the same broken detection mechanism — which is why the issue stayed open after that PR landed.

This PR enumerates the plugin list from var/installed_plugins.json and calls linkInstaller.installPlugin(name) (no version) for each entry. The installer resolves the registry-latest and overwrites the existing pinned copy, so an outdated plugin is brought to head; plugins already at latest are no-ops apart from a pnpm cache hit.

Also adds an update/up action to bin/plugins.ts so users can run pnpm run plugins update directly, mirroring the existing install / remove / list actions. bin/updatePlugins.sh becomes a one-line wrapper for backwards compatibility.

Test plan

Verified locally with a fresh develop checkout:

Scenario	Before	After
ep_markdown@11.0.5 installed, latest 11.0.18	All plugins are up-to-date (no-op)	Updating plugins to latest from registry: ep_markdown → 11.0.18
Already at latest, re-run	All plugins are up-to-date	re-pulls latest, no version change
Only ep_etherpad-lite in installed_plugins.json	All plugins are up-to-date	No plugins installed — nothing to update
var/installed_plugins.json missing	All plugins are up-to-date	No installed_plugins.json found — nothing to update
pnpm run plugins update (new direct invocation)	(action did not exist)	works the same as the wrapper

• tsc --noEmit -p . clean in both src/ and bin/
• Manual reproduction matrix above

🤖 Generated with Claude Code

[qodo-code-review]

ⓘ You've reached your Qodo monthly free-tier limit. Reviews pause until next month — upgrade your plan to continue now, or link your paid account if you already have one.

[qodo-free-for-open-source-projects]

Review Summary by Qodo

Fix plugin updates and add update action to plugins.ts

🐞 Bug fix ✨ Enhancement

Walkthroughs

Description

• Fix plugin update detection by reading from installed_plugins.json instead of pnpm outdated
• Add update/up action to bin/plugins.ts for direct plugin updates via pnpm
• Simplify updatePlugins.sh to one-line wrapper calling the new update action
• Handle edge cases: missing installed_plugins.json, no plugins installed, exclude core package

Diagram

flowchart LR
  A["updatePlugins.sh"] -->|calls| B["pnpm plugins update"]
  B -->|invokes| C["bin/plugins.ts update action"]
  C -->|reads| D["var/installed_plugins.json"]
  D -->|extracts plugin names| E["Filter out ep_etherpad-lite"]
  E -->|re-installs each| F["linkInstaller.installPlugin"]
  F -->|resolves registry-latest| G["Updates plugin to latest version"]

Loading

File Changes

1. bin/plugins.ts ✨ Enhancement +42/-1

Add update action to plugins.ts

• Added up and update to possibleActions array for new action support
• Implemented new update() function that reads installed plugins from var/installed_plugins.json
• Filters out ep_etherpad-lite core package and handles missing file gracefully
• Re-invokes linkInstaller.installPlugin for each plugin without version pin to resolve
 registry-latest
• Added switch cases for up and update actions to invoke the new update function

bin/plugins.ts

---

2. bin/updatePlugins.sh 🐞 Bug fix +1/-9

Simplify updatePlugins.sh to wrapper script

• Removed broken pnpm outdated detection logic that couldn't see installed plugins
• Replaced entire implementation with single-line wrapper calling `pnpm --filter bin run plugins
 update`
• Maintains backwards compatibility while delegating to new TypeScript update action

bin/updatePlugins.sh

---

[qodo-free-for-open-source-projects]

Code Review by Qodo

🐞 Bugs (2) 📘 Rule violations (1) 📎 Requirement gaps (0)

Context used

✅ Tickets: 🎫 `updatePlugin.sh` doesn't update plugins

1. update() lacks regression test 📘 Rule violation ☼ Reliability

Description

This bug fix adds new plugin update behavior but does not include an automated regression test that
would fail if the fix were reverted. This increases the risk of the updatePlugins regression
reappearing unnoticed.

Code

bin/plugins.ts[R81-112]

+// Re-install every plugin in installed_plugins.json without a version pin so
+// the registry-latest gets resolved and overwrites the existing pinned copy
+// in src/plugin_packages/. ep_etherpad-lite is excluded because the core is
+// vendored, not plugin-installed.
+const update = ()=> {
+  (async () => {
+    const path = settings.root+"/var/installed_plugins.json";
+    let plugins: {name: string}[];
+    try {
+      plugins = JSON.parse(fs.readFileSync(path, "utf-8")).plugins || [];
+    } catch (err: any) {
+      if (err.code === 'ENOENT') {
+        console.log("No installed_plugins.json found — nothing to update");
+        return;
+      }
+      throw err;
+    }
+    const names = plugins
+      .map((p) => p.name)
+      .filter((n) => n && n !== 'ep_etherpad-lite');
+    if (names.length === 0) {
+      console.log("No plugins installed — nothing to update");
+      return;
+    }
+    console.log(`Updating plugins to latest from registry: ${names.join(', ')}`);
+    await checkForMigration();
+    for (const name of names) {
+      await linkInstaller.installPlugin(name);
+    }
+    await persistInstalledPlugins();
+  })();
+}

Evidence

PR Compliance ID 2 requires at least one regression test for each bug fix. The diff shows only
production script changes (new update() action and wrapper script) with no accompanying test
changes.

bin/plugins.ts[81-112]
bin/updatePlugins.sh[5-5]
Best Practice: Repository guidelines

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
A bug fix was made to `bin/updatePlugins.sh`/`bin/plugins.ts` to correctly update installed plugins, but there is no automated regression test to ensure this behavior remains correct.

## Issue Context
The regression test should fail if the update action returns to the prior broken behavior (for example, not reading `var/installed_plugins.json` or not invoking the installer for installed plugins).

## Fix Focus Areas
- bin/plugins.ts[81-112]
- bin/updatePlugins.sh[5-5]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

2. Unvalidated plugin names 🐞 Bug ⛨ Security

Description

update() installs every name from var/installed_plugins.json (except ep_etherpad-lite) without
enforcing the ep_ prefix. If that file is corrupted or modified, running plugins update can
install arbitrary packages, unlike checkForMigration() which explicitly restricts to ep_
plugins.

Code

bin/plugins.ts[R98-109]

+    const names = plugins
+      .map((p) => p.name)
+      .filter((n) => n && n !== 'ep_etherpad-lite');
+    if (names.length === 0) {
+      console.log("No plugins installed — nothing to update");
+      return;
+    }
+    console.log(`Updating plugins to latest from registry: ${names.join(', ')}`);
+    await checkForMigration();
+    for (const name of names) {
+      await linkInstaller.installPlugin(name);
+    }

Evidence

The new update flow only filters out ep_etherpad-lite then calls installPlugin() on each
remaining name; the existing migration flow does a prefix check
(plugin.name.startsWith(plugins.prefix)) before installing from the file.

bin/plugins.ts[81-112]
src/static/js/pluginfw/installer.ts[81-134]
src/static/js/pluginfw/plugins.ts[36-154]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
`bin/plugins.ts` updates plugins by trusting `var/installed_plugins.json` and installing every entry by name. This should be restricted to actual Etherpad plugins (`ep_` prefix) to prevent accidental or malicious installation of arbitrary packages.

### Issue Context
`checkForMigration()` already enforces `plugin.name.startsWith(plugins.prefix)` before installing from `installed_plugins.json`, but `update()` does not.

### Fix Focus Areas
- Add an explicit `ep_`/`plugins.prefix` validation filter before invoking `installPlugin()`.
- Consider de-duplicating names (e.g., via `new Set(names)`) to avoid repeated installs if the file contains duplicates.

### Fix Focus Areas (code locations)
- bin/plugins.ts[81-112]
- src/static/js/pluginfw/installer.ts[81-134]
- src/static/js/pluginfw/plugins.ts[36-154]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

3. Update overwrites non-registry installs 🐞 Bug ≡ Correctness

Description

update() re-installs each plugin via linkInstaller.installPlugin(name) (registry resolution),
even for plugins originally installed via --path or --github. Because
persistInstalledPlugins() only persists {name, version} (no install source/spec), running update
can silently replace local/GitHub installs with the registry package of the same name.

Code

bin/plugins.ts[R81-110]

+// Re-install every plugin in installed_plugins.json without a version pin so
+// the registry-latest gets resolved and overwrites the existing pinned copy
+// in src/plugin_packages/. ep_etherpad-lite is excluded because the core is
+// vendored, not plugin-installed.
+const update = ()=> {
+  (async () => {
+    const path = settings.root+"/var/installed_plugins.json";
+    let plugins: {name: string}[];
+    try {
+      plugins = JSON.parse(fs.readFileSync(path, "utf-8")).plugins || [];
+    } catch (err: any) {
+      if (err.code === 'ENOENT') {
+        console.log("No installed_plugins.json found — nothing to update");
+        return;
+      }
+      throw err;
+    }
+    const names = plugins
+      .map((p) => p.name)
+      .filter((n) => n && n !== 'ep_etherpad-lite');
+    if (names.length === 0) {
+      console.log("No plugins installed — nothing to update");
+      return;
+    }
+    console.log(`Updating plugins to latest from registry: ${names.join(', ')}`);
+    await checkForMigration();
+    for (const name of names) {
+      await linkInstaller.installPlugin(name);
+    }
+    await persistInstalledPlugins();

Evidence

The CLI supports installing from path and GitHub, but the persisted state only records name/version.
The new update action uses only the name and always calls the registry-based install method, so it
cannot preserve the original install source.

bin/plugins.ts[27-66]
bin/plugins.ts[81-112]
bin/commonPlugins.ts[6-17]
src/static/js/pluginfw/LinkInstaller.ts[40-62]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
`plugins update` currently assumes every installed plugin should be updated from the npm registry. This breaks installs performed via `--path` or `--github` because the source information is not persisted and update always uses `installPlugin(name)`.

### Issue Context
- `install()` supports registry, `--path`, and `--github` installs.
- `persistInstalledPlugins()` only writes `{name, version}` to `installed_plugins.json`.
- `update()` reads only `{name}` then re-installs from the registry.

### Fix Focus Areas
- Extend `installed_plugins.json` persistence to record the install source/spec (registry version pin vs path vs GitHub repo).
- Update `update()` to:
 - Reinstall using the original source (path/GitHub) when that metadata exists, OR
 - Skip non-registry installs with a clear warning and require an explicit flag to replace them with registry packages.

### Fix Focus Areas (code locations)
- bin/plugins.ts[27-66]
- bin/plugins.ts[81-112]
- bin/commonPlugins.ts[6-17]
- src/static/js/pluginfw/LinkInstaller.ts[40-62]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

[qodo-free-for-open-source-projects]

1. update() lacks regression test 📘 Rule violation ☼ Reliability

This bug fix adds new plugin update behavior but does not include an automated regression test that
would fail if the fix were reverted. This increases the risk of the updatePlugins regression
reappearing unnoticed.

Agent Prompt

## Issue description
A bug fix was made to `bin/updatePlugins.sh`/`bin/plugins.ts` to correctly update installed plugins, but there is no automated regression test to ensure this behavior remains correct.

## Issue Context
The regression test should fail if the update action returns to the prior broken behavior (for example, not reading `var/installed_plugins.json` or not invoking the installer for installed plugins).

## Fix Focus Areas
- bin/plugins.ts[81-112]
- bin/updatePlugins.sh[5-5]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

[qodo-free-for-open-source-projects]

2. Unvalidated plugin names 🐞 Bug ⛨ Security

update() installs every name from var/installed_plugins.json (except ep_etherpad-lite) without
enforcing the ep_ prefix. If that file is corrupted or modified, running plugins update can
install arbitrary packages, unlike checkForMigration() which explicitly restricts to ep_
plugins.

Agent Prompt

### Issue description
`bin/plugins.ts` updates plugins by trusting `var/installed_plugins.json` and installing every entry by name. This should be restricted to actual Etherpad plugins (`ep_` prefix) to prevent accidental or malicious installation of arbitrary packages.

### Issue Context
`checkForMigration()` already enforces `plugin.name.startsWith(plugins.prefix)` before installing from `installed_plugins.json`, but `update()` does not.

### Fix Focus Areas
- Add an explicit `ep_`/`plugins.prefix` validation filter before invoking `installPlugin()`.
- Consider de-duplicating names (e.g., via `new Set(names)`) to avoid repeated installs if the file contains duplicates.

### Fix Focus Areas (code locations)
- bin/plugins.ts[81-112]
- src/static/js/pluginfw/installer.ts[81-134]
- src/static/js/pluginfw/plugins.ts[36-154]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools