Repository navigation
The second audit's non-blocking half, and assignments A to E - #59
Merged
Merged
Conversation
Harted
force-pushed
the
chore/audit2
branch
from
September 23, 2026 21:34
a83ee0f to
e7c56da
Compare
`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
force-pushed
the
chore/audit2-annoying
branch
from
September 23, 2026 21:35
cf7da84 to
3313c0f
Compare
Harted
marked this pull request as ready for review
September 23, 2026 21:35
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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/audit2rather thanmain.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.mdunder Unreleased.How to test
yarn verify. The E2E workflow runs on this branch on all five runners.