Release 4.3.0 - #1127
Merged
Merged
Release 4.3.0#1127
Conversation
…t (DOPE-603) Review findings on #1081. The first is a data-loss path this PR's own ungating opened up; the rest are accuracy and coverage. **Half-migrated projects lost data types.** `executeSaveFile` — Ctrl+S, File▸Save, save-on-close — writes exactly one `datatypes/<Name>.dt` and never touches `project.json`, so a pre-DOPE-385 project ended up with one type in a file and the rest still inline. The loader then took the files as authoritative wholesale and dropped every type that had no file yet. Reproduced on a 14-type project: one `.dt` beside an untouched `project.json` and the build fails with `Undefined type 'POINT'`, `'OUTER'`, `'BUF'`… With the flag off this was unreachable, because the data-type branch fell through to a full `project.json` write. Fixed on both sides, because either alone still loses data: - The loader now MERGES. A `.dt` wins for the type it declares; anything left only in the inline list rides along beside it. A half-migrated project — from a single-file save, an old staging build, or a batch that failed part-way — keeps every type. A name owned by an unparseable `.dt` stays excluded, so a stale inline copy can't reappear next to the raw file the save echoes back. - The save now migrates the whole set at once. `parseProjectFiles` reports `dataTypesNeedMigration` when a project still carries its types inline with no `.dt` on disk; the store carries it, and while it is set a single-file data-type save writes every `.dt` and then rewrites `project.json`. `project.json` goes last, so a failed write leaves the inline list intact and the merge covers it. Cleared on any successful full save. **The deletion-filter comment was wrong about web.** It claimed both platforms apply deletions after the writes. Verified against `autonomy-edge`: the Edge save DTO has no `deletions` field, and deletion is by omission — anything absent from the uploaded envelope is dropped from the bucket — applied before the uploads. Reworded to say what each platform actually does. **Case-only renames self-deleted.** The filter compared paths exactly, so `Motor` → `motor` wrote the new name and unlinked it under the old one on the editor's case-insensitive targets. Now compared case-insensitively. **An unparseable `.dt` reported the wrong problem.** Ctrl+S on one failed with `Data type "X" not found.` — the file is on disk, it just cannot be read. The message now says so. Tests: the migration matrix (owed / already migrated / failed write), the merge matrix (override, case-insensitive override, unparsed shadowing, legacy kept), the flag matrix, plus the POU and case-only-rename cases the deletion filter claimed to cover and did not. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01PFqC3hMCBGrca8z4JzQLfi
Two more review findings.
**A multi-dimensional array could be located here and not there.**
`getArrayTotalElements` returns the product of every dimension, so
`AT %MW0 : ARRAY [0..3, 0..3] OF WORD` was accepted, reserved 16 slots and
validated its WORD base type — while the compiler refuses it outright:
Located variable 'MD' at %MW0 cannot be placed: a 2-dimensional array has
no single linear run of addresses to occupy.
That is the same accept-here/reject-there divergence this branch exists to
close, so the editor now refuses it too, in both paths that can produce it: a
location edit, and the type-only patch the array modal dispatches when a user
adds a dimension to an already-located array.
**The auto-increment walk re-scanned every variable on every step.**
Replacing the hoisted `Set` with a `checkIfLocationExists` call per iteration
made it O(iterations x variables x regex): the walk steps one element slot at a
time, so placing an `ARRAY [0..999]` on a taken address would run ~1000
iterations over every variable, each re-parsing its address, synchronously
inside the store's `produce`. The occupied spans are now parsed once before the
loop and each step is a plain interval comparison.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…yOne Hoisting the occupied spans out of the auto-increment loop left `incrementLocationByOne` with two unreachable statements: `collides()` returns false for anything that does not parse, so the loop body only ever ran on a parseable address, and neither its `return null` nor the caller's `break` could be taken. Dead code that no test could honestly cover -- and openplc-web requires 100% statement coverage on this directory, so it would have failed there while passing here (the editor asks for 97). Walking the linear index instead removes the function altogether. Every size class advances identically once linearised, so `%QX0.7 -> %QX1.0` and `%IB0 -> %IB1` are both `+ 1` and the carry never has to be spelled out. The address is parsed once instead of being re-formatted and re-parsed per step. Behaviour is unchanged, including the alias case: a location that does not parse has nothing to step, so it is left as it stands. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01PFqC3hMCBGrca8z4JzQLfi
…aths (DOPE-603) Three findings from the automated review of the previous commit. The first is a real security bug that this PR's ungating made live. **Path traversal through a legacy data type name (CWE-22).** `PLCDataTypeSchema` declares `name: z.string()` with no constraint, and the save flow interpolates that name straight into a path — `datatypes/<name>.dt`. A crafted `project.json` with `name: "../../../../tmp/pwned"` resolves to `/tmp/pwned.dt`, outside the project. Confirmed by resolving the join. Dormant until now: no shipped build ever wrote a `.dt`, so the sink never executed. Making `.dt` the default turns it live on the next save of any project the user opens, and an imported or shared project is external input. POUs are NOT exposed the same way — a POU's name is taken from its file name via `getBaseNameFromPath`, never from `project.json`, so it is a basename by construction. The legacy inline data type list is the one place a raw string from `project.json` becomes a path segment. Fixed at the boundary, which is where CLAUDE.md says external payloads get validated: `parseProjectFiles` now drops any legacy inline type whose name is not a plain IEC identifier, with a warning naming it. A type sourced from a `.dt` cannot reach the check — its name comes from the file name and has already been through the text parser's identifier rule. Defence in depth at the write boundary as well: `writeEntry` refuses any path that does not resolve inside the project directory. Every relative path in that payload is built by interpolating a name, so one missed boundary anywhere would write outside the project; this covers POUs, servers and remote devices too. **The migration under-reported what it wrote.** `migrateDataTypesToFiles` writes every `.dt` plus `project.json`, but `recordSavedFiles` was handed only the edited type's spec, so the version-control slice left the other migrated files looking dirty and kept a stale `rawLoadedContent` for them. It now returns its write set — including a partial one on failure, since those files did land. **Dropped two `as unknown as` assertions** the guidelines forbid, introduced in the previous commit's tests. Tests: the traversal matrix (parent traversal, absolute path, both separators, space, leading digit) plus the valid-identifier case, and the migration's version-control record set. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01PFqC3hMCBGrca8z4JzQLfi
…(DOPE-599) IEC 61131-3 identifiers are case-insensitive, but the resource-global duplicate-name check compared names verbatim, so `Motor` and `motor` were both accepted as global variables while the same pair was correctly refused for a POU local. `checkIfGlobalVariableExists` is gone in favour of `checkIfVariableExists`, which already folds case. Folding case alone makes a case-only rename collide with the variable being renamed, so `checkIfVariableExists` gained an `exclude` parameter — the same reference-equality skip `checkIfLocationExists` already uses — and both update validators now pass the variable under edit. That also fixes the rename the UI actually reaches: `updateVariableValidation` received `variableToUpdate` but never used it for the name check, so renaming a global to a different case of its own name was refused. The legacy `workspace/utils/variables.ts` copy carried the same bug in both of its predicates. Its auto-suffix filters fold case as well now: a case-different name would otherwise filter to nothing and index past the end of the sorted array. Covered at both levels — the validators directly, and through the real store so the immer-draft reference equality is exercised on the path the table uses. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VBTYbWQcUqJ87UXLJenaL6
…9-global-variable-name-case
Reported by forum user hh against 4.2.10; all three still reproduced on
development. Grouped because each is self-contained and small.
1. Duplicating a POU never registered it as a user library. `libraries.user`
backs the "User-defined POUs" tree, the FBD/LD block pickers and Monaco
completion, so the copy existed in the project but could not be placed in
a diagram or completed in ST. The create path already made this call; the
duplicate path did not.
2. The Block Properties OK button was gated on `selectedNode !== node`, which
compares node identity and only changes when a different block type is
picked from the library tree. Editing the name, input count, execution
order or execution control never enabled OK, so the edit could not be
applied. Execution order was the visible casualty: the field accepted
input that could never be saved, which is also why the reporter could find
no trace of execution order in the generated code. Replaced with a dirty
check against the values the dialog opened with.
3. Renaming a variable compared the candidate name against the whole table,
including the row being renamed, so a variable collided with itself and any
rename still matching its old name -- `ABCD` -> `ABCd` -- was refused.
`checkIfVariableExists` now takes the same `exclude` argument
`checkIfLocationExists` has carried for this exact reason. The
case-sensitivity of the comparison is deliberately untouched; that belongs
to the name-collision gate work in DOPE-577.
Enabling OK for a same-variant edit (2) reaches a path that was previously
dead, and it is not safe on a block still drawn with a two-sided VAR_IN_OUT
pin: rebuilding the node drops that output-side pin, so the wires that read it
would be left aimed at a handle the new node no longer has, and
`handleBlockSubmit` threw on the missing connector and aborted before closing
the dialog. Converting the block as a side effect of an execution-order edit
would contradict the project-open warning ("nothing is changed until you do"),
so the submit now refuses and points at the update badge -- mirroring how the
update path itself refuses an ambiguous in-out feed. Once the badge is
clicked, `rewireInOutReads` re-points the reads and the same edit applies
normally. The connector dereference is left optional as a backstop.
Covered by unit tests in both repos (mirrored) and, in openplc-web, a
Playwright spec driving a real browser. The e2e spec lives only in
openplc-web: the editor's harness launches a built Electron app rather than
a dev server.
DOPE-606
…rift chore(release): bring the version fields back into agreement (DOPE-601)
…dt-standard task(data-types): make .dt the standard format and drop the DATATYPES_DT_FILES gate (DOPE-603)
…6-small-editor-fixes
Follow-up to review on #1084 / openplc-web#733. Drop fix #3 (case-only variable rename) entirely. DOPE-599 makes the identical change to `checkIfVariableExists` and additionally removes `checkIfGlobalVariableExists`, routing globals through the same exclude-aware check. Keeping both would have produced a conflict on one function that reads as comment-only, where resolving it in this branch's favour would silently revert DOPE-599's global fix. `variables.ts` is back to `development` here and the rename tests move with it. Narrow the OK gate to execution order. The description overstated the defect: input count, the execution-control switch and a name that resolves to a library block all call `setNode`, which replaces `node` and so already enabled OK. Only the execution-order field and its arrows call `setFormState` alone. Keying the check on `formState.name` was also actively harmful -- `handleBlockSubmit` never reads it, so a name matching no library block would enable OK, rebuild the node with a fresh uuid and discard what was typed without a word. Check the legacy VAR_IN_OUT guard against `selectedNode` as well as `node`. Picking a replacement variant rebuilds `node` under current rules, clearing the legacy pin from that copy while the edges being remapped are still the original node's -- so the swap path, the one already reachable on `development`, walked straight past the refusal. Confirmed in a browser before and after. Stop registering programs in `libraries.user` on both the create and duplicate paths. A program is instantiated by the Resource, never called from another POU, so it is not a library block; mapping it to `function` put it in the block pickers while project load excluded it again, so a placed one referenced a library entry that vanished on reopen. The three `addLibrary` call sites now agree. Updates the tests that pinned the old mapping. Left alone: the e2e job is absent from CI in both repos, which predates this branch. DOPE-606
…itor-fixes fix(editor): three small defects from the community forum [DOPE-606]
The editor half of RTOP-283's plumbing: a typed path from the renderer to the
bootloader's control API on port 8445, so a version can be changed from the
editor without SSH.
A separate BootloaderApiClient rather than a port argument on
RuntimeApiClient. It listens on its own port, and it issues its OWN sessions
-- the two services share a credential database, not a token -- so threading
a port and a second token authority through the runtime client would have
made every existing call site carry a concept it never needs. Sessions are
keyed by device address, so switching devices cannot silently reuse another
device's token, and a rejected token is dropped rather than left to fail
every subsequent call identically.
Responses are validated with zod, like the runtime client's: these are HTTP
bodies from a device on the network, and `JSON.parse(...) as T` would claim a
shape without checking it and surface a missing field somewhere else
entirely. The bootloader answers failures as {"error": "..."} with messages
already written for a person, so they are surfaced verbatim instead of being
replaced by a status code.
Two timeouts are deliberate rather than uniform: getCapabilities is short
because it runs on every connect and a device with no bootloader must not
make the editor wait, while login is generous because verifying a password
runs PBKDF2 at 600k iterations, which on a Pi-class CPU is slow.
getCapabilities is unauthenticated by design -- the editor asks it to decide
whether to offer a version change at all, and a refusal or timeout is the
ordinary answer for a native install or an orchestrator-managed vPLC, not an
error worth showing anybody.
RTOP-283
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01VwWQN5S2ZKbhJz8kxMV9vF
A new screen under the Device tree that answers "what is this device and how is it doing". It takes over the scan-cycle, EtherCAT and plugin statistics that used to sit at the bottom of device Configuration -- they were never configuration, and were reachable only by scrolling past a pin-mapping table. The statistics are moved rather than duplicated, and the two polling toggles moved with them: the screen that displays data should be the one asking a device for it, or stats get polled where nobody is looking and are missing where they are shown. The header adds what the device reports about itself -- version, host, architecture, kernel, whether it is containerised, and who may change its version -- from the new /api/device-info, which older runtimes answer with a 404 and so simply contribute less to show. "Change runtime version" appears only when a bootloader is actually present. On a native install or an orchestrator-managed vPLC nothing on the device can perform a swap, and a button that cannot work is worse than none; the Updates field says who owns it instead, and points at openplc-web for a managed device. Signing in to the bootloader reuses the credentials the operator already gave the runtime, since the two services read one user database. The modal polls rather than awaiting: a pull runs for many minutes on a slow device. It adopts an update already in flight, so opening it mid-update shows that progress instead of an empty form inviting a second attempt the device will refuse; a failed poll mid-swap is ignored rather than reported, because the runtime container is being replaced and a brief interruption is expected. Absent progress is rendered indeterminate rather than 0%, since the daemon reports no size for layers it already holds and 0% would read as a stall. Upgrade and downgrade are one action, with no version floor. RTOP-283 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VwWQN5S2ZKbhJz8kxMV9vF
Follow-up to DOPE-606, from reviewing how the field reads now that it can actually be saved. **Ladder no longer has an execution order.** Rung position is the order in ladder, but the LD Block Properties dialog exposed the same field FBD does, and `emitLdBody` buckets sinks GLOBALLY across rungs -- so numbering a block in rung 2 hoisted it above rung 1. Confirmed on a P1AM-100 with a two-rung program (`ADD(seed,1)->acc` then `MUL(acc,10)->acc`): both blocks unnumbered gave acc=10 (rung order), numbering rung 2's block gave acc=1. The field is gone from the LD dialog, and the dialog now writes 0 rather than preserving whatever a project carried, so an edit clears a stale value instead of keeping it alive. **0 now reads as "None".** The transpiler treats anything > 0 as ordered and sorts the rest by layout position, so 0 is the ABSENCE of an order rather than the lowest one -- a bare "0" read as "runs first" and meant the opposite. The field takes digits only: a leading `-` is dropped rather than rejected on submit, a typed 0 (or an emptied field) falls back to None, and it blanks on focus so the user types over "None" instead of around it. The arrows step by one and floor at None. The old ceiling reused `maxInputs`, an input-count constant; execution order is an ordering key rather than a count, so there is no upper clamp now. **A numbered block shows a badge.** Small round badge on the bottom-right corner, only when the block carries an order, so an ordinary diagram stays uncluttered and a numbered one shows its sequence without opening a dialog per block. It is anchored to the block's own width/height rather than the wrapper's edges (the wrapper is a few pixels taller) and inset by half the 6px corner radius: centring on the mathematical corner puts it past the curve, where the drawn border has already turned away, and it reads as hanging low. Verified in a browser at each step. Covered by e2e/dope-606-execution-order-ux.spec.ts in the openplc-web mirror. DOPE-606
The same test file as openplc-web's, running unchanged under Jest here and Vitest there. It pins what a screenshot cannot: that the version-change action appears only when a bootloader actually answers, that the update policy is rendered in words rather than as a bare enum, that a device in recovery announces itself with the bootloader's own reason, and that the statistics polling this screen turned on is turned off again when it unmounts. The mocks go through the @root alias rather than relative paths because Jest resolves a mock path relative to its setup file, not the test -- a relative path fails here while working under Vitest. The alias resolves to the same module in both, which is what lets one file serve both apps. RTOP-283 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VwWQN5S2ZKbhJz8kxMV9vF
…rrays feat(located): allow ARRAY at a physical address, and detect range collisions
…it rewiring blocks Follow-up to DOPE-606, and the DOPE-611 fix folded in with it: every execution-order edit runs through the code that rewires the block, so shipping the two apart would deliver a feature that corrupts diagrams the moment it is used. **Editing a block no longer rewires it (DOPE-611).** Every edge was forced onto `inputConnector` / `outputConnector`, which are just the FIRST pin on each side -- so a block with two or more inputs had all its input wires collapsed onto IN1 and the wires to IN2, IN3 ... were lost. Two wires on one pin transpile as a parallel combination, so `SUB(acc, 3)` came out as `SUB(IN1 := 3 OR acc)` and failed to compile; with same-typed pins it would have changed the logic and still built. A pin that still exists on the rebuilt node now keeps its wire, which covers the whole same-variant case and the matching pins of a variant swap; anything genuinely gone is dropped and reported rather than piled onto the first pin. In ladder the rung chain still follows the connectors, because there an edge into a block is power flow rather than a data read. **Ladder no longer has an execution order.** Rung position is the order in ladder, but the LD dialog exposed the same field FBD does, and `emitLdBody` buckets sinks GLOBALLY across rungs -- so numbering a block in rung 2 hoisted it above rung 1. Confirmed on a P1AM-100 with a two-rung program (`ADD(seed,1)->acc` then `MUL(acc,10)->acc`): unnumbered gave acc=10 (rung order), numbering rung 2's block gave acc=1. The field is gone, and the dialog writes 0 rather than preserving what a project carried, so an edit clears a stale value. **0 now reads as "None".** The transpiler treats anything > 0 as ordered and sorts the rest by layout position, so 0 is the ABSENCE of an order rather than the lowest one -- a bare "0" read as "runs first" and meant the opposite. Digits only: a leading `-` is dropped, a typed 0 or an emptied field falls back to None, and the field blanks on focus so the user types over "None" instead of around it. The arrows step by one and floor at None. The old ceiling reused `maxInputs`, an input-count constant; execution order is an ordering key, so there is no upper clamp now. **A numbered block shows a badge.** Bottom-right corner, only when the block carries an order. Anchored to the block's own width/height rather than the wrapper's edges (the wrapper is a few pixels taller) and inset by half the 6px corner radius: centring on the mathematical corner puts it past the curve, where the drawn border has already turned away, and it reads as hanging low. Verified end to end on a P1AM-100: setting execution order 1 on the SUB block through the dialog, saving, and compiling now yields `SUB ADD MUL` with both of SUB's input wires intact, and the board computes acc = 10 as predicted. Before this change the same edit produced `SUB(IN1 := 3 OR acc)` and would not build. DOPE-606 DOPE-611
CI's format check runs `prettier --check`, which collapses the helper's signature onto one line. No behaviour change.
… text Typing a version invited a typo that only the device rejected, minutes later, in the daemon's words. The picker now lists the runtime's release tags and marks the one already installed, so the field can only produce a version that exists. A failed listing falls back to typing rather than blocking: an offline device holding a side-loaded image is a version that can legitimately be installed. Also stop claiming a deployment shape the runtime never reported. Every runtime released before /api/device-info answers it with HTTP 200 and its catch-all body, not a 404; read as device info that became an empty object, and an absent `containerized` rendered as "Native" -- a false statement about every runtime currently in the field. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VwWQN5S2ZKbhJz8kxMV9vF
…the right device The header was fed by the runtime's /api/device-info, an endpoint only a runtime carrying this feature serves. Every device in the field runs one that does not, so the header sat blank on exactly the devices this screen exists for. It now comes from the bootloader, which is present wherever an update is possible at all and reads these from the Docker daemon on the host rather than from inside a container's namespace. Drop the Deployment and Updates fields with it. Under the new source both could only ever hold one value, which a reader already knew from the version button being there at all; three of describePolicy's four branches had become unreachable. Operating system, CPU cores and memory take their place -- facts that differ from one device to the next. Also stop labelling the header with the wrong machine. It showed runtimeConnection.ipAddress, which is the IP saved in the project's Device configuration, not the device on screen: a project carrying an old address named a host elsewhere on the network. The connected device's own name wins now, and the dialled address is used only when there is no orchestrator in the picture. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VwWQN5S2ZKbhJz8kxMV9vF
…sertions Three findings from the review of the DOPE-611 remap. **The ladder rung chain is matched against `selectedNode`, and matched first.** Two bugs, one line apart. The connectors the existing edges ride were read from the dialog's local `node`, which an execution-control toggle or a variant swap has already rebuilt with DIFFERENT connectors -- so the rung edge matched nothing and was dropped. Turning execution control off on a ladder block lost the rung connection and with it the block itself, which is worse than the dropped wire the review described. And the rung is now matched BEFORE any pin-name match: `AND`'s `IN1` connector survives on `ADD` as an ordinary data pin, so a name-first check left the rung squatting on `IN1` beside the branch `reconcileBranches` had just rebuilt there, giving two wires on one pin. **Edges are replaced in place, never appended.** `reconcileBranches` runs before the remap and rewrites the branch edges' ids, so an id taken from `edges` may no longer be in `newEdges`. Appending it back resurrected a stale edge alongside the reconciled one. **A self-loop keeps both endpoints.** An edge whose source and target are both this block appears in both lists; remapping in two id-keyed passes rewrote the id on the first and then failed to find the edge on the second, leaving the other endpoint on a node that no longer exists. One pass over the affected edges, keyed by id, resolves both endpoints together. This one is pre-existing -- `development` leaves a dangling edge on the same fixture -- and the canvas does permit creating a self-loop, so it is fixed rather than filed. **No type assertions.** `CLAUDE.md:298` forbids `as`, and every field these casts reached for is already typed: `executionOrder: number`, `inputHandles: CustomHandleProps[]`, `inputConnector: CustomHandleProps | undefined`. The handle type is taken from the node itself rather than imported, so it cannot drift from the builder. Also drops a duplicated BOOL/EN-ENO rule: ladder's own `getBlockVariantAndExecutionControl` already forces execution control on when a block's top pins are not BOOL, and `buildBlockNode` runs it for every block placed on a rung. Passing the form's value through means a rebuilt block obeys the same rule as a freshly placed one, with one implementation rather than two -- and it uses `validateVariableType` rather than the naive string compare the duplicate had. Verified in a browser at each step, each fix confirmed to fail without it: toggling execution control on a ladder block loses the block on the old code; a self-loop leaves a dangling edge on `development`; the ADD swap doubles up on `IN1` without the ordering change. DOPE-606 DOPE-611
… render Reported as "connection to Unknown lost" after a successful downgrade. The lost connection was real, but it was a symptom: the dialog was flooding the device with requests until everything around it fell over. onFinished is passed as an inline arrow, so it is a new function on every parent render. It was a dependency of poll, which was a dependency of the effect that starts the poll timer -- and that effect assigned pollingRef.current without clearing what was there, while its cleanup only set a `cancelled` flag. Each parent render started another timer and abandoned the last. Each timer's tick re-rendered the parent, which started another, so the count compounded: hundreds of requests a second, 503s from everything upstream, an editor that looked dead. Hold the callback in a ref so poll is stable, route every start through one helper that clears first, and clear the timer in the cleanup. A regression test drives ten re-renders and pins the tick count; against the old code it sees 36 polls where 4 are due. Separately, stop reading a deliberate swap as a fault. The runtime is stopped and replaced during an update, so the status poller now stands down while one is in flight instead of counting the gap as five failures. And name the device: this path passed null, which rendered as the literal "Unknown". Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VwWQN5S2ZKbhJz8kxMV9vF
…s for engine gaps pdf.js's worker code calls several very recent JS platform APIs (Uint8Array.prototype.toHex/toBase64, Uint8Array.fromBase64, Map.prototype.getOrInsert/getOrInsertComputed, Promise.try) that Electron's bundled V8 doesn't implement yet, and this app's CSP makes a blob:-constructed Worker an open question anyway. `ProjectPort.preparePdfPreviewWorker` lets each platform choose: web still runs pdf.js in a real Worker, Electron registers the worker module on `window.pdfjsWorker` (pdf.js's own hook for running that code in-process) and polyfills the missing APIs directly in that shared realm. Also excludes pdfjs-dist from the webpack DLL prebuild — bundling it both there and via the dynamic import above instantiated the module twice, which broke pdf.js's own worker handshake — and adds console.error logging where preview failures were previously swallowed silently. DOPE-594
…ll, zoom, page tracking The wizard's preview step was a fixed 600x560 box with a single page shown at a time — too small to actually review a diagram, and single-page Prev/Next made no sense once the preview could be made to fit and scroll. The modal now takes up most of the screen; landscape documents scroll continuously through every page fit to width, portrait documents lay pages out in a wrapping grid (two per row at 100%); both support zoom via corner controls (usual 25%-400% range) or Cmd/Ctrl+scroll; and a page indicator tracks the page currently in view, with Prev/Next repositioning the scroll instead of swapping content. DOPE-594
…ments, or wires A scale-to-fit FBD/text block sized itself against the FULL page height, so it overflowed the instant a header (already placed) ate into that budget, and the whole diagram jumped to a page of its own, leaving the header alone above blank space. `paginate.ts` now reserves the header's space up front. Separately, `gapAlignedCuts` lets an unbreakable chain of elements overflow its own band rather than bisect one — that overflow used to draw past the page edge and get silently clipped there. FBD tiles and LD rungs now shrink to fit as a last resort instead. For Ladder specifically: the closing power rail sitting alone in its own band (a cut landing in the small gap before it) is now merged into the band before it; and a wire cut by a genuine, node-avoiding split is now drawn — clipped — in both resulting bands, so it visually continues on the next line instead of vanishing on the far side. DOPE-594
APP_VERSION and the root package.json read 4.3.0 while release/app/package.json sat at 4.2.2 and both lockfiles trailed further behind. electron-builder reads release/app/package.json — directories.app points there — so a local package build stamped the wrong version on the binary. Set with `npm version 4.3.0 --no-git-tag-version --allow-same-version` at the root and in release/app, which updates each lockfile too. Version fields only; no dependency added, removed or upgraded. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
… the node cap
The generator returned `dropped` and `overflowed` and the only production caller
read neither -- `generateOpcUaHeaderContent` destructured `{ nodes }` and threw
the rest away. Every way a variable could fail to reach the address space was
therefore silent, and the symptom that reaches us is "my variable is missing in
UaExpert".
STRING / WSTRING were the worst case. `TYPE_TAGS` hands out 19 / 20, but
`kTagToUaType[]` in opcua_nodes.cpp stops at TAG_DT (18). Every index into that
table is guarded by `tag >= kTagCount`, so this was never a memory hazard -- it
was invisible: the row shipped to flash, `materialise_nodes` skipped it, and the
variable was absent from the address space with nothing said at build time or
run time. Reproduced on a LOGO! 8.2 with a project exposing a STRING and a
WSTRING:
before: OPCUA_NODE_COUNT 9, no build output; Browse returns 7 nodes,
with a tell-tale gap at node ids 3 and 4
after: two build warnings naming ua_msg and ua_wmsg, OPCUA_NODE_COUNT 7,
Browse returns the same 7 -- now with contiguous ids
So `dropped` becomes `{ path, reason }[]`, `resolveTag` refuses tags 19 / 20
with a reason that names DOPE-645, and `generateOpcUaHeaderContent` takes the
same optional `warn` sink `generateS7CommHeaderContent` already had. The
pipeline passes the one it already builds for `buildOpcUaRuntimeConfig`.
The variables themselves are not lost, only unexposed: the debugger still reads
both (`INSTANCE0.UA_MSG` forced to "hello string" reads back over the Modbus
debug channel). Serving them over OPC-UA needs strucpp's pointer accessor so
`read_node` can address the string in place instead of through its 8-byte scalar
buffer -- DOPE-645.
Separately, `maxNodes` goes. It truncated the table to a ceiling that nothing
enforced downstream: `OPCUA_MAX_NODES` was emitted into the header and no
translation unit ever read it, and the device bounds what actually costs RAM
(`nodePoolSlots`, and the per-request operation limits, which all stay). A
project that outgrew 256 nodes silently lost its tail. The per-request limits,
maxSessions and maxArrayLength are untouched -- those bound one request, not
project size.
Verified on hardware after the change that the exposed nodes still behave:
ua_target written to 4242 as `eng` and confirmed over `openplc-cli debug read`,
the same write as anonymous refused with BadUserAccessDenied and the 4242 left
standing.
Refs openplc-editor#1091, Autonomy-Logic/openplc-packages#48
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01NegApcNt2SzRk3FDAs5T9p
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NegApcNt2SzRk3FDAs5T9p
`read_node` read into a `uint8_t buf[8]` and `UA_Variant_setScalarCopy`'d out of
it, so it allocated from the ~19 KB arena on every value of every read and could
not have held a string anyway. It now addresses the value in place through
strucpp's `type_ops[].ptr` (v0.6.7 runtime, reached by a new
`openplc_debug_ptr()` on the C-ABI glue) and publishes it with
`UA_VARIANT_DATA_NODELETE`. A read is now allocation-free, which is what makes
strings affordable rather than merely possible: a 253-byte WSTRING never fitted
the old buffer.
STRING maps to UA String and WSTRING to UA ByteString carrying UTF-16LE code
units -- transcoding to UTF-8 would need a scratch buffer the size of the string
and would end the in-place property, so the client gets the bytes. Writes take
the reverse path into strucpp's `[len][payload]` wire form, the one place a copy
is unavoidable.
Two things were easy to get wrong and are pinned by comments:
* A UA String is a {length, data} HEADER, and the variant points at the header,
so the header has to outlive the callback exactly as the characters do.
`Service_Read` fills every result before encoding any of them, so a single
static header would make two strings in one request both report whichever was
read last. Hence a slot per node the server accepts in one request
(`OPCUA_MAX_NODES_PER_READ`, 160 bytes of .bss).
* Zero-copy is sound ONLY because `scheduler()` is a cooperative single-threaded
super-loop: OPC-UA runs in the tail of the cycle and the scan cannot move the
value underneath it. If OPC-UA ever gets its own task this must go back to
copying -- the same reason strucpp does not export `handle_ptr` to Runtime v4.
`OPCUA_TAG_STRING` / `OPCUA_TAG_WSTRING` join opcua_types.h for plain-C callers
(scalar vs header is a branch, not a table lookup) and are static_asserted
against strucpp's TypeTag in the glue, so that duplication cannot drift.
`UNEXPOSABLE_TAGS` in the generator is now empty but kept: it is the seam that
turns "a tag the firmware has no mapping for" into a build warning naming the
variable instead of a node that silently vanishes.
Verified end to end on a LOGO! 8.2, every write cross-checked over the Modbus
debug channel rather than trusting the OPC-UA client's reply:
Browse 9/9 nodes, ua_msg and ua_wmsg present
read empty STRING/WSTRING '' / b'' (an empty string is a VALUE --
rejecting len 0 as BadNoData was a
bug this test caught)
write ua_msg as eng 'from opcua' confirmed via openplc-cli debug read
write ua_wmsg as eng 'wide via ua' confirmed via openplc-cli debug read
BOTH strings in one Read distinct, correct values (the aliasing case)
anonymous writes ua_msg REFUSED BadUserAccessDenied
200-char STRING ACCEPTED, truncated to the 126 cap
odd-length WSTRING bytes REFUSED BadTypeMismatch
Int32 into a STRING REFUSED BadTypeMismatch
Sustained-load note, NOT from this change: the server drops the connection after
roughly 330 requests hammered back to back. Reproduced identically on the
pre-change firmware (338 / 333 / 343), so it is pre-existing and consistent with
the ~368 reads/s ceiling measured during the original hardening.
Refs openplc-editor#1091, Autonomy-Logic/openplc-packages#48, DOPE-645
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01NegApcNt2SzRk3FDAs5T9p
Finding 7 had no automated coverage -- it is a handler inside a large React
organism, so the fix shipped on reasoning alone.
The teardown that drops the Ethernet debug link for an upload runs BEFORE the
`try`, while the restore used to sit at the END of the `try`. So an IPC failure
or an adapter throw from `compileProgram` -- as opposed to a `{ success: false }`
return -- skipped the restore and left the debugger disconnected for good, with
only "Build error: ..." in the console and nothing to say the connection was
gone. The fix moved the restore into `finally`.
Driven through the flash-request event rather than the build popover: it reaches
the same `handleBuild`, and keeps the test about the teardown contract instead
of about menu markup.
Four cases, and the first is the regression:
compileProgram THROWS reconnects, and does NOT sit through the 6 s
reboot settle (a build that threw never reached
the device)
returns success: false reconnects
succeeds waits out the settle, THEN reconnects -- the
settle deliberately stayed on the success path
inside the `try`
nothing was connected neither disconnects nor reconnects; the restore
is guarded on its own flag, not on "is ethernet"
Confirmed to actually catch it: with the restore moved back inside the `try`,
the throwing case fails and the other three still pass.
Ported to vitest for openplc-web, where `default.tsx` is byte-identical and the
same regression applies.
Refs openplc-editor#1091, Autonomy-Logic/openplc-packages#48
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01NegApcNt2SzRk3FDAs5T9p
`user_access_level_cb` returned 0 for any node whose `nodeContext` was null --
which is every namespace-zero node, since only our own rows carry a context. The
comment claimed this was the conservative choice ("expose nothing rather than
everything"). It was not conservative; it was wrong, because the caller does:
node->accessLevel & getUserAccessLevel(...)
The node's own AccessLevel attribute is ALREADY the gate. That is exactly why
the library's own default returns 0xFF -- it is deferring, not granting. ANDing
with 0 instead refused all of namespace zero with BadUserAccessDenied, including
`Server_ServerStatus_State` (ns=0;i=2259).
That node is what practically every OPC-UA client's connection watchdog polls,
asyncua included, about a second after connect. So the visible symptom was the
session dying roughly a second in, whatever it was or was not doing -- and a
client that never polls ns0 would not have noticed at all.
I previously reported this as a pre-existing flood limit of "~330 back-to-back
requests", having reproduced the same number on the committed firmware. That
conclusion was wrong: the A/B compared 8ba994b against my working tree, and
46cbde1 -- which introduced this callback -- is an ancestor of BOTH, so both
sides carried the bug. There was never a request-count limit. 330 was simply how
many requests fitted into the one second before the watchdog read failed, which
is why the count tracked the request rate exactly (368 at full speed, 100 at
5 ms spacing, 43 at 20 ms, 19 at 50 ms) and why an IDLE connection died at the
same 1.0 s having issued nothing at all.
Measured on a LOGO! 8.2, before and after:
read ns=0;i=2259 BadUserAccessDenied -> OK (0 = Running)
ns0 ServerStatus/Namespace/ServerArray all refused -> all read
idle connection dead at 1.0 s -> alive at 15 s
sustained reads died at ~330 -> 3000 with no error,
360 req/s
200 batched reads x 9 died mid-run -> 1800 values, 1348/s,
0 string mismatches
Role enforcement on our own rows is unchanged and re-verified: anonymous still
refused write on a viewer:r node, `eng` still accepted, empty-username and
wrong-password sessions still refused.
Refs openplc-editor#1091, Autonomy-Logic/openplc-packages#48
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01NegApcNt2SzRk3FDAs5T9p
…s unreadable
Two review findings from openplc-editor#1091.
**No default IP, ever.** An ethernet-upload build fell back to a hardcoded
`192.168.2.4` when neither the Network screen nor `runtimeIpAddress` supplied
one, and baked it into the firmware. That produced an image pointing at a device
the user never named, on the ONE class of board where a wrong address means it
cannot be reached again -- and the build reported success. The fallback is gone;
a build with no address now stops and names the Network screen.
Upload and connect already refused correctly (`if (!port) throw` in the upload
path, prompt-or-error in `resolveDeviceLinkCandidates`), so the invented address
only ever existed at the firmware-baking site. Nothing else had to change.
**An unreadable device config is reported, not silenced -- and never fatal.**
Both `catch` blocks around `devices/configuration.json` swallowed a MALFORMED
file exactly as they swallowed a missing one. Treating it as absent is the right
behaviour and stays: a project must still open and build with a corrupt device
file, the same way a malformed POU degrades rather than taking the workspace
down with it. Doing it silently is not. ENOENT stays quiet -- a project that was
never configured has nothing to report -- while a parse error now reaches the
app console naming the file and the position, because those are settings the
user believes are in effect.
The two interact, and that is the point: a malformed config on an ethernet board
now warns about the file AND then stops for the missing address, instead of
silently flashing 192.168.2.4.
**`target.uploadMethod` is validated at the manifest boundary.** `target` is
`.passthrough()`, so any value rode through into a field typed
`'serial' | 'ethernet'` that `BoardInfoResolver.#fromVppDevice` copies verbatim.
A manifest saying `uploadMethod: "etherner"` would be carried as if it were a
member of that union, match 'ethernet' nowhere, and silently take the serial
path -- on a board whose only link is Ethernet. Now `z.enum(['serial',
'ethernet']).optional()`; every installed VPP declares `ethernet` or nothing, so
the shipped catalogue is unaffected. `target` stays `.passthrough()` for every
other VPP-defined key.
Verified by building, not by inspection:
valid config, ethernet board builds
malformed config, ethernet board warns about the file, then refuses for
the missing IP -- not for the parse error
valid config with no IP, ethernet refuses, naming the Network screen
malformed config, SERIAL board warns and BUILDS (ok:true) -- a parse
error must never block a build
LOGO! 8.2 upload with a valid config still flashes
11 new schema tests cover the accepted values, the rejected ones (typo, wrong
case, empty, number, boolean) and that unrelated `target` keys still pass
through. Full suite green: 8478 passed.
Refs openplc-editor#1091
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01NegApcNt2SzRk3FDAs5T9p
…ceholder `192.168.2.4` is the address of the LOGO! 8.2 sitting on my desk. It reached the device screen's IP field as the example value users are shown, which is both wrong as guidance and an odd subnet to suggest — 192.168.0.x is the common one. The debugger's IP prompt had drifted differently again, to 192.168.1.100. Both now read 192.168.0.2. Placeholders only; neither field ever had a default, and after the previous commit no code path invents an address at all. The same leak is still in `com.siemens.logo/screens/network.json` in openplc-packages (ip 192.168.2.4, gateway 192.168.2.1) — the only VPP that does not follow the 192.168.0.x convention its six siblings use. Not fixed here: it is a different repo, and changing a shipped screen means bumping that package's version. Refs openplc-editor#1091 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NegApcNt2SzRk3FDAs5T9p
…way in Turning the Network section off on a LOGO! and building produced firmware with defines BYTE-IDENTICAL to the enabled case -- `OPLC_NET_ENABLED` and all -- then uploaded it. The mandate added for the Modbus/network split forces the network on for any ethernet-upload board, and it was doing that even when the project explicitly said off. The screen said one thing and the device did another, and the only reason this surfaced is that the board stayed reachable afterwards. A mandate is not a licence to overrule the user silently. Disabling the network on a board whose only access path is Ethernet is the same class of mistake as disabling serial on a Mega, and it now gets the same answer: refuse, and say why. Only an EXPLICIT `network.enabled === false` refuses. An absent network block still gets the mandate, because that is a project which never configured anything rather than one that said no -- and the missing-IP check already stops the genuinely unconfigured case. Verified by building: network.enabled false, ethernet board REFUSED, naming the Network section network enabled, ethernet board builds network block absent, ethernet board builds (mandate applies) network.enabled false, SERIAL board builds (unaffected) LOGO! 8.2 upload after the change still flashes Full suite green: 8478 passed. Refs openplc-editor#1091 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NegApcNt2SzRk3FDAs5T9p
…ble-badging
Three UI defects from the PR review.
**Folding "Device" left its VPP screens behind.** `Network` and `Backplane
Configuration` rendered as SIBLINGS of `<ProjectTreeBranch branchTarget='device'>`,
not children, so collapsing the branch could not hide them -- only `Configuration`
went away. Moved inside the branch. Driven off `vendorScreens`, so it is whatever
the VPP declares rather than a fixed list, on editor and web alike.
**The debugger told you to go connect instead of offering to.** Pressing Debug
with no session raised "Connection Required / OK" and left the user to find the
button. It now asks, and on Yes performs the connect itself:
simulator start it (same action as the sidebar's Start, which
already attaches the debugger once the firmware lands)
device useDeviceConnect().connect()
runtime v4 useRuntimeConnect().connect() -- raises its login modal,
so this is deliberately a modal opening a modal
nothing selected "select a device to connect to" and stop
One flow for desktop and web. The only difference is WHICH action connects, and
that is a property of the target, not the platform -- the no-selection case is a
real state on web (the Orchestrators screen picks the device) and unreachable on
desktop, and is evaluated identically on both rather than branched on platform.
That required the runtime connect to have more than one caller, so it moved out
of `board.tsx` into `useRuntimeConnect`. A second copy in the activity bar is
exactly how the two would drift: the version gate, the login/first-user choice
and the licence teardown all have to behave the same wherever connect is invoked.
The screen now consumes the hook; five selectors and two imports it no longer
uses were removed with it.
**A block output and its connected variable both drew a badge.** The
de-duplication already existed -- `connectedOutputNames`, commented "avoids
double badges" -- but it read `data.connectedVariables`, a denormalised cache
that does not carry output entries in practice. A CTU with `current_count` wired
to CV records only its inputs (`R`, `PV`), so the skip never fired. Now derived
from the rung's own nodes, which is the authority and needs no migration for
projects already saved.
An output variable node with an EMPTY name is a bare pin stub (every TON carries
one for ET). That is not a connected variable and must not suppress anything --
the block's badge is the only place its value appears. Replayed against the
reported project:
TON0 outputs=[Q,ET] suppressed=[] shown=[Q,ET]
TOF0 outputs=[Q,ET] suppressed=[] shown=[Q,ET]
CTU0 outputs=[Q,CV] suppressed=[CV] shown=[Q]
The Ethernet-restore suite needed mocks for the two connect hooks the activity
bar now calls; this is about the restore, not about connecting.
Editor 8478 tests pass, web 8320. Surface match, 0 diffs.
Refs openplc-editor#1091
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01NegApcNt2SzRk3FDAs5T9p
openplc-web has not started since 7eabc84db. `opcua-credentials.ts` sits on the
shared compile surface, so the web app bundles and EVALUATES it in the browser,
and its top-level
import { pbkdf2Sync, randomBytes } from 'node:crypto'
threw at module load -- Vite externalises node builtins and touching any member
of the shim is fatal. Blank page, no interactive elements, before a single
component rendered. My commit, and exactly the drift the shared surface exists
to prevent.
A lazy import would only have moved the failure. The derivation runs inside the
pipeline's `isRuntimeV4` branch, and reaching a Runtime v4 device through the
orchestrator is what web is FOR, so the browser genuinely has to derive
credentials -- a deferred import would have turned a boot crash into a compile
crash, which is worse for being later and rarer.
So it uses `globalThis.crypto.subtle`: the one PBKDF2 both platforms already
have, native in the browser and in Node since 15. One implementation rather than
a platform port, and native rather than pure-JS -- at the default 600_000
iterations a JS fallback would block the UI thread for seconds. `randomBytes`
becomes `crypto.getRandomValues`, and base64 is done without `Buffer`, which the
browser does not have.
The cost is that deriving is now async: `subtle` has no synchronous form, so
`deriveOpcUaCredential` and `materialiseOpcUaCredentials` return promises and
the pipeline's two call sites await them. Worth noting `tsc` did NOT catch those
call sites -- both pass the result through `as never`, which swallowed the
promise silently; the unit tests caught it.
Output is unchanged, which is the part that matters: a credential already stored
on a device has to keep verifying. Checked against the old path with a fixed
salt --
node:crypto : b6uAWgTmq4hP7O+nkIhoy5R5A6tkuTJ8PrsJYaIGfrE=
WebCrypto : b6uAWgTmq4hP7O+nkIhoy5R5A6tkuTJ8PrsJYaIGfrE=
-- and pinned as a vector test rather than a comparison, so it still guards on a
platform that has no `node:crypto` to compare against.
Verified: Vite now serves the module with 0 `__vite-browser-external`
references; a LOGO! 8.2 build still emits a real credential
(`{ "eng", "plain:s3cret", 2 }`) rather than a stringified promise; editor 8479
tests and web 8321 pass.
Related, not fixed here: `backend/shared/utils/vpp/verify-package-signature.ts`
carries the same node:crypto / node:fs / node:path pattern. It has no importer
in web, so it never enters the browser's module graph and cannot crash it today
-- but it is the same latent hazard on the same shared surface.
Refs openplc-editor#1091
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01NegApcNt2SzRk3FDAs5T9p
Testing the offer-to-connect in a browser showed it never fired for the
simulator: TWO gates kept the button from reaching its own handler.
disabled={isDebuggerProcessing || isSimulatorBoard} with the tooltip
"Use Start to debug"
if (isSimulatorBoard) return first line of the handler
Between them the control was a dead end -- the answer to "I want to debug" was a
greyed button pointing at a different button. The offer added in the previous
commit was unreachable code.
The disable is gone and the early return MOVED to just after the offer, rather
than being deleted. What follows it is the device path -- debug compile, MD5
verify against flashed firmware, channel connect -- and none of that describes
an emulator. A running simulator already carries its debug session (Start
attaches it as the firmware event lands, via
`simulatorRun.launch({ attachDebugger })`, and there is no attach-to-running
entry point to call here), so the button's remaining job for it is the toggle-off
handled at the top.
Verified in a browser against openplc-web with the local-runtime proxy:
Debugger, simulator stopped "Simulator Not Running -- start it now?"
No "Debugger session cancelled.", nothing started
Yes simulator builds, launches, debugger attaches;
live values on the diagram
The same run confirmed the other two fixes on web, which is what found this one:
folding Device hid its children, and in the ladder POU `CTU0.CV` now carries a
single badge beside `test_something` while `TON0.ET` / `TOF0.ET` keep theirs
(4s220ms / 6s) because their output stubs have no name.
Worth recording: the FBD block already derived its `connectedOutputNames` from
the rung EDGES rather than from `data.connectedVariables`. Ladder was the lone
outlier reading the stale cache, so the previous commit did not invent an
approach -- it brought ladder into line with the one that was already right.
Refs openplc-editor#1091
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01NegApcNt2SzRk3FDAs5T9p
…e target Pressing Debugger with no session told the user to go find another button. It now offers to establish one, and the same flow covers every target -- the simulator starts, a locally-addressed device and an orchestrator-reached Runtime v4 both connect. A runtime's connect raises its own login, so this is deliberately a modal opening a modal. Three things this depends on: - Selecting a device in Orchestrators PUBLISHES the selection (`setSelectedDevice`) without connecting to it. Selection and connection are separate decisions; only Connect connects. - The offer NAMES the device, because "would you like to connect?" with no name is how a mis-selection becomes a session on someone else's machine. - `runtimeConnect` sets the device context before connecting, so the login modal is raised against the device that was actually picked. `isRuntime` no longer derives from the board alone: a device chosen in the Orchestrators list IS a Runtime v4 target before the board target catches up, and deriving it from the board sent a selected-but-unconnected device down the serial path and answered "Could not reach the device on simulator". The simulator guard moved below the offer rather than being removed -- standing above it, it made the button a dead end on a simulator target. Mirrored byte-for-byte on openplc-web; compare-surfaces reports 0 diffs across 1147 files. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NegApcNt2SzRk3FDAs5T9p
…ring POU
Ticking a shared global in the OPC-UA address space failed the build:
Cannot resolve OPC-UA variable address.
Variable: main:test_global
Expected debug path: INSTANCE0.TEST_GLOBAL
A VAR_EXTERNAL is a REFERENCE to a CONFIGURATION VAR_GLOBAL, never storage of
its own, so STruC++ emits it under its bare name -- the real debug map from a
Runtime v4 build carries `TEST_GLOBAL` and `INSTANCE0.TEST_VAR`, and no
`INSTANCE0.TEST_GLOBAL` for it to find. The variable picker listed it under the
POU that declares it, which is where the instance prefix came from, and the
build then died on a variable the picker itself had offered.
The decision had drifted into two answers. The debugger derives global-ness
from the variable CLASS (`buildVariableDebugPath(class === 'external', …)`);
OPC-UA derived it from `pouName`, and `resolve-indices.ts` already carried a
comment recording the split. Only the class-based answer matches what the
compiler emits.
- The picker now attributes a VAR_EXTERNAL to the global scope, so new address
spaces are right at the source and ticking the variable under its POU or
under GVL is one node, not two.
- `resolve-indices` falls back from `INSTANCE0.<path>` to the bare path, so
address spaces saved BEFORE this still build instead of being unfixable
without re-picking the variable. The fallback cannot mis-bind: a variable a
program really owns is always in the debug map under its instance, so it is
unreachable for one.
- The "is this the global scope?" test was spelled three different ways in one
file (one of which accepted `gvl` but not `config`); it is now one predicate.
- The address-space tab resolves both spellings to the same tree entry, so a
legacy config shows the variable ticked and re-ticking it cannot add a second
node for one address.
Verified by running the real `buildOpcUaRuntimeConfig` over the real
`debug-map.json` from the device build with the pre-fix spelling: it threw
before and now resolves `test_global` to arr 0 / elem 0 -- the address the
working build produced and the OPC-UA read/write test exercised over both the
plain and the Basic256Sha256/SignAndEncrypt endpoints.
Six regression tests; three of them fail without this change.
Mirrored on openplc-editor/openplc-web; compare-surfaces reports 0 diffs.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01NegApcNt2SzRk3FDAs5T9p
Running each CI job locally turned up three failures on the two commits before this one, none of which the targeted test runs would have shown. - **Editor unit tests exited 1 with every test passing.** `use-runtime-connect.ts` shipped with no tests at all, and 152 uncovered lines dragged the whole `src/frontend/hooks/` root under all four of its thresholds (34.27/29.4/29.35/ 33.79 against 35/30/30/35). 14 cases now cover it — login vs first-user modal, the remembered version, both error paths, the device context being set BEFORE the first call (and in that order, which is the bug that left `getUsersInfo` addressed at nothing), the no-IP/no-device stop, the version mismatch refusal, the continue-anyway offer taken both ways, and what a disconnect drops. The root is back over threshold at 39.37/33.6/33.78/38.95. - **Lint failed on 2 errors**, both `simple-import-sort` in the files the previous commit touched. - **Format failed on the same files**, and then on the new test. The test file is byte-identical across both repos, like its siblings, but web excludes it from vitest for the reason the four entries above it are excluded: `jest.mock(...)` of the platform provider does not survive Vitest's hoisting, so unhoisted the real provider loads and the file dies before a case runs. The editor's jest run owns the coverage — and `frontend/hooks` is a threshold root there, which is exactly where it is enforced. Verified by running every job as CI does. Editor: tsc --noEmit, eslint, prettier --check, validate:arch, `jest --collectCoverage --ci` (8499 passed). Web: tsc --build, eslint, prettier --check, validate:arch, `pnpm run test` (8327 passed). All four sync-gate scripts pass; compare-surfaces is 0 diffs across 1147 files. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NegApcNt2SzRk3FDAs5T9p
Starting the simulator on any project with an enabled OPC-UA server failed at
the preprocessor:
opcua_auth.h:25:10: fatal error: open62541.h: No such file or directory
The Simulator declares `opcuaServer` / `s7Server` / `modbusTcpServer` in
hals.json on purpose — those flags keep the server options offered in the UI so
a project authored for Runtime v4 is not stripped of its configuration while
someone simulates it. They answer "may the UI offer a server?", never "can this
firmware host one?". Since 41dd333b9 taught `resolveTargetCapabilities` to
materialise nested profiles even when a manifest omits them, the compile
pipeline's `capability && profile` test started passing for the Simulator, and
it emitted a real `opcua_config.h` with `OPCUA_ENABLED 1` in front of an
`#include <open62541.h>` that its toolchain has no header for.
The simulator runs USER LOGIC ONLY — an emulated ATmega2560 with no Ethernet
and no serial peripheral, so no server it is handed is reachable, the same
reason Python function blocks are dropped for it. Modbus was already excluded
this way; OPC-UA and S7 were not. One term now gates all three.
Not a fix to `resolveTargetCapabilities`: that change is right, and the
Simulator declares its capability block in hals.json anyway, so "did a manifest
declare it?" does not separate these cases. What separates them is whether the
firmware can host a server at all.
Verified on the emulated target: a project carrying Modbus + OPC-UA + S7 now
compiles to 16638 bytes / 4189 bytes SRAM — byte-identical to the same project
with no servers at all — and the existing "server configurations will be
ignored" warning still tells the user. Driven from the browser on openplc-web
it compiles, runs (emulated clock advancing 3s per 3s), reports variables
through the debugger, and a forced DINT reads back as 12345.
Real targets are untouched: the term is `!isInProcessSimulator`, true
everywhere else. LOGO! 8.2 still builds at 240884 bytes with OPCUA_ENABLED 1,
and Runtime v4 still receives its full address space.
Mirrored on openplc-editor/openplc-web; compare-surfaces reports 0 diffs.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01NegApcNt2SzRk3FDAs5T9p
…blank passwords Addresses items 2, 3, 8 and 9 from the second review; items 1, 4 and 10 were confirmed non-issues and left as-is (notes below). **2 — a short follow-up frame dispatched on a discarded frame's bytes (TCP).** `modbus_tcp.cpp` read a request into `mb_frame` and only then discarded it if its length disagreed with the MBAP, leaving the bytes behind; `process_mbpacket` indexed `mb_frame[2..]` without ever consulting `mb_frame_len`. A frame declaring 100 bytes but carrying 6 ending in the 0x4C magic, followed by a bare `[unit][4C]`, rebooted the device on the stale operands. Two guards now: a per-FC minimum length in `process_mbpacket` (reusing `mb_pdu_request_len`, which already knows each FC's shape), and the TCP discard path wipes the buffer. Verified on a LOGO! 8.2: the bare 0x4C is refused (`cc 03`) and the device does not reboot, while a well-formed 8-byte reboot still returns `4c 7e` and works. **3 — unauthenticated S7 clients could stop the PLC.** `on_control` bound run/stop to the S7 server, which has no authentication. Removed: the control handler is no longer registered, and the Settimino fork already refuses control when none is set. SZL identity stays, so clients still connect and read; run/stop remains where it is gated (the Modbus debug channel on the editor unit id, and the physical mode switch). **8 — OPCUA_KDF_ITERATIONS was emitted and read by nothing.** The iteration count travels inside the hash string (`pbkdf2:sha256:<n>$...`), which both verifiers parse — baremetal `opcua_auth.cpp` and Runtime v4 `user_manager.py`. The define disagreed with the firmware's own ceiling and invited the reading that it configures the device. Removed. Confirmed Runtime v4 reads the count from the hash, not a define. **9 — a blank-password user silently promoted anonymous to engineer.** A password user with no credential was dropped at build (`generate-opcua-header` filters on `password_hash`), and dropping the last one flipped `OPCUA_ANONYMOUS_ROLE` to engineer with `OPCUA_ALLOW_ANONYMOUS 1` — full write to anyone. Now: the editor's user modal refuses a blank password unless there is a stored credential to keep (the exemption was firing when switching a cert user to Password), the generator warns on every dropped user and warns hard when the drop hands anonymous the engineer role. Item 1 (reboot FC over TCP) is gated by the VPP hardware layer: `hardwareRebootToBootloader` is a weak no-op that only the LOGO overrides, and the LOGO gates it behind `hardwareProgrammingLocked`. Item 4 (PBKDF2 stalling the scan) cannot occur today: every shipped VPP declares `passwordScheme: "plain"`, so the firmware bakes `plain:` credentials and never runs the KDF in `login_cb`. Item 10 (library re-clone) does not happen: `installThirdParty` filters on what is already installed. Mirrored editor/web; compare-surfaces 0 diffs. Editor 8503 tests, web 8331. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NegApcNt2SzRk3FDAs5T9p
…r-recanonicalisation fix(frontend): re-canonicalise the code buffer after a successful commit (DOPE-622)
DOPE-622 rewrote `restoreBufferName` to serialise the canonical text instead of preserving the typed form, which removed this helper's last production caller. Only its own suite imported it. `declaredNameRegex` went with it — the helper was its only user. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Jjy1Ja62BFtay579KYhBC7
**12 — `--host` now reaches the baked firmware IP.** `compiler-module.ts`
resolved `vppModbusState?.network?.ip_address || configuredIp` and never
consulted the caller's `runtimeIpAddress`, so `openplc-cli upload --host X`
flashed X while baking the project's stored address into `MBTCP_IP`. The
caller's value now wins, matching `communicationPort`. Verified on a LOGO!:
project storing 192.168.2.99, `--host 192.168.2.5` → `MBTCP_IP 192,168,2,5`.
**13 — ethernet seeding defaults `enable_dhcp`/`interface`, no longer forces
them.** They were overwritten unconditionally while subnet/gateway/dns deferred
with `||`; a user who ticked DHCP got static firmware silently. Now defaulted
only when unset.
**14 — the serial upload handoff restores in `finally`.** The port was released
before the `try` but reconnected inside it, so a throwing or `{success:false}`
build left a USB target disconnected with no log line — the exact shape the
ethernet handoff already fixed, one branch over. Both restores now live in
`finally`, guarded on their own flag, independent of success.
**15 — the device switch publishes to the store.** `handleConfirmDeviceSwitch`
called only the local `setSelectedDevice`; the store's
`runtimeConnection.selectedDevice` stayed null after a switch, so the debugger
reported "No Device Selected". Now published like every other selection path.
Verified in the browser on openplc-web with two local-runtime devices: connect
to deviceA, switch to deviceB, and the store's selectedDevice is deviceB.
**16 — VPP screen defaults persist on view, not only on edit.** A user who
opened a screen, agreed with every default and changed nothing left them
unstored, so the build used the library value (Pico chip-select 17 → pin 10).
Seeded on mount now. Per-target storage (`vendorScreenDataByBoard`) already
keeps each board's data separate, so a Pico selection cannot bleed into a Mega.
**17 — the post-upload IP advance is DHCP-aware.** It overwrote
`runtimeIpAddress` from the stale stored address even under DHCP, then dialed
the wrong host. For a DHCP target it now asks the user for the address the
device came up on (the existing debugger-ip-input modal).
**18 — S7 data-block descriptions are escaped into generated C.** A newline in
`db.description` broke out of the `//` comment into code. New `cComment`
sanitiser; `cString` now also escapes `\r`; DB number/size/index coerced to ints.
**19 — the OPC-UA global-scope fallback warns and is narrowed.** It bound a
program-local to a same-named global silently; every hit now warns.
`isGlobalScopePou` no longer case-folds, so a user POU named `Config` is not
routed to the global scope.
**20 — the ethernet upload-arg plumbing is tested.** A platform-port test pins
that `uploadMethod`/`ipAddress` reach the handler — the two fields that were
"declared but never populated".
**Nits:** `materialiseOpcUaCredentials` is typed concretely (drops
`as unknown as T` and one `as never`; the remaining cast is an honest
cross-module shape bridge to generateRuntimeConfs); the S7 server lookup filters
on `enabled`; two credential tests await their async calls; `opcua_nodes.cpp`
frees `referencesSize` kinds, not a hardcoded 2, so an appended Organizes ref
cannot leak; the dead `-DOPENPLC_SIMULATOR` define is removed (the simulator is
identified by the `isInProcessSimulator` capability); `use-runtime-connect`
guards against a double connect; `block.tsx`'s connected-output scan moved out
of a per-tick selector into a memo keyed on the flows reference; and
`uploadMethod` reads through a typed `BoardInfoLike` field instead of casts.
Mirrored editor/web; compare-surfaces 0 diffs. Editor 8507 tests, web 8334.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01NegApcNt2SzRk3FDAs5T9p
…e-declared-type-name chore(frontend): drop rewriteDeclaredTypeName, now unused
These files advanced on development after this branch was cut (the DOPE-545 datatypes .dt work: reject-handling and code-buffer re-canonicalisation on commit, plus a parser refactor that dropped rewriteDeclaredTypeName). The PR never touched them, so the Shared Surface Sync check saw the editor PR-merge (development's newer versions) against the web PR head (the older ones) and flagged diffs. Adopt development's exact version on both mirrored repos: - data-type/index.tsx, variables-editor/index.tsx (+ their tests) - utils/PLC/data-type-text-parser.ts (+ its test) No production code referenced the dropped export. Keeps the mirror byte-identical without pulling in unrelated development divergence. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NegApcNt2SzRk3FDAs5T9p
v0.6.8 carries the live-pointer debug accessor this branch's baremetal OPC-UA zero-copy read depends on — `type_ops`/`ptr_ops` with handle_ptr and read_ptr — plus the string-capacity guard (the debug dispatch addresses strings as STRING(254)). Verified: the installed 0.6.8 runtime headers are byte-identical to the strucpp source, and the 0.6.8 compiler compiles and emits the debug table with the expected behaviour. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NegApcNt2SzRk3FDAs5T9p
…ice-vplc EDGE-639: rename Orchestrator to Edge Device and its children to vPLC
…ntime RTOP-285: OpenPLC runtime on Siemens LOGO! 8.2 over Ethernet + Modbus TCP debug
Contributor
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. 🗂️ Base branches to auto review (1)
Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Advanced Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Gustavohsdp
approved these changes
Sep 18, 2026
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
7 participants
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.
Release 4.3.0 —
development→main.Ships the LOGO! 8.2 baremetal runtime work (RTOP-285), the DOPE-442 Modbus-config unification, and the EDGE-639/640 Orchestrator→Edge Device / vPLC rename that have accumulated on development since 4.2.11.
Version is 4.3.0 across all files (package.json, app-version, release/app, package-lock root). This corrects the erroneous 4.4.0 dev bump back to the intended 4.3.0. Production main is currently 4.2.11.
Mirror release pair — merge together with the sibling repo. After both merge: tag
v4.3.0on openplc-editormainkicks the desktop build; openplc-web builds automatically on this merge tomain.🤖 Generated with Claude Code
https://claude.ai/code/session_01NegApcNt2SzRk3FDAs5T9p