Delete the host scaffolding that guards nothing

[Pixnop]

Summary

Four pieces of host-layer machinery that defend a door already bricked up, or spell out what the language and the type already say: a config sanitiser downstream of a normaliser that makes its input impossible, a pooled worker thread for a chmod walk, a four-branch ternary where two branches are the same branch, and a five-case switch listing the five method names it is indexing.

Total estimated saving: about 60 lines, one worker script, one full extra serialisation of the config on every save.

1. configManager strips underscore keys that normalizeConfig has already made impossible

src/config/configManager.ts:50

writeConfig runs the whole config through JSON.parse(JSON.stringify(config, replacer)) to drop keys starting with an underscore. The only assignment to pendingConfig is line 85, pendingConfig = normalizeConfig(config), and normalizeConfig:411 returns a fixed object literal built field by field, whose nested values come from normalizeInstallation, normalizeGameVersion, normalizeBackup, normalizeIcon and normalizeAccounts, all also fixed literals. No underscore key can reach writeConfig. The renderer's own markers (_image, _playing, _installing, _deleting, and the rest) never survive the normaliser.

The guard dates to commit b284266 (January 2025), when saveConfig wrote the incoming config straight to disk with no normalisation at all. The fixed-literal normaliser landed later and subsumed the job.

Replace with: await writeJsonAtomic(configPath, normalizedConfig).

Savings: 6 lines, and one full serialise plus parse of the entire config (installations, game versions, backups, icons, accounts) on every save. writeJsonAtomic already stringifies, so today every save serialises the document twice.

Risk and test: if normalizeConfig ever switches to a spread of the incoming object instead of a fixed literal, the guard would be needed again. Pin the invariant where it belongs: tests/ipc/configManager.test.ts:679 already writes a config carrying underscore fields through saveConfig and asserts they are absent on disk, which exercises the normaliser, not the writer, and passes with or without the guard. Verified: 83/83 in that file and 1082 passing across the config-related suites after the cut.

Worth: high.

2. A pooled worker thread for a chmod walk

src/ipc/workers/changePermsWorker.ts:1

CHANGE_PERMS spins a worker thread to run changePermissions, which is existsSync, lstatSync, readdirSync and chmodSync over a folder tree. That is pure I/O, not CPU: the four other workers (download, extract, compress, inno payload read) each stream or decode and belong in a thread. This one costs a worker script, a ?modulePath import (pathsHandlers.ts:27), a WORKER_TIMEOUTS_MS entry, a WORKER_POOL_MAX_IDLE entry set to 0 with a comment admitting pooling buys nothing (pathsHandlers.ts:116, :128), and the full runTrackedWorker message protocol, for one call site fired once per Linux install (TaskManagerContext.tsx:330). workerHost.ts's own docstring calls out that the shared protocol exists partly to let "the synchronous permissions handler share a shape with the four asynchronous ones".

Replace with: an async walk in the handler using node:fs/promises lstat/readdir/chmod, awaited directly. It never blocks the main event loop. While there, drop the nodeFileSystem delegating object (permissions.ts:27-32): node:fs structurally satisfies PermissionsFileSystem.

Savings: one worker script, the two table entries and the ?modulePath import, the delegating object, and one concept: four CPU workers instead of five with an odd one out.

Risk and test: two costs the original write-up understated, weigh them before starting. First, removing the WORKER_TIMEOUTS_MS entry removes the 10 minute bound on a stuck chmod walk, and the renderer awaits this call before marking the extract task complete, so a replacement timeout has to come back with the async version. Second, changePermissions is synchronous by design and its test drives a synchronous fake filesystem, so the port shape changes with it: that is most of the diff, and it means an async rewrite of tests/ipc/permissions.test.ts (174 lines, real-tempdir cases included) and of the CHANGE_PERMS block in tests/ipc/pathsHandlers.test.ts. The guards do not move: the symlink refusal (permissions.test.ts:130, a chmod follows links, so following one applies the launcher's bits outside the chosen folder) and the 100,000 entry cap stay exactly where they are.

Worth: medium.

3. A four-branch ternary in validateGameInstallation where two branches are the same branch

src/ipc/validation.ts:279

The gameVersionId spread reads null ? {gameVersionId: null} : typeof === "string" ? {gameVersionId: assertString(...)} : undefined ? {} : {gameVersionId: assertString(...)}. The second and fourth arms are character for character identical. History explains it: commit d679753 wrote three arms (null / string / omit), then 1f8b5b8 patched in a fourth to fix a real bug where an invalid id was silently dropped instead of rejected, duplicating the assertString call rather than restructuring.

Replace with: ...(value.gameVersionId === undefined ? {} : { gameVersionId: value.gameVersionId === null ? null : assertString(value.gameVersionId, "installation game version id", 128) }). The string-typeof arm is redundant because assertString itself throws for a non-string, which is the intent.

Savings: 6 lines, and one branch fewer to read at the trust boundary.

Risk and test: this is a validation boundary, so the three outcomes must stay exactly three: null preserved, undefined omitted, anything else asserted. Pinned by tests/security-boundaries.test.ts:72 (gameVersionId: 42 throws /Invalid installation game version id/) and the EXECUTE_GAME correlation check at gameHandlers.ts:265, which distinguishes undefined from null. Verified: prettier, both tsconfigs, eslint and the full suite (4091 tests) pass on the replacement.

Worth: low, but it is six lines at the boundary people read most.

4. A five-case switch that spells out the five method names it is indexing

src/utils/logManager.ts:17

logMessage switches on mode over "error", "warn", "info", "debug", "verbose", calling Logger.error/warn/info/debug/verbose respectively, plus a default that silently drops. ErrorTypes (src/preload/preload.d.ts:119) is exactly those five strings, and they are exactly electron-log's method names. The switch dates to the file's pre-fork commit and no PR ties meaning to its shape.

Replace with: Logger[mode]?.(redactSensitiveText(message)), the optional call standing in for the default branch. The redaction, which is the part that matters, is untouched.

Savings: 18 lines down to 2, in the most-called function in the host layer.

Risk and test: the optional call is what preserves today's silent drop for a mode that slipped past the type; without it an unknown mode would throw. An absent property key resolves to undefined and ?.() short-circuits, so the behaviour matches. The only untyped entry point is the LOG_MESSAGE IPC channel, and utilsHandlers.ts:23 already filters the mode against the same five strings before calling. Pinned by tests/log-provenance.test.ts and tests/rendererErrorLog.test.ts, which drive logMessage end to end; verified green, plus eslint and all three tsconfigs.

Worth: medium.

Suggested order

1. Item 4 (logManager), two lines, highest traffic, already verified.
2. Item 3 (the ternary), six lines at the boundary, already verified.
3. Item 1 (configManager), verified, but land the normaliser-side test note with it so the invariant is pinned where it lives.
4. Item 2 (the chmod worker) last and only if someone wants it: it is the one with a real test rewrite and a timeout to re-add, so it is the least free of the four.

Out of scope

The redaction in logMessage, the symlink refusal and the entry cap in changePermissions, the three outcomes of the gameVersionId check, and the normaliser's fixed-literal shape all stay: they are the behaviour, not the scaffolding. The hexagonal split (pure src/domain, src/ipc and src/main as host, the renderer through window.api and feature adapters), the path policy, the IPC validation at the boundary, the mutation-tested guards and the accessibility work are deliberate and are not cut here. tests/security-boundaries.test.ts, tests/log-provenance.test.ts, tests/text-contrast.test.ts and tests/i18n/i18n-parity.test.ts pin log provenance, no HTML sinks, contrast floors and locale parity, and stay green through all four items.