Add plugin install command to controller, host and ctl

Installing a plugin used to take two steps, npm install followed by plugin
add, and both silently did their work in whatever directory they were run
from. The new plugin install <package> subcommand runs npm install --save
and then discovers the plugin through the existing loadPluginList logic,
so the plugin list is updated in the same step.

The command refuses to run outside a Clusterio installation, detected by
the package.json the installer writes or one that depends on a Clusterio
package. plugin add with a bare package name warns in the same situation.

Reinstalling a plugin that is already listed reports it as such instead of
failing. Installing a package without the clusterio-plugin keyword fails.

Fixes #1008

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>

README.md

@@ -167,16 +167,12 @@ Your instances (save files etc) will be stored there.
 ### Installing Plugins
 
 For well-known plugins you can select them during installation and no further steps are necessary to make them work.
-Installing plugins that are not offered by the installer consists of two steps.
-First install the package the plugin is provided by via npm, for example
+Plugins that are not offered by the installer can be installed by the name of the npm package that provides them, for example
 
-    npm install @clusterio/plugin-subspace_storage
+    npx clusteriocontroller plugin install @clusterio/plugin-subspace_storage
 
-Then tell clusterio that this plugin exists by adding it as a plugin.
-
-    npx clusteriocontroller plugin add @clusterio/plugin-subspace_storage
-
-This adds it to the default `plugin-list.json` file which in the shared folder setup is loaded by controller, host and ctl.
+This installs the package with npm and adds it to the default `plugin-list.json` file which in the shared folder setup is loaded by controller, host and ctl.
+The command must be run from the directory clusterio was installed in and will refuse to run anywhere else.
 If you have hosts or ctl installed on separate computers (or directories) then you need to repeat the plugin install process for all of them.
 The clusteriocontroller, clusteriohost and clusterioctl commands has the plugin sub-command so you do not need to install clusteriocontroller to add plugins, instead use the clusterio command you have available.
 

packages/controller/README.md

@@ -25,17 +25,28 @@ Configure plugins available to be loaded by the controller.
 The available plugins will be loaded unless they have been disabled in the configuration, see the config command for disabling plugins.
 
 
+#### `plugin install <package>`
+
+Install a plugin from npm and add it to the plugin list.
+The package is passed to `npm install --save` so anything npm accepts works, including a version like `@clusterio/plugin-subspace_storage@2.0.0`.
+This command refuses to run outside the directory Clusterio was installed in, as the plugin has to end up in that directory's node_modules to be found.
+
+For example, installing the Subspace Storage plugin:
+
+    npx clusteriocontroller plugin install @clusterio/plugin-subspace_storage
+
+Since the `plugin-list.json` is shared between controller, host and ctl you usually only need to do this once per machine.
+
+
 #### `plugin add <path>`
 
 Add plugin either by require path or relative/absolute path to the plugin directory.
 A relative path must start with ./ or ../ (or .\ and ..\ on Windows) otherwise it will be assumed to be a require path for an installed package in node_modules.
+This is mainly useful for plugins under development, packages published on npm are better installed with `plugin install`.
 
-For example, installing the Subspace Storage plugin:
+For example, adding a plugin checked out next to the Clusterio installation:
 
-    npm install @clusterio/plugin-subspace_storage
-    npx clusteriocontroller plugin add @clusterio/plugin-subspace_storage
-
-Since the `plugin-list.json` is shared between controller, host and ctl you usually only need to do this once per machine.
+    npx clusteriocontroller plugin add ../my_plugin
 
 
 #### `plugin remove <name>`

packages/ctl/README.md

@@ -27,17 +27,28 @@ Configure plugins available to be loaded by ctl.
 The available plugins will be automatically loaded and the commands they provide will be added to the ctl utility.
 
 
+#### `plugin install <package>`
+
+Install a plugin from npm and add it to the plugin list.
+The package is passed to `npm install --save` so anything npm accepts works, including a version like `@clusterio/plugin-subspace_storage@2.0.0`.
+This command refuses to run outside the directory Clusterio was installed in, as the plugin has to end up in that directory's node_modules to be found.
+
+For example, installing the Subspace Storage plugin:
+
+    npx clusterioctl plugin install @clusterio/plugin-subspace_storage
+
+Since the `plugin-list.json` is shared between controller, host and ctl you usually only need to do this once per machine.
+
+
 #### `plugin add <path>`
 
 Add plugin either by require path or relative/absolute path to the plugin directory.
 A relative path must start with ./ or ../ (or .\ and ..\ on Windows) otherwise it will be assumed to be a require path for an installed package in node_modules.
+This is mainly useful for plugins under development, packages published on npm are better installed with `plugin install`.
 
-For example, installing the Subspace Storage plugin:
+For example, adding a plugin checked out next to the Clusterio installation:
 
-    npm install @clusterio/plugin-subspace_storage
-    npx clusterioctl plugin add @clusterio/plugin-subspace_storage
-
-Since the `plugin-list.json` is shared between controller, host and ctl you usually only need to do this once per machine.
+    npx clusterioctl plugin add ../my_plugin
 
 
 #### `plugin remove <name>`

packages/host/README.md

@@ -26,17 +26,28 @@ Configure plugins available to be loaded by the host.
 The available plugins will be loaded unless they have been disabled in the configuration, see the config command for disabling plugins.
 
 
+#### `plugin install <package>`
+
+Install a plugin from npm and add it to the plugin list.
+The package is passed to `npm install --save` so anything npm accepts works, including a version like `@clusterio/plugin-subspace_storage@2.0.0`.
+This command refuses to run outside the directory Clusterio was installed in, as the plugin has to end up in that directory's node_modules to be found.
+
+For example, installing the Subspace Storage plugin:
+
+    npx clusteriohost plugin install @clusterio/plugin-subspace_storage
+
+Since the `plugin-list.json` is shared between controller, host and ctl you usually only need to do this once per machine.
+
+
 #### `plugin add <path>`
 
 Add plugin either by require path or relative/absolute path to the plugin directory.
 A relative path must start with ./ or ../ (or .\ and ..\ on Windows) otherwise it will be assumed to be a require path for an installed package in node_modules.
+This is mainly useful for plugins under development, packages published on npm are better installed with `plugin install`.
 
-For example, installing the Subspace Storage plugin:
+For example, adding a plugin checked out next to the Clusterio installation:
 
-    npm install @clusterio/plugin-subspace_storage
-    npx clusteriohost plugin add @clusterio/plugin-subspace_storage
-
-Since the `plugin-list.json` is shared between controller, host and ctl you usually only need to do this once per machine.
+    npx clusteriohost plugin add ../my_plugin
 
 
 #### `plugin remove <name>`

packages/lib/src/shared_commands.ts

@@ -2,13 +2,16 @@
  * Implementation of commands shared between controller/host/ctl.
  * @module lib/shared_commands
  */
+import fs from "fs/promises";
 import path from "path";
+import { spawn } from "child_process";
 
 import * as libConfig from "./config";
 import * as libFileOps from "./file_ops";
 import { logger } from "./logging";
 import * as libHelpers from "./helpers";
 import { LockFile } from "./LockFile";
+import { loadPluginList } from "./load_plugin_list";
 
 
 function print(...content: any[]) {
@@ -26,6 +29,7 @@ function print(...content: any[]) {
  */
 export function pluginCommand(yargs: any) {
 	yargs
+		.command("install <package>", "Install plugin from npm and add it")
 		.command("add <path>", "Add plugin by require path")
 		.command("remove <name>", "Remove plugin by name")
 		.command("list", "List all plugins and their path")
@@ -35,6 +39,65 @@ export function pluginCommand(yargs: any) {
 	;
 }
 
+const corePackages = ["@clusterio/controller", "@clusterio/host", "@clusterio/ctl"];
+
+/**
+ * Check if the current directory is a Clusterio installation
+ *
+ * A directory counts as an installation if it has a package.json created
+ * by the installer or one that depends on a Clusterio package.
+ */
+export async function isClusterioInstall(dir = process.cwd()) {
+	let packageJson: { name?: string, dependencies?: Record<string, string> };
+	try {
+		packageJson = JSON.parse(await fs.readFile(path.join(dir, "package.json"), "utf8"));
+	} catch (err: any) {
+		if (err.code === "ENOENT") {
+			return false;
+		}
+		throw err;
+	}
+	if (packageJson.name === "clusterio-install") {
+		return true;
+	}
+	return corePackages.some(name => packageJson.dependencies?.[name] !== undefined);
+}
+
+async function readDependencies(): Promise<Record<string, string>> {
+	const packageJson = JSON.parse(await fs.readFile("package.json", "utf8"));
+	return packageJson.dependencies ?? {};
+}
+
+async function packageNameFromSpec(packageSpec: string) {
+	try {
+		const packageJson = JSON.parse(await fs.readFile(path.join(packageSpec, "package.json"), "utf8"));
+		if (typeof packageJson.name === "string") {
+			return packageJson.name;
+		}
+	} catch {
+		// Not a local directory, treat it as name@version
+	}
+	return packageSpec.replace(/(?!^)@.*$/, "");
+}
+
+function npmInstall(packageSpec: string) {
+	const npm = process.platform === "win32" ? "npm.cmd" : "npm";
+	return new Promise<void>((resolve, reject) => {
+		const child = spawn(npm, ["install", "--save", "--no-audit", "--no-fund", packageSpec], {
+			stdio: "inherit",
+			shell: process.platform === "win32",
+		});
+		child.on("error", reject);
+		child.on("exit", code => {
+			if (code === 0) {
+				resolve();
+			} else {
+				reject(new Error(`npm install exited with code ${code}`));
+			}
+		});
+	});
+}
+
 /**
  * Handle yargs plugin command
  *
@@ -50,11 +113,58 @@ export async function handlePluginCommand(
 	pluginListPath: string
 ) {
 	let command = (args._ as string[])[1];
+	const wrongDirHint = `${process.cwd()} is not a Clusterio installation, run this command from the directory ` +
+		"Clusterio was installed in";
 
-	if (command === "add") {
+	if (command === "install") {
+		if (!await isClusterioInstall()) {
+			logger.error(wrongDirHint);
+			process.exitCode = 1;
+			return;
+		}
+
+		const packageSpec = args.package as string;
+		const depsBefore = await readDependencies();
+		try {
+			await npmInstall(packageSpec);
+		} catch (err: any) {
+			logger.error(`Failed to install ${packageSpec}: ${err.message}`);
+			process.exitCode = 1;
+			return;
+		}
+		const depsAfter = await readDependencies();
+
+		const newPluginList = await loadPluginList(pluginListPath);
+		const added = [...newPluginList.keys()].filter(name => !pluginList.has(name));
+		for (const name of added) {
+			pluginList.set(name, newPluginList.get(name)!);
+			print(`Installed ${name}`);
+		}
+		if (added.length) {
+			return;
+		}
+
+		// Nothing new, either the plugin was already in the list or the package is not a plugin.
+		let packageNames = Object.keys(depsAfter).filter(name => depsAfter[name] !== depsBefore[name]);
+		if (!packageNames.length) {
+			packageNames = [await packageNameFromSpec(packageSpec)];
+		}
+		const existing = [...pluginList].filter(([, requirePath]) => packageNames.includes(requirePath));
+		if (existing.length) {
+			for (const [name] of existing) {
+				print(`${name} is already installed`);
+			}
+		} else {
+			logger.error(`${packageSpec} was installed but did not provide any plugins`);
+			process.exitCode = 1;
+		}
+
+	} else if (command === "add") {
 		let pluginPath = args.path as string;
 		if (/^\.\.?[\/\\]/.test(pluginPath)) {
 			pluginPath = path.resolve(pluginPath);
+		} else if (!path.isAbsolute(pluginPath) && !await isClusterioInstall()) {
+			logger.warn(wrongDirHint);
 		}
 
 		let pluginInfo: { name: string };

test/lib/shared_commands.js

@@ -0,0 +1,129 @@
+"use strict";
+const assert = require("assert").strict;
+const fs = require("fs/promises");
+const path = require("path");
+
+const lib = require("@clusterio/lib");
+
+
+describe("lib/shared_commands", function() {
+	describe("isClusterioInstall()", function() {
+		const baseDir = path.join("temp", "test", "shared_commands", "is_install");
+
+		beforeEach(async function() {
+			await fs.rm(baseDir, { force: true, recursive: true, maxRetries: 10 });
+			await fs.mkdir(baseDir, { recursive: true });
+		});
+
+		async function writePackageJson(content) {
+			await fs.writeFile(path.join(baseDir, "package.json"), JSON.stringify(content));
+		}
+
+		it("should be false without a package.json", async function() {
+			assert.equal(await lib.isClusterioInstall(baseDir), false);
+		});
+		it("should be false for an unrelated package.json", async function() {
+			await writePackageJson({ name: "something", dependencies: { "left-pad": "^1.0.0" } });
+			assert.equal(await lib.isClusterioInstall(baseDir), false);
+		});
+		it("should be true for the installer package.json", async function() {
+			await writePackageJson({ name: "clusterio-install", private: true });
+			assert.equal(await lib.isClusterioInstall(baseDir), true);
+		});
+		it("should be true when a Clusterio package is a dependency", async function() {
+			for (const name of ["@clusterio/controller", "@clusterio/host", "@clusterio/ctl"]) {
+				await writePackageJson({ dependencies: { [name]: "^2.0.0" } });
+				assert.equal(await lib.isClusterioInstall(baseDir), true);
+			}
+		});
+	});
+
+	describe("handlePluginCommand()", function() {
+		const oldCwd = process.cwd();
+		const baseDir = path.join("temp", "test", "shared_commands", "install");
+		const pluginListPath = "plugin-list.json";
+		const testPluginPath = path.resolve("test", "file", "test_plugin");
+		let pluginList;
+
+		beforeEach(async function() {
+			await fs.rm(baseDir, { force: true, recursive: true, maxRetries: 10 });
+			await fs.mkdir(baseDir, { recursive: true });
+			process.chdir(baseDir);
+			pluginList = new Map();
+			process.exitCode = undefined;
+		});
+
+		afterEach(function() {
+			process.chdir(oldCwd);
+			process.exitCode = undefined;
+		});
+
+		async function run(...argv) {
+			await lib.handlePluginCommand({ _: ["plugin", ...argv], package: argv[1], path: argv[1], name: argv[1] },
+				pluginList, pluginListPath);
+		}
+
+		it("should refuse to install outside a Clusterio installation", async function() {
+			await run("install", testPluginPath);
+			assert.equal(process.exitCode, 1);
+			assert.equal(pluginList.size, 0);
+			await assert.rejects(fs.access("node_modules"));
+		});
+
+		it("should install and add an npm plugin", async function() {
+			this.timeout(60000);
+			await fs.writeFile("package.json", JSON.stringify({ name: "clusterio-install", private: true }));
+			await run("install", testPluginPath);
+			assert.equal(process.exitCode, undefined);
+			assert.deepEqual([...pluginList], [["test_plugin", "test_plugin"]]);
+			assert.deepEqual(JSON.parse(await fs.readFile(pluginListPath, "utf8")), [["test_plugin", "test_plugin"]]);
+			const packageJson = JSON.parse(await fs.readFile("package.json", "utf8"));
+			assert.ok(packageJson.dependencies.test_plugin);
+		});
+
+		it("should report plugins that are already installed", async function() {
+			this.timeout(60000);
+			await fs.writeFile("package.json", JSON.stringify({ name: "clusterio-install", private: true }));
+			await run("install", testPluginPath);
+			assert.deepEqual([...pluginList], [["test_plugin", "test_plugin"]]);
+			await run("install", testPluginPath);
+			assert.equal(process.exitCode, undefined);
+			await run("install", `test_plugin@file:${testPluginPath}`);
+			assert.equal(process.exitCode, undefined);
+			assert.deepEqual([...pluginList], [["test_plugin", "test_plugin"]]);
+		});
+
+		it("should fail when the package is not a plugin", async function() {
+			this.timeout(60000);
+			await fs.writeFile("package.json", JSON.stringify({ name: "clusterio-install", private: true }));
+			const notAPlugin = path.resolve("not-a-plugin");
+			await fs.mkdir(notAPlugin);
+			await fs.writeFile(path.join(notAPlugin, "package.json"), JSON.stringify({ name: "not-a-plugin" }));
+			await run("install", notAPlugin);
+			assert.equal(process.exitCode, 1);
+			assert.equal(pluginList.size, 0);
+		});
+
+		it("should fail when npm install fails", async function() {
+			this.timeout(60000);
+			await fs.writeFile("package.json", JSON.stringify({ name: "clusterio-install", private: true }));
+			await run("install", path.resolve("does-not-exist"));
+			assert.equal(process.exitCode, 1);
+			assert.equal(pluginList.size, 0);
+		});
+
+		it("should add, list and remove plugins by path", async function() {
+			await run("add", testPluginPath);
+			assert.equal(process.exitCode, undefined);
+			assert.deepEqual([...pluginList], [["test_plugin", testPluginPath]]);
+			await run("add", testPluginPath);
+			assert.equal(process.exitCode, 1);
+			process.exitCode = undefined;
+			await run("remove", "test_plugin");
+			assert.equal(process.exitCode, undefined);
+			assert.equal(pluginList.size, 0);
+			await run("remove", "test_plugin");
+			assert.equal(process.exitCode, 1);
+		});
+	});
+});