Repository navigation
The second audit's blocking half - #57
Merged
Merged
Conversation
Harted
force-pushed
the
chore/audit-annoying
branch
from
September 23, 2026 21:32
ffabbc2 to
141061b
Compare
grep -c '^describe(' src/__tests__/conformance.test.ts returns 12. At nine, an
auditor reports three rules the suite already asserts.
Claude-Session: https://claude.ai/code/session_018YrcWbyHtV9JPT4ppY4EZP
`App.tsx` imports `containers/Client` statically and `Client.tsx` imports `client.zustand`, so `out/renderer/assets/` holds one js file and both windows evaluate that module scope. It called main five times from whichever window ran it, and `set_read_configuration(false)` is the one that costs: `modbusClient.ts` reads that flag, and off it polls one flat `[address, length]` block instead of the mapping's groups, while the main window's toggle still reads on. `init`'s own `set` also wrote `CLIENT_ZUSTAND_STORAGE_KEY` from the split window, because persist wraps `setState`. Closed from both sides. `client.zustand.ts` gates its module tail on `isServerWindow`, the question `layout.zustand.ts` and `server.zustand.ts` already ask. And `createIpcHandle` refuses the fourteen channels that drive main's one Modbus client when they come from a window that is not `windows.main`, so a caller added to that module later, or a client component mounted in the server window, is refused rather than served. `list_serial_ports` and `validate_serial_port` stay open, because they enumerate hardware and the RTU server's COM field reads the first of the two from the server window. The version fetch stays where it is. `get_app_version` asks `app` rather than the client, and `containers/Home.tsx` is the only reader of what it answers, which `App.tsx` does not draw in that window. So S-16 was not the precondition the cluster document called it. Claude-Session: https://claude.ai/code/session_018YrcWbyHtV9JPT4ppY4EZP
`RegisterParamsSchema` bounded `address` through `RegisterAddressSchema` and left three fields bare, and a config file and a persisted blob both arrive on it and nothing else asks. `length` reached `createStringRegisters`, which is `Buffer.alloc(length * 2)`: at 1e12 Electron 43 answers ERR_OUT_OF_RANGE, at 1e9 it allocates and then builds a billion words, and `getUsedAddresses` loops the same number. `value` reached `createRegisters`, where `writeUInt32BE(-1)` and `writeBigUInt64BE(2n ** 64n)` throw and `BigInt(1.5)` throws. `interval` had a floor and no ceiling, and above 2147483647 `setInterval` warns `TimeoutOverflowWarning` and sets the duration to 1. Each of the three rejected the invoke `init` awaits, which leaves every `ready` false and the server view faded out on that launch and every one after it. The bounds were already stated elsewhere and are applied here now. `getValueRangeError` sits beside `createRegisters` and reads `getMinMaxValues`, the table the add dialog's mask reads, over the seven types whose writer throws: `float` and `double` take anything, `unix` goes through `value >>> 0`, `datetime` through the clamp in `encodeIEC870DateTime`, and `none` and `utf8` never reach the encoder, so a rule over those would refuse a config that works. `getAddressFitError` is the pair of `address` and `length` and is the add dialog's own rule. A stored register outside the bounds is dropped rather than refused with the blob around it, which is what store version 5 and 6 already do for an address and an interval. Version 7 runs the same step, and `dropUnservableRegisters` reads `RegisterParamsSchema`, so tightening the schema is what tightens the drop. A config file is still refused whole, with the register and the field named, because the user picked the file. Claude-Session: https://claude.ai/code/session_018YrcWbyHtV9JPT4ppY4EZP
…claim back `/code-review` on `218a60f` found four things. `CLIENT_CHANNELS` was a hand written list with nothing asserting it stays complete, which is the defect class the commit closes. The thirteenth conformance rule reads the handler's own body: a listener naming `client` or `state` reaches what one window owns, and it has to be in the list or in the test's own three named exceptions. Proved by adding a `get_app_version` handler that reads `client.state`, which the rule printed. The test named `refuses a guarded client channel before it reads the payload` did not measure that ordering: its payload was valid, so both orders answer the same. It now sends a payload the schema also refuses, and asserts the message names the window and not Zod, with the main window's half beside it. Spec 31's assertion was already satisfied by the rows the test before it left, so detection rested on a scroll being slower than a localhost read. It clears the grid first, and the row is asserted absent before the Read. Measured red again with the guard out both sides. `Windows.isMain` had no test of the real implementation, because `ipc.test.ts` replaces it. `windows.test.ts` covers the main window, the other window, no main window and a destroyed one. Claude-Session: https://claude.ai/code/session_018YrcWbyHtV9JPT4ppY4EZP
`addRegister` called `createRegisters` unguarded, so a value the data
type cannot take threw straight out of it. `createIpcHandle` puts no try
around a listener, so the invoke rejected rather than answering, and
`syncServerRegisters` adds in a bare `for`, so the throw took every
register after it in that unit with it. `server.zustand.helpers.ts`
awaits that invoke before it writes `ready`, and `containers/Server.tsx`
wraps the whole server view in `<Fade in={ready}>`, so one such register
left the view blank on that launch and every one after it.
One register is refused now. `addRegister` answers `undefined`, which the
store already reads as nothing changed, and the window showing the server
is told which register and why. Nothing is written for it, and the rest of
the unit stands.
This is the class answering for its own input rather than a reachable
defect. `src/main/modules/__tests__/unencodableRegister.test.ts` measures
the three doors `AUDIT-clusters.md` names as still open behind it, the add
dialog, a repaired persisted store and the width overflow in `M-05`, and
`RegisterParamsSchema` refuses all seven inputs on both guarded channels
since `a301600`. `addRegister` is public and its only two callers cross
that boundary, so no shipped path reaches the refusal, and it gets no
changelog line.
The generator branch is not answered for here. `ValueGenerator` encodes
inside an unawaited async tick, so a `min` or `max` its data type cannot
take writes nothing and warns once per interval instead of throwing.
`RegisterParamsGeneratorPartSchema` leaves both bare on purpose, so that
door is open, and closing it is `M-02` on the annoying branch.
Claude-Session: https://claude.ai/code/session_018YrcWbyHtV9JPT4ppY4EZP
`containers/Server.tsx` faded the whole server view in on `Object.values(z.ready).some(...)`. `ready` is per uuid and written only by `syncUuidToBackend`, after every invoke in it got through, and `init` sets every uuid false before its loop. So one rejected invoke anywhere in that loop left the view blank on that launch and on every launch after it, with no message and nothing on screen to clear the configuration with. The verdicts of `M-03`, `SH-01`, `SH-02` and `M-16` all end at that state, and fixing any one of them removed a door and left the latch. Two questions, two flags. `initialized` says `init` has run, whatever came of it, and the view draws on that. `ready` keeps its meaning, which is whether main knows this uuid, and keeps gating `setPort`, `setUnitId` and `setLittleEndian`. A failed sync now costs the one server it belonged to. `init` catches as well as sets the flag, because it runs from module scope with nothing awaiting it, so a rejection there was an unhandled one. Main reports its own failures through `backend_message`, and a rejected invoke carries the channel name and nothing the user can act on. The other half of the same flag: `ready` is not in `partialize`, so the `window_update` rehydrate brought back `uuids`, `port` and `selectedUuid` from the split out window and left `ready` where this window's `init` had put it. A server made in that window came back with no entry at all and refused its three setters with no message until a restart. The window that made it ran `createServer` for it, so main does know it, and the rehydrate says so. Claude-Session: https://claude.ai/code/session_018YrcWbyHtV9JPT4ppY4EZP
…eplaced
`/code-review` on `a301600` found six things.
`min` and `max` stayed bare on the reasoning that a rule there would
refuse what the Add button sends and cost the whole persisted register
map, and both halves are measured false. `useMinMaxInteger` already masks
both fields to `getMinMaxValues(dataType)` over the same seven types that
`getValueRangeError` reads, so switching a generator from `uint32` to
`uint16` rewrites a max of 100000 to 65535 and reports it valid. And
store version 7 drops the one register rather than the map. The door was
live: `{ dataType: 'uint16', max: 1e9 }` from a config file hands a draw
of about 5e8 to `writeUInt16BE` inside an unawaited async tick, so the
register answers 0 for as long as the generator runs and main takes an
unhandled rejection every interval.
`addRegister` disposed the generator it was replacing and zeroed its
words before it tried the encode, so a refusal answered `undefined`, the
store read that as nothing changed, and the grid went on drawing a
generator that no longer ran. It encodes first.
`length` was bounded by the map and not by the bound already stated for
it. `MAX_UTF8_LENGTH` is that bound, 124, which `RegisterLengthInput` has
always masked to and which the protocol puts a ceiling under either way,
because FC3 and FC4 answer at most 125 registers. At 65536 one string
made `addRegister` send 65536 `register_value` messages in one
synchronous loop. `RemoveRegisterParamsSchema.length` takes the same
bound, because `removeRegister` loops `registerWidth` times writing into
the register array.
The fit issue named `length` whenever a register carried one, and
`registerWidth` reads `length` for `utf8` alone, so a `uint64` still
carrying one from an earlier edit was refused at a field with no bearing
on its width.
`utils.ts` reads the three type modules rather than the barrel. The
import `a301600` added made a cycle through `types/index.ts` that costs
nothing only while every name in it is a type the compiler erases.
Claude-Session: https://claude.ai/code/session_018YrcWbyHtV9JPT4ppY4EZP
`useOpen` awaited `resetServer(selectedUuid)` above its `try`, and `resetServer` reaches main and then clears the store. The parse came after. `migrateServerConfig` throws on both shapes a user picks by accident, a client config file and malformed JSON, and the `catch` only enqueues a snackbar, so the configuration that was on screen was gone in main and in the store before anything had read the file. The reset moves inside the `try`, after the migration returns. `migrateServerConfig(content)` needs nothing from the store, and the reason the reset exists, that a unit the file does not name must not keep answering a master, is kept by doing it later rather than first. `file.text()` moves in with it, and the flag it sets comes off in a `finally`, because a file that is gone by the time it is read left `opening` true and every server button disabled until the app was restarted. `05-file-io` opens `client-basic.json` into the server view. Without the change it reads `Holding Registers (0)` where the config it just loaded had two. Claude-Session: https://claude.ai/code/session_018YrcWbyHtV9JPT4ppY4EZP
Both migrations opened with `JSON.parse(raw)` and then `parsed.version ?? 1`. On a number, a string, a boolean or an array that is `undefined`, so 1, so the v1 to v2 step ran, and every field the server migration needs sits behind a `??`. So `5`, `[]`, `"hello"` and `true` each came out a valid empty v2 config, and the user was told the configuration had been updated from an older format. `null` reached them as `Cannot read properties of null (reading 'version')`. The client migration refused the same input, but only because `migrateClientV1toV2` feeds the whole blob into `registerMapping` and the final `safeParse` catches it, which is an accident of that one field. `parseConfigFile` is the guard, before `.version` is read, in both. It answers the version as well, and that half was not in the finding. `JSON.parse` answers `any`, so nothing had asked what was in that field, and a `version` of `"3"` or `true` took the v1 path with everything else. Typing the blob as a record of `unknown` is what surfaced it: six lines of `migrateServerConfig` and `migrateClientConfig` stopped compiling. Claude-Session: https://claude.ai/code/session_018YrcWbyHtV9JPT4ppY4EZP
`/code-review` on `53fcc7c` found four things, and the first of them means that commit did not do what it said. The `try` stood around the whole loop, so a refusal for the first uuid threw out of it and `createServer` was never called for the second at all: no listener on its port, none of its registers in main, and `ready` false with no message. The commit message and the changelog both claimed a refusal cost the one server it belonged to, and now it does. The catch sits in `openServer`, per uuid. That `try` also covered three mutative recipes and `cleanOrphanedServerState`, and swallowed a throw out of any of them with no line anywhere. The narrowed one covers the invokes its comment describes, and prints what it caught, because a throw out of the store's own recipes is the other kind and nothing else says so. `markRehydratedUuidsReady` wrote `true` over every uuid rather than the ones the rehydrate introduced. A uuid this window's own `init` wrote `false` for is one main refused, and handing the three setters back for a server main has no listener for is the defect in the other direction. It writes only where the entry is missing. The rehydrate chain takes a `catch`. `rehydrate` reads the key and runs `migrateServerState` over what it finds, and either can throw on a blob a hand edit left there. Claude-Session: https://claude.ai/code/session_018YrcWbyHtV9JPT4ppY4EZP
The `detectedVersion > CURRENT_..._CONFIG_VERSION` branch cast the parsed
JSON to the config type and returned it, in both migrations, so it was
the one door into the app that no schema stood in. `LoadButton` calls
`replaceRegisterMapping(config.registerMapping)` with no gate at all, so
`{ version: 9, registerMapping: 'nope' }` went into the persisted store
and was flushed to main. The snackbar read "This config was created with
a newer version of Modbux. Some features may not work correctly.", which
is a compatibility notice for something nothing had checked. Both
branches had a test and both fixtures were well formed, so the suite was
green on the half that works.
`repairPersisted` is the answer, which is what a persisted store already
gets: parse field by field against the current schema, keep what matches,
default the rest and name it. A config from a newer Modbux shares its
fields with this one, and the fields it shares are the ones worth
keeping.
`MigrationResult.reset` carries what did not come across, and the two
snackbars read `resetMessage`, which already had the wording for a blob a
newer version wrote, down to the case where it came across in full.
`FIELD_LABELS` gains the three fields a file carries and a store does
not, because the two name some of the same things differently:
`serverRegistersPerUnit` against `serverRegisters`, plus the two version
fields.
`emptyRegisterMapping` moves to `shared/default.ts`. The client store had
it and the client repair needs the same thing to default the field to.
Claude-Session: https://claude.ai/code/session_018YrcWbyHtV9JPT4ppY4EZP
`/code-review` on `f9184b8` measured what a `version` field that is not a number actually did, and the docblock, the test comment and the changelog line all said something else. A relational operator coerces: `"3" > 2` and `2.5 > 2` are both true, so those two took the future version branch and came back `migrated: false` with the persistent newer-version warning, never the v1 migration. `true` and `null` compared false against both and took the v1 path. Four values, three outcomes. The changelog claimed one symptom for all of them, and `null` was a fourth: the migration threw on it, so the user saw a red failure rather than "updated from an older format", after the server had already been emptied. `migrateClientConfig` reaches the same guard and had no case sending it a bad version. Claude-Session: https://claude.ai/code/session_018YrcWbyHtV9JPT4ppY4EZP
`/code-review` on `e8d01d9` found six things, and the first two mean that
commit traded one defect for another. `repairPersisted` reads a field
whole, and `serverRegistersPerUnit` is the one field a server config is
about. `DataTypeSchema` is a closed enum and `RegisterParamsSchema` a
closed union, so a config from a newer Modbux carrying one register with
a data type this version does not name failed the field, the field became
`{}`, and every register on every unit was gone, after `useOpen` had
emptied the store and main. Under the cast those registers landed.
`registerMapping` is the same shape on the client, and it is the one
thing in the client store built by hand.
`dropUnservableConfigRegisters` and `dropUnreadableConfigMapping` run
before the field repair, which is what `dropUnservableRegisters` and
`dropUnmappableRegisters` already do for the persisted store. They walk
`serverRegistersPerUnit[unit]` and `registerMapping[type]` where those
walk one level deeper, which is the difference
`migrateBoolShapeForUnit` already carries.
`warning: 'FUTURE_VERSION'` and `reset` were only ever set together, so
the caller asked twice and the second question was one no input answered
no. One field, `futureVersion`, carrying what did not come across.
Three on the wording. The field list opened a sentence, so it read
"Modbux. the registers did not come across"; the sentence is one clause
now. `version: 'the config version'` was a label nothing can reach,
because `parseConfigFile` refuses a version that is not an integer and an
absent one gives 1. And a notice that everything came across is not one
the user has to dismiss.
Claude-Session: https://claude.ai/code/session_018YrcWbyHtV9JPT4ppY4EZP
`ServerRegisterEntrySchema.value` was `z.number()`, and `applyRegisterValue` read the composite back with `getBigUint64` and stored `Number(...)`. Four words of 0xFFFF came out 18446744073709552000 rather than 18446744073709551615, and the next single word write read that back through `BigInt`, which `setBigUint64` takes modulo 2 ** 64 rather than throwing, so the entry collapsed to the low word alone. Measured: one more word after the flush left it holding 1. The batcher held the exact composite, so the loss only showed once a flush had cleared it and the fallback was the entry. A decimal string for the three types whose composite fills 64 bits as an integer, `ServerRegisterValue`. A `bigint` is not an option: the store is persisted through `JSON.stringify`, which refuses one outright. `double` is the fourth type four registers wide and stays a number, because its composite is a float64, which is exactly what a JS number is. `parseIEC870DateTimeValue` takes a `bigint` now. It read the four words back out of a number with `Math.floor` and `& 0xffff`, so the encoding of 2099/12/31 23:59:59.999 rounded up by a millisecond and the all-0xFFFF sentinel arrived as 2 ** 64, which the range gate answered rather than the sentinel branch. That branch is reachable from this side now. Store version 8 rewrites the value every blob on disk carries for those three types. A number still parses, so nothing is dropped by leaving them: the step is so the first word write after a launch reads an exact value. A value already past 2 ** 53 cannot be recovered, and the string says what was stored rather than what the device holds. Claude-Session: https://claude.ai/code/session_018YrcWbyHtV9JPT4ppY4EZP
Nothing between the keyboard and the wire clamped. `LengthField` passed no `max`, so `UintInput`'s default of 65535 was typeable; `setLength` stored it as given; `ScanUnitIDParametersSchema.length` was `z.number().int().positive()`; and `_scanUnitIds` handed it to `this._readers[registerType](address, length)`, which `modbus-serial` writes into the quantity field with nothing between. A device answers illegal-data-value or says nothing, and what the scan told you was about the request rather than about the device. The ceiling is the protocol's, stated once in `ranges.ts`. MODBUS Application Protocol Specification V1.1b3 section 6: FC01 and FC02 answer at most 2000 bits, FC03 and FC04 at most 125 registers, which is the 253 byte PDU minus the function code and the byte count. `maxReadQuantity` reads the register types selected, because one length goes out for every one of them and the strictest is the one the request has to fit. Three places ask it now. The scan's Length field masks to it, `ScanButton` clamps to it where the request is built, because a length typed under one set of types stays in the store when another is selected, and the schema is the backstop. `ScanRegisters.tsx ChunkSizeField` computed the same pair by hand and reads it instead. `LengthInput`, which caps a coil read at 125 in the other direction, is `CU-11` and stays for the annoying branch: too narrow is not illegal. Claude-Session: https://claude.ai/code/session_018YrcWbyHtV9JPT4ppY4EZP
`readOnly` was set from one place, `readOnly={collapse}` in
`ServerBoolRow`, and `ServerBooleans` renders that list only under
`{!collapse && ...}`. Both read the same store and React renders the
parent first, so no committed render carried `readOnly: true`. `collapse`
is persisted, so a restart into a collapsed section starts on the same
side. The same half guarded `ServerBoolRow`'s hover styles and its remove
button, and two tests in `ServerBit.test.tsx` asserted behaviour nothing
in the app reached, which is the severity table's own clause about dead
code that makes something look covered.
The prop, the `collapse` selector in `ServerBoolRow` and the two tests
go. `collapse.test.tsx` states why instead: the section shows its bits
while it is open and renders no bit at all once collapsed.
Claude-Session: https://claude.ai/code/session_018YrcWbyHtV9JPT4ppY4EZP
… cannot break a blob `/code-review` on `2376f64` found four things, and the first two matter. `ServerRegisterEntrySchema` is read by the persisted store and by `ServerConfigSchema`, and only the store version moved. So a config this build writes still claimed v2 while carrying a decimal string, and v2.3.0 takes its `detectedVersion === CURRENT` branch, fails `ServerConfigSchema` on `z.number()` and refuses the whole file rather than the one register. In the other direction a v2 file kept the rounded number, because no config migration ran the rewrite, so File then Open put the defect back. `CURRENT_SERVER_CONFIG_VERSION` is 3 and `migrateServerV2toV3` runs the same helper. `migrateBoolShapeInConfig` goes with it, because it ran on the current-version branch alone and a v2 file carrying the old `boolean` entries takes this step now. `String(entry.value)` could produce a string the new schema refuses: `String(0.5)` is `"0.5"` and `String(1e21)` is `"1e+21"`, and the union takes either as a number and neither as a string. `repairPersisted` reads `serverRegisters` whole, so one such entry cost every server and every unit, and `usedAddresses` survived beside it. The rewrite tests the digits, so it can never turn a blob that parsed into one that does not. The test is the digits rather than `Number.isSafeInteger`, because an integer above 2 ** 53 is exactly what this step is for. `toExact64Bits` answers `undefined` rather than `0n`. Returning zero turned an aborted merge into a composite rebuilt from zero, so one word write would have cost the other three registers. `applyRegisterValue` returns and the row shows a dash, which is what a register the server has not written shows. `ServerRegisters.test.tsx` feeds the datetime cell the decimal string the store now holds, including the far end of the window, which read 2100/01/01 through `Number`, and a fractional value no composite can be read out of. Claude-Session: https://claude.ai/code/session_018YrcWbyHtV9JPT4ppY4EZP
`_logTransaction` read `rawTransaction.nextDataAddress`, and `shared/types/client.ts` declared it required. modbus-serial files it on two of its twelve transaction records, inside `writeFC4` at index.js:880 and `writeFC6` at 983, and `writeFC1` delegates to `writeFC2` while `writeFC3` delegates to `writeFC4`. So FC3, FC4 and FC6 carry an address and FC1, FC2, FC5, FC15 and FC16 do not, and `_columns.tsx` binds `field: 'address'` with no formatter, so the Addr cell was blank for every coil read, every discrete input read and every write Modbux sends. `_requestedAddress` reads it off the frame instead. Every `writeFCx` builds one as the unit id, the function code, and the data address at offset 2 as a big-endian word: measured over FC2, FC4, FC5, FC6, FC15 and FC16, which is every code Modbux sends. `_writeBufferToPort` stashes the frame while debug mode is on, and `_connect` sets `isDebugEnabled`, so a frame that went out has one. `nextDataAddress` is the fallback. The type stops asserting what the library does not file, and `TransactionSchema.address` is optional with it, because a request that never went out carries neither and a blank cell is the honest answer there. Reading 0 would be an address. Claude-Session: https://claude.ai/code/session_018YrcWbyHtV9JPT4ppY4EZP
…rite `/code-review` on `3bcb7e3..a0e32b6` found four things, all low, and two of them are the same defect from two sides. `toExact64Bits` answering nothing aborts `applyRegisterValue` before it rewrites the entry, so a stored value no composite can be read out of froze that register in the grid for good: a master writes all four words, main serves the new value, and the row keeps showing the old one with nothing saying why. The `0n` it answered before self-healed on the first write. `stringifyExact64BitValues` writes `'0'` where the digits are not there, which is what a register the server has not written holds, and a fraction on a 64 bit type is not a value any device wrote. The `detectedVersion === CURRENT_SERVER_CONFIG_VERSION` branch skipped the rewrite the version bump exists for. `ServerConfigSchema` takes a number for a 64 bit value as well as a string, and the persisted store's own step is gated on the store version and does not run on a config load, so opening a v3 file holding the number folded the first single word write from the rounded value. `_requestedAddress`'s docblock said `isDebugEnabled` is set at construction. It is set in `connect`, which matters: the disconnect timeout builds a replacement `ModbusRTU`, and that one carries debug off until the next connect. `getDisplayValue` stated the same claim twice, two lines apart. Claude-Session: https://claude.ai/code/session_018YrcWbyHtV9JPT4ppY4EZP
Harted
force-pushed
the
chore/audit2
branch
from
September 23, 2026 21:34
a83ee0f to
e7c56da
Compare
Harted
marked this pull request as ready for review
September 23, 2026 21:34
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 blocking half. A second reading of the codebase area by area
produced 126 findings, a refutation reviewer answered every one and 124 held up,
and 14 of those were blocking. They are eleven causes, and all eleven are here.
This is built on #56, so the base is
chore/audit-annoyingrather thanmain.One commit per cause, each carrying the test that was red before it. Seven more
commits are what
/code-reviewfound in that work, and those are marked below.Merge this with a rebase, not a squash. Each commit is one concern with its
own message and its own test, and there are no fixups to hide.
What a user gets
Ten entries in
CHANGELOG.mdunder Unreleased. By subject:A saved setup opens, or says why it did not. Four things were never checked
when a configuration was read: how wide a text register is, what value a register
holds, how often a generator fires, and between which two numbers. Any one of
them left the server view blank on that launch and on every launch after, with no
message and nothing on screen to clear it with. A file is now refused with the
register and the field named, and a setup already saved has that register dropped
on the next launch and keeps the rest.
A file that is not a configuration is refused. A JSON file holding a number,
a string or a list came out as a valid empty configuration and reported that your
setup had been updated from an older format. A file holding nothing emptied the
server and then failed with a message about a property of null.
Opening the wrong file no longer empties the server. Modbux cleared the setup
on screen before it read the file, so a client config cost you the server you had.
The file is read first now.
A configuration from a newer Modbux is checked before it is loaded. It was
the one file nothing looked at. It is now read against what this version
understands, what matches is kept, and the message says which parts did not come
across. Registers and mapping entries are checked one at a time, so one entry
this version cannot read does not cost the other thousand.
The server view comes up. It was hidden until every server had been handed
over successfully, so one refusal anywhere left it faded out for good. Whether
initran and whether a particular server got through are two questions now, anda refusal costs only the server it belonged to. A server added in the split out
window can be changed once that window closes.
A 64-bit register shows what a master wrote. Four words were folded through a
number that carries 53 bits, so anything larger came back rounded, and the next
single word write collapsed the register to that word alone. Those values are
held as digits now, and files and settings already saved are rewritten on the
next launch.
The unit ID scan asks for what a device can answer. Its Length field took
anything up to 65535 and sent it as the quantity. It stops at 125 registers, or
2000 bits when only bit types are selected, with the pair stated once and the
reason from MODBUS V1.1b3 section 6 beside it.
The transaction log shows the address. The Addr column was blank for every
coil read, every discrete input read and every write Modbux sends, because the
address came from a field modbus-serial files on three of the eight request types
Modbux uses. It is read off the request frame now.
Splitting the server into its own window leaves the client alone. Read
Configuration kept showing as on while the reading fell back to the toolbar's
address and length, so every configured register above the tenth stopped being
read. Both windows load the same bundle and both were running the client's setup
code. The client store now asks which window it is in, and main refuses the
fourteen requests that steer its one Modbus client from any window but the main
one. A thirteenth conformance rule keeps that list complete.
Under it
Two commits change no behaviour.
c6ec7fbtakes out a read-only mode in the bitpanel that no render could reach, along with the two tests that asserted it.
48c3033gives main an answer for a register it cannot encode, andsrc/main/modules/__tests__/unencodableRegister.test.tsmeasures that no inputstill reaches it: the schema work closed every door first. Neither gets a
changelog line.
Seven commits are
/code-reviewfindings in this branch's own work:8d03353,0186a09,7311f80,a16484b,267b3b2,17d6103anda83ee0f. Three ofthose were serious. One fix did not do what its own commit message claimed, one
let a single unknown register cost every register, and one changed the config
file's shape without moving its version, which would have made 2.3.0 refuse a
whole file this build writes.
What is not here
The other 110 findings are in #59, which branches off this one. Three of them are retired by the work above.
How to test
Measured on
a83ee0f: lint 0, typecheck 0,yarn test1368 passed over 67files,
yarn test:e2e634 passed in 8.4 minutes.