Adds an externalTest(this) helper alongside slowTest that skips the test
when NO_EXTERNAL_TEST is set and raises the timeout to 60s, plus a
no-external-test script. Marks the tests hitting factorio.com, the mod
portal and GitHub with it.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The Web UI console rendered server level entries as bare parsed Factorio
output, while every other level was prefixed with its name. Nothing in a
line distinguished what the game wrote from what Clusterio wrote about it,
which matters more now that both modes of the filter carry a mix of the
two.
Emit the [server] prefix for those entries as well, matching what
TerminalFormat already does for the host and controller logs, and style the
level the same bold the terminal colourises it with.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The filter was a Switch showing only the label of the mode it was already
in, so with the default of chat mode it read as a button labelled "Chat"
with no hint that a Log mode existed or that anything was being hidden.
Swap it for a Segmented control listing both Chat and Log, so the mode that
is not active is always visible, and add a tooltip describing what each one
covers.
Fixes#853
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The console's chat filter kept only Factorio output parsed as an action,
which dropped the server's replies to those actions. Running a console
command with a typo in it echoed the command and then appeared to print
nothing, because "Cannot execute command. Error: ..." is a plain message
rather than an [ACTION] line. Factorio's engine level errors were dropped
the same way, so a failed multiplayer connection left no trace either.
Filter by where a line came from instead. Factorio interleaves two streams
on stdout: the date stamped console log, which carries what was done to the
server and the replies to it, and the seconds stamped engine log, which is
verbose diagnostic output. Keep the console log whole and take only
warnings and errors from the engine log.
Apply the same rule to Clusterio's own entries, which were previously kept
in full and drowned the Factorio output they were mixed with. Keep those at
warn or worse and drop the routine bookkeeping. Replayed over a development
cluster's history this drops 415 informational lines and surfaces engine
errors that were never visible in either mode.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The plugin list reported "Error loading module" for anything without a
loaded package, which includes plugins the controller has picked up since
the page was opened. Nothing had been tried for those, so reporting a
failure was both wrong and unhelpful: the fix is to reload, not to
investigate.
Report those as needing a reload instead. Plugins the controller cannot
serve at all are excluded during startup before they reach the loaded
plugin map, so those are checked for first and still count as an error.
Fixes#729
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The web interface loads plugin bundles, registers their messages and
adds their config fields once in bootstrap, and has no way to undo any
of it. A controller restarted to pick up a plugin being installed,
enabled or disabled comes back running a different set, and a controller
started in recovery mode runs none of them while still accepting control
connections, so open pages carry on sending messages it no longer
handles.
The controller already states the plugins it is running in the handshake
it sends on every connection, which is what hosts track as serverPlugins
and use to decide what to forward and which instance plugins to load.
Compare that against the set the page loaded and stop rendering on a
mismatch, asking to be reloaded instead.
The comparison is against loaded rather than enabled because loaded is
what the handshake reports and what the controller is actually running;
enabled also takes load_plugin into account, which only applies from the
next restart and would otherwise latch a permanent mismatch.
Closes#804
Address review feedback: modsByName is built from annotatedMods so the
advisories array always exists, and each source mod is iterated once with
no duplicate dependencies, so the existence check can never fail. Push the
recommended_dependency advisory directly.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01G8b9dEaJtpvydzz4ttM7JH
Nothing covered whether plugins are loaded at all, or whether the messages
they declare are routed. The integration tests only loaded the first party
plugins, so what coverage existed came incidentally from testing those.
Add a plugin under test/file which exists purely to be loaded by the tests.
It is plain JavaScript rather than TypeScript so that it needs no build
step, which the plugin loader is happy with as it requires the entry point
and reads the version from package.json like any other plugin.
Covered so far: the plugin loads on the controller and on the host, its
title and version are reported, the command it adds is available in
clusterioctl, a request from control reaches the controller, a request from
control reaches a host, and an event sent with broadcastEventToHosts
arrives at the hosts.
Left for a follow up, as all three need a running instance: a request to an
instance, a Lua module reached over rcon, and log statements being tagged
with the plugin and host or instance id.
Part of #342, and a step towards #721 as core no longer needs a first party
plugin to have the plugin system under test.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
An event broadcast from an instance to all hosts only reached the host
running that instance. The host side of an instance connection has the
host's own address as its source, so a broadcast aimed at hosts counts as
addressed to it, and a link that a message is addressed to handles it and
stops. The event was delivered locally and never routed onwards.
A broadcast is addressed to every link of the type it targets, so arriving
at one of them does not mean it has arrived everywhere. Keep routing such a
message after handling it locally. Only links that route are affected, as
instances and controls have no router and would otherwise throw on every
broadcast they receive.
The only links whose source type matches a broadcast target and which route
are the host and its instance connections, both matching a broadcast to
hosts, so add the missing case for that to the host router. It forwards to
the controller, which already passes broadcasts on to every other host, and
stops when the message came from the controller to begin with.
Drop the exclusion this bug had in the routing test matrix, which now
covers it along with the plugin filtering variants.
Fixes#575
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
execFile was called with shell: true and an array of arguments, which Node
deprecates in DEP0190 because it concatenates the arguments by space
without escaping anything, giving the impression they have been made safe.
The warning was printed to the console on every install.
The arguments here are already escaped for the shell by escapeArg, so join
them into the command instead of handing them to execFile to concatenate.
Nothing about what ends up being run changes.
Point the escaping test at the same form so it covers what the installer
actually does, and stops emitting the warning itself. Its several hundred
round trip cases still pass, which is what confirms the escaping is
unaffected.
Fixes#851
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
forwardRequest records a request so that its response can be sent back the
way it came, but recorded it before handing the message to the connector.
When the connector threw, because the session to the next hop was gone, the
record stayed behind for a request that had never left.
Routers answer the origin themselves when forwarding throws, so the request
was already accounted for. The leftover record then had it answered a
second time when the link cleared its pending requests on disconnect,
producing a duplicate response and "Received response N without a pending
request" at the far end.
Remove the record when the forward throws. Doing it here rather than in the
routers covers both the host, which catches and responds, and the
controller router, which does not.
Fixes#650
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
A subscribe arrives as a request, so the source address the subscription
controller handles it with carries the request id used to route the
response back to the caller. That address was stored verbatim as the
destination for the subscription, so every event broadcast to it, and the
replay event sent when the subscription is created, went out addressed to
a request that had long since been answered.
A request id identifies one request in flight and has no meaning on an
event. The receiving link keys response routing off it, so events carrying
one are put through paths meant for response traffic.
Strip the request id when taking the subscription's destination from the
request that created it. Add Address.withoutRequestId for that, as the
same address is used in three places here.
The subscription map is keyed on addressIndex, which already excludes the
request id, so subscriptions were never duplicated by this and only the
address events are sent to was wrong.
Fixes#847
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
downloadFile opens a write stream before fetching, but only closed the
stream and removed the file it was writing to when the server answered
with a bad status. A request that never reached the server, or a
connection dropped part way through the body, left both the temporary file
and an open file handle behind.
Mods are downloaded through this in a Promise.all over every mod in a pack,
so one bad moment on the network scatters temporary files across the mods
directory. They are not harmless: createTempWriteStream picks the first
free name, so each leftover file permanently shifts the name used for
later downloads of the same mod, as the existing "should not overwrite
existing temp files" test describes.
Move the fetch and the body write into a try block and discard the stream
and its file on the way out. Destroy rather than end the stream, as the
content is being thrown away, and skip both if the stream has already
closed so that a failure after the body was written does not wait forever
for a close event that has already fired.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The mod portal takes the Factorio account token as a query parameter on the
download URL. When a download failed, downloadFile put the whole URL into the
error message, which ModStore then logged verbatim, printing the token into the
log at error level on every failed mod download.
Add a redactUrl helper that replaces the value of credential-carrying query
parameters with REDACTED, and use it when downloadFile builds its error.
Parameter names and the rest of the URL are kept so the message stays useful
for diagnosing which request failed; username is deliberately left intact as it
is not a secret and identifies the account in use.
Fixes#795
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
