Skip to content

The second audit's non-blocking half, and assignments A to E - #59

Merged
Harted merged 137 commits into
mainfrom
chore/audit2-annoying
Sep 23, 2026
Merged

Harted merged 137 commits into
mainfrom
chore/audit2-annoying

Conversation

@Harted

@Harted Harted commented Sep 23, 2026 •

Copy link
Copy Markdown
Member

Summary

The second audit's annoying and cosmetic half, and assignments A to E on top of it.

This is built on #57, so the base is chore/audit2 rather than main.

  • The triage of round two: 110 annoying and cosmetic findings, built as one commit per cause.
  • A to E: the server module split, a unit holding only the addresses it was given, one record per server, the client's volatile fields moved out of the persisted store, and undo and redo.
  • After E: reduced motion follows the system setting, a stopped scan shows its last rows, and three e2e and unit flakes are fixed.

Merge this with a rebase, not a squash. Each commit is one concern with its own message.

What a user gets

The entries are in CHANGELOG.md under Unreleased.

How to test

yarn verify. The E2E workflow runs on this branch on all five runners.

@Harted
Harted changed the base branch from chore/audit2 to main September 23, 2026 21:34
`meme` is `memo` with `deepEqual`, and the conformance suite asserts that every
component in the renderer is wrapped in it. It collects a declaration when the
name is rendered as a JSX tag or exported as its file's default. A component
handed to MUI as a prop is neither, so the walk never reached one.

The walk reads both spellings MUI has now, `slots={{ footer: X }}` and
`inputComponent: X`, and keeps them in two sets rather than one. Each set is
asserted to have found something, so a walk that stopped matching one spelling
cannot pass on the other's count.

That adds ten names the first two sets do not reach: `AddressInput`,
`CustomFooter`, `Footer`, `IntervalInput`, `MaxInput`, `MinInput`, `PortInput`,
`RegisterLengthInput`, `TimeoutInput` and `ValueInput`. Eight of the ten were
already wrapped. `TransactionGrid`'s `CustomFooter` and `ServerConfig`'s
`PortInput` were not, and they are what this wraps.

Each rule was mutated on its own and turned exactly its own test red: the
`slots` key, the `inputComponent` key, the cast peel that
`PortInput as unknown as ElementType` needs, the JSX half, and the two wraps
themselves.
…t it reads

Two findings from `/code-review` on the commit before this one, both about the
rule rather than about what runs.

The walk read `inputComponent` only as a property inside `slotProps`, so
`inputComponent={X}` handed straight to an element was silent. A component
reaching the tree that way would have been left out of the population, and both
new meters would have gone on passing on the names the nested form already
reaches. The collector is one exported function now, and it reads all three
spellings.

The shorthand of either prop needs no branch and gets none. `slots={{ footer }}`
names its local for the slot, so the local is lowercase, and the population
takes capitalised declarations only. A branch for it would be one no input can
reach.

`CONTRIBUTING.md` still described the population as JSX or a default export,
which is what the rule read before the previous commit. A contributor writing a
new `inputComponent` input would have learned the rest from a red suite.

The three spellings get four tests over synthetic sources, because the renderer
uses two of them and the third would otherwise be a branch nothing exercises.
Each branch was disabled in turn and turned exactly its own test red.
The skill already says to edit by a unique string rather than by the first
match, and names the symptom of a botched edit as vitest reporting no tests.
There is a second symptom and it is the dangerous one: a replace that matches
nothing leaves the file alone, every test passes, and that is indistinguishable
from a rule the test does not discriminate.

Hit once while proving R9's third branch. The reply to it was a second test
nobody needed until the file was diffed against the copy.
`servertcp_handler.js` calls a three-argument accessor inside try/catch and
discards what it returns, so a throw out of one is answered exception 4, slave
device failure. `_get` and `_set` were `async`, which turns that throw into a
rejection nothing holds: no frame is written and the client waits out its
timeout instead. `serverserial.js` requires the same handlers, so RTU read the
vector the same way.

Neither body throws on a shipped path today, which is why this gets no CHANGELOG
line. What the two new tests throw from is the caller's own code, the callback
the library hands the accessor and `windows.send` under `_set`, and both were
red before with the unhandled rejection printed beside the failure.

`ValueGenerator._updateValue` is the same construction from the other side. The
constructor calls it and `setInterval` calls it, neither awaits it, so a throw
inside it was an unhandled rejection that left the generator ticking. It throws
out of both callers now and a test names each.

Fifty `await vector.x(...)` calls in `modbusServer.test.ts` awaited the promise
that is gone, and nineteen test bodies were `async` for that await alone. Both
are dropped, because a call that reads as awaitable is what the next test would
copy.
`createServer` walked ten thousand ports with no upper bound and then returned
the port it had stopped on, whether or not anything was listening there. The
store writes that answer into the port field, so the field named a port no
socket was on.

The near way out is the port range rather than the count. `PortSchema` caps a
configured port at 65535, so a server asked for 65535 with that port taken
increments to 65536, and `net.createServer().listen(65536, '0.0.0.0')` throws
there:

    sync throw: ERR_SOCKET_BAD_PORT | options.port should be >= 0 and < 65536. Received type number (65536).

That throw is inside `_isPortAvailable`'s executor, so the promise rejects and
the invoke rejects with it. At startup `openServer` catches that into
`console.error`, which leaves the server neither up nor reported.

`if (!isPort(actualPort)) break` ends the walk on the message that was already
there, and the answer on that path is `undefined`. Both callers in
`server.zustand.ts` already return on `undefined`, because the channel has a
schema and a refused payload answers the same way.

Two tests, both red first: the walk from 65535 constructed a `ServerTCP` on
65536 and answered it, and the exhausted walk answered 15020.
Five findings from `/code-review` on the commit before this one.

Both new vector tests threw from `windows.send`, which cannot throw:
`Windows.send` wraps every `contents.send` in a try/catch, and the comment there
says why. The callback the library hands the accessor is what does throw, at
`responseBuffer.writeUInt16BE(value, ...)` in FC3's answer, so both tests throw
from there now.

`ValueGenerator`'s test threw from the same place. `createRegisters` is what
throws inside an update: `createRegisters('uint16', 70000, false)` answers `The
value of "value" is out of range. It must be >= 0 and <= 65535. Received 70000`.
The constructor test drives that, and the tick test is gone, because a generator
whose range constructs cannot draw a value the type refuses.

Three comments described the old construction and its consequence:

- `server.ts` gave the refine's reason as a quiet unhandled rejection every
  interval. The throw now leaves `addRegister`, which is the loud half and a
  stronger reason to keep the refine, and it names what a `setInterval` draw
  does instead.
- `addRegister`'s docblock said the generator branch cannot be answered for.
  It can throw out of this method now, and the reason it gets no guard is that
  `RegisterParamsSchema` refuses the range, so no input reaches it.
- `schemas.test.ts` named the same consequence over the same rule.

Two `await`s on vector calls escaped the sweep, both in `creates nothing from a
write to a unit it does not host`.
One finding from `/code-review` on the commit before this one. `createServer`
closes the listener a uuid has before it starts walking, so a walk that ends in
`undefined` leaves that uuid with none and nothing puts it back. Loading a
config that moves a running server onto a taken 65535 reaches it: the listener
that was working is gone, `ready` stays false, and one message is the whole
trace.

`setPort` already answers this situation by rebinding the port it came from, and
this reads the port before the close and does the same. The answer is that port,
which is the one a listener is on. A restore that fails says so and answers
`undefined`.

Two tests, each turned red by taking out exactly the branch it names.
`set_register_mapping` answered `void` where `update_connection_config` and
`update_register_config` answer `true | undefined`, and the spec states that
convention in the doc comment above the first of them. It answers now, and
`ipc.test.ts` reads all three through `initIpc`: `true` for a payload taken,
`undefined` for one refused.

The renderer bails on the refusal where a caller leans on main holding the
mapping. `flushRegisterMappingToMain` takes the mapping and answers whether main
took it, `replaceRegisterMapping` writes the store after that answer rather than
before it, and the Read Configuration toggle stops before `showMapping` and
before it turns the flag on. The debounced cell edit reads no answer and says
why: nothing waits on it, a refusal reports itself as a `backend_message`, and
the next edit sends the whole mapping again.

Neither the Clear button nor the Load button grows a branch for it. No shipped
payload reaches the refusal, so a branch there is one no test could turn red.

`read`'s comment said it returns `RegisterData[] | undefined` above
`return: void`. The rows go out as a `register_data` event, and the comment says
that.

`rtu_server_status` carried `{ active: boolean }` while `get_rtu_server_status`
answered `boolean`, and the spec's comment on the second called that the same
shape as `get_client_state`, which carries one type on both halves. The event
carries `boolean` now, three send sites and one listener, and `scan_progress`
was already a bare primitive.

Two tests were red first: the store keeping the mapping it had when main refuses
the new one, and the toggle staying off. `stubRenderer` answers the new channel
with the schema `main/ipc.ts` guards it with, because a stub answering
`undefined` would make every store test read a refusal.
`App.tsx` renders `<UpdateBanner />` above the view switch and both windows load
the same bundle, so the split out server window asked GitHub for the latest
release too and drew the same banner. `setWindowOpenHandler` was on the main
window alone, so the release link opened a second Modbux window on the release
page instead of the browser.

The banner asks nothing and draws nothing in the split out window, which is one
request per launch rather than two. The server window gets the handler the main
window has, because both windows load the same bundle and a link that opens in
the browser from one has to from the other.

`FORCE_SHOW_BANNER` was `false` beside two reads of it, so `!FORCE_SHOW_BANNER`
was always true and `FORCE_SHOW_BANNER ||` always fell through. Neither branch
could turn a test red, and both are gone.
`/code-review` on that commit found no defect and three claims.

Two of them gave the banner's guard its reason as `setWindowOpenHandler` being
on the main window alone, which the same commit stopped being true. The reason
is one notice per launch, in the window Modbux opened with.

The third called the server window's new handler the answer to a link that
window has. It has none: `appType` is pinned to `server` there, so `Home`'s two
external links never render, and the banner's link left with the banner. The
handler is what a window without one does instead, which is to answer
`window.open` with a Modbux window on whatever the link points at.
Five findings from `/code-review` on `afd5748`, the commit that made the Read
Configuration toggle wait for main.

The store is written after that round trip, so between the press and the answer
the toggle still read off and a second press arrived as one more turn-on: a
second flush, a second `showMapping` and a second read, which main refuses with
the warning the `disabled` gate exists to avoid. One press is taken while the
first is in flight, and a test presses twice against an answer it holds open.

Two comments and two tests, all about what a refusal costs:

- `replaceRegisterMapping` turns read configuration off before it asks and does
  not turn it back on. That is the price of a refusal and the comment says so:
  both sides keep the mapping from before, and main's message names the channel.
- Both new store tests ran with `ready` false, where the action returns at
  `setReadConfiguration`'s own guard, so they asserted the accept and refuse gate
  and nothing else. They run the whole action now and assert read configuration
  went off.

`ipc.test.ts` read the answer for three channels named by hand. The refusal half
holds for every guarded channel and is read from the handlers instead, so a
channel that starts answering a value is held to `undefined` on a refusal
without joining a list.

The Load and Clear buttons still grow no branch. `migrateClientConfig` validates
the mapping against the schema main guards the channel with, so the refusal is
unreachable from either.
Round B of the audit's non-blocking half. Five items whose whole proof is that
they came out, so typecheck and the unit suite are the meter and nothing here
changes what the app does.

`applyRegisterValue` held five branches no input reaches. `registerWidth`
answers 1, 2 or 4 for the eleven types that get that far, `byteLength` follows
from it, and `byteOffset + 2 <= byteLength` follows from the guard above it, so
the three `if`s were arithmetic restating itself. Both `default` arms went with
them and the switches narrow their union now: `newComposite` is declared without
a value, so a fourteenth `DataType` answers TS2454 three times instead of
returning a silent zero.

The two `try`/`catch` blocks around those switches went too. What was recorded
as their reason had moved: `BigInt(1.5)` throws inside `toExact64Bits`, which
catches it and answers `undefined`. Measured over nineteen stored values, among
them 1.5, NaN, both infinities, `'abc'`, a 32 digit decimal string and
`2n ** 70n`, through all eight setters, the eight getters over a buffer of 0xFF,
and seven `numberValue`s through the one word overwrite: zero throws.

`getMinMaxValues` cased twelve of the thirteen `DataType` members and answered
the thirteenth out of `default:`. `case 'none':` is where that belongs. The test
that reached the arm was titled for an unknown type; it names the two types that
hold no number and asserts both.

Nineteen names were exported from the file that is the only file naming them.
Twelve keep the declaration and lose the keyword. Seven had no reader anywhere
and are gone: `onceEvent`, `SetUnitIdParams`, whose channel `set_unit_id`
returns nothing from `grep -rn src e2e`, and five aliases over a schema that is
used without them.

Five declarations were read and could not run. `AppState`'s constructor was
empty above the four field initialisers that do the construction.
`valueColumn`'s third parameter is passed by none of its eight call sites.
`registerValueToString` opened a `let` and overwrote it on the next line.
`layout.zustand` kept a `setShowLog` with no caller, where `toggleShowLog` is
the one the toolbar uses. And the client store's migration held an empty
`if (version < 2)`: that step wrote `readLocalTime` into `registerConfig`, which
`79fa174` took out of the client, so a line says a v1 store has nothing to
carry.

`ScanUnitIDResultSchema.refusedRegisterTypes` lost `.default([])`. The schema
parses nothing, being read once by the type it infers while `modbusClient`
builds the object literally, so the default read as a guard on a payload that
never arrives at a boundary.

`types/ipc.ts` imported `SharedProps` from notistack as a value, in the one
module all three processes compile against. It is `import type` now. esbuild
dropped it either way:
`grep -c notistack out/main/index.js out/preload/index.js` answers 0 for both,
measured before the change and after it.

The two pkexec helpers each held their own copy of what a fix answers: two
unions `diff` calls identical, and two interfaces differing by one optional
field. `PkexecFixResult` in `types/pkexec.ts` is the one shape. The port fix
extends it with the value it re-reads from the kernel, and the group fix takes
it as it stands.
Round C of the audit's non-blocking half. Four items in main, with
`modbusClient.test.ts` and `modbusServer.test.ts` as the meter.

`syncServerRegisters` opened by disposing and clearing the generators of both
register types, and then called `resetRegisters` for both, which disposes and
clears the same generators before it replaces the data array. The prologue is
gone and the two calls stand. Two tests titled for a branch inside that prologue
name the branch in `resetRegisters` they still reach.

`stopAllTcpServers` was `_closeAndForget` written out again, message included,
ending in a `clear()` of both maps rather than a delete per uuid. It is the loop
now. The key sets cannot differ: `_bindServer` writes `_servers` and `_port` on
consecutive lines, `_closeAndForget` deletes both, and nothing else writes
either.

Four callback-to-promise wrappers had one body between them. FC5, FC6, FC15 and
FC16 share a signature in `ModbusRTU.d.ts` apart from the value they carry, so
`_awaitWrite` is that conversion once and each call site hands over its own
method with the callback still open.

Encoding a value, writing the words into `serverData` and sending one
`register_value` per register was written twice, in `addRegister`'s fixed branch
and in `ValueGenerator._updateServerData`. `modbusServer/registers.ts` holds both
halves. What the copies did with a throw had drifted apart, and that difference
is deliberate, so it stays where it can be seen: `_encode` is `encodeRegisters`
inside a `try`, the generator calls it bare, and `addRegister`'s docblock says
why.

The transaction log's comment block said a serial transaction files under the
string "undefined". `open` sets `_transactionIdWrite` to 1 for every transport
(`index.js:679`), and only `tcpport.js`, `tcprtubufferedport.js` and
`udpport.js` increment it, so a serial request files under 1 and the next one
lands on top of it. The mock said the same wrong thing, and two tests wrote
`undefined` by hand to reach what it claimed. The mock models the increment
instead, and both tests assert two serial requests sharing the key 1. Setting
the mock to increment on a serial port turns both red.

`/code-review` on round B read this work in the tree beside it and found two
things. `ValueGenerator` kept a literal 10 where `encodeRegisters` falls back to
`DEFAULT_UTF8_LENGTH`, which is 10, so the copy was the thing that could drift.
And the two optionals on `ModbusRTUInternals._port` read as guards nothing
reaches. They describe modbus-serial and the docblock says so now: a client
built and not connected has no `_port`, a serial port has no
`_transactionIdWrite` until `open` sets it, and `_nextTransactionIdKey` runs
behind `_requireConnected`, so it meets neither.
`/code-review` on `6f0c682`, the commit that established what a serial request
files under.

`_logTransaction` said the table holds everyone else's requests and named a
write and a poll overlapping as the reason the caller passes its own key. That
overlap is refused: `write` and `read` both return at `_readLoopOwner`, and
`_poll` awaits each read before it arms the next. On a serial port the key is 1
for every request, so naming it discriminates nothing and the delete would take
an entry still in flight. The docblock says that, and says what keeps it from
happening: the client sends one request at a time.

`encodeRegisters` opened with "The words a value of this type occupies", which
is the question `registerWidth` answers. It says which of the two it is.
`interpolation.tsx`'s four fields, x1, x2, y1 and y2, shared a mask with four
on the server side and wrote no `scale`, so `IMask.MaskedNumber` fell back to
its own default of 2. The mask cuts while you type, so an endpoint of 0.0625
went in as 0.06 and every row scaled through it came out 4% off. The other four
wrote `scale: integer ? 0 : 7`.

`decimalMask(integer)` is the one copy, beside `integerMask`, and it takes all
five sites. `decimalMask.test.ts` holds the scale at 7 and at 0, and was seen
red with the 2 back in place.

On this commit alone: lint 0, typecheck 0, 68 files and 1392 passed, measured
in a worktree at it.

This left `0f67bff` afterwards. It was the one behaviour that commit changed,
and a commit saying `refactor` while a user sees a different number is not what
it says it is.

Claude-Session: https://claude.ai/code/session_018YrcWbyHtV9JPT4ppY4EZP
Round D of the annoying half: R20, T17, T18, T26, T27 and T31. R16 was the
seventh and is the commit before this one, because it is a fix.

T17 is `ServerPanel`, which holds the outer box, the Paper, the title and the
scroll box, and takes `openFlex`, `openMinWidth` and `contentSx`. The scroll
box lives there rather than in each caller because its `top: 38` is
`ServerPartTitle`'s `height: 38`, and the two have to move together. The
`collapse` guard went with it, so neither caller reads the field.

T18 is `utf8RegisterLength` and `utf8MaxBytes` in the dialog's helpers, plus
`registerLength` in `FIELD_DEFAULTS`. The width comes off `registerWidth`,
which C1 settled as the one table, and the five literal tens are gone.

T26 is `setSerialOption` and `setRegisterConfigField` in `client.zustand.ts`.
Nine setters that differed in nothing but a key are one line each. The eight
that stay written out each add something: a payload of another shape, a string
to convert, a validity flag, a grid to clear, a read to ask for.

T27 is `boolArraysOf` in the server store's helpers. `resetBools` and
`syncBoolsWithBackend` each built two arrays of 65536 from one unit's map, and
main writes both by index on every sync, so the type that is not being cleared
has to arrive carrying what it had.

T31 is the three layout toggles writing `!state.showLog` rather than reading
the same field back through `get()`, which no longer reaches the factory.

R20 is nineteen short names spelled out, across the two stores, the server
store's helpers and `ServerBooleans`. Four more left with T27's fold.

Three test files added and two extended, and each was seen red first: a wrong
key in the setter fold, a pair that answers the cleared arrays twice, a
fallback that no longer catches an empty field, a toggle stuck on, and the
collapse guard taken out.

Measured over this commit and the fix before it together, which is how round D
was built: lint 0, typecheck 0, 71 files and 1407 passed. E2E 02, 04, 06, 09,
15, 16, 18, 26 and 30: 242 passed.

Claude-Session: https://claude.ai/code/session_018YrcWbyHtV9JPT4ppY4EZP
Round D's own line sat under Changed while its body described a wrong value.
The same Unreleased block has a Fixed section, and a user looking there for a
reading that came out low would not have found it.

Claude-Session: https://claude.ai/code/session_018YrcWbyHtV9JPT4ppY4EZP
R17, R25, T23 and T28.

The v1 server config wrote the bool shape migration out a third time, and the
copy differed from `migrateBoolShapeForUnit`: it built a new record per bool
type, so a `coils` or `discrete_inputs` holding a number or a true became an
empty record and loaded. The triage read both outcomes as refused by the final
`safeParse`, which holds for a string and not for those two. The helper runs
over the units before the loop reads them now, and `V1ServerRegisters` says so.
A changelog line, because Load reaches it.

`migrateServerModeState` wrote the four serial defaults as a literal beside
`defaultSerialPortOptions`, which `getDefaultSerialConfig` spreads. Three
copies, two agreeing by construction and this one by coincidence.

`isNumberRegister` and `isBooleanRegister` are built from `.options` and take
the seven sites that wrote one of the two pairs out by hand, five in the client
toolbar and grid, one in `migrateBoolShapeForUnit` and one in the register scan.

T23 is a measurement and a comment. The finding's 41.37 ms at 2000 registers
describes a buffer no read can carry: FC03 and FC04 answer at most 125
registers, so the widest `utf8` tail is 250 bytes. Capping each word at
`MAX_UTF8_READ_REGISTERS` took a 125 register conversion from 2.88 ms to
2.58 ms, median of 40, and cut what the grid draws for a string read with read
configuration off from 250 characters to 48. Jens: the protocol is the ceiling,
so there is nothing to cap.

T28's cycle between `client.zustand` and `data.zustand` stands. A module-scope
`useClientZustand.getState()` beside `data.zustand`'s `create` threw "Cannot
read properties of undefined (reading 'getState')" before
`client.zustand.test.ts` ran a case, and the comment says so where an edit would
add one.

Claude-Session: https://claude.ai/code/session_018YrcWbyHtV9JPT4ppY4EZP
…about

The review of `ddcde7b` found three.

The half-open door the commit closed for the bool records was one level up as
well. `if (!serverRegisters) continue` let a truthy non-object through, and
reading `coils` off a number answers `undefined`, so the unit loaded empty:
`{"serverRegistersPerUnit":{"1":5}}` opened with unit 1 gone, and
`{"serverRegistersPerUnit":"ab"}` opened two empty units keyed 0 and 1, both
reporting the file had been updated from an older format. Each unit is read as
`unknown` and named after the check now, because narrowing the V1 type with
`isRecord` takes the bool records' own type with it. A list still loads as an
empty unit: `isRecord` takes one, and widening that guard reaches seven other
callers. The changelog line grows to say all of it.

`maxReadQuantity` was an eighth site asking the same question by hand, at the
cost of a Zod parse per element. The three remaining
`Array<'input_registers' | 'holding_registers'>` literals stay: they describe
the v1 file shape, which should not follow the enum.

The serial defaults test asserted the values and not the copy, so it passed
whether `migrateServerModeState` spreads `defaultSerialPortOptions` or hands the
object itself over. `default.ts:17` states the copy, so the identity is its own
half.

Claude-Session: https://claude.ai/code/session_018YrcWbyHtV9JPT4ppY4EZP
Round E of the annoying half: T19, T20, T21, T22, T24 and T32. Not split:
every one of the six changes what the app does, so there is no refactor here
for T20 to be a fix inside of.

T20 is `Math.floor` on the register count in `convertRegisterData`.
modbus-serial slices the response's buffer at the byte count the frame
declares and never compares that field with the frame length it checked, so a
device declaring an odd one leaves a trailing byte. The count was then 1.5,
`inRange32` was 0 < 0.5, and the read stopped on "Attempt to access memory
outside buffer bounds", taking the registers that did arrive with it.

T21 narrows `BitMapConfigSchema`'s key to `/^(?:[0-9]|1[0-5])$/`. It was
`!isNaN(Number(v))` refined to the range, the shape `RegisterAddressKeySchema`
was rewritten away from, and seven spellings got in that
`ServerBitMapDetail` and `BitMapDetailPanel` never ask for. Both write and
read the key as `String(bitIndex)`, so nothing shipped writes one and no
migration is owed.

T22 is `${key}.corrupt` rather than `${key}.corrupt-${now()}`, and
`keepCorrupt` loses its `now`. Every launch that repaired anything left
another whole blob in the store's own localStorage budget, and grep over `src`
and `e2e` finds no reader and no deleter.

T24 prefixes the port on `humanizeSerialError`'s third return too. It was on
the two branches Modbux writes the text for and off the one that hands the
driver's own text through.

T19 sorts the register rows by `params.address`, and drops a `replace` of a
space `NumberRegistersSchema` has no member holding. `Object.values` is
ascending over integer-index keys only and `RegisterAddressKeySchema` accepts
'007', which is not one, so a config carrying one drew that row where it was
written while `ServerBoolList` next door sorts.

T32 runs `repairPersistedStore` after the `window_update` rehydrate as well as
at load. That path ran `migrateServerState` alone, so a key hand-edited while
the split out window was up was installed unvalidated. `migrate` rewrites
`persistedVersion` on the way through, which is what the second call reads.
Jens took it over the comment the finding offered instead.

A changelog line for T20, T22 and T24. The other three need a hand-edited file
to reach.

Every item was seen red first, over seven test files: an odd buffer, seven
bitmap keys, a second corrupt copy, an unrecognised serial error, a row keyed
'007', and a rehydrate of a blob whose `port` does not parse.

lint 0, typecheck 0, 71 files and 1444 passed. E2E 06, 10, 21, 26 and 30: 99
passed. E2E 02-standalone: 25 passed.

Claude-Session: https://claude.ai/code/session_018YrcWbyHtV9JPT4ppY4EZP
…sses share

What `/code-review` found on `de9607e`, three of five.

The second `repairFromTheKey` read a stale `persistedVersion`. persist runs
`migrate` only where the blob's version differs from the store's
(`zustand/middleware.js`), so a re-read of a key already at the current version
leaves the number the launch put there. A launch off a newer blob puts a higher
one there and the store re-persists at the current version, so every close of
the split out window then reported "saved by a newer version of Modbux and was
read in full" again, with no field having failed. It reaches no hand edit: one
newer blob, then split and close. The rehydrate clears `persistedVersion` before
it re-reads, so an unmigrated key answers undefined.

That also took the copy `keepCorrupt` exists to keep. A second reset calls it,
and the key is fixed now, so the healthy blob overwrote the unreadable one.

'7' and '007' both pass `RegisterAddressKeySchema` and name address 7, so a row
key built from the address was the same string twice. Both rows draw and React
says the behaviour is unsupported: "Encountered two children with the same key,
`server_register_holding_registers_7`." Measured, because the finding read it as
one row with shared `expanded` state and that is not what happens. The key is
the map key now, which is unique by construction. `Object.entries` sorts on the
entry's address the same way.

The changelog line for `keepCorrupt` is gone. `git show main:src/shared/repairPersisted.ts`
does not resolve: the timestamped key arrived in `8ca2b10` on `chore/audit` and
is in no tag, so no release ever wrote one copy per launch.

The fifth is in TODO.md rather than here. A repair on this path defaults a field
while main keeps serving, and the load path's `init()` is not what the close can
call.

lint 0, typecheck 0, 71 files and 1446 passed. E2E 06, 10, 21, 26, 30 and
02-standalone: 124 passed.

Claude-Session: https://claude.ai/code/session_018YrcWbyHtV9JPT4ppY4EZP
Round A of the audit's non-blocking half, and the last of its six rounds. Every
item changes what the text says rather than what the app does, so the meter is
typecheck and the unit suite. It ran last because a comment adjusted first
describes the code the rounds after it move.

Three comments described something else. `resetServer` said `createServer` is
called "for the case where there is none, such as after a spell in RTU mode",
and after a spell in RTU mode is when it is not called at all. `_bindServer`
writes `_port` and `_servers` on consecutive lines and `_closeAndForget` deletes
both, so a stored port means a listener on that port, which `createServer`
answers at its first line. `f339d93` is where that became true and
`modbusServer.test.ts` has asserted it since. `MessageReceiver` said the server
window runs the server store "and no message listener", and `Server.tsx:55`
renders it. `migrations/server/zustand.ts` carried
`migrateServerRegistersState`'s docblock over `migrateServerModeState` as well
as over the function it belongs to.

Three names pointed at the wrong thing. `DEFAULT_MOBUS_PORT` misspells Modbus,
and nothing outside `modbusServer.ts` reads it, so it lost its `export` with the
rename. `src/shared` held two `shared.ts` and two `utils.ts`. Both `utils.ts` reach the
`@shared` barrel, through `index.ts` and through `types/index.ts`, and
`migrations/server/config.ts` imported from both `shared.ts` in one block, as
`'../shared'` and `'./shared'`. `migrations/server/shared.ts` holds the v1
server types and is `v1.ts` now; `types/utils.ts` holds two type-level helpers
and is `types/helpers.ts`.

`IpcEventTarget` says which of its three a caller names and what decides it. A
boundary is handed the asker, so `ipc.ts` answers `event.sender`; a module is
not, so it names the view that draws what failed. The two rules pick the same
window wherever the window that asked is the one drawing, and the deliberate
exception is a client channel refused from a window that is not the main one.
`send`'s docblock lost the two paragraphs that now sit on the type.

A scan's three pauses carry their cost. The widest unit id scan the fields
accept is 256 ids over four register types, which is 1024 progress calls and
5.12 s of pausing plus 2.56 s of the per-id pause; a register scan of 65536
addresses at a chunk of 100 is 656 chunks and 6.56 s. `8aed54a` brought all
three in with the scans and recorded no reason for any of them, so shortening
one is measured against a device rather than settled in a comment.

`littleBigEndian.md` is tracked, sat under `src/main/modules` and was named by
nothing. It documents `littleEndian32`, which lives in `src/shared/utils.ts`, so
it moved beside that file and the two name each other now.

`applyRegisterValue` looked back three addresses under a comment naming int64
and double. `MAX_NUMBER_REGISTER_WIDTH` is the 4 those two have, read off
`registerWidth` rather than written beside it. Set to 3, the four
`exact64BitValue.test.ts` cases go red: a word at offset 3 stops finding its
base.

lint 0, typecheck 0, 71 files and 1446 passed. E2E 06, 07, 08 and 09: 114
passed.

Claude-Session: https://claude.ai/code/session_018YrcWbyHtV9JPT4ppY4EZP
What `/code-review` found on `ded1cfb`, the only finding.

`MAX_NUMBER_REGISTER_WIDTH`'s docblock said it is how far back a word has to
look, and `applyRegisterValue`'s comment said the base is within that many
addresses back. Both are one wide: the loop visits `address` down to
`address - 3`, and a fourth step would reach a base a 4 register value starts
too early to cover, where `offsetRegisters >= registersCount` drops the word
instead of composing it. Two sentences, no code: the loop was already written
the narrow way.

lint 0, typecheck 0, 71 files and 1446 passed.

Claude-Session: https://claude.ai/code/session_018YrcWbyHtV9JPT4ppY4EZP
R4 of the audit's non-blocking half. Two calls ran from `client.zustand`'s
module tail with nothing awaiting them and no catch, in a file whose `init`
carries one for exactly that reason.

The app version was the one call that tail made from the split out server
window, and a docblock said why it could: `get_app_version` asks `app` rather
than the client, and Home is the only reader. Home is not the only reader.
`SaveButton` and `OpenSaveClear` both read `version` into the config they save,
and `OpenSaveClear` is the server view's, which is exactly what the split out
window draws. The fetch is `layout.zustand`'s now, beside the field it fills and
guarded on nothing, so the client store's tail is inside its guard end to end.

`stopScanningUnitIds` got the catch it owed. Both catches say what happened and
no more: main reports its own failures through `backend_message`, so a rejected
invoke carries the channel name and nothing the user can act on.

`clientStoreInServerWindow.test.ts` asserted `['getAppVersion']` under the title
"tells main nothing at module scope". It asserts nothing now, which is what the
title says. `appVersion.test.ts` is the pair: the version reaches the layout
store in both windows, and it is asked for once. Gating the new fetch the way
the client store's tail is gated takes the server window's copy away and two of
the three go red; removing it takes all three.

lint 0, typecheck 0, 72 files and 1449 passed. E2E 01, 05, 10 and 14: 51 passed.

Claude-Session: https://claude.ai/code/session_018YrcWbyHtV9JPT4ppY4EZP
What `/code-review` found on `c4092af`, the only finding.

"Everything this tail does is inside the guard now" is not true, and the commit
body said it too. The persisted repair sits below the guard and runs in both
windows, and so do the four `onEvent` registrations. What the guard covers is
every call this tail makes to main, which is what the docblock says now. The
repair's `setState` writes the shared key through persist, the way `init`'s
does, and the paragraph above already blames that mechanism.

`appVersion.test.ts`'s third case was titled "by the layout store" and counts to
one, which a fetch moved back into the client store would still satisfy. The two
cases above it are what pin the fetch, so the title says what this one asserts.

lint 0, typecheck 0, 72 files and 1449 passed.

Claude-Session: https://claude.ai/code/session_018YrcWbyHtV9JPT4ppY4EZP
R3 of the audit's non-blocking half. Two generics whose cast is what stopped
TypeScript asking, one on each side of the preload.

`createIpcHandle` parses `args[0]` and calls the listener with `[result.data]`,
and `PayloadSchema<C>` names `args[0]` alone, so a channel declaring two
arguments loses the second where a schema guards it and hands it over
unvalidated where none does. Measured: giving `request_logout` an `args` of
`[string, number]` under the old signature typechecks clean, and under
`OneArgumentChannel` it is TS2345 at the `ipcHandle` call, naming the channel.
Every spec entry declares `[]` or a one element tuple today, so the constraint
refuses nothing that exists.

`src/preload/index.ts` builds `window.api` with `Object.fromEntries` over
`IPC_CHANNELS.map(snakeToCamel)` and casts the result into the mapped type, so
two channels reaching one method name would drop a method with typecheck green.
The lowercase segments rule beside this one is what makes that impossible rather
than unlikely: every capital in a method name marks a segment boundary, so the
channel is readable back off it. That was an argument, and an argument with no
meter goes stale, so the conformance suite calls the real `snakeToCamel` and
counts. Adding `requestLogout` beside `request_logout` turns it red along with
three rules it already had.

lint 0, typecheck 0, 72 files and 1450 passed.

Claude-Session: https://claude.ai/code/session_018YrcWbyHtV9JPT4ppY4EZP
What `/code-review` found on `bc6ff4c`, all three.

The collision meter counted channels against each other and missed the name
already on the object. `src/preload/index.ts` builds `api` as
`{ isServerWindow, ...handlers }` with the spread last, so a channel named
`is_server_window` passes the lowercase rule, collides with no other channel,
and puts a function where `client.zustand`, `layout.zustand` and
`server.zustand` read a boolean. Every window would then read as the split out
one: no `init` in the client store, and no home button in the main window.
Added to IPC_CHANNELS it turns the rule red, along with three the suite already
had.

`snakeToCamel` is imported into this file now, and the caller rule had its own
copy of the same replace. The commit's own argument is that an unmetered copy
goes stale, so that rule calls the imported one too.

`OneArgumentChannel` refuses `[T?]` and `T[]` as well as a second argument,
which the docblock did not say. Both refusals are wanted: `args` is read as a
count, so a channel says how many it takes rather than how many it might.

lint 0, typecheck 0, 72 files and 1450 passed.

Claude-Session: https://claude.ai/code/session_018YrcWbyHtV9JPT4ppY4EZP
Every Unreleased entry was measured against the tree and against v2.3.0.
Four described a defect no release carried, because it arose and was fixed
on this branch; two of those are dropped and two fold into the entry they
belong to. One claimed a state only mocks reach and is cut. The rest say
what 2.3.0 did rather than what an intermediate commit did, and drop the
figures and clauses nothing measured.
Both counted a scan of 1 through 247, a range the unit id fields have not
had since they were widened to 255. The widest scan is 0 through 255 over
four types, 1024 events. The test comment names scanUnitIds rather than a
line number.
27-serial-line-settings switched the server back to TCP and left the
client on RTU, and only 28-connect-cancel's cleanup put it back. 25 and 28
kept their own list of socat paths without /opt/homebrew/bin, so on the
Apple Silicon runner 28 skipped and 29-read-on-read-configuration found no
host field. Run 35843977908 failed there; 27 followed by 29 reproduces it
locally with 27's old cleanup. Both specs use e2e/fixtures/socat.ts now,
and 27 switches the client back itself.

On Linux 28 read "Connected over Modbus RTU" in the snackbar container
before its cancel ran, next to the pty errors 27 left, so it failed on a
message it did not raise. It closes every snackbar before the cancel now,
which also keeps preventDuplicate from dropping a new one.
On the macOS runners a closed snackbar stayed in the DOM hidden, and
closeSnackbars waited 30 s to click it. A probe on run 35850109803 showed
the window focused and three snackbars going to one, the last one with a
close button Playwright called not visible. Only visible buttons are
pressed now, and afterwards no container may still hold "Connected over",
which is what preventDuplicate would drop a new one against.
The cancel test read the snackbar container for "Connected over", and
every way of clearing it first failed on some runner: a snackbar from an
earlier spec, a closed one left in the DOM hidden, and on the Intel runner
two visible ones that were still there after ten seconds of clicking close.
The defect the spec guards left the button on Disconnect, so it samples the
button for two seconds instead. With disconnect's _connectGeneration bump
removed, it reads ["", "Disconnect"] and fails; restored, 25 to 29 pass.
The only upload ran at the end of the job, after the dist/ run had emptied
test-results/, so a failure in the out/ run reached the artifact as the
passing dist/ run's logs. Run 35854594226 lost the Windows main-process log
that way. The out/ run now uploads its own on failure, as
playwright-out-<runner>.
…trace

keepOutput moves into launch.ts and runs inside launchElectron, so the
privileged port and hardware specs log main as well; the presentation
fixture calls it itself.

Playwright's trace option records only the contexts it creates, so a failed
test's trace held its assertions and no DOM. The fixture traces the app's
context with DOM snapshots, one chunk per test, and writes the chunk, with a
screenshot of every open window, only for a test that failed. A probe that
failed on purpose left electron-trace.zip, window-0.png and the main log.
01-main 01 to 28 took 6.6 min without it and 7.7 min with it, one run each;
with the screencast as well it took 8.5 min, so that stays off.
One app per worker carried each spec's leftovers into the next. 27 left the
client on RTU and only 28's cleanup put it back, and run alone, 12, 15 and
18 failed while passing in the suite. The fixture now launches the app when
a file's first test asks for it and closes it when the next file's does.

The three that leaned on an earlier spec set up for themselves: 12 loads
server-integration.json as 11 does, 15 loads client-basic.json as 14 left
it, and 18 no longer clicks Clear on a grid that connecting a fresh app has
already emptied. The afterAll hooks in 10 and 31 closed a server window for
the next file and are gone; a beforeAll or afterAll cannot take the app now.

yarn test:e2e: 647 passed in 11.3 min, against 646 in 7.5 min on 574e4b6
before this and the trace commit.
The read configuration test counted the rows in the DOM and asked for 20.
The grid virtualises, so that count is the window's height: 29 locally and
19 on the macOS arm and Windows runners of run 35873899922, where a fresh
app per file no longer inherits a window an earlier spec had drawn. It
checks aria-rowcount instead, which is every row and the header, 81 for
the 80 registers configured.
A fresh app per file made the suite 20 to 35 percent slower on the CI
runners, with the trace commit counted in, so the worker keeps one app and
resetApp brings it back to a fresh launch instead: it closes every window
but the main one, disconnects the client, deletes every server but 502 and
clears it, puts it back on TCP, then clears the storage and reloads, which
hands main the default client configuration through the store's init.

Every spec file under the fixture calls it in a beforeAll, and a test in a
file that did not is refused, so a new spec cannot leave it out. The
afterAll hooks in 10 and 31 stay gone; the next file's reset closes the
server window. yarn test:e2e: 647 passed.
The Windows e2e flake of run 35881205447 left a main log that ends in
window created, "Debugger ending", window closed and exit code 0, with
none of the lines the two app.quit() callers write. So the quit came from
somewhere this file does not log. before-quit now logs the stack that
asked for it and quit the exit code; a quit Playwright asks for shows
playwright-core's electron loader and an evaluate in that stack.
Every spec file resets the app in its beforeAll now, so a failure in one
file says nothing about the next, and the run can report every failing
file rather than the first. maxFailures goes from the base config. A probe
that failed on purpose, followed by 01-home-navigation, gave 1 failed and 7
passed. The presentation tour keeps maxFailures 1, because each scene
builds on the one before it.

A correction to a3ea1ea: its message says 18 passed on the macOS arm and
Windows runners before because a fresh app per file no longer inherited a
window an earlier spec had drawn. That cause was not measured; why 18
passed there before is not known.
Modbux's server refuses a unit it does not host at once, so the scan of
all 256 could end before the five disabled fields were read. It did in 3
of 7 runs of yarn test:e2e on Linux. That test now scans a device that
never answers, so every read waits out its 500ms timeout.

The two scans before it waited for Start Scanning, which the button
reads before main takes the request too. A scan still running there was
stopped by the next test's click. Both now wait for their last row.

Removing the Stop button's call turns the stop test red at Stop
Scanning, and a Length field left enabled turns it red at that field.
Recording the app's trace made 06, 11 and 21 together take 1.2 and 1.3
min against 1.0 min without, alternating runs on one machine, which
accounts for most of what the suite gained on CI today. The fixture traces
only when E2E_TRACE=1, which e2e.yml sets on both runs; a local run leaves
it off, and a spec that fails is run again on its own with it on.

A failed test gets a screenshot of every open window either way. A probe
that failed on purpose left one png without E2E_TRACE, and a png and
electron-trace.zip with it.
A failed reconnect ended the burst. The open's catch reported the error and
set the client disconnected, and a failed open fires no `close` to schedule
the next attempt. Measured on the Arduino on 23 Sep 2026 with the USB cable
pulled while polling: one attempt failing on "No such file or directory",
then nothing, and still disconnected 30 s after the cable was back.

A failed reconnect now reports its reason as an error, then a
"Reconnecting (n/5)..." warning, and schedules the next attempt until the
limit. Giving up clears the reconnect flag, so a manual connect after it
counts from zero and says Connected rather than Reconnected.

The first hardware run with the fix showed a second defect: the 10 s
stability timer of the connection that dropped fired mid-burst and reset the
count, 2/5 followed by 1/5. The drop clears it now.

Verified on the Arduino: cable out, attempts failing on the missing path,
cable back, "Reconnected over Modbus RTU" 0.6 s later and polling 1 s after
that. That run predates the timer clear and the split message, which the
five new unit tests cover; each rule was mutated and seen red. Lint,
typecheck, yarn test (1793 passed) and e2e 25 and 28 (13 passed) green.
Five comments in rtu.ts describe what `@serialport/stream` and
modbus-serial's `serverserial.js` do, and none said which version they read.
They now name 13.0.0 and 8.0.25, the versions in node_modules, so a bump
that changes that behaviour leaves a comment that says what it was true of.
Comments only.
`ServerBoolList` and `ServerRegisterRows` selected the whole map of their
register type and derived the sorted addresses from it. Mutative gives that
map a new identity on every value written into it, so the coil list drew
itself again on every toggle and the register list on every generator tick,
sorting its keys each time and comparing every row's props.

Both now select a string of their keys, which compares equal until a key
comes or goes, and each register row selects its own entry by key, as the
bool rows already did. A type-checker scan of the renderer's 269 store
selectors found 20 answering an object or array; these two were the ones
that used only the keys of what they selected.

Measured with a render counter in vitest: one extra list render per value
written, for both lists, before the change; none after. Reverting either
file to HEAD turns its test red. Lint, typecheck, yarn test (1797 passed)
and e2e 03, 04, 08, 21 and 30 green.
Both scans awaited `_sendScanProgress`, which slept 5 ms after sending, and
added a pause of their own: 10 ms per unit id before its result row, and
5 ms per register scan chunk. `8aed54a` brought all three in with the scans
and recorded no reason. A scan spent them rather than reported them, so a
device that answers quickly was asked at a fraction of its pace.

The pauses are gone. Progress goes out at most every 100 ms, and always on
the last step, without waiting. The unit id result goes out as soon as the
id is done.

Measured on localhost against Modbux's own server, a register scan of
10000 addresses: chunk 100, 2.0 s before and 0.54 s after; chunk 10,
13.1 s before and 1.9 to 2.6 s after. On the Arduino a scan of 100
registers at chunk 2 took 26.1 s before and after: at 9600 baud the device
takes about 520 ms a request, and no request failed either way.

Three stop tests relied on the pauses to keep a scan running long enough to
stop; their mock device now takes 1 ms per answer, as a real one takes
time. Three new tests cover the pauseless scan, the throttled progress and
the result per unit id; reverting to HEAD turns all three red, and removing
the throttle or its last-step clause turns the progress test red. Lint,
typecheck, yarn test (1800 passed) and e2e 11 and 12 (66 passed) green.
Found by opening a client config from May 2025 on the server: the server
cleared itself, loaded nothing and reported "Configuration updated from
older format". That file has no version and no `registerMapping`, so
`migrateServerConfig` read it as a v1 server config and built a server with
no units. The same on main (2fab763) and v2.3.0, measured.

The other mix-ups were refused on main with a Zod message naming a missing
field. On this branch one of them was worse: a server config is v3 now and
the client's own version is 2, so a server file opened in the client took
the newer-version path and loaded an empty mapping with a warning.

Both migrations now call `refuseOtherSidesConfig` first. A server config is
the file holding `serverRegistersPerUnit`, or `serverRegisters`, which a
server wrote before it had units (before 14893ff). The server refuses a file
with neither; the client refuses one with either. A client config from
before versioned files is a bare mapping, so the client keeps taking it.

`serverRegisters` also opens now, as unit 0, where it loaded as an empty
server before.

The test "still takes a file with no version at all" fed `{ name, coils }`
in as a server file, a shape no server ever wrote; it now carries
`serverRegistersPerUnit`. configKind.test.ts covers each direction and
format; each refusal and the unit 0 fallback was mutated and seen red.
Lint, typecheck, yarn test (1811 passed), e2e 05 and 14 (36 passed) green.
Address 65535 with a length of 2 in the unit id scan asked every unit for
65535 and 65536. The toolbar's Length field is bounded by the addresses
left (`registersFrom(address)`), and the scan's was bounded by
`maxReadQuantity` alone, so did `ScanUnitIDParametersSchema`; main passes
the length through. A sweep for the same pair found one more: an int32
written at 65535 went out as FC16 for two registers, and
`WriteParametersSchema` checked the address alone.

The scan's Length field now masks to the smaller of the two bounds, and
the mask rewrites a stored length when the address moves. Both schemas
refuse a read or write that runs past 65535 and name the field. The
register scan, the toolbar read and the server's add-register dialog were
bounded already.

A clamp in the scan request as well turned out unreachable: with the field
mounted the store already holds the bounded length, so removing it left
every test green. It is not in.

Tests: the scan and write schemas at the edge in both directions, and the
dialog sending 65535 with a length of 1. Each rule was mutated and seen
red. Lint, typecheck and yarn test (1822 passed) green. The e2e specs 09,
11 and 12 did not run: a dev app held port 502.
A unit id scan sends one result per unit id, and the table drew itself
again on each: with 255 ids the window lagged behind the scan. The results
are now held back and written every 100 ms, on the timer the rows of a
register scan already use, with what is left written when the scan ends
and dropped when the results are cleared.

`addScanUnitIdResult` became `addScanUnitIdResults`, taking the batch.
Three tests cover one write per flush in the order the table shows, the
flush on the state that ends the scan, and nothing outliving a clear; each
rule was mutated and seen red.
Turning read configuration on asked main for a read (c69455b), so the grid
showed values rather than the mapping's zeros. Jens found switching back and
forth flaky: the read the switch started could land after the switch went
back, and a read of the toolbar's range then filled a grid drawing the
mapping. The switch now tells main the flag and asks for nothing; Read or
the poll fills the values.

The toggle still greys while a read, a write or a scan owns the client,
because that answer would land after the switch the same way; the comment
says so now.

`setLittleEndian` keeps reading through `readWhenMainCan`, and its tests
take over the states the toggle's covered (disconnected, polling, a
register scan). Spec 29 pins the new behaviour: a second after the switch
the grid still holds zeros, and Read brings 1 and 51. Spec 31 presses Read.
Unit suite 1820 passed; e2e 11, 12, 14, 17 to 21, 23, 29 and 31 (252
passed).
…pped

With the scan pauses gone (ca27c9a) a register scan in chunks of one
against a server that answers at once sent a transaction per request
faster than the renderer wrote them. On the E2E run on ca27c9a the Stop
click in 11-scan-registers waited 30 s for the window on ubuntu arm and
macOS Intel. Transactions are now held and written every 100 ms, on the
timer the scan rows and the unit id results use; the unit id results and
the transactions share one helper, `heldOnTimer`.

The same run failed 12-scan-unitids on three runners for a different
reason: the test waited for the row of unit 200, and the table shows the
newest result on top, so a scan faster than the check left 200 below what
the grid draws. It waits for 255, the last id, instead.

Tests: one write per flush in log order, and nothing outliving a clear;
each rule mutated and seen red. A check that skipped an empty flush was
unreachable, since an empty batch changes nothing, and is gone. Unit suite
1820 passed; e2e 11 and 12 among 252 passed locally.
`modbus-serial` words exception 11 as "Gateway target device failed to
respond (retry request again later)". The spec (V1.1b3, section 7) says no
response came from the target and the device is usually not present on the
network, and Modbux's own server sends 11 for a unit it does not host, where
retrying changes nothing. Filed upstream as yaacov/node-modbus-serial#631.

`errorText` in the client reads an error's message with that one hint
replaced, and every site that passed a library message on (the snackbar,
the transaction log, read configuration's error rows, the scan results)
reads it through `errorText`. Exception 6 keeps "retry request again later",
which is right there.

Tests: exception 11 reaches the snackbar in the spec's words, exception 6
is left alone, and an error with no message still reads "Connection error";
the replacement and the empty-message fallback were each mutated and seen
red. Lint, typecheck, yarn test (1823 passed).
…ckbar

Both failed on the E2E runs at ca27c9a and 57ae017, one mode each, and
neither for a reason in the code under test; the traces say why.

25, RTU deliberate disconnect: the disconnect worked (the button read
Connect 30 ms after the click), but "Disconnected from server" never
showed. The TCP half's snackbar with the same text was still leaving, and
`preventDuplicate` in main.tsx drops a message equal to one notistack still
holds during its exit transition. A socat pty closes within milliseconds,
so the second message landed in that window. The test now waits until the
first is gone before it disconnects.

18, Huawei manual client: row 40736 was never in the DOM. The grid
virtualises, and at the 1024 by 768 window macos-26 had, a group of 31
rows reaches past what it draws. `configureRegister` scrolls to the row
first, and the comment that said 33 rows avoid virtualization no longer
does. The same spec failed on macos-26 at f5832ad the same way.

Both specs pass locally (44 passed).
`electron-devtools-installer` (4.0.0, devDependency) loads React Developer
Tools before the first window opens, the way Electron's docs recommend; they
list React Developer Tools among the extensions tested to work. The branch
sits behind `import.meta.env.DEV`, which a build drops with the import: a
search of out/ for the installer and the extension's id finds nothing, so
neither a build nor the e2e suite downloads anything.

Checked by Jens under yarn dev: the Components and Profiler tabs find React
after one reload with DevTools open.
@Harted
Harted force-pushed the chore/audit2-annoying branch from cf7da84 to 3313c0f Compare September 23, 2026 21:35
@Harted
Harted marked this pull request as ready for review September 23, 2026 21:35
@Harted
Harted merged commit 3313c0f into main Sep 23, 2026
1 check passed
@Harted
Harted deleted the chore/audit2-annoying branch September 23, 2026 21:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant