From eb1e2e2a5e4510e71a02f49ec400ffed7f24d0fd Mon Sep 17 00:00:00 2001 From: bbassie <17990055+bbassie@users.noreply.github.com> Date: Thu, 29 Oct 2026 22:52:09 +0000 Subject: [PATCH 01/11] Wait for a start in progress before stopping an instance A stop request sent while the instance was starting ran alongside the start. During save preparation it did nothing and the instance came up running anyway. Once Factorio was launched it put the server into the stopping state, the start then failed with "Expected state running but state is stopping" and its cleanup killed the server mid shutdown. Stop and restart requests now wait for any start, stop or save in progress to finish first, then stop the instance if it is running. Co-Authored-By: Claude Opus 5.5 --- packages/host/src/Instance.ts | 14 ++++++++++-- test/host/Instance.js | 40 +++++++++++++++++++++++++++++++++++ test/integration/instance.js | 22 +++++++++++++++++++ 3 files changed, 74 insertions(+), 2 deletions(-) diff --git a/packages/host/src/Instance.ts b/packages/host/src/Instance.ts index f70080d3..6cee6a7b 100644 --- a/packages/host/src/Instance.ts +++ b/packages/host/src/Instance.ts @@ -1308,6 +1308,16 @@ end`.replace(/\r?\n/g, " "); } } + /** Stop the instance once a start, stop or save in progress has finished */ + async _stopWhenSettled() { + // Stopping concurrently with these races them and asserts on the server state. + // Checked right before stop() as another waiter may have started stopping. + while (transitoryStatuses.has(this._status)) { + await this._waitForSettledStatus(); + } + await this.stop(); + } + async kill() { if (this._status === "stopped") { return; @@ -1494,7 +1504,7 @@ end`.replace(/\r?\n/g, " "); } async handleInstanceRestartRequest(request: lib.InstanceRestartRequest) { - await this.stop(); + await this._stopWhenSettled(); // Stopping invalidates this instance object, a new one must be created by the host // To do this we simulate a waking an instance and sending a start request const instanceConnection = await this._host._connectInstance(this.id); @@ -1502,7 +1512,7 @@ end`.replace(/\r?\n/g, " "); } async handleInstanceStopRequest() { - await this.stop(); + await this._stopWhenSettled(); } async handleInstanceKillRequest() { diff --git a/test/host/Instance.js b/test/host/Instance.js index 0b106b78..80cca616 100644 --- a/test/host/Instance.js +++ b/test/host/Instance.js @@ -268,6 +268,46 @@ describe("class Instance", function() { }); }); + describe(".handleInstanceStopRequest()", function() { + let calls; + beforeEach(function() { + calls = []; + instance.server._state = "running"; + instance.server.stop = async () => { calls.push("stop"); instance.server._state = "stopping"; }; + instance.server.kill = async () => { calls.push("kill"); }; + instance.sendSaveListUpdate = async () => {}; + }); + + it("should stop once a start in progress finishes", async function() { + instance.notifyStatus("starting"); + let stopped = instance.handleInstanceStopRequest(); + await wait(10); + assert.deepEqual(calls, [], "stopped while starting"); + instance.notifyStatus("running"); + await stopped; + assert.deepEqual(calls, ["stop"]); + }); + + it("should do nothing if a start in progress fails", async function() { + instance.notifyStatus("starting"); + let stopped = instance.handleInstanceStopRequest(); + await wait(10); + instance.notifyStatus("stopped"); + await stopped; + assert.deepEqual(calls, []); + }); + + it("should not kill the server when stopped while stopping", async function() { + instance.notifyStatus("running"); + let first = instance.handleInstanceStopRequest(); + let second = instance.handleInstanceStopRequest(); + await first; + instance.notifyStatus("stopped"); + await second; + assert.deepEqual(calls, ["stop"]); + }); + }); + describe(".checkModPackVersion()", function() { function modPack(factorioVersion) { return lib.ModPack.fromJSON({ name: "pack", factorio_version: factorioVersion }); diff --git a/test/integration/instance.js b/test/integration/instance.js index 25bdf4f0..c12bee72 100644 --- a/test/integration/instance.js +++ b/test/integration/instance.js @@ -474,4 +474,26 @@ describe("start-all and stop-all commands", function() { assert.equal(await getInstanceStatus(testInstId3), "stopped"); }); }); + + describe("stop while starting", function() { + it("stops the instance once it has started", async function() { + this.timeout(30000); + const control = getControl(); + const started = control.sendTo({ instanceId: testInstId1 }, new lib.InstanceStartRequest()); + // Unroutable until the host has connected the instance + for (;;) { + try { + await control.sendTo({ instanceId: testInstId1 }, new lib.InstanceStopRequest()); + break; + } catch (err) { + if (!(err instanceof lib.UnavailableError)) { + throw err; + } + await lib.wait(10); + } + } + await started; + assert.equal(await getInstanceStatus(testInstId1), "stopped"); + }); + }); }); From 666dd484121f32d32f8c91d7ec7c166ff33c5456 Mon Sep 17 00:00:00 2001 From: bbassie <17990055+bbassie@users.noreply.github.com> Date: Thu, 29 Oct 2026 22:52:09 +0000 Subject: [PATCH 02/11] Show the instance control button as loading while starting or stopping The button offered Stop while the instance was starting whenever the start came from somewhere else, such as Load save, another tab or ctl. It now shows the action in progress as loading until the status settles. Kill stays available from the dropdown. Co-Authored-By: Claude Opus 5.5 --- .../web_ui/src/components/InstanceControlButton.tsx | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/packages/web_ui/src/components/InstanceControlButton.tsx b/packages/web_ui/src/components/InstanceControlButton.tsx index 1090f2b3..e62a9608 100644 --- a/packages/web_ui/src/components/InstanceControlButton.tsx +++ b/packages/web_ui/src/components/InstanceControlButton.tsx @@ -25,8 +25,10 @@ export default function InstanceControlButton(props: InstanceControlButtonProps) const instanceId = props.instance.id; const instanceStatus = props.instance.status; - const showStart = ["stopped", "stopping"].includes(instanceStatus); - const showStop = ["running", "starting"].includes(instanceStatus); + // While starting or stopping the button shows the action in progress as loading. + const showStart = ["stopped", "starting"].includes(instanceStatus); + const showStop = ["running", "stopping"].includes(instanceStatus); + const transitioning = ["starting", "stopping"].includes(instanceStatus); function actionFinish() { setSwitching(false); @@ -37,7 +39,7 @@ export default function InstanceControlButton(props: InstanceControlButtonProps) const actions: VariableDropdownButtonProps["actions"] = []; - if (account.hasPermission("core.instance.start") && showStart !== switching) { + if (account.hasPermission("core.instance.start") && showStart) { actions.push({ key: "start", label: "Start", @@ -101,7 +103,7 @@ export default function InstanceControlButton(props: InstanceControlButtonProps) {...props} type="primary" actions={actions} - loading={switching} + loading={switching || transitioning} disabled={!(showStart || showStop)} /> Date: Fri, 30 Oct 2026 00:04:15 +0000 Subject: [PATCH 03/11] Resync research after a new controller session While the host has no controller session, events between the instance and the controller are dropped in both directions, and research_sync only synced technologies on instance start. A technology finished during the outage stayed unresearched on the controller and on every other instance until the instance restarted. On a new session the instance now runs the same technology sync as on start, before taking the cluster queue. Contributions made while the controller is unreachable are summed per technology and sent on connect or resume, since progress from several instances can't be rebuilt from a state sync that keeps the highest progress. A finish made while the connection was dropped triggers the sync on resume too. sync_technologies now runs while the game is running, so it skips writing progress to a technology that finished after the dump. Fixes #1123 Co-Authored-By: Claude Opus 5.5 --- plugins/research_sync/instance.ts | 124 +++++++++++++++-------- plugins/research_sync/module/sync.lua | 3 +- plugins/research_sync/test/controller.js | 49 +++++++++ plugins/research_sync/test/instance.js | 114 ++++++++++++++++++++- 4 files changed, 246 insertions(+), 44 deletions(-) diff --git a/plugins/research_sync/instance.ts b/plugins/research_sync/instance.ts index 02771abf..464e91ea 100644 --- a/plugins/research_sync/instance.ts +++ b/plugins/research_sync/instance.ts @@ -24,9 +24,13 @@ type IpcFinished = { type IpcQueue = { queue: unknown, player?: string }; export default async function(context: InstancePluginContext) { - const { instance, logger, plugin } = context; + const { instance, host, logger, plugin } = context; let syncStarted = false; let queueEventsApplied = 0; + // Contributions made while the controller is unreachable, summed per technology. + // Unlike finished research they can't be recovered from the game state. + const pendingContributions = new Map(); + let finishedWhileDisconnected = false; // Commands are sent one at a time so they execute in order let rconQueue: Promise = Promise.resolve(); @@ -69,11 +73,68 @@ export default async function(context: InstancePluginContext) { } async function researchContribution(tech: IpcContribution) { - instance.sendTo("controller", new ContributionEvent(tech.name, tech.level, tech.contribution)); + if (host.connector.connected) { + instance.sendTo("controller", new ContributionEvent(tech.name, tech.level, tech.contribution)); + return; + } + let pending = pendingContributions.get(tech.name); + if (pending && pending.level === tech.level) { + pending.contribution += tech.contribution; + } else if (!pending || pending.level < tech.level) { + // A lower level finished here, the technology sync reports it + pendingContributions.set(tech.name, { ...tech }); + } } async function researchFinished(tech: IpcFinished) { - instance.sendTo("controller", new FinishedEvent(tech.name, tech.level)); + if (host.connector.connected) { + instance.sendTo("controller", new FinishedEvent(tech.name, tech.level)); + return; + } + finishedWhileDisconnected = true; + } + + function sendPendingContributions() { + for (let tech of pendingContributions.values()) { + instance.sendTo("controller", new ContributionEvent(tech.name, tech.level, tech.contribution)); + } + pendingContributions.clear(); + } + + async function syncTechnologies() { + let dumpJson = await sendOrderedRcon("/sc research_sync.dump_technologies()"); + let techsToSend = []; + let instanceTechs = new Map(); + for (let tech of JSON.parse(dumpJson)) { + techsToSend.push(new TechnologySync( + tech.name, + tech.level, + tech.progress || null, + tech.researched, + )); + instanceTechs.set(tech.name, tech); + } + + let controllerTechs = await instance.sendTo("controller", new SyncTechnologiesRequest(techsToSend)); + syncStarted = true; + let techsToSync = []; + for (let controllerTech of controllerTechs) { + let { name, level, progress, researched } = controllerTech; + let instanceTech = instanceTechs.get(name); + if ( + !instanceTech + || instanceTech.level !== level + || (instanceTech.progress || null) !== progress + || instanceTech.researched !== researched + ) { + techsToSync.push(controllerTech); + } + } + + if (techsToSync.length) { + let syncJson = lib.escapeString(JSON.stringify(techsToSync)); + await sendOrderedRcon(`/sc research_sync.sync_technologies("${syncJson}")`, true); + } } instance.server.on("ipc-research_sync:contribution", (tech: IpcContribution) => { @@ -128,52 +189,35 @@ export default async function(context: InstancePluginContext) { }); instance.hooks.start.attach(plugin.name, async () => { - let dumpJson = await sendOrderedRcon("/sc research_sync.dump_technologies()"); - let techsToSend = []; - let instanceTechs = new Map(); - for (let tech of JSON.parse(dumpJson)) { - techsToSend.push(new TechnologySync( - tech.name, - tech.level, - tech.progress || null, - tech.researched, - )); - instanceTechs.set(tech.name, tech); - } - - let controllerTechs = await instance.sendTo("controller", new SyncTechnologiesRequest(techsToSend)); - syncStarted = true; - let techsToSync = []; - for (let controllerTech of controllerTechs) { - let { name, level, progress, researched } = controllerTech; - let instanceTech = instanceTechs.get(name); - if ( - !instanceTech - || instanceTech.level !== level - || (instanceTech.progress || null) !== progress - || instanceTech.researched !== researched - ) { - techsToSync.push(controllerTech); - } - } - - if (techsToSync.length) { - let syncJson = lib.escapeString(JSON.stringify(techsToSync)); - await sendOrderedRcon(`/sc research_sync.sync_technologies("${syncJson}")`, true); - } + await syncTechnologies(); // A first start counts as turning following on, which seeds an empty cluster queue await syncQueue(true); }); - // QueueEvents sent while the controller session was down are lost, so take the cluster queue again. - // An edit made on this instance during the outage was never sent and is replaced: the cluster queue wins. + // Events are lost both ways while the controller session is down. Contributions are kept above, finished + // research comes from the technology sync, and the cluster queue replaces queue edits made meanwhile. instance.hooks.controllerConnectionEvent.attach(plugin.name, async (event) => { - if (event !== "connect" || !syncStarted || !["starting", "running"].includes(instance.status)) { + if (event !== "connect" && event !== "resume") { return; } try { - await syncQueue(false); + sendPendingContributions(); + if (!syncStarted || !["starting", "running"].includes(instance.status)) { + return; + } + if (event === "connect" || finishedWhileDisconnected) { + finishedWhileDisconnected = false; + try { + await syncTechnologies(); + } catch (err) { + finishedWhileDisconnected = true; + throw err; + } + } + if (event === "connect") { + await syncQueue(false); + } } catch (err: any) { unexpectedError(err); } diff --git a/plugins/research_sync/module/sync.lua b/plugins/research_sync/module/sync.lua index 65809005..3b675c43 100644 --- a/plugins/research_sync/module/sync.lua +++ b/plugins/research_sync/module/sync.lua @@ -332,7 +332,8 @@ function research_sync.sync_technologies(data) end tech.researched = true progress = nil - elseif tech_data[progressIndex] then + -- may have finished here since the dump this sync is answering + elseif tech_data[progressIndex] and not tech.researched then send_contribution(tech) progress = tech_data[progressIndex] set_technology_progress(tech, progress) diff --git a/plugins/research_sync/test/controller.js b/plugins/research_sync/test/controller.js index faef14c5..69584a15 100644 --- a/plugins/research_sync/test/controller.js +++ b/plugins/research_sync/test/controller.js @@ -164,6 +164,55 @@ describe("research_sync controller", function() { }); }); + // What instances send on reconnecting: the contributions summed during the outage, then a technology sync + describe("reconnect", function() { + const contribute = (name, level, contribution) => mock.getHandler(controller, ContributionEvent)( + new ContributionEvent(name, level, contribution), src + ); + const syncTechs = async (...techs) => new Map( + (await mock.getHandler(controller, SyncTechnologiesRequest)(new SyncTechnologiesRequest(techs), src)) + .map(tech => [tech.name, tech]) + ); + const finishedEvents = () => sent.filter(([, m]) => m instanceof FinishedEvent).map(([, m]) => m); + + it("should count progress made on every instance during the outage", async function() { + await contribute("automation", 1, 0.25); + // A and B each contributed 0.25 from the controller's 0.25 + await contribute("automation", 1, 0.25); + await syncTechs(new TechnologySync("automation", 1, 0.5, false)); + await contribute("automation", 1, 0.25); + let techs = await syncTechs(new TechnologySync("automation", 1, 0.5, false)); + assert.equal(techs.get("automation").progress, 0.75); + }); + it("should broadcast research finished during the outage once", async function() { + await contribute("automation", 1, 0.5); + await syncTechs(new TechnologySync("automation", 1, null, true)); + await syncTechs(new TechnologySync("automation", 1, null, true)); + assert.deepEqual(finishedEvents(), [new FinishedEvent("automation", 1)]); + }); + const tech = (level, progress) => new TechnologySync("mining-productivity-3", level, progress, false); + + it("should take a higher infinite research level from an instance", async function() { + await contribute("mining-productivity-3", 4, 0.9); + let techs = await syncTechs(tech(5, 0.2)); + assert.deepEqual(finishedEvents(), [new FinishedEvent("mining-productivity-3", 4)]); + assert.deepEqual(techs.get("mining-productivity-3"), tech(5, 0.2)); + }); + it("should return a higher infinite research level to an instance", async function() { + await syncTechs(tech(6, 0.1)); + sent.length = 0; + let techs = await syncTechs(tech(4, 0.5)); + assert.equal(finishedEvents().length, 0); + assert.deepEqual(techs.get("mining-productivity-3"), tech(6, 0.1)); + }); + it("should ignore contributions to a level finished elsewhere", async function() { + await syncTechs(tech(5, null)); + await contribute("mining-productivity-3", 4, 0.5); + let techs = await syncTechs(); + assert.deepEqual(techs.get("mining-productivity-3"), tech(5, null)); + }); + }); + it("should persist the queue across loads", async function() { await update(["a", "b"]); await controller.hooks.save.invoke(); diff --git a/plugins/research_sync/test/instance.js b/plugins/research_sync/test/instance.js index 844d16f2..64138a78 100644 --- a/plugins/research_sync/test/instance.js +++ b/plugins/research_sync/test/instance.js @@ -5,11 +5,12 @@ import * as mock from "../../../test/mock.js"; import loadInstancePlugin from "../dist/node/instance.js"; import { plugin as info } from "../dist/node/index.js"; import { - QueueEvent, QueueSyncRequest, QueueUpdateEvent, SyncTechnologiesRequest, + ContributionEvent, FinishedEvent, QueueEvent, QueueSyncRequest, QueueUpdateEvent, SyncTechnologiesRequest, + TechnologySync, } from "../dist/node/messages.js"; describe("research_sync instance", function() { - let instance, sent; + let instance, host, sent; const clusterQueue = ["a", "b"]; before(function() { @@ -20,7 +21,7 @@ describe("research_sync instance", function() { } }); beforeEach(async function() { - ({ instance } = await mock.loadInstancePlugin(loadInstancePlugin, info)); + ({ instance, host } = await mock.loadInstancePlugin(loadInstancePlugin, info)); sent = []; instance.sendTo = async (dst, message) => { sent.push(message); @@ -95,6 +96,7 @@ describe("research_sync instance", function() { await instance.hooks.start.invoke(); sent.length = 0; instance.server.reset(); + instance.server.rconCommandResults.set("/sc research_sync.dump_technologies()", "[]"); queueState({ follow: true, first: false }); }); @@ -108,6 +110,31 @@ describe("research_sync instance", function() { await instance.hooks.controllerConnectionEvent.invoke("resume"); assert.equal(sent.length, 0); }); + it("should sync technologies before the queue on a new session", async function() { + await instance.hooks.controllerConnectionEvent.invoke("connect"); + assert.ok(sent[0] instanceof SyncTechnologiesRequest); + assert.ok(sent[1] instanceof QueueSyncRequest); + }); + it("should apply research finished elsewhere during the outage", async function() { + instance.server.rconCommandResults.set( + "/sc research_sync.dump_technologies()", + JSON.stringify([{ name: "automation", level: 1, progress: 0.5, researched: false }]), + ); + let controllerTechs = [new TechnologySync("automation", 1, null, true)]; + let sendTo = instance.sendTo; + instance.sendTo = async (dst, message) => { + if (message instanceof SyncTechnologiesRequest) { + sent.push(message); + return controllerTechs; + } + return sendTo(dst, message); + }; + await instance.hooks.controllerConnectionEvent.invoke("connect"); + let syncRequest = sent.find(m => m instanceof SyncTechnologiesRequest); + assert.deepEqual(syncRequest.technologies, [new TechnologySync("automation", 1, 0.5, false)]); + let syncJson = lib.escapeString(JSON.stringify(controllerTechs)); + assert.ok(instance.server.rconCommands.includes(`/sc research_sync.sync_technologies("${syncJson}")`)); + }); it("should do nothing while stopped", async function() { instance.status = "stopped"; await instance.hooks.controllerConnectionEvent.invoke("connect"); @@ -122,6 +149,9 @@ describe("research_sync instance", function() { // reply resolves the request's await instance.sendTo = async (dst, message) => { sent.push(message); + if (message instanceof SyncTechnologiesRequest) { + return []; + } mock.getHandler(instance, QueueEvent)(new QueueEvent(["new"], null, "B")); return ["old"]; }; @@ -130,6 +160,84 @@ describe("research_sync instance", function() { }); }); + describe("controller outage", function() { + beforeEach(async function() { + queueState({ follow: false }); + await instance.hooks.start.invoke(); + sent.length = 0; + host.connector.connected = false; + }); + const contributions = () => sent.filter(m => m instanceof ContributionEvent); + const techSyncs = () => sent.filter(m => m instanceof SyncTechnologiesRequest); + function contribute(name, level, contribution) { + instance.server.emit("ipc-research_sync:contribution", { name, level, contribution }); + } + + it("should send contributions while connected", async function() { + host.connector.connected = true; + contribute("automation", 1, 0.25); + instance.server.emit("ipc-research_sync:finished", { name: "automation", level: 1 }); + assert.ok(sent[0] instanceof ContributionEvent); + assert.ok(sent[1] instanceof FinishedEvent); + }); + it("should sum contributions and send them on a new session", async function() { + contribute("automation", 1, 0.25); + contribute("automation", 1, 0.25); + contribute("logistics", 1, 0.125); + assert.equal(sent.length, 0); + host.connector.connected = true; + await instance.hooks.controllerConnectionEvent.invoke("connect"); + assert.deepEqual( + contributions().map(m => [m.name, m.level, m.contribution]), + [["automation", 1, 0.5], ["logistics", 1, 0.125]], + ); + assert.ok(sent.indexOf(contributions()[1]) < sent.indexOf(techSyncs()[0])); + }); + it("should keep only contributions to the highest level", async function() { + contribute("mining-productivity-3", 3, 0.5); + contribute("mining-productivity-3", 4, 0.25); + contribute("mining-productivity-3", 3, 0.5); + host.connector.connected = true; + await instance.hooks.controllerConnectionEvent.invoke("resume"); + assert.deepEqual( + contributions().map(m => [m.name, m.level, m.contribution]), + [["mining-productivity-3", 4, 0.25]], + ); + }); + it("should send contributions once", async function() { + contribute("automation", 1, 0.25); + host.connector.connected = true; + await instance.hooks.controllerConnectionEvent.invoke("resume"); + await instance.hooks.controllerConnectionEvent.invoke("resume"); + assert.equal(contributions().length, 1); + }); + it("should not sync technologies on resume when nothing finished", async function() { + contribute("automation", 1, 0.25); + host.connector.connected = true; + await instance.hooks.controllerConnectionEvent.invoke("resume"); + assert.equal(techSyncs().length, 0); + }); + it("should sync technologies on resume when research finished", async function() { + instance.server.emit("ipc-research_sync:finished", { name: "automation", level: 1 }); + assert.equal(sent.length, 0); + host.connector.connected = true; + await instance.hooks.controllerConnectionEvent.invoke("resume"); + assert.equal(techSyncs().length, 1); + await instance.hooks.controllerConnectionEvent.invoke("resume"); + assert.equal(techSyncs().length, 1); + }); + it("should retry the technology sync after it fails on resume", async function() { + instance.server.emit("ipc-research_sync:finished", { name: "automation", level: 1 }); + host.connector.connected = true; + let sendTo = instance.sendTo; + instance.sendTo = async () => { throw new Error("gone"); }; + await instance.hooks.controllerConnectionEvent.invoke("resume"); + instance.sendTo = sendTo; + await instance.hooks.controllerConnectionEvent.invoke("resume"); + assert.equal(techSyncs().length, 1); + }); + }); + it("should forward queue changes from the game", async function() { instance.server.emit("ipc-research_sync:queue", { queue: {}, player: "Bob" }); assert.equal(sent.length, 1); From 814a4a201a3908edbc59f7f3e3831e41fab72647 Mon Sep 17 00:00:00 2001 From: bbassie <17990055+bbassie@users.noreply.github.com> Date: Fri, 30 Oct 2026 00:05:23 +0000 Subject: [PATCH 04/11] Replace the PluginDetails booleans with a single state loaded, enabled, pendingInstall, removed, noEntrypoint and nameTaken allowed far more combinations than can happen, and every consumer had to know which took priority. PluginDetails now has one state, with loaded, loadsOnRestart and removed as getters over it. 2.0.3 web UIs and ctl send PluginListRequest straight to hosts and require loaded and enabled, so those are still sent next to the optional state. Plugins listed by 2.0.3 hosts get their state from loaded and enabled. The web UI planner gets the same treatment: MachinePlugin has one state in place of disabled, noEntrypoint, dependencyTaken and nameTaken. ctl host plugin list shows the state. Closes #1134 Co-Authored-By: Claude Opus 5.5 --- packages/ctl/src/commands_host.ts | 2 +- packages/lib/src/data/messages_plugin.ts | 94 +++++++++++---- packages/lib/src/plugin_loader.ts | 13 +-- .../src/components/PluginClusterStatus.tsx | 14 +-- packages/web_ui/src/util/plugin_status.ts | 108 ++++++++++-------- test/host/Host.test.js | 3 +- test/integration/clusterio.js | 6 +- test/lib/data/SystemInfo.js | 2 +- test/lib/plugin_loader.js | 26 +++-- test/messages/plugin.test.js | 48 ++++++-- test/web_ui/plugin_status.js | 24 ++-- test/web_ui/plugin_status_matrix.js | 92 +++++++-------- 12 files changed, 261 insertions(+), 171 deletions(-) diff --git a/packages/ctl/src/commands_host.ts b/packages/ctl/src/commands_host.ts index 3d20e050..91bd83c5 100644 --- a/packages/ctl/src/commands_host.ts +++ b/packages/ctl/src/commands_host.ts @@ -271,7 +271,7 @@ hostPluginCommands.add(new lib.Command({ let hostId = await lib.resolveHost(ctl, args.host); const plugins = await ctl.sendTo({ hostId }, new lib.PluginListRequest()); print(asTable(plugins.map(p => ({ - title: p.title, version: p.version, loaded: p.loaded, enabled: p.enabled, npmPackage: p.npmPackage, + title: p.title, version: p.version, state: p.state, npmPackage: p.npmPackage, })))); }, })); diff --git a/packages/lib/src/data/messages_plugin.ts b/packages/lib/src/data/messages_plugin.ts index 0514feee..46fb0cd1 100644 --- a/packages/lib/src/data/messages_plugin.ts +++ b/packages/lib/src/data/messages_plugin.ts @@ -4,57 +4,94 @@ import type { PluginNodeEnvInfo } from "../plugin.js"; import { Address, MessageRequest } from "./messages_core.js"; import type { IUser } from "./UserDetails.js"; +export const pluginStateSchema = Type.Union([ + Type.Literal("loaded"), + Type.Literal("disabled"), + Type.Literal("unloadOnRestart"), + Type.Literal("loadOnRestart"), + Type.Literal("noEntrypoint"), + Type.Literal("pendingInstall"), + Type.Literal("removed"), + Type.Literal("removedLoaded"), + Type.Literal("nameTaken"), +]); +/** + * State of a plugin on a machine + * + * - loaded: Running and loaded again on the next restart + * - disabled: Not loaded as `.load_plugin` is false + * - unloadOnRestart: Running, but disabled since startup + * - loadOnRestart: Not running, enabled since startup or skipped by recovery mode + * - noEntrypoint: Has no entrypoint for this machine, so it is never loaded here + * - pendingInstall: Installed since startup, loaded on the next restart + * - removed: Uninstalled since startup while not running + * - removedLoaded: Uninstalled since startup, running until the next restart + * - nameTaken: Not loaded as another package provides a plugin with the same name + */ +export type PluginState = Static; + /* This is similar to other plugin definitions but with lots removed */ export class PluginDetails { constructor( public name: string, public title: string, public version: string, - public loaded: boolean, - public enabled: boolean, + public state: PluginState, public description?: string, public npmPackage?: string, /** Version installed on disk, differs from version when an update is waiting for a restart */ public installedVersion?: string, - /** Installed after the machine started, will be loaded on the next restart */ - public pendingInstall?: boolean, - /** Uninstalled after the machine started, will not be loaded on the next restart */ - public removed?: boolean, /** Name in the root package.json dependencies, differs from npmPackage for aliases */ public dependencyName?: string, - /** Has no entrypoint for this machine, so it is never loaded here */ - public noEntrypoint?: boolean, - /** Not loaded as another package provides a plugin with the same name */ - public nameTaken?: boolean, ) {} static jsonSchema = Type.Object({ name: Type.String(), title: Type.String(), version: Type.String(), + // Missing from 2.0.3 and older, which require loaded and enabled instead + state: Type.Optional(pluginStateSchema), loaded: Type.Boolean(), enabled: Type.Boolean(), description: Type.Optional(Type.String()), npmPackage: Type.Optional(Type.String()), installedVersion: Type.Optional(Type.String()), - pendingInstall: Type.Optional(Type.Boolean()), - removed: Type.Optional(Type.Boolean()), dependencyName: Type.Optional(Type.String()), - noEntrypoint: Type.Optional(Type.Boolean()), - nameTaken: Type.Optional(Type.Boolean()), }); static fromJSON(json: Static) { return new this( - json.name, json.title, json.version, json.loaded, json.enabled, - json.description, json.npmPackage, json.installedVersion, json.pendingInstall, json.removed, - json.dependencyName, json.noEntrypoint, json.nameTaken, + json.name, json.title, json.version, json.state ?? this.loadState(json.loaded, json.enabled), + json.description, json.npmPackage, json.installedVersion, json.dependencyName, ); } - static fromNodeEnvInfo(pluginInfo: PluginNodeEnvInfo, loaded: boolean, enabled: boolean) { + toJSON(): Static { + return { + name: this.name, + title: this.title, + version: this.version, + state: this.state, + loaded: this.loaded, + enabled: this.loadsOnRestart, + description: this.description, + npmPackage: this.npmPackage, + installedVersion: this.installedVersion, + dependencyName: this.dependencyName, + }; + } + + /** State of a plugin known since startup */ + static loadState(loaded: boolean, enabled: boolean): PluginState { + if (loaded) { + return enabled ? "loaded" : "unloadOnRestart"; + } + return enabled ? "loadOnRestart" : "disabled"; + } + + static fromNodeEnvInfo(pluginInfo: PluginNodeEnvInfo, state: PluginState) { return new this( - pluginInfo.name, pluginInfo.title, pluginInfo.version, loaded, enabled, + pluginInfo.name, pluginInfo.title, pluginInfo.version, state, pluginInfo.description, pluginInfo.npmPackage, ); } @@ -63,10 +100,25 @@ export class PluginDetails { static fromPackageJson(packageName: string, packageJson: any, npmPackage?: string, dependencyName?: string) { const description = typeof packageJson.description === "string" ? packageJson.description : undefined; return new this( - packageName, packageName, packageJson.version, false, false, - description, npmPackage, packageJson.version, true, undefined, dependencyName, + packageName, packageName, packageJson.version, "pendingInstall", + description, npmPackage, packageJson.version, dependencyName, ); } + + /** Running on the machine now */ + get loaded() { + return ["loaded", "unloadOnRestart", "removedLoaded"].includes(this.state); + } + + /** Running on the machine after the next restart */ + get loadsOnRestart() { + return ["loaded", "loadOnRestart", "pendingInstall"].includes(this.state); + } + + /** Uninstalled since startup */ + get removed() { + return this.state === "removed" || this.state === "removedLoaded"; + } } export class PluginListRequest { diff --git a/packages/lib/src/plugin_loader.ts b/packages/lib/src/plugin_loader.ts index 04c4cf70..1aec0eee 100644 --- a/packages/lib/src/plugin_loader.ts +++ b/packages/lib/src/plugin_loader.ts @@ -128,14 +128,12 @@ export async function listPluginDetails( for (const pluginInfo of pluginInfos) { const dir = await realDir(path.dirname(pluginInfo.packagePath)); knownDirs.add(dir); + const loaded = isLoaded(pluginInfo); // Without an entrypoint for this machine the plugin has no load_plugin field here - const loadable = hasEntrypoint(pluginInfo); const plugin = PluginDetails.fromNodeEnvInfo( - pluginInfo, isLoaded(pluginInfo), loadable && isEnabled(pluginInfo), + pluginInfo, + hasEntrypoint(pluginInfo) ? PluginDetails.loadState(loaded, isEnabled(pluginInfo)) : "noEntrypoint", ); - if (!loadable) { - plugin.noEntrypoint = true; - } if (Object.hasOwn(dependencies, pluginInfo.requirePath)) { plugin.dependencyName = pluginInfo.requirePath; } @@ -147,7 +145,7 @@ export async function listPluginDetails( packageJson = await readPackageJson(path.dirname(pluginInfo.packagePath), false); if (packageJson === undefined) { // Uninstalled since startup, it won't be found on the next restart - plugin.removed = true; + plugin.state = loaded ? "removedLoaded" : "removed"; } } catch { // Ignored: npm is still writing it @@ -185,8 +183,7 @@ export async function listPluginDetails( const plugin = PluginDetails.fromPackageJson(packageName, packageJson, npmPackage, dependencyName); if (takenDirs.has(dir)) { // Skipped on startup as its plugin name is taken, will be again - plugin.pendingInstall = undefined; - plugin.nameTaken = true; + plugin.state = "nameTaken"; } plugins.push(plugin); } diff --git a/packages/web_ui/src/components/PluginClusterStatus.tsx b/packages/web_ui/src/components/PluginClusterStatus.tsx index 2faa6716..8c17234f 100644 --- a/packages/web_ui/src/components/PluginClusterStatus.tsx +++ b/packages/web_ui/src/components/PluginClusterStatus.tsx @@ -160,7 +160,7 @@ export function PluginVersions(props: { status: PluginStatus, latest?: string }) .sort(compareVersions) .reverse(); const count = (filter: (machine: MachinePlugin) => boolean) => machines.filter(filter).length; - const missing = count(machine => (!machine.plugin || Boolean(machine.plugin.removed)) && !machine.nameTaken); + const missing = count(machine => machine.state === "missing" || machine.state === "dependencyTaken"); return <> {versions.map(version => { @@ -175,14 +175,14 @@ export function PluginVersions(props: { status: PluginStatus, latest?: string }) ; })} {count(isPending) > 0 && } - {count(machine => machine.disabled) > 0 && machine.disabled)} type="secondary" + {count(machine => machine.state === "disabled") > 0 && machine.state === "disabled")} type="secondary" />} - {count(machine => machine.noEntrypoint) > 0 && machine.noEntrypoint)} type="secondary" + {count(machine => machine.state === "noEntrypoint") > 0 && machine.state === "noEntrypoint")} type="secondary" />} - {count(machine => machine.nameTaken) > 0 && machine.nameTaken)} type="secondary" + {count(machine => machine.state === "nameTaken") > 0 && machine.state === "nameTaken")} type="secondary" />} {missing > 0 && } {unknown.length > 0 && } diff --git a/packages/web_ui/src/util/plugin_status.ts b/packages/web_ui/src/util/plugin_status.ts index bf007669..d2e03cfe 100644 --- a/packages/web_ui/src/util/plugin_status.ts +++ b/packages/web_ui/src/util/plugin_status.ts @@ -16,18 +16,26 @@ export type Machine = { }; export type MachinePlugins = Machine & { plugins: lib.PluginDetails[] }; +/** + * What happens with a plugin on one machine, in order of precedence + * + * - nameTaken: Another package here provides the plugin by the same name, startup skips this one + * - dependencyTaken: Not installed, and the group's npm package name is a dependency here for + * another plugin, such as an alias + * - missing: Not installed + * - noEntrypoint: Installed but never loaded on this machine as it has no entrypoint for it + * - disabled: Installed but not loaded after the next restart + * - enabled: Installed and loaded after the next restart + */ +export type MachinePluginState = + "nameTaken" | "dependencyTaken" | "missing" | "noEntrypoint" | "disabled" | "enabled"; + /** A plugin on one machine, with the version it runs now and the version it will run after a restart */ export type MachinePlugin = Machine & { plugin?: lib.PluginDetails, + state: MachinePluginState, running?: string, afterRestart?: string, - disabled: boolean, - /** Installed but never loaded on this machine as it has no entrypoint for it */ - noEntrypoint: boolean, - /** The group's npm package name is a dependency here for another plugin, such as an alias */ - dependencyTaken: boolean, - /** Another package here provides the plugin by the same name, startup skips this one */ - nameTaken: boolean, /** A host from before installedVersion was reported, it can't install a specific version */ oldHost: boolean, }; @@ -53,45 +61,51 @@ export function compareVersions(a: string, b: string) { return strcmp(a, b); } +/** Not uninstalled since startup and not skipped by name, so its plugin name is in use */ +function holdsName(plugin: lib.PluginDetails) { + return !plugin.removed && plugin.state !== "pendingInstall" && plugin.state !== "nameTaken"; +} + +function machinePluginState( + plugins: lib.PluginDetails[], plugin: lib.PluginDetails | undefined, key: string, names: Set, +): MachinePluginState { + if (plugin?.state === "nameTaken") { + return "nameTaken"; + } + const absent = !plugin || plugin.removed; + if ((absent || plugin.state === "pendingInstall") && plugins.some( + candidate => pluginStatusKey(candidate) !== key && names.has(candidate.name) && holdsName(candidate) + )) { + return "nameTaken"; + } + if (absent) { + return plugins.some(candidate => candidate.dependencyName === key && !candidate.removed) + ? "dependencyTaken" : "missing"; + } + if (plugin.state === "noEntrypoint") { + return "noEntrypoint"; + } + return plugin.loadsOnRestart ? "enabled" : "disabled"; +} + /** The plugin with the given key on one machine, names are the plugin names known for that key */ function machinePlugin(machine: MachinePlugins, key: string, names: Set): MachinePlugin { const { plugins, ...rest } = machine; // A package removed and installed again is listed twice, the removed one may still be running const candidates = plugins.filter(candidate => pluginStatusKey(candidate) === key); const plugin = candidates.find(candidate => !candidate.removed) ?? candidates[0]; - const absent = !plugin || Boolean(plugin.removed); - const dependencyTaken = absent && plugins.some( - candidate => candidate.dependencyName === key && !candidate.removed - ); - const nameTaken = Boolean(plugin?.nameTaken) || (absent || Boolean(plugin.pendingInstall)) && plugins.some( - candidate => ( - pluginStatusKey(candidate) !== key && names.has(candidate.name) - && !candidate.removed && !candidate.pendingInstall && !candidate.nameTaken - ) - ); - const noEntrypoint = Boolean(plugin?.noEntrypoint); + const state = machinePluginState(plugins, plugin, key, names); const oldHost = !machine.isController && plugins.length > 0 && plugins.every( candidate => candidate.installedVersion === undefined && !candidate.removed ); - const entry: MachinePlugin = { - ...rest, plugin, disabled: false, noEntrypoint, dependencyTaken, nameTaken, oldHost, + return { + ...rest, + plugin, + state, + oldHost, + running: candidates.find(candidate => candidate.loaded)?.version, + afterRestart: state === "enabled" && plugin ? plugin.installedVersion ?? plugin.version : undefined, }; - // What runs now follows loaded, what runs after a restart follows enabled and what is on disk - entry.running = candidates.find(candidate => candidate.loaded)?.version; - if (nameTaken) { - return entry; - } - if (plugin?.pendingInstall) { - // Plugins load by default, so a new install runs after the next restart - entry.afterRestart = plugin.installedVersion ?? plugin.version; - } else if (plugin && !plugin.removed && !noEntrypoint) { - if (plugin.enabled) { - entry.afterRestart = plugin.installedVersion ?? plugin.version; - } else { - entry.disabled = true; - } - } - return entry; } /** Group the plugin lists of every machine by plugin */ @@ -107,7 +121,7 @@ export function computePluginStatuses( if (!titles.has(key) || plugin.loaded) { titles.set(key, plugin.title); } - if (!plugin.pendingInstall && !plugin.nameTaken) { + if (plugin.state !== "pendingInstall" && plugin.state !== "nameTaken") { pluginNames.set(key, (pluginNames.get(key) ?? new Set()).add(plugin.name)); } } @@ -154,15 +168,15 @@ export function planUpdate( if (installed && compareVersions(plugin.installedVersion ?? plugin.version, version) >= 0) { continue; } - const kind = !installed || plugin.pendingInstall ? "install" : "update"; + const kind = !installed || plugin.state === "pendingInstall" ? "install" : "update"; const skip = (reason: string, skipKind: PackageOperation["kind"] = kind) => { plan.skipped.push({ kind: skipKind, target, reason }); }; if (machine.offline) { skip("offline"); - } else if (machine.nameTaken) { + } else if (machine.state === "nameTaken") { skip("another package provides this plugin", "install"); - } else if (machine.dependencyTaken) { + } else if (machine.state === "dependencyTaken") { skip(`another package is installed as ${status.npmPackage}`, "install"); } else if (installed && !plugin.npmPackage) { // An alias, private or local package, installing the group's npm package would replace it @@ -211,18 +225,20 @@ export function machineStatusLabels(machine: MachinePlugin) { if (machine.offline) { labels.push("Offline"); } - if (machine.nameTaken) { + if (machine.state === "nameTaken") { labels.push("Another package provides this plugin"); - } else if (!machine.plugin || machine.plugin.removed) { - labels.push(machine.dependencyTaken ? "Another package is installed by this name" : "Not installed"); + } else if (machine.state === "dependencyTaken") { + labels.push("Another package is installed by this name"); + } else if (machine.state === "missing") { + labels.push("Not installed"); } else { - if (!machine.plugin.npmPackage) { + if (!machine.plugin?.npmPackage) { labels.push("Not installed from npm"); } - if (machine.noEntrypoint) { + if (machine.state === "noEntrypoint") { labels.push("No entrypoint for this machine"); } - if (machine.disabled) { + if (machine.state === "disabled") { labels.push("Disabled"); } } diff --git a/test/host/Host.test.js b/test/host/Host.test.js index 9ec08e08..0dcdc1f9 100644 --- a/test/host/Host.test.js +++ b/test/host/Host.test.js @@ -302,8 +302,7 @@ describe("host/src/Host", function() { await host.loadPlugins(); const plugins = await host.handlePluginListRequest(new lib.PluginListRequest()); const plugin = plugins.find(candidate => candidate.name === "controller_only"); - assert.equal(plugin.loaded, false); - assert.equal(plugin.noEntrypoint, true); + assert.equal(plugin.state, "noEntrypoint"); const [entry] = computePluginStatuses([ { name: "host", isController: false, address: { hostId: 1 }, plugins: [plugin] }, ]).get("controller_only").machines; diff --git a/test/integration/clusterio.js b/test/integration/clusterio.js index afb554ef..ef8fec1c 100644 --- a/test/integration/clusterio.js +++ b/test/integration/clusterio.js @@ -443,14 +443,12 @@ describe("Integration of Clusterio", function() { it("should load the plugin on the controller", async function() { const details = await pluginDetails("controller"); - assert(details.enabled, "test_plugin is not enabled on the controller"); - assert(details.loaded, "test_plugin is not loaded on the controller"); + assert.equal(details.state, "loaded", "test_plugin is not loaded on the controller"); }); it("should load the plugin on the host", async function() { const details = await pluginDetails({ hostId: 4 }); - assert(details.enabled, "test_plugin is not enabled on the host"); - assert(details.loaded, "test_plugin is not loaded on the host"); + assert.equal(details.state, "loaded", "test_plugin is not loaded on the host"); }); it("should report the plugin's title and version", async function() { diff --git a/test/lib/data/SystemInfo.js b/test/lib/data/SystemInfo.js index ae453afe..510f8915 100644 --- a/test/lib/data/SystemInfo.js +++ b/test/lib/data/SystemInfo.js @@ -41,7 +41,7 @@ describe("lib/data/SystemInfo", function() { testRoundTripJsonSerialisable(SystemInfo, [ base, [...base, []], - [...base, [new PluginDetails("name", "Title", "1.0.0", true, true, undefined, "name", "1.1.0")]], + [...base, [new PluginDetails("name", "Title", "1.0.0", "loaded", undefined, "name", "1.1.0")]], ]); }); }); diff --git a/test/lib/plugin_loader.js b/test/lib/plugin_loader.js index c7491b0b..ba999482 100644 --- a/test/lib/plugin_loader.js +++ b/test/lib/plugin_loader.js @@ -414,12 +414,12 @@ describe("lib/plugin_loader", function() { const loaded = byPackage.get("plugin-npm"); assert.equal(loaded.version, "0.0.0"); assert.equal(loaded.installedVersion, "0.0.1"); - assert.equal(loaded.pendingInstall, undefined); + assert.equal(loaded.state, "loaded"); // Dependencies of plugin packages are found the same way as on startup for (const packageName of ["plugin-aliased", "plugin-deep", "plugin-transitive"]) { const pending = byPackage.get(packageName); - assert.equal(pending.pendingInstall, true, packageName); + assert.equal(pending.state, "pendingInstall", packageName); assert.equal(pending.installedVersion, "0.0.1", packageName); assert.equal(pending.loaded, false, packageName); } @@ -513,13 +513,12 @@ describe("lib/plugin_loader", function() { }; let plugins = await lib.listPluginDetails([local], () => true, () => true); const taken = plugins.find(plugin => plugin.name === "plugin-npm"); - assert.equal(taken.nameTaken, true); - assert.equal(taken.pendingInstall, undefined); + assert.equal(taken.state, "nameTaken"); // Loaded on the next restart once the plugin with that name is gone const gone = { ...local, packagePath: path.resolve("plugins", "gone", "package.json") }; plugins = await lib.listPluginDetails([gone], () => true, () => true); - assert.equal(plugins.find(plugin => plugin.name === "plugin-npm").pendingInstall, true); + assert.equal(plugins.find(plugin => plugin.name === "plugin-npm").state, "pendingInstall"); }); it("should list a dependency made direct since startup as pending", async function() { @@ -531,7 +530,7 @@ describe("lib/plugin_loader", function() { await fs.writeFile("package.json", JSON.stringify(root)); const plugins = await lib.listPluginDetails([], () => true, () => true); const hidden = plugins.find(plugin => plugin.name === "plugin-hidden"); - assert.equal(hidden.pendingInstall, true); + assert.equal(hidden.state, "pendingInstall"); assert.equal(hidden.npmPackage, "plugin-hidden"); } finally { await fs.writeFile("package.json", rootPackageJson); @@ -573,7 +572,7 @@ describe("lib/plugin_loader", function() { assert.equal(npm.npmPackage, undefined); assert.equal(npm.dependencyName, "plugin-npm"); const deep = plugins.find(plugin => plugin.name === "plugin-deep"); - assert.equal(deep.pendingInstall, true); + assert.equal(deep.state, "pendingInstall"); assert.equal(deep.npmPackage, undefined); } finally { await fs.writeFile("package.json", rootPackageJson); @@ -637,7 +636,7 @@ describe("lib/plugin_loader", function() { npmPackage: "plugin-npm", }], () => true, () => true); assert.equal(plugin.npmPackage, undefined); - assert.equal(plugin.removed, undefined); + assert.equal(plugin.state, "loaded"); } finally { await fs.writeFile(packageJsonPath, original); } @@ -650,7 +649,7 @@ describe("lib/plugin_loader", function() { await fs.writeFile(packageJsonPath, JSON.stringify({ ...JSON.parse(original), private: true })); const plugins = await lib.listPluginDetails([], () => true, () => true); const deep = plugins.find(plugin => plugin.name === "plugin-deep"); - assert.equal(deep.pendingInstall, true); + assert.equal(deep.state, "pendingInstall"); assert.equal(deep.npmPackage, undefined); } finally { await fs.writeFile(packageJsonPath, original); @@ -705,7 +704,7 @@ describe("lib/plugin_loader", function() { assert.equal(alias.npmPackage, undefined); const published = new lib.PluginDetails( - "published", "Published", "1.0.0", true, true, undefined, "plugin-aliased", "1.0.0", + "published", "Published", "1.0.0", "loaded", undefined, "plugin-aliased", "1.0.0", ); const statuses = computePluginStatuses([ { name: "A", isController: false, address: { hostId: 1 }, plugins: [published] }, @@ -740,9 +739,12 @@ describe("lib/plugin_loader", function() { }]; const plugins = await lib.listPluginDetails(pluginInfos, () => true, () => true); const byPackage = new Map(plugins.map(plugin => [plugin.npmPackage ?? plugin.name, plugin])); - assert.equal(byPackage.get("plugin-gone").removed, true); + assert.equal(byPackage.get("plugin-gone").state, "removedLoaded"); assert.equal(byPackage.get("plugin-gone").installedVersion, undefined); - assert.equal(byPackage.get("plugin-npm").removed, undefined); + assert.equal(byPackage.get("plugin-npm").state, "loaded"); + + const [disabled] = await lib.listPluginDetails([pluginInfos[0]], () => false, () => false); + assert.equal(disabled.state, "removed"); }); it("should load existing plugin list", async function() { diff --git a/test/messages/plugin.test.js b/test/messages/plugin.test.js index 9ac42427..416058fa 100644 --- a/test/messages/plugin.test.js +++ b/test/messages/plugin.test.js @@ -38,20 +38,46 @@ describe("messages/plugin", function() { describe("PluginDetails", function() { it("has round trip json serialisation", function() { testRoundTripJsonSerialisable(lib.PluginDetails, [ - ["Name", "Title", "Version", true, true], - [ - "Name", "Title", "Version", false, false, "Description", "Npm Package", - "Installed Version", true, true, "Dependency Name", true, true, - ], + ["Name", "Title", "Version", "loaded"], + ["Name", "Title", "Version", "removedLoaded", "Description", "Npm Package", "Installed Version", "Dep"], ]); }); + it("sends loaded and enabled for 2.0.3 and older", function() { + const cases = [ + ["loaded", true, true], + ["disabled", false, false], + ["unloadOnRestart", true, false], + ["loadOnRestart", false, true], + ["noEntrypoint", false, false], + ["pendingInstall", false, true], + ["removed", false, false], + ["removedLoaded", true, false], + ["nameTaken", false, false], + ]; + for (const [state, loaded, enabled] of cases) { + const json = JSON.parse(JSON.stringify(new lib.PluginDetails("Name", "Title", "Version", state))); + assert.deepEqual([json.loaded, json.enabled], [loaded, enabled], state); + } + }); + it("reads the state of plugins listed by 2.0.3 and older", function() { + const cases = [ + [true, true, "loaded"], + [true, false, "unloadOnRestart"], + [false, true, "loadOnRestart"], + [false, false, "disabled"], + ]; + for (const [loaded, enabled, state] of cases) { + const json = { name: "Name", title: "Title", version: "1.0.0", loaded, enabled }; + assert.equal(lib.PluginDetails.fromJSON(json).state, state); + } + }); it("can be created from NodeEnvInfo interface", function() { const pluginDetails = lib.PluginDetails.fromNodeEnvInfo({ name: "Name", title: "Title", version: "Version", description: "Description", npmPackage: "Npm Package", - }, true, true); + }, "loaded"); assert.deepEqual(pluginDetails, new lib.PluginDetails( - "Name", "Title", "Version", true, true, + "Name", "Title", "Version", "loaded", "Description", "Npm Package", )); }); @@ -60,8 +86,8 @@ describe("messages/plugin", function() { "package", { version: "1.0.0", description: "Description" }, "package", "package", ); assert.deepEqual(pluginDetails, new lib.PluginDetails( - "package", "package", "1.0.0", false, false, - "Description", "package", "1.0.0", true, undefined, "package", + "package", "package", "1.0.0", "pendingInstall", + "Description", "package", "1.0.0", "package", )); const invalid = lib.PluginDetails.fromPackageJson("package", { version: "1.0.0", description: 1 }); assert.equal(invalid.description, undefined); @@ -99,7 +125,7 @@ describe("messages/plugin", function() { const response = await controlConnection.handlePluginListRequest(new lib.PluginListRequest()); assert.deepEqual(response, [new lib.PluginDetails( - "Name", "Title", "Version", true, true, + "Name", "Title", "Version", "loaded", "Description", "Npm Package", "Version", )]); }); @@ -114,7 +140,7 @@ describe("messages/plugin", function() { const response = await host.handlePluginListRequest(new lib.PluginListRequest()); assert.deepEqual(response, [new lib.PluginDetails( - "Name", "Title", "Version", true, true, + "Name", "Title", "Version", "loaded", "Description", "Npm Package", "Version", )]); }); diff --git a/test/web_ui/plugin_status.js b/test/web_ui/plugin_status.js index ddd351af..c945eaa2 100644 --- a/test/web_ui/plugin_status.js +++ b/test/web_ui/plugin_status.js @@ -13,11 +13,11 @@ describe("web_ui/util/plugin_status", function() { return { name, isController: false, address: { hostId: name.length }, plugins }; } function loaded(npmPackage, version) { - return new lib.PluginDetails("plugin", "Plugin", version, true, true, undefined, npmPackage, version); + return new lib.PluginDetails("plugin", "Plugin", version, "loaded", undefined, npmPackage, version); } function pending(packageName, npmPackage, version) { return new lib.PluginDetails( - packageName, packageName, version, false, false, undefined, npmPackage, version, true, + packageName, packageName, version, "pendingInstall", undefined, npmPackage, version, ); } function skippedText(result) { @@ -50,7 +50,7 @@ describe("web_ui/util/plugin_status", function() { it("should not update a loaded package that isn't from npm", function() { const result = plan([ machine("a", [loaded("plugin-aliased", "1.0.0")]), - machine("b", [new lib.PluginDetails("plugin-aliased", "Plugin", "1.0.0", true, true)]), + machine("b", [new lib.PluginDetails("plugin-aliased", "Plugin", "1.0.0", "loaded")]), ]); assert.deepEqual(result.operations.map(op => op.target.name), ["a"]); assert.deepEqual(skippedText(result), [{ kind: "update", text: "b (not installed from npm)" }]); @@ -58,7 +58,7 @@ describe("web_ui/util/plugin_status", function() { it("should not send a specific version to hosts that can't parse it", function() { // Hosts from before installedVersion was reported take name@version as a package name - const old = new lib.PluginDetails("plugin", "Plugin", "1.0.0", true, true, undefined, "plugin-aliased"); + const old = new lib.PluginDetails("plugin", "Plugin", "1.0.0", "loaded", undefined, "plugin-aliased"); const result = plan([ machine("a", [loaded("plugin-aliased", "1.0.0")]), machine("old", [old]), @@ -70,7 +70,7 @@ describe("web_ui/util/plugin_status", function() { }); it("should not install over a loaded alias that has the package name", function() { - const alias = new lib.PluginDetails("real_plugin", "Real", "1.0.0", true, true); + const alias = new lib.PluginDetails("real_plugin", "Real", "1.0.0", "loaded"); alias.dependencyName = "plugin-aliased"; const statuses = computePluginStatuses([ machine("a", [loaded("plugin-aliased", "1.0.0")]), @@ -110,7 +110,7 @@ describe("web_ui/util/plugin_status", function() { describe("planMachineUpdate()", function() { it("should plan for the given machine only", function() { const loaded = new lib.PluginDetails( - "plugin", "Plugin", "1.0.0", true, true, undefined, "plugin-aliased", "1.0.0", + "plugin", "Plugin", "1.0.0", "loaded", undefined, "plugin-aliased", "1.0.0", ); const statuses = computePluginStatuses([ { name: "a", isController: false, address: { hostId: 1 }, plugins: [loaded] }, @@ -136,7 +136,7 @@ describe("web_ui/util/plugin_status", function() { } function loadedPlugin(version = "1.0.0", installedVersion = version) { return new lib.PluginDetails( - "plugin", "Plugin", version, true, true, undefined, "plugin", installedVersion, + "plugin", "Plugin", version, "loaded", undefined, "plugin", installedVersion, ); } @@ -146,24 +146,22 @@ describe("web_ui/util/plugin_status", function() { assert.deepEqual(labels([]), ["Not installed"]); assert.deepEqual(labels([loadedPlugin()], { offline: true }), ["Offline"]); const disabled = new lib.PluginDetails( - "plugin", "Plugin", "1.0.0", false, false, undefined, "plugin", "1.0.0", + "plugin", "Plugin", "1.0.0", "disabled", undefined, "plugin", "1.0.0", ); assert.deepEqual(labels([disabled]), ["Disabled"]); - const local = new lib.PluginDetails("plugin", "Plugin", "1.0.0", true, true); + const local = new lib.PluginDetails("plugin", "Plugin", "1.0.0", "loaded"); assert.deepEqual(labels([local]), ["Not installed from npm"]); }); }); describe("computePluginStatuses()", function() { it("should not expect a plugin without an entrypoint for the machine to run after restart", function() { - const plugin = new lib.PluginDetails("controller_only", "Plugin", "1.0.0", false, false); - plugin.noEntrypoint = true; + const plugin = new lib.PluginDetails("controller_only", "Plugin", "1.0.0", "noEntrypoint"); const [status] = computePluginStatuses([ { name: "host", isController: false, address: { hostId: 1 }, plugins: [plugin] }, ]).values(); const [entry] = status.machines; - assert.equal(entry.noEntrypoint, true); - assert.equal(entry.disabled, false); + assert.equal(entry.state, "noEntrypoint"); assert.equal(entry.running, undefined); assert.equal(entry.afterRestart, undefined); }); diff --git a/test/web_ui/plugin_status_matrix.js b/test/web_ui/plugin_status_matrix.js index 778820a2..a8a3c420 100644 --- a/test/web_ui/plugin_status_matrix.js +++ b/test/web_ui/plugin_status_matrix.js @@ -9,62 +9,64 @@ const target = "2.0.0"; const versions = ["1.0.0", "2.0.0", "3.0.0"]; function details(fields) { - const plugin = new lib.PluginDetails( - fields.name, fields.name, fields.version, fields.loaded, fields.enabled, undefined, fields.npmPackage, - fields.installedVersion, fields.pendingInstall, fields.removed, fields.dependencyName, fields.noEntrypoint, - fields.nameTaken, + return new lib.PluginDetails( + fields.name, fields.name, fields.version, fields.state, undefined, fields.npmPackage, + fields.installedVersion, fields.dependencyName, ); - return plugin; } function* states() { yield { label: "absent", plugins: [] }; for (const version of versions) { for (const installed of [version, ...versions.filter(other => other !== version)]) { - for (const enabled of [true, false]) { - for (const loaded of enabled ? [true] : [true, false]) { - const state = `${enabled ? "enabled" : "disabled"}${loaded ? " loaded" : ""}`; - yield { - label: `npm ${version} on disk ${installed} ${state}`, - entry: { source: "npm", version, installed, enabled, loaded }, - plugins: [details({ - name: "plugin", version, loaded, enabled, npmPackage: "pkg", - installedVersion: installed, dependencyName: "pkg", - })], - }; - } + for (const [state, loaded, enabled] of [ + ["loaded", true, true], + ["unloadOnRestart", true, false], + ["loadOnRestart", false, true], + ["disabled", false, false], + ]) { + yield { + label: `npm ${version} on disk ${installed} ${state}`, + entry: { source: "npm", version, installed, enabled, loaded }, + plugins: [details({ + name: "plugin", version, state, npmPackage: "pkg", + installedVersion: installed, dependencyName: "pkg", + })], + }; } } - yield { - label: `npm ${version} removed`, - entry: { source: "npm", version, removed: true }, - plugins: [details({ - name: "plugin", version, loaded: true, enabled: true, npmPackage: "pkg", - removed: true, dependencyName: "pkg", - })], - }; + for (const loaded of [true, false]) { + yield { + label: `npm ${version} removed${loaded ? " while loaded" : ""}`, + entry: { source: "npm", version, removed: true, loaded }, + plugins: [details({ + name: "plugin", version, state: loaded ? "removedLoaded" : "removed", npmPackage: "pkg", + dependencyName: "pkg", + })], + }; + } yield { label: `npm ${version} no entrypoint`, entry: { source: "npm", version, installed: version, noEntrypoint: true }, plugins: [details({ - name: "plugin", version, loaded: false, enabled: false, npmPackage: "pkg", - installedVersion: version, dependencyName: "pkg", noEntrypoint: true, + name: "plugin", version, state: "noEntrypoint", npmPackage: "pkg", + installedVersion: version, dependencyName: "pkg", })], }; yield { label: `npm ${version} pending`, entry: { source: "npm", version, installed: version, pending: true }, plugins: [details({ - name: "pkg", version, loaded: false, enabled: false, npmPackage: "pkg", - installedVersion: version, pendingInstall: true, dependencyName: "pkg", + name: "pkg", version, state: "pendingInstall", npmPackage: "pkg", + installedVersion: version, dependencyName: "pkg", })], }; yield { label: `alias ${version} pending`, entry: { source: "alias", version, installed: version, pending: true }, plugins: [details({ - name: "pkg", version, loaded: false, enabled: false, - installedVersion: version, pendingInstall: true, dependencyName: "pkg", + name: "pkg", version, state: "pendingInstall", + installedVersion: version, dependencyName: "pkg", })], }; yield { @@ -72,12 +74,12 @@ function* states() { entry: { source: "npm", version, installed: target, replaced: true }, plugins: [ details({ - name: "plugin", version, loaded: true, enabled: true, npmPackage: "pkg", - removed: true, dependencyName: "pkg", + name: "plugin", version, state: "removedLoaded", npmPackage: "pkg", + dependencyName: "pkg", }), details({ - name: "pkg", version: target, loaded: false, enabled: false, npmPackage: "pkg", - installedVersion: target, pendingInstall: true, dependencyName: "pkg", + name: "pkg", version: target, state: "pendingInstall", npmPackage: "pkg", + installedVersion: target, dependencyName: "pkg", }), ], }; @@ -86,7 +88,7 @@ function* states() { label: "alias loaded as pkg", taken: "dependency", plugins: [details({ - name: "real_plugin", version: "1.0.0", loaded: true, enabled: true, + name: "real_plugin", version: "1.0.0", state: "loaded", installedVersion: "1.0.0", dependencyName: "pkg", })], }; @@ -95,10 +97,10 @@ function* states() { label: `npm ${version} skipped as its name is taken`, taken: "name", plugins: [ - details({ name: "plugin", version: "1.0.0", loaded: true, enabled: true, installedVersion: "1.0.0" }), + details({ name: "plugin", version: "1.0.0", state: "loaded", installedVersion: "1.0.0" }), details({ - name: "pkg", version, loaded: false, enabled: false, npmPackage: "pkg", - installedVersion: version, dependencyName: "pkg", nameTaken: true, + name: "pkg", version, state: "nameTaken", npmPackage: "pkg", + installedVersion: version, dependencyName: "pkg", }), ], entry: { source: "npm", version, installed: version }, @@ -108,17 +110,17 @@ function* states() { label: "local plugin with the same name", taken: "name", plugins: [details({ - name: "plugin", version: "1.0.0", loaded: true, enabled: true, installedVersion: "1.0.0", + name: "plugin", version: "1.0.0", state: "loaded", installedVersion: "1.0.0", })], }; yield { label: "local plugin with the same name and pkg pending", taken: "name", plugins: [ - details({ name: "plugin", version: "1.0.0", loaded: true, enabled: true, installedVersion: "1.0.0" }), + details({ name: "plugin", version: "1.0.0", state: "loaded", installedVersion: "1.0.0" }), details({ - name: "pkg", version: "1.0.0", loaded: false, enabled: false, npmPackage: "pkg", - installedVersion: "1.0.0", pendingInstall: true, dependencyName: "pkg", + name: "pkg", version: "1.0.0", state: "pendingInstall", npmPackage: "pkg", + installedVersion: "1.0.0", dependencyName: "pkg", }), ], }; @@ -145,7 +147,7 @@ function expected(state) { } let running; let afterRestart; - if (entry?.loaded || entry?.removed || entry?.replaced) { + if (entry?.loaded || entry?.replaced) { running = entry.version; } if (entry && !entry.removed && !entry.noEntrypoint && (entry.pending || entry.replaced || entry.enabled)) { @@ -159,7 +161,7 @@ function semverGte(a, b) { } const reference = { name: "ref", isController: true, address: "controller", plugins: [details({ - name: "plugin", version: "1.0.0", loaded: true, enabled: true, npmPackage: "pkg", + name: "plugin", version: "1.0.0", state: "loaded", npmPackage: "pkg", installedVersion: "1.0.0", dependencyName: "pkg", })] }; From b989815829ec8f0c36a37ff676ccdc6b3095213a Mon Sep 17 00:00:00 2001 From: bbassie <17990055+bbassie@users.noreply.github.com> Date: Fri, 30 Oct 2026 00:16:28 +0000 Subject: [PATCH 05/11] Speed up CI setup and coverage upload pnpm/action-setup installs a bootstrap pnpm and then switches to the packageManager version, about 10 s per job; npm installs it in 3 s. setup-node took 4 s probing package managers for a cache it never used. The pnpm store cache took 4 s to restore and 5 s to save, more than downloading the packages costs, and the build cache no longer pays off with TypeScript 7: pnpm install is 13-23 s without either cache against 17-31 s with them. The Codecov action searched the workspace and uploaded the raw V8 coverage in coverage/tmp next to lcov.info, 72 MB taking 11 s. The version compatibility job ran tsc --build again after pnpm install had already built everything. Cancel superseded runs of branches and pull requests. Co-Authored-By: Claude Opus 5.5 --- .github/actions/pnpm-install/action.yml | 37 ++++--------------------- .github/workflows/lua-lint.yml | 4 +++ .github/workflows/node.js.yml | 10 +++++-- 3 files changed, 18 insertions(+), 33 deletions(-) diff --git a/.github/actions/pnpm-install/action.yml b/.github/actions/pnpm-install/action.yml index 7a5037e5..42112fc2 100644 --- a/.github/actions/pnpm-install/action.yml +++ b/.github/actions/pnpm-install/action.yml @@ -1,5 +1,5 @@ name: Install dependencies -description: Set up pnpm and Node.js, restore caches and run pnpm install +description: Set up pnpm and Node.js and run pnpm install inputs: node-version: @@ -12,40 +12,15 @@ inputs: runs: using: composite steps: - - uses: pnpm/action-setup@v6 - uses: actions/setup-node@v7 with: node-version: ${{ inputs.node-version }} + package-manager-cache: false - # The lockfile is not committed, so the usual lockfile hash key does not - # apply. Instead a new store snapshot is saved whenever a package.json - # changes or a week passes, and every run restores the most recent one. - - id: store - shell: bash - run: | - echo "path=$(pnpm store path --silent)" >> "$GITHUB_OUTPUT" - echo "week=$(date -u +%G-%V)" >> "$GITHUB_OUTPUT" - - uses: actions/cache@v6 - with: - path: ${{ steps.store.outputs.path }} - key: pnpm-store-${{ runner.os }}-${{ steps.store.outputs.week }}-${{ hashFiles('**/package.json', 'pnpm-workspace.yaml') }} - restore-keys: pnpm-store-${{ runner.os }}- - - # The prepare scripts run tsc --build, which is incremental when its - # previous output is present. Every run saves its output and the next run - # starts from the most recent one. - - if: inputs.ignore-scripts != 'true' - uses: actions/cache@v6 - with: - path: | - packages/*/dist - plugins/*/dist - key: build-${{ runner.os }}-${{ github.sha }} - restore-keys: build-${{ runner.os }}- + # pnpm/action-setup installs a bootstrap pnpm and then switches to the + # packageManager version, which takes about 10 seconds. + - shell: bash + run: npm install --global --no-audit --no-fund "$(node -p 'require("./package.json").packageManager.split("+")[0]')" - shell: bash run: pnpm i --no-frozen-lockfile ${{ inputs.ignore-scripts == 'true' && '--ignore-scripts' || '' }} - # Drop packages the restored store had but this install did not use, so - # the snapshot does not grow with every version bump. - - shell: bash - run: pnpm store prune diff --git a/.github/workflows/lua-lint.yml b/.github/workflows/lua-lint.yml index 7e29a49f..0d1c9cde 100644 --- a/.github/workflows/lua-lint.yml +++ b/.github/workflows/lua-lint.yml @@ -4,6 +4,10 @@ on: push: pull_request: +concurrency: + group: ${{ github.workflow }}-${{ github.ref == 'refs/heads/master' && github.sha || github.ref }} + cancel-in-progress: true + env: EMMYLUA_CHECK_VERSION: "0.25.1" diff --git a/.github/workflows/node.js.yml b/.github/workflows/node.js.yml index d7a76b97..9a0508f6 100644 --- a/.github/workflows/node.js.yml +++ b/.github/workflows/node.js.yml @@ -7,6 +7,12 @@ on: push: pull_request: +# Superseded runs of a branch or pull request are cancelled. Runs on master +# each get their own group so every commit there is tested. +concurrency: + group: ${{ github.workflow }}-${{ github.ref == 'refs/heads/master' && github.sha || github.ref }} + cancel-in-progress: true + env: NODE_VERSION: 24.x @@ -54,6 +60,8 @@ jobs: uses: codecov/codecov-action@v7 with: files: ./coverage/lcov.info + # Otherwise the raw V8 coverage in coverage/tmp is uploaded as well + disable_search: true token: ${{ secrets.CODECOV_TOKEN }} - name: Upload test logs for debugging if: ${{ !cancelled() }} @@ -87,8 +95,6 @@ jobs: uses: ./.github/actions/pnpm-install with: node-version: ${{ env.NODE_VERSION }} - - name: Build - run: pnpm run build - name: Test ${{ matrix.controller }} controller with ${{ matrix.host }} host run: > node scripts/version-compat-test.js From 5df375d4909c901d2513a882c2e09be894c2c7bb Mon Sep 17 00:00:00 2001 From: bbassie <17990055+bbassie@users.noreply.github.com> Date: Fri, 30 Oct 2026 18:41:24 +0000 Subject: [PATCH 06/11] Take FieldData from antd instead of rc-field-form web_ui depended on rc-field-form 1.x for a single type import, while antd 5 uses rc-field-form 2.x. Derive the type from antd's FormProps and drop the dependency. Co-Authored-By: Claude Opus 5.5 --- packages/web_ui/package.json | 1 - packages/web_ui/src/components/ModPackViewPage.tsx | 5 +++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/packages/web_ui/package.json b/packages/web_ui/package.json index 3f010034..91b86911 100644 --- a/packages/web_ui/package.json +++ b/packages/web_ui/package.json @@ -45,7 +45,6 @@ "os-browserify": "^0.3.0", "path-browserify": "^1.0.1", "process": "^0.11.10", - "rc-field-form": "^1.44.0", "react": "catalog:", "react-dom": "catalog:", "react-markdown": "catalog:", diff --git a/packages/web_ui/src/components/ModPackViewPage.tsx b/packages/web_ui/src/components/ModPackViewPage.tsx index 3aba7619..575b3361 100644 --- a/packages/web_ui/src/components/ModPackViewPage.tsx +++ b/packages/web_ui/src/components/ModPackViewPage.tsx @@ -1,11 +1,10 @@ import React, { Fragment, memo, useCallback, useEffect, useContext, useRef, useState } from "react"; import { useNavigate, useParams } from "react-router-dom"; -import { FieldData } from "rc-field-form/lib/interface"; import { Button, Card, ColorPicker, Checkbox, Col, ConfigProvider, Descriptions, Form, Input, Pagination, Popconfirm, Row, Table, Tag, Typography, Select, Skeleton, Space, Spin, Switch, Modal, Tooltip, - TablePaginationConfig, Alert, notification, + TablePaginationConfig, Alert, notification, FormProps, } from "antd"; import { @@ -35,6 +34,8 @@ const { logger } = lib; const { Text } = Typography; const strcmp = new Intl.Collator(undefined, { numeric: true, sensitivity: "base" }).compare; +type FieldData = NonNullable[number]; + type ModChange = { type: "name" | "description" From 41e3f50dbe9f3d243ae4c20d185891e340bfd412 Mon Sep 17 00:00:00 2001 From: bbassie <17990055+bbassie@users.noreply.github.com> Date: Fri, 30 Oct 2026 18:42:36 +0000 Subject: [PATCH 07/11] Move typescript to web_ui devDependencies web_ui only needs typescript to build its own types. The runtime dependency is a leftover from webpack's ts-loader, Vite doesn't use it, and it pulls in a 30 MB native compiler for anyone installing web_ui. Co-Authored-By: Claude Opus 5.5 --- packages/web_ui/package.json | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/web_ui/package.json b/packages/web_ui/package.json index 91b86911..13c6551d 100644 --- a/packages/web_ui/package.json +++ b/packages/web_ui/package.json @@ -52,7 +52,6 @@ "react-router-dom": "catalog:", "semver": "catalog:", "stream-browserify": "^3.0.0", - "typescript": "catalog:", "util": "^0.12.5", "vite": "catalog:" }, @@ -60,6 +59,7 @@ "@types/node": "catalog:", "@types/react": "catalog:", "@types/react-dom": "^18.3.7", - "@types/semver": "catalog:" + "@types/semver": "catalog:", + "typescript": "catalog:" } } From c9db2fb29e5d2bfd27c32590962fdbaaa16448bb Mon Sep 17 00:00:00 2001 From: bbassie <17990055+bbassie@users.noreply.github.com> Date: Fri, 30 Oct 2026 18:42:59 +0000 Subject: [PATCH 08/11] Use native FormData in controller route tests Node's FormData and Blob do what the form-data package did here, and fetch sets the multipart Content-Type header itself. Co-Authored-By: Claude Opus 5.5 --- package.json | 1 - test/controller/routes.js | 26 ++++++++++---------------- 2 files changed, 10 insertions(+), 17 deletions(-) diff --git a/package.json b/package.json index b8c32482..8ae0c1d4 100644 --- a/package.json +++ b/package.json @@ -46,7 +46,6 @@ "@types/node": "catalog:", "c8": "^12.0.0", "express": "catalog:", - "form-data": "^4.0.6", "jsonwebtoken": "catalog:", "mocha": "^11.8.0", "oxlint": "^1.88.0", diff --git a/test/controller/routes.js b/test/controller/routes.js index d0af23da..0f60a9d2 100644 --- a/test/controller/routes.js +++ b/test/controller/routes.js @@ -1,6 +1,5 @@ import assert from "node:assert/strict"; import events from "node:events"; -import FormData from "form-data"; import http from "node:http"; import { wait, expositionContentType } from "@clusterio/lib"; @@ -308,27 +307,25 @@ describe("controller/src/routes", function() { let form; form = new FormData(); form.append("instance_id", "123"); - form.append("file", "totally a zip file", { filename: "test.zip", contentType: "text/plain" }); + form.append("file", new Blob(["totally a zip file"], { type: "text/plain" }), "test.zip"); response = await fetch(endpoint, { method: "POST", headers: { "X-Access-Token": testToken, - ...form.getHeaders(), }, - body: form.getBuffer(), + body: form, }); assert.equal(response.status, 400); assert.deepEqual(await response.json(), { request_errors: ["invalid file Content-Type"] }); form = new FormData(); - form.append("file", "totally a zip file", { filename: "test.zip", contentType: "application/zip" }); + form.append("file", new Blob(["totally a zip file"], { type: "application/zip" }), "test.zip"); form.append("instance_id", "123"); response = await fetch(endpoint, { method: "POST", headers: { "X-Access-Token": testToken, - ...form.getHeaders(), }, - body: form.getBuffer(), + body: form, }); assert.equal(response.status, 400); assert.deepEqual( @@ -336,14 +333,13 @@ describe("controller/src/routes", function() { ); form = new FormData(); form.append("instance_id", "123"); - form.append("file", "totally a zip file", { filename: "test.txt", contentType: "application/zip" }); + form.append("file", new Blob(["totally a zip file"], { type: "application/zip" }), "test.txt"); response = await fetch(endpoint, { method: "POST", headers: { "X-Access-Token": testToken, - ...form.getHeaders(), }, - body: form.getBuffer(), + body: form, }); assert.equal(response.status, 400); assert.deepEqual( @@ -351,14 +347,13 @@ describe("controller/src/routes", function() { ); form = new FormData(); form.append("instance_id", "invalid"); - form.append("file", "totally a zip file", { filename: "test.zip", contentType: "application/zip" }); + form.append("file", new Blob(["totally a zip file"], { type: "application/zip" }), "test.zip"); response = await fetch(endpoint, { method: "POST", headers: { "X-Access-Token": testToken, - ...form.getHeaders(), }, - body: form.getBuffer(), + body: form, }); assert.equal(response.status, 400); assert.deepEqual( @@ -410,14 +405,13 @@ describe("controller/src/routes", function() { let form; form = new FormData(); form.append("instance_id", "123"); - form.append("file", "totally a zip file", { filename: "test.zip", contentType: "application/zip" }); + form.append("file", new Blob(["totally a zip file"], { type: "application/zip" }), "test.zip"); response = await fetch(endpoint, { method: "POST", headers: { "X-Access-Token": testToken, - ...form.getHeaders(), }, - body: form.getBuffer(), + body: form, }); assert.equal(response.status, 500); assert.deepEqual(await response.json(), { errors: ["Something went wrong"], request_errors: [] }); From 8738c74d7f228f4b8763483382c865d2ea227947 Mon Sep 17 00:00:00 2001 From: bbassie <17990055+bbassie@users.noreply.github.com> Date: Fri, 30 Oct 2026 18:43:04 +0000 Subject: [PATCH 09/11] Remove leftover typedoc.json files TypeDoc was dropped in #1147, but these configs still ship in the published packages. Co-Authored-By: Claude Opus 5.5 --- packages/controller/typedoc.json | 4 ---- packages/ctl/typedoc.json | 4 ---- packages/host/typedoc.json | 4 ---- packages/lib/typedoc.json | 4 ---- packages/web_ui/typedoc.json | 4 ---- 5 files changed, 20 deletions(-) delete mode 100644 packages/controller/typedoc.json delete mode 100644 packages/ctl/typedoc.json delete mode 100644 packages/host/typedoc.json delete mode 100644 packages/lib/typedoc.json delete mode 100644 packages/web_ui/typedoc.json diff --git a/packages/controller/typedoc.json b/packages/controller/typedoc.json deleted file mode 100644 index ff5edcb9..00000000 --- a/packages/controller/typedoc.json +++ /dev/null @@ -1,4 +0,0 @@ -{ - "entryPoints": ["index.ts"], - "includeVersion": true -} diff --git a/packages/ctl/typedoc.json b/packages/ctl/typedoc.json deleted file mode 100644 index ff5edcb9..00000000 --- a/packages/ctl/typedoc.json +++ /dev/null @@ -1,4 +0,0 @@ -{ - "entryPoints": ["index.ts"], - "includeVersion": true -} diff --git a/packages/host/typedoc.json b/packages/host/typedoc.json deleted file mode 100644 index ff5edcb9..00000000 --- a/packages/host/typedoc.json +++ /dev/null @@ -1,4 +0,0 @@ -{ - "entryPoints": ["index.ts"], - "includeVersion": true -} diff --git a/packages/lib/typedoc.json b/packages/lib/typedoc.json deleted file mode 100644 index ff5edcb9..00000000 --- a/packages/lib/typedoc.json +++ /dev/null @@ -1,4 +0,0 @@ -{ - "entryPoints": ["index.ts"], - "includeVersion": true -} diff --git a/packages/web_ui/typedoc.json b/packages/web_ui/typedoc.json deleted file mode 100644 index 5aab5474..00000000 --- a/packages/web_ui/typedoc.json +++ /dev/null @@ -1,4 +0,0 @@ -{ - "entryPoints": ["src/index.ts"], - "includeVersion": true -} From ed2057e4a23edbe6319ae320800293abff52eb09 Mon Sep 17 00:00:00 2001 From: Bastiaan <17990055+bbassie@users.noreply.github.com> Date: Fri, 30 Oct 2026 20:57:48 +0100 Subject: [PATCH 10/11] Merge pull request #1149 from bbassie/feature/compat-ignore-control Ignore control in the version compat message check --- docs/contributing.md | 1 + scripts/version-compat-test.js | 33 ++++++++++++++++++++++++++------- 2 files changed, 27 insertions(+), 7 deletions(-) diff --git a/docs/contributing.md b/docs/contributing.md index 48094a79..5a9adfdb 100644 --- a/docs/contributing.md +++ b/docs/contributing.md @@ -254,6 +254,7 @@ If there are no changelog entries to add, remove the code block and start the ch The version-compat job in CI runs the local controller with the latest published host, and the published controller with the local host. It fails if a link message changed its src, dst, data or response schema since that release. +Control is left out of src and dst, and messages that only go between control and the controller are not checked, as the web UI is served by the controller it talks to. If the break is intended, add an entry to [scripts/version-compat-breaks.json](../scripts/version-compat-breaks.json) in the same pull request. Breaking changes are only accepted if the next release is intended to include them, see our decision on [breaking changes](/docs/decisions.md#breaking-changes). The entry is keyed by the published version directly before the breaking change was introduced, which is the latest release at the time. diff --git a/scripts/version-compat-test.js b/scripts/version-compat-test.js index b300833e..e9e7aa32 100644 --- a/scripts/version-compat-test.js +++ b/scripts/version-compat-test.js @@ -12,7 +12,10 @@ * added and removed messages are only warned about since breaking * compatibility is unavoidable there. Optional properties that only one * version has are left out of the comparison, as the other version ignores - * them when received and does not need them when sent. + * them when received and does not need them when sent. Control clients are + * left out too, as the web UI is served by the controller it talks to, so + * control is removed from src and dst and messages that only go between + * control and the controller are skipped. * * Intended breaks are listed in scripts/version-compat-breaks.json, see * scripts/version-compat-breaks.js for the format. @@ -196,6 +199,15 @@ const dumpMessageSpecs = ` console.log(JSON.stringify(specs)); `; +function withoutControl(spec) { + const strip = types => types.filter(type => type !== "control"); + return { ...spec, src: strip(spec.src), dst: strip(spec.dst) }; +} + +function isControlOnly(spec) { + return [...spec.src, ...spec.dst].every(type => type === "control" || type === "controller"); +} + async function messageSpecs(libPath) { const result = await execFile(process.execPath, ["-e", dumpMessageSpecs, libPath]); return new Map(Object.entries(JSON.parse(result.stdout))); @@ -215,12 +227,19 @@ async function checkMessageCompat(publishedVersion, knownBreaks) { console.log(`| comparing ${localSpecs.size} local with ${publishedSpecs.size} published link messages`); const changed = []; - for (const [name, localSpec] of localSpecs) { - const publishedSpec = publishedSpecs.get(name); - if (!publishedSpec) { - annotate(`Link ${name} does not exist in ${publishedVersion} which cannot receive it`); + for (const [name, fullLocalSpec] of localSpecs) { + const fullPublishedSpec = publishedSpecs.get(name); + if (!fullPublishedSpec) { + if (!isControlOnly(fullLocalSpec)) { + annotate(`Link ${name} does not exist in ${publishedVersion} which cannot receive it`); + } continue; } + if (isControlOnly(fullLocalSpec) && isControlOnly(fullPublishedSpec)) { + continue; + } + const localSpec = withoutControl(fullLocalSpec); + const publishedSpec = withoutControl(fullPublishedSpec); const differs = Object.keys(localSpec).filter( field => stableStringify(localSpec[field]) !== stableStringify(publishedSpec[field]) ); @@ -238,8 +257,8 @@ async function checkMessageCompat(publishedVersion, knownBreaks) { changed.push({ name, fields, localSpec, publishedSpec }); } } - for (const name of publishedSpecs.keys()) { - if (!localSpecs.has(name)) { + for (const [name, publishedSpec] of publishedSpecs) { + if (!localSpecs.has(name) && !isControlOnly(publishedSpec)) { annotate(`Link ${name} was removed after ${publishedVersion} which may still send it`); } } From 0f7c8be0608d2bd8b07a481b18ee639d4f763a5f Mon Sep 17 00:00:00 2001 From: Bastiaan <17990055+bbassie@users.noreply.github.com> Date: Fri, 30 Oct 2026 21:24:07 +0100 Subject: [PATCH 11/11] Use only the jimp modules the host export needs (#1162) jimp pulls in every image format and plugin, about 60 packages and 26 MB that only export.ts uses. Build the Jimp class from @jimp/core with the PNG format and the crop and resize plugins instead. The generated spritesheet is byte-identical to the one from jimp. Co-authored-by: Claude Opus 5.5 --- packages/host/package.json | 6 +++++- packages/host/src/export.ts | 12 +++++++++--- 2 files changed, 14 insertions(+), 4 deletions(-) diff --git a/packages/host/package.json b/packages/host/package.json index 80f839c6..b552445f 100644 --- a/packages/host/package.json +++ b/packages/host/package.json @@ -38,8 +38,12 @@ "dependencies": { "@clusterio/lib": "workspace:^", + "@jimp/core": "^1.6.1", + "@jimp/js-png": "^1.6.1", + "@jimp/plugin-crop": "^1.6.1", + "@jimp/plugin-resize": "^1.6.1", + "@jimp/types": "^1.6.1", "@sinclair/typebox": "catalog:", - "jimp": "^1.6.1", "pidusage": "^4.1.0", "rcon-client": "^4.2.5", "semver": "catalog:", diff --git a/packages/host/src/export.ts b/packages/host/src/export.ts index cb7f73d5..5df9d1df 100644 --- a/packages/host/src/export.ts +++ b/packages/host/src/export.ts @@ -1,7 +1,11 @@ // Export of item icons and locale import fs from "node:fs/promises"; import path from "path"; -import { Jimp, JimpMime, type JimpInstance } from "jimp"; +import { createJimp } from "@jimp/core"; +import png from "@jimp/js-png"; +import * as crop from "@jimp/plugin-crop"; +import * as resize from "@jimp/plugin-resize"; +import type {} from "@jimp/types"; // needed to name jimp types in export.d.ts import yazl from "yazl"; import * as lib from "@clusterio/lib"; @@ -147,7 +151,9 @@ async function loadFile( return await modZip.zip.readFile(path.posix.join(modZip.root, filePath)); } -type Image = JimpInstance; +// Only the parts of jimp the export uses +const Jimp = createJimp({ formats: [png], plugins: [crop.methods, resize.methods] }); +type Image = InstanceType; type IconCache = Map; async function loadIcon( server: FactorioServer, @@ -618,7 +624,7 @@ async function buildExport(server: FactorioServer, zipCache: ZipCache) { // Crop sheet to actual used height and write single spritesheet + metadata. const usedRows = Math.ceil(state.pos / (SHEET_WIDTH / SHEET_ICON_SIZE)); state.sheet.crop({ x: 0, y: 0, w: SHEET_WIDTH, h: Math.max(usedRows * SHEET_ICON_SIZE, 1) }); - zip.addBuffer(await state.sheet.getBuffer(JimpMime.png), "export/spritesheet.png"); + zip.addBuffer(await state.sheet.getBuffer("image/png"), "export/spritesheet.png"); addJson("export/metadata.json", state.metadata satisfies lib.ExportMetadata); zip.end();