You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
A pass over the whole repository looking for one thing only: code that is bigger than the job it does. Reinvented standard library, the same shape written out once per place that needs it, options nobody passes, scaffolding around a door that is already bricked up, and files nothing reaches.
36 findings survived verification (most were applied in a throwaway worktree and run against the real suite before being written up) and are grouped below into 8 issues by pattern and area, not one per finding. Rough total if all of them land: about 1500 lines, one script, one npm script, one dependency, and roughly 50 duplicated concepts collapsed to their single copy.
Nothing in these issues touches the rules this repo pins on purpose. See "Out of scope" at the bottom of each one.
The issues
Fold the repeated domain helpers into one copy each #484, Fold the repeated domain helpers into one copy each (area: domain, 4 findings): isRecord written out 8 times, refuse 7 times, three byte-identical outcome interfaces in ports.ts, and GameBuildVariant declared both in the domain and as an ambient global.
Stop hand-rolling in the domain what the repo already imports #485, Stop hand-rolling in the domain what the repo already imports (area: domain, 4 findings): 65 lines of semver precedence while semver is a production dependency three files over, two options bags no production caller fills, two exported functions for one executable table, and one predicate restating the function below it.
One bounded read, one symlink walk, one file name rule in the host #486, One bounded read, one symlink walk, one file name rule in the host (area: host, 3 findings): three security-relevant guards that exist in two copies each, one of which has already drifted and costs another file twelve lines of workaround.
Delete the host scaffolding that guards nothing #487, Delete the host scaffolding that guards nothing (area: host, 4 findings): 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 identical, and a five-case switch listing the five method names it indexes.
Hoist the log tag into one constant per file #488, Hoist the log tag into one constant per file (area: host, 2 findings): 199 hand-typed provenance prefixes across 17 files, already drifted into two spellings for the same file in two of them, while the renderer and one host file already use the constant.
Fold the repeated renderer shells into one component each #489, Fold the repeated renderer shells into one component each (area: renderer, 5 findings): the button written twice, the confirm dialog eight times, the single-select dropdown four times, the multi-select filter twice, and two controls still spelling out the shared menu chrome by hand.
One launch path, one filter object, one task runner #490, One launch path, one filter object, one task runner (area: renderer, 6 findings): the launch that a hook was extracted to deduplicate and never was, nine filter states drilled as nineteen props, four near-identical task runners, a folder picker written three times, a hook wrapping a hook, and one user-visible wrong sentence the copy-paste left behind.
Delete the dead tooling and use what node already ships #491, Delete the dead tooling and use what node already ships (area: ci, 8 findings): an unreferenced PowerShell script pointing at the pre-rename executable, a docs page whose citations have all drifted, an unreachable test polyfill, two hand-rolled stdlib reimplementations, a test that checks seven keys then checks all of them, a render harness written four times, and a dependency nothing imports.
Where to start
#488 and #491 are the cheapest: mechanical, no behaviour change, and #491 is mostly deletion. #486 and #490 carry the most value per line because both close drifts a player or a maintainer can already feel (a bare error class the login classifier has to parse out of message text, a crash report link missing on one of two launch paths). #489 is the largest diff and is best taken one item at a time.
Not this, already tracked elsewhere
Triage the first SonarCloud pass: 5 bugs, 199 code smells #107 stays the umbrella for the first SonarCloud pass (5 bugs, 199 code smells). Some of the duplication above will overlap with what Sonar flags; triage stays there, the code changes are here.
The hexagonal split (pure src/domain, src/ipc and src/main as the host, the renderer through window.api and feature adapters), the path policy, the IPC validation at the trust boundary, the mutation-tested guards and the accessibility work are deliberate and are not debt. tests/security-boundaries.test.ts, tests/log-provenance.test.ts, tests/text-contrast.test.ts and tests/i18n/i18n-parity.test.ts pin rules the project chose (log provenance, no HTML sinks, contrast floors, locale parity) and every issue above keeps them green; where one of them reads a file by path that a refactor moves, updating the test's reading path is part of that item, never relaxing its thresholds.
Summary
A pass over the whole repository looking for one thing only: code that is bigger than the job it does. Reinvented standard library, the same shape written out once per place that needs it, options nobody passes, scaffolding around a door that is already bricked up, and files nothing reaches.
36 findings survived verification (most were applied in a throwaway worktree and run against the real suite before being written up) and are grouped below into 8 issues by pattern and area, not one per finding. Rough total if all of them land: about 1500 lines, one script, one npm script, one dependency, and roughly 50 duplicated concepts collapsed to their single copy.
Nothing in these issues touches the rules this repo pins on purpose. See "Out of scope" at the bottom of each one.
The issues
isRecordwritten out 8 times,refuse7 times, three byte-identical outcome interfaces inports.ts, andGameBuildVariantdeclared both in the domain and as an ambient global.semveris a production dependency three files over, two options bags no production caller fills, two exported functions for one executable table, and one predicate restating the function below it.Where to start
#488 and #491 are the cheapest: mechanical, no behaviour change, and #491 is mostly deletion. #486 and #490 carry the most value per line because both close drifts a player or a maintainer can already feel (a bare error class the login classifier has to parse out of message text, a crash report link missing on one of two launch paths). #489 is the largest diff and is best taken one item at a time.
Not this, already tracked elsewhere
expectedGameExecutablesby name and line range as infrastructure for its planned feature, and Stop hand-rolling in the domain what the repo already imports #485 proposes deleting that function, so whoever takes it should leave a line on Optimum as a VS Version: recognise it now, install it from the launcher next #457.Out of scope, everywhere
The hexagonal split (pure
src/domain,src/ipcandsrc/mainas the host, the renderer throughwindow.apiand feature adapters), the path policy, the IPC validation at the trust boundary, the mutation-tested guards and the accessibility work are deliberate and are not debt.tests/security-boundaries.test.ts,tests/log-provenance.test.ts,tests/text-contrast.test.tsandtests/i18n/i18n-parity.test.tspin rules the project chose (log provenance, no HTML sinks, contrast floors, locale parity) and every issue above keeps them green; where one of them reads a file by path that a refactor moves, updating the test's reading path is part of that item, never relaxing its thresholds.