diff --git a/.github/workflows/ci-unit-tests.yml b/.github/workflows/ci-unit-tests.yml index efa90bc74..c19cebca2 100644 --- a/.github/workflows/ci-unit-tests.yml +++ b/.github/workflows/ci-unit-tests.yml @@ -20,6 +20,14 @@ jobs: - name: Install dependencies run: npm ci --ignore-scripts + # --ignore-scripts skips the postinstall, so strucpp (a GitHub-release + # package, not an npm dep) is never fetched — yet the TS sources import + # `strucpp/libs/iec-types.json` and its runtime headers. Install just + # strucpp (no xml2st platform binary, no native rebuild) so the suites + # can load and the coverage gate actually runs. + - name: Install strucpp + run: npm run setup:strucpp + - name: Run tests with coverage run: npx jest --config jest.config.json --collectCoverage --ci diff --git a/CLAUDE.md b/CLAUDE.md index 78a71638d..516aa0764 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -263,6 +263,27 @@ When adding new code to covered directories, you must add corresponding tests to ## Important Patterns +### When bumping the app version: +`APP_VERSION` in `src/frontend/data/constants/app-version.ts` is the **single +source of truth** for the human-facing version, shared **byte-for-byte** between +openplc-editor and openplc-web (enforced by the mirror gate / `compare-surfaces.py`). +The About modal renders it directly; the web build writes it into `version.json`. + +**Bump `APP_VERSION` — never `package.json` alone.** Make the identical one-line +edit in BOTH repos, and set `package.json.version` to the same value in both so +they can't drift. Roles: `APP_VERSION` is what the user sees in the About dialog; +`package.json.version` is what electron-builder stamps on the desktop binary and +what the release tag `vX.Y.Z` must match. Bumping only `package.json` leaves the +About dialog stuck on the old version — **this mistake shipped 4.2.7 and 4.2.8 +with About still showing 4.2.6.** If the two ever disagree, `APP_VERSION` is +authoritative; fix it to match. + +Release order: bump `APP_VERSION` + `package.json` (both repos, same value) → PR +to `development` → merge → promote `development`→`main` on both → tag `vX.Y.Z` on +the editor's `main` to trigger the "Build and Release" workflow. Web auto-deploys +on its `main` push. (Ideally `package.json.version` should be derived from +`APP_VERSION` in the release workflow so a single bump can never drift.) + ### When adding a new port: 1. Define the interface in `src/middleware/shared/ports/` 2. Add it to `PlatformPorts` in `src/middleware/shared/providers/types.ts` @@ -304,20 +325,31 @@ openplc-web). Pure functions, no IPC, no electron coupling. replaces the old `generateIecAddress` helper. Pass `alsoUsed` for in-flight allocations within a batch. - **Alias registry** (`alias-registry.ts`): derived index on top of - the pool. `byAlias` / `byAddress` maps plus `duplicateAliases` - (first-wins). Pure function — rebuild on demand, cost is - O(producers). -- **Variable sync** (`sync-variable-aliases.ts`): pure function that - walks variables and either adopts (no alias bound but address - matches), refreshes (alias address moved), or orphans (alias gone). - Called from every producer-mutation site, target switch, - project load, and pre-compile via the - `projectActions.syncVariableAliases()` store action. - -The variable cell renders `alias ?? location` and shows an amber -warning glyph + tooltip when the alias is orphaned. Aliases are -intended to be unique system-wide; the registry's -`isAliasNameAvailable(name, ignoring?)` is the system-wide validator. + the pool. `byAlias` map plus `duplicateAliases` (first-wins). Pure + function — rebuild on demand, cost is O(producers). +- **Compile-time resolution** (`registry/resolve.ts`): a variable's + `location` holds EITHER an alias name OR a literal `%addr` + (single-field model). `buildAliasIndex(registry)` builds the + `alias → address` map; `resolveLocation(field, index)` resolves a + variable's `location` for the compiler: + - literal `%…` → used verbatim (manual locations honoured exactly); + - alias that still exists → its current address; + - alias that is gone → `''` (variable becomes unlocated). + The compiler/runtime never see aliases: the editor resolves them in + a pre-compile snapshot via the + `projectActions.getCompileReadyProjectData()` store action. When a + producer alias is renamed, `projectActions.renameAlias(old, new)` + cascades onto every bound variable's `location`. + +The variable cell renders `location` verbatim — the alias name when +alias-bound, the `%addr` when a manual literal — and shows an amber +warning glyph + tooltip when an alias-bound location no longer resolves +(orphaned) or when a manual `%addr` collides with an alias another +project variable is bound to (duplicate-location risk). Aliases are +intended to be unique system-wide; every IO-mapping / pin / +remote-device editor calls the registry's +`validateAliasEdit(registry, name, ignoring)` gate before persisting a +new alias. ## Environment diff --git a/binary-versions.json b/binary-versions.json index 879364ceb..a0d6978b0 100644 --- a/binary-versions.json +++ b/binary-versions.json @@ -4,7 +4,7 @@ "repository": "Autonomy-Logic/xml2st" }, "strucpp": { - "version": "v0.5.10", + "version": "v0.5.13", "repository": "Autonomy-Logic/STruCpp" } } diff --git a/docs/iec-address-registry.md b/docs/iec-address-registry.md new file mode 100644 index 000000000..445bc9a42 --- /dev/null +++ b/docs/iec-address-registry.md @@ -0,0 +1,243 @@ +# Central IEC Address Registry — Architecture + +> Status: **approved design, in implementation** +> Scope: `openplc-editor` + `openplc-web` (shared surface — byte-identical) +> Supersedes the scattered per-producer address allocators and the +> derived `address-pool` / `alias-registry` / `sync-variable-aliases` trio. + +## 1. Motivation + +IEC 61131-3 addresses are a **single, finite, shared resource**. Every +producer — pin mapping, VPP I/O modules, Modbus remote devices, EtherCAT +slaves, and anything added later — draws from the same address spaces +(`%IX`, `%QX`, `%IW`, `%QW`, `%MW`, …). Today each producer: + +- stores its own addresses in its own domain records, +- allocates them with its own copy of the "find next free" loop + (EtherCAT even has a *separate* bit-offset allocator), and +- owns its channels' aliases independently, reconciled to program + variables only opportunistically. + +The result is duplication, drift, and no project-wide gap reclamation +(deleting a consumer leaves holes). Aliases — the thing user code should +depend on — are the most fragmented of all. + +**This design makes a single store the source of truth for consumers, +their address assignments, and their aliases.** Producers become thin +clients that *register channels* and *read addresses back*. User program +variables reference aliases (or manual literal addresses); the compiler +resolves them. + +## 2. Concepts + +- **Consumer** — anything that needs addresses: a VPP module in a slot, a + Modbus IO group, an EtherCAT slave, a pin block. Identified by a stable + `id`, tagged with a `kind`, and given a deterministic `order`. +- **Channel** — one address request inside a consumer. Has a **stable, + address-independent `channelId`**, an address **class**, an optional + **alias**, and an optional **pinned** literal address (fixed hardware). + > The stable `channelId` is the linchpin: aliases and variable bindings + > attach to the channel, **not** to the address, so when reallocation + > moves the channel the alias follows it automatically. +- **Address class** — `{ direction: I|Q|M, size: X|B|W|D|L }`. Maps to a + prefix (`%IX`, `%QB`, `%QW`, …). **Each prefix is an independent linear + space — no byte/word/dword overlap** (matches the OpenPLC runtime, which + uses separate typed buffers per prefix). +- **Assignment** — the derived `(consumerId, channelId) → address` map, + recomputed by `recalculate()`. +- **Alias** — a user-facing name, **unique system-wide**, owned by the + registry and attached to a channel. + +## 3. The registry (source of truth) + +Serialized as a dedicated project section: + +```ts +interface IecAddressRegistry { + consumers: RegistryConsumer[] // the record + assignments: Record // derived cache: key(cid,chid) -> address +} +interface RegistryConsumer { + id: string + kind: 'pin-mapping' | 'vpp-io' | 'modbus-tcp-remote' | 'ethercat' | string + label?: string + order: number // deterministic allocation order + channels: RegistryChannel[] +} +interface RegistryChannel { + channelId: string // stable, address-independent + class: { direction: 'I'|'Q'|'M'; size: 'X'|'B'|'W'|'D'|'L' } + alias?: string // unique system-wide; empty = none + pinned?: string // fixed hardware address → reserved, not allocated +} +``` + +Producers keep only their **domain** config (a Modbus group still has its +function code, cycle time, etc.) plus the `consumerId` they registered +under. They no longer store IEC addresses or aliases. + +## 4. Generic API + +```ts +// pure core (framework-agnostic); a Zustand slice wraps it 1:1 +createRegistry(): IecAddressRegistry +addConsumer(reg, consumer): IecAddressRegistry // append + recalculate +removeConsumer(reg, consumerId): IecAddressRegistry // drop + recalculate (fills gaps) +updateConsumer(reg, consumerId, patch): IecAddressRegistry // change channels/order + recalculate +setAlias(reg, consumerId, channelId, alias): SetAliasResult // system-wide uniqueness gate (the ONE gate) +recalculate(reg): { registry, conflicts } // reassign all — deterministic, gapless, idempotent +// queries +addressOf(reg, consumerId, channelId): string | undefined +buildAliasIndex(reg): Map +resolveLocation(field, aliasIndex): string // compile-time: alias|literal -> address +``` + +`recalculate()` is **idempotent** (same input → same output) so it is safe +to call on every mutation, on project load, and pre-compile. + +## 5. Allocation algorithm + +1. **Reserve** every `pinned` channel at its literal address (pin-mapping, + or any explicitly-fixed channel). Two pinned channels on the same + address are reported as a conflict (first wins). +2. **Allocate** every non-pinned channel, walking consumers in `order` + (ties broken by `id`) and channels in declaration order, taking the + **lowest free index** in that channel's prefix space. +3. Bit prefixes (`%IX`/`%QX`/`%MX`) address as `byte.bit` + (`linear = byte*8 + bit`); all other sizes are index-addressed. + +Determinism (stable order + stable channel order) guarantees reproducible +results across sessions, so a re-open never gratuitously renumbers. + +## 6. Aliases and variable binding + +- **Aliases live only in the registry**, attached to channels. Uniqueness + is enforced in the single `setAlias` gate — no producer can create a + duplicate. +- **A program variable's location field holds either an alias name or a + literal IEC address.** The compiler only understands IEC addresses, so + the editor resolves at compile time (`resolveLocation`): + - **alias** → the alias's current address from the registry; + - **alias no longer exists** → empty location (variable is unlocated — + this already matches today's orphan behavior); + - **literal `%…`** → used **verbatim**. +- **Manual literal addresses are fully manual.** The allocator does **not** + reserve or avoid them. If the user types `%QX0.3` and later reallocation + assigns `%QX0.3` to some channel, that is the user's responsibility — + we honor exactly what they typed. This keeps the manual escape hatch + simple and predictable. +- Because aliases attach to the stable channel, moving `%QX0.3 → %QX0.5` + updates `alias → address` automatically; every variable using that alias + now resolves to `%QX0.5` with no user action. **That is the whole point: + user code is address-agnostic.** + +## 7. Migrate-on-open (existing projects → registry format) + +Old projects store addresses and aliases scattered across producer records +and have no `addressRegistry` section. On project open, when the section is +absent, run a **one-shot pure migration**: + +1. **Build consumers** from legacy producer state, one consumer per natural + unit, preserving channel order: + - `pin-mapping` → one pinned channel per board pin (alias from `pin.alias`). + - `vpp-io` → one consumer per slot/module; channels from its + `io-mapping` entries (class from `entry.iecAddress`, alias from + `entry.alias`). + - `modbus-tcp-remote` → one consumer per IO group; channels from + `ioPoints` (class from function code, alias from `point.alias`). + - `ethercat` → one consumer per slave; channels from `channelMappings` + (alias from `mapping.alias`). + Each channel's **initial `pinned` = its current legacy address**, so the + first `recalculate()` reproduces exactly today's addresses (nothing + moves on open). +2. **Adopt variables onto aliases.** For each program variable whose + `location` matches an aliased channel address, rewrite `location` to the + **alias name** (self-upgrade — today's `adopt`). Variables on a + non-aliased address stay as manual literals; variables already using an + alias are unchanged. +3. **Unpin.** After adoption, clear the temporary `pinned` seeds on the + allocatable (non-hardware) channels so subsequent edits can compact + gaps. Genuine hardware pins (pin-mapping) stay pinned. +4. **Strip** the now-migrated address/alias fields from producer records + and write the `addressRegistry` section. Bump a project schema version + so the migration runs exactly once. + +The migration is a pure function `migrateProjectToRegistry(project)` with +its own exhaustive tests (round-trip: legacy project → registry → +addresses identical to the originals; aliases preserved; variables +adopted). + +## 8. Compilation + +At compile, the pipeline resolves each variable's location through +`resolveLocation(field, buildAliasIndex(registry))` and emits the concrete +`%…` (or an empty location, dropping the `AT %…`, when an alias orphaned). +Literals pass through unchanged. No producer-specific address logic remains +in the emitters. + +## 9. Module layout (shared surface) + +``` +middleware/shared/utils/iec-address/registry/ + types.ts # Consumer, Channel, AddressClass, IecAddressRegistry, reports + address-space.ts # prefix<->class, bit linearization, parse/format, lowest-free + allocate.ts # allocateAddresses(consumers) -> { assignments, conflicts } + registry.ts # createRegistry / add / remove / update / setAlias / recalculate / queries + resolve.ts # buildAliasIndex, resolveLocation (compile-time) + migrate.ts # migrateProjectToRegistry (legacy -> registry) [Phase 2] + index.ts +frontend/store/slices/iec-address/ # Zustand slice wrapping the pure core [Phase 2] +``` + +The pure core carries no framework or Electron coupling and is +byte-identical across both repos. The old `address-pool.ts` / +`alias-registry.ts` / `sync-variable-aliases.ts` are removed once every +producer and the compiler have moved to the registry. + +## 10. Phased delivery + +- **Phase 1** — pure core (§3–§6 minus migration): types, address-space, + allocator, registry ops, `resolveLocation`. Fully unit-tested. No wiring. +- **Phase 2** — `migrateProjectToRegistry` + the Zustand slice + wire into + project load (schema-version gated). +- **Phase 3** — Modbus + EtherCAT register as consumers; delete their + bespoke allocators. **Delivers project-wide gap reclamation (bug #4).** +- **Phase 4** — VPP registers as consumers (module-channel resolver + injected); delete the two allocator effects. +- **Phase 5** — pin-mapping as pinned consumer; unify alias editing on the + single `setAlias` gate. +- **Phase 6** — compiler resolves via the registry; delete the legacy + pool/registry/sync modules and the now-redundant producer fields. + +Each phase ships as a paired editor+web PR keeping the shared surface +byte-identical. + +## 11. Implementation status + +Shipped on `feat/central-iec-address-registry` (editor + web, byte-identical): + +- **Pure core** — types, address-space, allocator (capability-scoped, + gapless, deterministic), registry ops, `resolveLocation`, migration + transform, and the session **alias-memory** (`memoryKey` + + `restoreAliasesFromMemory`, keyed `moduleId:slot:channel`; held in the store, + never serialized, reset on a fresh project). +- **Central action** `recalculateIecAddresses` reallocates **VPP + Modbus + + EtherCAT** together (`ALLOCATED_KINDS`), capability-scoped, restoring aliases + from the session memory, and writes addresses + aliases back to each + producer (VPP `io-mapping`, Modbus `ioPoints`, EtherCAT `channelMappings`). + Invoked from every producer mutation and on target switch. +- **Pins** participate as **fixed constraints** — hardware addresses are never + reallocated; their aliases persist per-board and flow through the same + registry uniqueness gate. Nothing to route. +- **Aliases** resolve to concrete IEC addresses **in the editor** (each + variable's `location` is kept resolved); the compiler/runtime are untouched. + +**Deliberately deferred** (superseded but still present; removal needs the +class-carrying-entry refactor + a `syncVariableAliases` migration, both best +verified in-app): the VPP effects' provisional `nextFreeAddress` seeding +(overwritten by the central recalc), the EtherCAT `esi-parser` overlap +allocator (superseded by the independent-prefix model — confirmed against the +runtime's separate typed buffers), and the legacy `address-pool` / +`alias-registry` read model (still backs `syncVariableAliases` and the +per-producer alias-uniqueness gates — consistent with the registry because +both derive from the same producer state). diff --git a/package-lock.json b/package-lock.json index 739684025..089b47780 100644 --- a/package-lock.json +++ b/package-lock.json @@ -1,12 +1,12 @@ { "name": "open-plc-editor-jwplc", - "version": "4.2.7-jwplc.1", + "version": "4.2.8-jwplc.1", "lockfileVersion": 3, "requires": true, "packages": { "": { "name": "open-plc-editor-jwplc", - "version": "4.2.7-jwplc.1", + "version": "4.2.8-jwplc.1", "hasInstallScript": true, "license": "GPL-3.0", "dependencies": { diff --git a/package.json b/package.json index e2d83b053..4aa3c4321 100644 --- a/package.json +++ b/package.json @@ -1,7 +1,7 @@ { "name": "open-plc-editor-jwplc", "description": "OpenPLC Editor - JWPLC Edition, build maintained by JW Control for JWPLC Basic integration", - "version": "4.2.7-jwplc.1", + "version": "4.2.8-jwplc.1", "license": "GPL-3.0", "author": { "name": "Autonomy Logic" @@ -17,6 +17,7 @@ "format": "cross-env NODE_ENV=development prettier --write \"./src/**/*.{ts,tsx}\"", "postinstall": "ts-node scripts/download-binaries.ts && ts-node scripts/check-native-dep.js && electron-builder install-app-deps && npm run build:dll", "setup:binaries": "ts-node scripts/download-binaries.ts", + "setup:strucpp": "ts-node scripts/download-binaries.ts --strucpp-only", "package": "ts-node scripts/clean.js dist && npm run build && electron-builder build --publish never && npm run build:dll", "rebuild": "electron-rebuild --parallel --types prod,dev,optional --module-dir release/app", "prestart": "ts-node scripts/download-binaries.ts && cross-env NODE_ENV=development TS_NODE_TRANSPILE_ONLY=true webpack --config ./configs/webpack/webpack.config.main.dev.ts", diff --git a/release/app/package-lock.json b/release/app/package-lock.json index bc5523367..ac80b9fe2 100644 --- a/release/app/package-lock.json +++ b/release/app/package-lock.json @@ -1,12 +1,12 @@ { "name": "open-plc-editor-jwplc", - "version": "4.2.7-jwplc.1", + "version": "4.2.8-jwplc.1", "lockfileVersion": 3, "requires": true, "packages": { "": { "name": "open-plc-editor-jwplc", - "version": "4.2.7-jwplc.1", + "version": "4.2.8-jwplc.1", "hasInstallScript": true, "license": "GPL-3.0", "dependencies": { diff --git a/release/app/package.json b/release/app/package.json index ef9101713..29c502359 100644 --- a/release/app/package.json +++ b/release/app/package.json @@ -1,10 +1,10 @@ { "name": "open-plc-editor-jwplc", - "version": "4.2.7-jwplc.1", + "version": "4.2.8-jwplc.1", "description": "OpenPLC Editor - JWPLC Edition, build maintained by JW Control for JWPLC Basic integration", "license": "GPL-3.0", "author": { - "name": "Autonomy Logic" + "name": "open-plc-editor-jwplc" }, "main": "./dist/main/main.js", "scripts": { diff --git a/run-jwplc-editor-dev.local.bat b/run-jwplc-editor-dev.local.bat new file mode 100644 index 000000000..3113e749d --- /dev/null +++ b/run-jwplc-editor-dev.local.bat @@ -0,0 +1,21 @@ +@echo off +setlocal +cd /d "%~dp0" + +echo ====================================== +echo OpenPLC Editor - JWPLC Edition DEV +echo ====================================== +echo. + +where node +node -v +where npm.cmd +call npm.cmd -v + +echo. +echo Iniciando editor en modo desarrollo... +echo. + +call npm.cmd run dev + +pause diff --git a/scripts/download-binaries.ts b/scripts/download-binaries.ts index b0318c183..e7a18860c 100644 --- a/scripts/download-binaries.ts +++ b/scripts/download-binaries.ts @@ -54,11 +54,17 @@ function cacheFile(platform: Platform, arch: Arch): string { // CLI argument parsing // --------------------------------------------------------------------------- -function parseArgs(): { platform: Platform; arch: Arch; force: boolean } { +function parseArgs(): { platform: Platform; arch: Arch; force: boolean; strucppOnly: boolean } { const args = process.argv.slice(2) let platform = process.platform as string let arch = process.arch as string let force = false + // strucpp-only: install just the platform-independent strucpp package (its + // libs/ + runtime headers are imported by the TS sources). Skips the xml2st + // platform binary — used by the unit-test CI job, which runs `npm ci + // --ignore-scripts` (so the postinstall never fetched strucpp) but doesn't + // need the heavyweight platform binaries or a native rebuild. + let strucppOnly = false for (let i = 0; i < args.length; i++) { if (args[i] === '--platform' && args[i + 1]) { @@ -67,6 +73,8 @@ function parseArgs(): { platform: Platform; arch: Arch; force: boolean } { arch = args[++i] } else if (args[i] === '--force') { force = true + } else if (args[i] === '--strucpp-only') { + strucppOnly = true } } @@ -79,7 +87,7 @@ function parseArgs(): { platform: Platform; arch: Arch; force: boolean } { process.exit(1) } - return { platform: platform as Platform, arch: arch as Arch, force } + return { platform: platform as Platform, arch: arch as Arch, force, strucppOnly } } // --------------------------------------------------------------------------- @@ -289,7 +297,7 @@ async function downloadStrucpp(tool: ToolEntry): Promise { // --------------------------------------------------------------------------- async function main(): Promise { - const { platform, arch, force } = parseArgs() + const { platform, arch, force, strucppOnly } = parseArgs() if (!fs.existsSync(VERSIONS_FILE)) { console.error(`binary-versions.json not found at ${VERSIONS_FILE}`) @@ -298,33 +306,33 @@ async function main(): Promise { const versions: BinaryVersions = JSON.parse(fs.readFileSync(VERSIONS_FILE, 'utf-8')) - console.log(`[download-binaries] platform=${platform} arch=${arch} force=${force}`) + console.log(`[download-binaries] platform=${platform} arch=${arch} force=${force} strucppOnly=${strucppOnly}`) - const targetBinDir = binDir(platform, arch) - fs.mkdirSync(targetBinDir, { recursive: true }) - - const cached = force ? null : getCachedMetadata(platform, arch) - const downloadXml2stNeeded = force || needsXml2st(versions, cached, platform, arch) - const downloadStrucppNeeded = force || needsStrucpp(versions) + // strucpp is platform-independent (its libs/ + runtime headers are imported + // by the TS sources) — download it whenever it's missing or outdated. + if (force || needsStrucpp(versions)) { + await downloadStrucpp(versions.strucpp) + } else { + console.log(` strucpp ${versions.strucpp.version} already installed, skipping.`) + } - if (!downloadXml2stNeeded && !downloadStrucppNeeded) { - console.log(`[download-binaries] All tools up to date, skipping.`) + // --strucpp-only stops here: the caller (e.g. the unit-test CI job) needs the + // strucpp package but not the platform binary or the platform cache marker. + if (strucppOnly) { + console.log(`[download-binaries] strucpp-only: skipped xml2st platform binary.`) return } - if (downloadXml2stNeeded) { + const targetBinDir = binDir(platform, arch) + fs.mkdirSync(targetBinDir, { recursive: true }) + + const cached = force ? null : getCachedMetadata(platform, arch) + if (force || needsXml2st(versions, cached, platform, arch)) { await downloadXml2st(versions.xml2st, platform, arch, targetBinDir) } else { console.log(` xml2st ${versions.xml2st.version} already installed, skipping.`) } - // strucpp is platform-independent — only download once regardless of platform/arch - if (downloadStrucppNeeded) { - await downloadStrucpp(versions.strucpp) - } else { - console.log(` strucpp ${versions.strucpp.version} already installed, skipping.`) - } - writeCache(versions, platform, arch) console.log(`[download-binaries] Done.`) } diff --git a/src/__architecture__/validate.ts b/src/__architecture__/validate.ts index 437418d2e..e498384df 100644 --- a/src/__architecture__/validate.ts +++ b/src/__architecture__/validate.ts @@ -155,6 +155,10 @@ function getLayer(filePath: string): LayerName | null { // openplc-web parity explicit (this folder is byte-identical // between repos). if (rel.startsWith('middleware/shared/utils/')) return 'utils' + // Shared runtime-auth (RuntimeTokenManager) is pure, dependency-free logic + // reachable from adapters/backend/main on both platforms — same `utils` rule + // set, byte-identical between repos. + if (rel.startsWith('middleware/shared/runtime-auth/')) return 'utils' if (rel.match(/^middleware\/adapters\/[^/]+\/components\//)) return 'adapter-components' if (rel.startsWith('middleware/adapters/')) return 'adapters' diff --git a/src/backend/editor/compiler/__tests__/editor-compiler-platform-port.test.ts b/src/backend/editor/compiler/__tests__/editor-compiler-platform-port.test.ts index 590b5a0a7..1de587f46 100644 --- a/src/backend/editor/compiler/__tests__/editor-compiler-platform-port.test.ts +++ b/src/backend/editor/compiler/__tests__/editor-compiler-platform-port.test.ts @@ -149,9 +149,9 @@ describe('createEditorCompilerPlatformPort', () => { cleanBuild: false, mainProcessBridge: { makeRuntimeApiRequest: jest.fn(), + makeRuntimeApiUpload: jest.fn(), }, compressSourceFolder: jest.fn(), - sendRuntimeUpload: jest.fn(), pollTimeoutMs: 1000, pollIntervalMs: 10, startTimeoutMs: 1000, @@ -321,7 +321,7 @@ describe('createEditorCompilerPlatformPort', () => { })) as unknown as EditorCompilerPlatformPortContext['mainProcessBridge']['makeRuntimeApiRequest'] const port = createEditorCompilerPlatformPort( makeHandlers(), - makeContext({ mainProcessBridge: { makeRuntimeApiRequest } }), + makeContext({ mainProcessBridge: { makeRuntimeApiRequest, makeRuntimeApiUpload: jest.fn() } }), ) const result = await port.checkRuntimeVersion( { context: { kind: 'editor-https', ip: '10.0.0.1', jwt: 'token' } }, @@ -338,7 +338,7 @@ describe('createEditorCompilerPlatformPort', () => { const log = jest.fn() const port = createEditorCompilerPlatformPort( makeHandlers(), - makeContext({ mainProcessBridge: { makeRuntimeApiRequest } }), + makeContext({ mainProcessBridge: { makeRuntimeApiRequest, makeRuntimeApiUpload: jest.fn() } }), ) const result = await port.checkRuntimeVersion( { context: { kind: 'editor-https', ip: '10.0.0.1', jwt: 'token' } }, @@ -355,7 +355,7 @@ describe('createEditorCompilerPlatformPort', () => { const log = jest.fn() const port = createEditorCompilerPlatformPort( makeHandlers(), - makeContext({ mainProcessBridge: { makeRuntimeApiRequest } }), + makeContext({ mainProcessBridge: { makeRuntimeApiRequest, makeRuntimeApiUpload: jest.fn() } }), ) const result = await port.checkRuntimeVersion( { context: { kind: 'editor-https', ip: '10.0.0.1', jwt: 'token' } }, diff --git a/src/backend/editor/compiler/compiler-module.ts b/src/backend/editor/compiler/compiler-module.ts index fe5d59831..c8b914fc9 100644 --- a/src/backend/editor/compiler/compiler-module.ts +++ b/src/backend/editor/compiler/compiler-module.ts @@ -50,10 +50,19 @@ import type { KnownPou } from '@root/backend/shared/utils/PLC/split-program-st' type LibraryCompileBridge = { makeRuntimeApiRequest: ( ipAddress: string, - jwtToken: string, endpoint: string, responseParser?: (data: string) => T, ) => Promise<{ success: true; data?: T } | { success: false; error: string }> + // Required to satisfy compileProgram's bridge contract; never invoked on the + // library path (it compiles with runtimeIpAddress=null, so no upload runs). + makeRuntimeApiUpload: (opts: { + ipAddress: string + fileBuffer: Buffer + filename: string + contentType: string + cleanBuild: boolean + onUploadAccepted?: (responseBody: string) => void + }) => Promise<{ success: true; data: string } | { success: false; error: string }> loadEnabledArchives: (enabledNames: string[]) => { archives: unknown[]; missing: string[] } } @@ -1903,73 +1912,8 @@ class CompilerModule { }) } - /** - * Send a compiled program file to the runtime's `/api/upload-file` - * over HTTPS via a multipart/form-data POST. Pure transport — no - * polling, no PLC start, no UI logging. Used as the `uploadProgram` - * callback fed to the shared `deployRuntimeProgram` orchestrator. - * - * v3 callers pass `program.st` + `text/plain`; v4 callers pass the - * compiled zip + `application/zip`. `cleanBuild` toggles the - * `?clean=1` flag the runtime honours by wiping `build/` and ccache - * before compiling. - */ - private async sendRuntimeUpload(opts: { - hostname: string - jwtToken: string - filename: string - contentType: string - fileBuffer: Buffer - cleanBuild: boolean - onUploadAccepted?: (responseBody: string) => void - }): Promise<{ success: boolean; error?: string }> { - const boundary = '----WebKitFormBoundary' + Math.random().toString(36).substring(2) - const header = Buffer.from( - `--${boundary}\r\n` + - `Content-Disposition: form-data; name="file"; filename="${opts.filename}"\r\n` + - `Content-Type: ${opts.contentType}\r\n\r\n`, - ) - const footer = Buffer.from(`\r\n--${boundary}--\r\n`) - const body = Buffer.concat([header, opts.fileBuffer, footer] as unknown as ReadonlyArray) - - return new Promise<{ success: boolean; error?: string }>((resolve) => { - const req = https.request( - { - hostname: opts.hostname, - port: 8443, - path: opts.cleanBuild ? '/api/upload-file?clean=1' : '/api/upload-file', - method: 'POST', - headers: { - 'Content-Type': `multipart/form-data; boundary=${boundary}`, - 'Content-Length': body.length, - Authorization: `Bearer ${opts.jwtToken}`, - }, - ...getRuntimeHttpsOptions(), - } as https.RequestOptions, - (res: IncomingMessage) => { - let data = '' - res.on('data', (chunk: Buffer) => { - data += chunk.toString() - }) - res.on('end', () => { - if (res.statusCode === 200) { - opts.onUploadAccepted?.(data) - resolve({ success: true }) - } else { - resolve({ success: false, error: data || `HTTP ${res.statusCode}` }) - } - }) - }, - ) - req.setTimeout(300_000, () => { - req.destroy() - resolve({ success: false, error: 'Upload request timed out after 5 minutes' }) - }) - req.on('error', (err: Error) => resolve({ success: false, error: err.message })) - req.write(body) - req.end() - }) - } + // Runtime upload moved to MainProcessBridge.makeRuntimeApiUpload so it shares + // the single token authority (transparent refresh + retry on an expired JWT). // !! Deprecated: This method is a outdated implementation and should be removed. async createXmlFile( @@ -2457,10 +2401,17 @@ class CompilerModule { mainProcessBridge: { makeRuntimeApiRequest: ( ipAddress: string, - jwtToken: string, endpoint: string, responseParser?: (data: string) => T, ) => Promise<{ success: true; data?: T } | { success: false; error: string }> + makeRuntimeApiUpload: (opts: { + ipAddress: string + fileBuffer: Buffer + filename: string + contentType: string + cleanBuild: boolean + onUploadAccepted?: (responseBody: string) => void + }) => Promise<{ success: true; data: string } | { success: false; error: string }> /** * Resolve a list of project-enabled library names to parsed * `.stlib` archives. Bundled libraries are always-on and @@ -2710,7 +2661,6 @@ class CompilerModule { cleanBuild: cleanBuild ?? false, mainProcessBridge, compressSourceFolder: (folderPath: string) => this.compressSourceFolder(folderPath), - sendRuntimeUpload: (opts) => this.sendRuntimeUpload(opts), pollTimeoutMs: CompilerModule.COMPILATION_STATUS_TIMEOUT_MS, pollIntervalMs: CompilerModule.COMPILATION_STATUS_POLL_INTERVAL_MS, startTimeoutMs: POST_BUILD_START_TIMEOUT_MS, diff --git a/src/backend/editor/compiler/editor-compiler-platform-port.ts b/src/backend/editor/compiler/editor-compiler-platform-port.ts index e5e184d8a..5dc1a2116 100644 --- a/src/backend/editor/compiler/editor-compiler-platform-port.ts +++ b/src/backend/editor/compiler/editor-compiler-platform-port.ts @@ -106,28 +106,26 @@ export interface EditorCompilerPlatformPortContext { mainProcessBridge: { makeRuntimeApiRequest: ( ipAddress: string, - jwtToken: string, endpoint: string, responseParser?: (data: string) => T, ) => Promise<{ success: true; data?: T } | { success: false; error: string }> + /** Upload the runtime-v4 program bundle. Owns token refresh internally + * (via the token authority), so the upload self-heals on expiry like every + * other runtime call. */ + makeRuntimeApiUpload: (opts: { + ipAddress: string + fileBuffer: Buffer + filename: string + contentType: string + cleanBuild: boolean + onUploadAccepted?: (responseBody: string) => void + }) => Promise<{ success: true; data: string } | { success: false; error: string }> } /** Compress the source folder into the runtime v4 upload zip. * Delegated through context so the port adapter doesn't pull * in the `archiver`-dependent compressSourceFolder method (which * has its own private state on CompilerModule). */ compressSourceFolder: (folderPath: string) => Promise - /** Send the upload request to a runtime device. Wraps - * CompilerModule.sendRuntimeUpload with the right multipart - * payload structure. */ - sendRuntimeUpload: (opts: { - hostname: string - jwtToken: string - filename: string - contentType: string - fileBuffer: Buffer - cleanBuild: boolean - onUploadAccepted?: (responseBody: string) => void - }) => Promise<{ success: boolean; error?: string }> /** Timeout for the post-upload compile-status poll. */ pollTimeoutMs: number /** Interval for the post-upload compile-status poll. */ @@ -384,9 +382,8 @@ export function createEditorCompilerPlatformPort( const deployOutcome = await deployRuntimeProgram({ uploadProgram: () => - context.sendRuntimeUpload({ - hostname: deviceContext.ip, - jwtToken: deviceContext.jwt, + context.mainProcessBridge.makeRuntimeApiUpload({ + ipAddress: deviceContext.ip, filename: 'program.zip', contentType: 'application/zip', fileBuffer, @@ -405,7 +402,7 @@ export function createEditorCompilerPlatformPort( status: string logs: string[] exit_code: number | null - }>(deviceContext.ip, deviceContext.jwt, '/api/compilation-status', (data: string) => { + }>(deviceContext.ip, '/api/compilation-status', (data: string) => { return JSON.parse(data) as { status: string; logs: string[]; exit_code: number | null } }) if (!result.success) return { success: false, error: result.error } @@ -414,7 +411,6 @@ export function createEditorCompilerPlatformPort( fetchStartResponse: async () => { const result = await context.mainProcessBridge.makeRuntimeApiRequest( deviceContext.ip, - deviceContext.jwt, '/api/start-plc', (data: string) => { const parsed = JSON.parse(data) as { status?: string } @@ -486,9 +482,8 @@ export function createEditorCompilerPlatformPort( const fileBuffer = Buffer.from(args.programSt, 'utf-8') const deployOutcome = await deployRuntimeProgram({ uploadProgram: () => - context.sendRuntimeUpload({ - hostname: deviceContext.ip, - jwtToken: deviceContext.jwt, + context.mainProcessBridge.makeRuntimeApiUpload({ + ipAddress: deviceContext.ip, filename: 'program.st', contentType: 'text/plain', fileBuffer, @@ -507,7 +502,7 @@ export function createEditorCompilerPlatformPort( status: string logs: string[] exit_code: number | null - }>(deviceContext.ip, deviceContext.jwt, '/api/compilation-status', (data: string) => { + }>(deviceContext.ip, '/api/compilation-status', (data: string) => { return JSON.parse(data) as { status: string; logs: string[]; exit_code: number | null } }) if (!result.success) return { success: false, error: result.error } @@ -516,7 +511,6 @@ export function createEditorCompilerPlatformPort( fetchStartResponse: async () => { const result = await context.mainProcessBridge.makeRuntimeApiRequest( deviceContext.ip, - deviceContext.jwt, '/api/start-plc', (data: string) => { const parsed = JSON.parse(data) as { status?: string } @@ -556,7 +550,6 @@ export function createEditorCompilerPlatformPort( fetchVersion: async () => { const result = await context.mainProcessBridge.makeRuntimeApiRequest<{ version: string }>( deviceContext.ip, - '', // unauthenticated probe '/api/version', (data: string) => JSON.parse(data) as { version: string }, ) diff --git a/src/backend/editor/package-manager/__tests__/verify-installed-signatures.test.ts b/src/backend/editor/package-manager/__tests__/verify-installed-signatures.test.ts new file mode 100644 index 000000000..a5783a4d7 --- /dev/null +++ b/src/backend/editor/package-manager/__tests__/verify-installed-signatures.test.ts @@ -0,0 +1,215 @@ +import { createHash, sign as cryptoSign } from 'node:crypto' +import { existsSync, mkdirSync, mkdtempSync, rmSync, writeFileSync } from 'node:fs' +import { tmpdir } from 'node:os' +import { dirname, join } from 'node:path' + +import { app } from 'electron' + +import { canonicalize, SIGNATURE_FILENAME } from '../../../shared/utils/vpp/verify-package-signature' + +// Heavy/side-effecting deps that the module pulls in transitively but this +// suite has no use for. Mocking them keeps `import { PackageManagerModule }` +// free of winston file transports and extract-zip's ESM entry point. +jest.mock('electron', () => ({ app: { getPath: jest.fn(() => '/mock/path') } })) +jest.mock('extract-zip', () => ({ __esModule: true, default: jest.fn() })) +jest.mock('../../services/logger-service', () => ({ + logger: { warn: jest.fn(), info: jest.fn(), error: jest.fn() }, +})) + +// Replace the real trusted-key store with a freshly generated test keypair so +// fixtures can be signed with a private key we actually hold. The factory may +// not reference outer-scope variables, so it generates the pair inline and +// re-exports the private PEM for the test to sign with. +jest.mock('../../../shared/utils/vpp/trusted-keys', () => { + const { generateKeyPairSync } = jest.requireActual('node:crypto') + const { publicKey, privateKey } = generateKeyPairSync('ed25519') + return { + TRUSTED_PACKAGE_KEYS: { 'test-key': publicKey.export({ type: 'spki', format: 'pem' }).toString() }, + __TEST_PRIVATE_PEM: privateKey.export({ type: 'pkcs8', format: 'pem' }).toString(), + } +}) + +// eslint-disable-next-line @typescript-eslint/no-var-requires +const PRIVATE_PEM: string = (require('../../../shared/utils/vpp/trusted-keys') as { __TEST_PRIVATE_PEM: string }) + .__TEST_PRIVATE_PEM + +import { PackageManagerModule } from '../package-manager-module' + +const KEY_ID = 'test-key' + +const sha256 = (s: string): string => + createHash('sha256') + .update(Uint8Array.from(Buffer.from(s, 'utf-8'))) + .digest('hex') + +const DEFAULT_FILES: Record = { + 'manifest.json': '{"formatVersion":"1.0"}', + 'hal/arduino/hal.cpp': 'void hardwareInit() {}', +} + +interface FixtureOpts { + files?: Record + /** Override fields on the signed payload (e.g. a foreign keyId). */ + payloadOverride?: Record + /** Mutate a file's bytes AFTER signing, to simulate tampering. */ + tamperFile?: { rel: string; content: string } + /** Skip writing signature.json entirely (a side-loaded package). */ + omitSignature?: boolean +} + +describe('PackageManagerModule.verifyInstalledSignatures', () => { + let userDataDir: string + let packagesDir: string + + beforeEach(() => { + userDataDir = mkdtempSync(join(tmpdir(), 'pkg-sweep-')) + packagesDir = join(userDataDir, 'packages') + ;(app.getPath as jest.Mock).mockReturnValue(userDataDir) + }) + + afterEach(() => { + jest.clearAllMocks() + rmSync(userDataDir, { recursive: true, force: true }) + }) + + /** Write a package directory under packagesDir and register it. */ + function installFixture(packageId: string, opts: FixtureOpts = {}): string { + const dir = join(packagesDir, packageId) + const files = opts.files ?? DEFAULT_FILES + const fileHashes: Record = {} + for (const [rel, content] of Object.entries(files)) { + const full = join(dir, rel) + mkdirSync(dirname(full), { recursive: true }) + writeFileSync(full, content) + fileHashes[rel] = sha256(content) + } + + if (!opts.omitSignature) { + const payload = { + formatVersion: '1.0', + alg: 'ed25519', + keyId: KEY_ID, + packageId, + version: '1.0.0', + signedAt: '2026-06-01T00:00:00.000Z', + files: fileHashes, + ...opts.payloadOverride, + } + const signature = cryptoSign( + null, + Uint8Array.from(Buffer.from(canonicalize(payload), 'utf-8')), + PRIVATE_PEM, + ).toString('base64') + writeFileSync(join(dir, SIGNATURE_FILENAME), JSON.stringify({ ...payload, signature }, null, 2)) + } + + if (opts.tamperFile) { + writeFileSync(join(dir, opts.tamperFile.rel), opts.tamperFile.content) + } + + return dir + } + + /** Write registry.json with the given packageId -> path entries. */ + function writeRegistry(entries: Record): void { + mkdirSync(packagesDir, { recursive: true }) + const packages: Record = {} + for (const [id, e] of Object.entries(entries)) { + packages[id] = { version: '1.0.0', installedAt: '2026-06-01T00:00:00.000Z', path: e.path, devices: [] } + } + writeFileSync(join(packagesDir, 'registry.json'), JSON.stringify({ formatVersion: '1.0', packages }, null, 2)) + } + + function readRegistryIds(): string[] { + const raw = JSON.parse(require('node:fs').readFileSync(join(packagesDir, 'registry.json'), 'utf-8')) as { + packages: Record + } + return Object.keys(raw.packages) + } + + it('keeps a package that is validly signed by a trusted key', () => { + const dir = installFixture('com.test.valid') + writeRegistry({ 'com.test.valid': { path: dir } }) + + const warn = jest.fn() + const removed = new PackageManagerModule().verifyInstalledSignatures(warn) + + expect(removed).toEqual([]) + expect(warn).not.toHaveBeenCalled() + expect(existsSync(dir)).toBe(true) + expect(readRegistryIds()).toEqual(['com.test.valid']) + }) + + it('removes a package that has no signature.json', () => { + const dir = installFixture('com.test.unsigned', { omitSignature: true }) + writeRegistry({ 'com.test.unsigned': { path: dir } }) + + const warn = jest.fn() + const removed = new PackageManagerModule().verifyInstalledSignatures(warn) + + expect(removed).toEqual(['com.test.unsigned']) + expect(warn).toHaveBeenCalledWith(expect.stringContaining('com.test.unsigned')) + expect(existsSync(dir)).toBe(false) + expect(readRegistryIds()).toEqual([]) + }) + + it('removes a package whose signature names an untrusted key', () => { + // keyId is not present in TRUSTED_PACKAGE_KEYS. + const dir = installFixture('com.test.selfsigned', { payloadOverride: { keyId: 'untrusted-key' } }) + writeRegistry({ 'com.test.selfsigned': { path: dir } }) + + const removed = new PackageManagerModule().verifyInstalledSignatures(jest.fn()) + + expect(removed).toEqual(['com.test.selfsigned']) + expect(existsSync(dir)).toBe(false) + }) + + it('removes a package whose files were tampered after signing', () => { + const dir = installFixture('com.test.tampered', { + tamperFile: { rel: 'hal/arduino/hal.cpp', content: 'void hardwareInit() { /* altered */ }' }, + }) + writeRegistry({ 'com.test.tampered': { path: dir } }) + + const removed = new PackageManagerModule().verifyInstalledSignatures(jest.fn()) + + expect(removed).toEqual(['com.test.tampered']) + expect(existsSync(dir)).toBe(false) + }) + + it('drops a stale registry entry whose directory is missing', () => { + writeRegistry({ 'com.test.ghost': { path: join(packagesDir, 'com.test.ghost') } }) + + const removed = new PackageManagerModule().verifyInstalledSignatures(jest.fn()) + + expect(removed).toEqual(['com.test.ghost']) + expect(readRegistryIds()).toEqual([]) + }) + + it('de-lists an out-of-tree registry path without touching disk', () => { + // Registry path outside packagesDir: the entry is dropped and files there + // are left untouched. + const outside = mkdtempSync(join(tmpdir(), 'outside-')) + writeFileSync(join(outside, 'keepme.txt'), 'data') + writeRegistry({ 'com.test.escape': { path: outside } }) + + const removed = new PackageManagerModule().verifyInstalledSignatures(jest.fn()) + + expect(removed).toEqual(['com.test.escape']) + expect(readRegistryIds()).toEqual([]) + expect(existsSync(join(outside, 'keepme.txt'))).toBe(true) + rmSync(outside, { recursive: true, force: true }) + }) + + it('removes only the invalid package in a mixed registry', () => { + const good = installFixture('com.test.good') + const bad = installFixture('com.test.bad', { omitSignature: true }) + writeRegistry({ 'com.test.good': { path: good }, 'com.test.bad': { path: bad } }) + + const removed = new PackageManagerModule().verifyInstalledSignatures(jest.fn()) + + expect(removed).toEqual(['com.test.bad']) + expect(existsSync(good)).toBe(true) + expect(existsSync(bad)).toBe(false) + expect(readRegistryIds()).toEqual(['com.test.good']) + }) +}) diff --git a/src/backend/editor/package-manager/package-manager-module.ts b/src/backend/editor/package-manager/package-manager-module.ts index 0a1cc0b6e..7319b5837 100644 --- a/src/backend/editor/package-manager/package-manager-module.ts +++ b/src/backend/editor/package-manager/package-manager-module.ts @@ -7,6 +7,7 @@ import { PackageManifestSchema } from '../../../middleware/shared/ports/package- import { validatePathId } from '../../shared/utils/path-safety' import { TRUSTED_PACKAGE_KEYS } from '../../shared/utils/vpp/trusted-keys' import { verifyPackageSignature } from '../../shared/utils/vpp/verify-package-signature' +import { logger } from '../services/logger-service' import { assertPathContained } from '../utils/path-containment' import type { ImportResult, InstalledPackage, PackageManifest, PackageRegistry } from './types' @@ -132,6 +133,73 @@ class PackageManagerModule { } } + /** + * Re-verify every registry-listed package against the trusted keys and remove + * any whose signature does not validate, returning the ids removed. For each + * failing entry the package directory is deleted (when its recorded path + * resolves inside packagesDir) and the registry entry is dropped, emitting a + * warning per removal. No-op when REQUIRE_SIGNATURE is false. Directories with + * no registry entry are not listed and are left as-is. + */ + verifyInstalledSignatures(warn: (message: string) => void = (m) => logger.warn(m)): string[] { + if (!REQUIRE_SIGNATURE) return [] + + const registry = this.readRegistry() + const removed: string[] = [] + let mutated = false + + for (const [packageId, info] of Object.entries(registry.packages)) { + const reason = this.signatureRejectionReason(packageId, info?.path) + if (!reason) continue + + // Drop the registry entry; delete on-disk contents only when the recorded + // path resolves inside packagesDir. + try { + assertPathContained(this.packagesDir, info.path, 'registry package path') + if (existsSync(info.path)) { + rmSync(info.path, { recursive: true, force: true }) + } + } catch { + // Out-of-tree or unusable path: leave disk untouched, just de-list. + } + + delete registry.packages[packageId] + mutated = true + removed.push(packageId) + warn(`VPP package "${packageId}" was removed at startup due to an invalid or missing signature: ${reason}`) + } + + if (mutated) this.writeRegistry(registry) + return removed + } + + /** + * Returns null when the installed package recorded at `packagePath` is + * genuinely signed by a trusted key, or a short human-readable reason when it + * is not (bad id shape, path escaping packagesDir, missing files, or any + * failure surfaced by `verifyPackageSignature`). + */ + private signatureRejectionReason(packageId: string, packagePath: string | undefined): string | null { + try { + validatePathId(packageId, 'registry package id') + } catch { + return 'invalid package id' + } + if (typeof packagePath !== 'string' || packagePath.length === 0) { + return 'registry entry has no package path' + } + try { + assertPathContained(this.packagesDir, packagePath, 'registry package path') + } catch { + return 'package path resolves outside the packages directory' + } + if (!existsSync(packagePath)) { + return 'package files are missing' + } + const verification = verifyPackageSignature(packagePath, TRUSTED_PACKAGE_KEYS) + return verification.valid ? null : (verification.error ?? 'invalid signature') + } + listInstalled(): InstalledPackage[] { const registry = this.readRegistry() return Object.entries(registry.packages).map(([packageId, info]) => ({ diff --git a/src/backend/shared/compile/__tests__/pipeline.test.ts b/src/backend/shared/compile/__tests__/pipeline.test.ts index 1c8cf9877..170260375 100644 --- a/src/backend/shared/compile/__tests__/pipeline.test.ts +++ b/src/backend/shared/compile/__tests__/pipeline.test.ts @@ -5,7 +5,7 @@ * methods. Each branch (simulator / runtime v4 / runtime v3 / * arduino-direct, with `compileOnly` variants for each) is exercised * here by mocking the port + the heavy shared dependencies - * (`runProgramBuildPipeline`, `XmlGenerator`). + * (`runProgramBuildPipeline`). * The actual content-authoring steps (defines, confs, composers) are * covered by their own unit tests; this suite focuses on the * orchestration — call ordering, branch dispatch, error propagation, @@ -21,9 +21,6 @@ import type { // Mocks for heavy shared deps. Use `jest.fn()` so individual tests // can override `.mockReturnValueOnce` / `.mockResolvedValueOnce`. -jest.mock('../../utils/PLC/xml-generator', () => ({ - XmlGenerator: jest.fn(), -})) jest.mock('../../library/program-build-pipeline', () => ({ runProgramBuildPipeline: jest.fn(), })) @@ -59,14 +56,12 @@ jest.mock('../steps/generate-confs', () => ({ })), })) -import { XmlGenerator } from '../../utils/PLC/xml-generator' import { runProgramBuildPipeline } from '../../library/program-build-pipeline' import { isStrucppCompatibleRuntime } from '../../firmware/runtime-version-gate' import { generateRuntimeConfs } from '../steps/generate-confs' import { runCompilePipeline, type RunCompilePipelineArgs, type PipelineProgressEvent } from '../pipeline' -const mockedXmlGen = XmlGenerator as jest.MockedFunction const mockedConfs = generateRuntimeConfs as jest.MockedFunction const mockedStrucpp = runProgramBuildPipeline as jest.MockedFunction const mockedVersionGate = isStrucppCompatibleRuntime as jest.MockedFunction @@ -78,7 +73,7 @@ const mockedVersionGate = isStrucppCompatibleRuntime as jest.MockedFunction = {}): jest.Mocked { return { computeMd5: jest.fn().mockResolvedValue('a'.repeat(32)), - transpileXmlToSt: jest.fn().mockResolvedValue({ ok: true, programSt: 'PROGRAM main\nEND_PROGRAM' }), + transpileToSt: jest.fn().mockResolvedValue({ ok: true, programSt: 'PROGRAM main\nEND_PROGRAM' }), installArduinoCore: jest.fn().mockResolvedValue({ ok: true }), installArduinoLib: jest.fn().mockResolvedValue({ ok: true }), compileArduino: jest.fn().mockResolvedValue({ ok: true, binary: new Uint8Array([1, 2, 3]) }), @@ -136,8 +131,6 @@ function captureEvents() { beforeEach(() => { jest.clearAllMocks() - // Default-mock: XML generation succeeds. - mockedXmlGen.mockReturnValue({ ok: true, data: '', message: 'ok' } as never) // Default-mock: strucpp succeeds with empty file map. mockedStrucpp.mockReturnValue({ success: true, @@ -166,14 +159,13 @@ describe('runCompilePipeline — simulator path', () => { expect(result.success).toBe(true) expect(result.binary).toBeInstanceOf(Uint8Array) expect(result.uploaded).toBe(false) - expect(port.transpileXmlToSt).toHaveBeenCalledTimes(1) - // The pipeline owns xml2st flag semantics: every strucpp target - // gets `xml2stArgs: ['--keep-structs']`. Regression guard for - // the editor/web STRUCT drift bug — see compiler-platform-port.ts - // comment. Future flags get added to this array, not to a - // typed boolean on the port (the port stays format-agnostic). - expect(port.transpileXmlToSt).toHaveBeenCalledWith( - expect.objectContaining({ xml2stArgs: ['--keep-structs'] }), + expect(port.transpileToSt).toHaveBeenCalledTimes(1) + // The pipeline hands the transpiler the (preprocessed) project IR and a + // log callback; the port impl owns xml2st-vs-JSON backend selection and any + // format-specific flags internally (see transpiler-mode.ts). The pipeline + // stays format-agnostic — it only passes { projectData }. + expect(port.transpileToSt).toHaveBeenCalledWith( + expect.objectContaining({ projectData: expect.anything() }), expect.any(Function), ) expect(port.compileArduino).toHaveBeenCalledTimes(1) @@ -272,9 +264,8 @@ describe('runCompilePipeline — blank FBD variable guard', () => { const result = await runCompilePipeline(makeArgs({ projectData }), port, emit) expect(result.success).toBe(false) - // Validation runs before XML generation / xml2st. - expect(mockedXmlGen).not.toHaveBeenCalled() - expect(port.transpileXmlToSt).not.toHaveBeenCalled() + // Validation runs before the transpile step. + expect(port.transpileToSt).not.toHaveBeenCalled() // The user-facing error names the POU and the kind of block. const validateError = events.find((e) => e.stage === 'validate' && e.level === 'error') expect(validateError?.message).toContain('POU "main"') @@ -771,19 +762,9 @@ describe('runCompilePipeline — boardEntry shape variants', () => { // --------------------------------------------------------------------------- describe('runCompilePipeline — failure propagation', () => { - it('returns success=false when XmlGenerator reports failure', async () => { - mockedXmlGen.mockReturnValueOnce({ ok: false, data: undefined, message: 'malformed pou' } as never) - const port = makePort() - const { events, emit } = captureEvents() - const result = await runCompilePipeline(makeArgs(), port, emit) - expect(result.success).toBe(false) - expect(events.some((e) => e.stage === 'xml' && /malformed pou/.test(e.message))).toBe(true) - expect(port.transpileXmlToSt).not.toHaveBeenCalled() - }) - - it('returns success=false when transpileXmlToSt reports failure', async () => { + it('returns success=false when transpileToSt reports failure', async () => { const port = makePort({ - transpileXmlToSt: jest.fn().mockResolvedValue({ + transpileToSt: jest.fn().mockResolvedValue({ ok: false, errors: [{ message: 'bad xml', line: 1, column: 1, severity: 'error' }], }), @@ -828,6 +809,17 @@ describe('runCompilePipeline — failure propagation', () => { expect(result.success).toBe(false) }) + it('returns success=false when a simulator build produces no .hex binary', async () => { + // Simulator targets require the .hex artefact in memory (the loader can't + // find it on disk). A compile that reports ok but omits `binary` must fail + // with a precise error rather than silently succeeding. + const port = makePort({ compileArduino: jest.fn().mockResolvedValue({ ok: true }) }) + const { events, emit } = captureEvents() + const result = await runCompilePipeline(makeArgs(), port, emit) + expect(result.success).toBe(false) + expect(events.some((e) => e.stage === 'arduino-compile' && /did not produce a \.hex/.test(e.message))).toBe(true) + }) + it('returns success=false when uploadRuntimeV4 reports failure', async () => { const port = makePort({ uploadRuntimeV4: jest.fn().mockResolvedValue({ ok: false, errors: [] }), @@ -945,7 +937,7 @@ describe('runCompilePipeline — side effects', () => { it('emits per-error events with structured compileError payloads on transpile failure', async () => { const port = makePort({ - transpileXmlToSt: jest.fn().mockResolvedValue({ + transpileToSt: jest.fn().mockResolvedValue({ ok: false, errors: [ { message: 'bad syntax', line: 5, column: 3, severity: 'error' }, @@ -1048,7 +1040,7 @@ describe('runCompilePipeline — side effects', () => { // ports with `vi.fn()` never invoke the callback, leaving the // lambda body uncovered — this test pins the wiring explicitly. const port = makePort({ - transpileXmlToSt: jest.fn().mockImplementation(async (_args, log) => { + transpileToSt: jest.fn().mockImplementation(async (_args, log) => { log('xml2st spawned subprocess', 'info') log('xml2st: parsed 5 POUs', 'info') return { ok: true, programSt: 'PROGRAM main\nEND_PROGRAM' } diff --git a/src/backend/shared/library/__tests__/build-pipeline.test.ts b/src/backend/shared/library/__tests__/build-pipeline.test.ts index 1222dd6b0..fa0f37d12 100644 --- a/src/backend/shared/library/__tests__/build-pipeline.test.ts +++ b/src/backend/shared/library/__tests__/build-pipeline.test.ts @@ -1,9 +1,11 @@ /** * Tests for the library build pipeline. * - * The XmlGenerator is mocked because it depends on the frontend - * xml-generator helpers; we exercise the orchestration here, not - * actual XML serialisation (covered by xml-generator's own tests). + * `prepareXmlForLibraryBuild` no longer generates PLCopen XML — the + * old xml2st flow was replaced by an in-process JSON → ST transpiler. + * The function now only validates the manifest and returns the stubbed + * project data (plus the POU inventory the splitter needs); the actual + * transpile happens later via `LibraryBuildPort.transpileToSt`. * Strucpp is mocked via the runtime's test escape hatch — the build * pipeline must remain pure (no real strucpp load) for these tests. */ @@ -15,12 +17,6 @@ import type { StrucppRuntime } from '../strucpp-runtime' // Mocks // --------------------------------------------------------------------------- -const mockXmlGenerator = jest.fn() -jest.mock('../../utils/PLC/xml-generator', () => ({ - XmlGenerator: (...args: unknown[]) => mockXmlGenerator(...args), -})) - -// Import after mocks import { __setStrucppRuntimeForTests } from '../strucpp-runtime' import { __TESTING__, @@ -289,7 +285,6 @@ describe('prepareXmlForLibraryBuild', () => { expect('error' in result).toBe(true) if (!('error' in result)) return expect(result.error).toContain('library.json is invalid') - expect(mockXmlGenerator).not.toHaveBeenCalled() }) it('formats multi-line error reports with one bullet per validation issue', () => { @@ -299,35 +294,17 @@ describe('prepareXmlForLibraryBuild', () => { expect(bulletCount).toBeGreaterThanOrEqual(3) }) - it('returns a structured error when XML generation fails', () => { - mockXmlGenerator.mockReturnValue({ ok: false, message: 'no main pou', data: undefined }) - const result = prepareXmlForLibraryBuild(makeLibraryProject(), VALID_MANIFEST_JSON) - expect('error' in result).toBe(true) - if (!('error' in result)) return - expect(result.error).toContain('no main pou') - }) - - it('falls back to "unknown error" when XmlGenerator omits a message', () => { - mockXmlGenerator.mockReturnValue({ ok: false, data: undefined }) + it('returns stubbed projectData + knownPous (including stub) + parsed manifest on success', () => { const result = prepareXmlForLibraryBuild(makeLibraryProject(), VALID_MANIFEST_JSON) - if (!('error' in result)) throw new Error('expected error') - expect(result.error).toContain('unknown error') - }) - - it('treats ok=true but empty data as an error', () => { - mockXmlGenerator.mockReturnValue({ ok: true, message: 'XML generated', data: '' }) - const result = prepareXmlForLibraryBuild(makeLibraryProject(), VALID_MANIFEST_JSON) - expect('error' in result).toBe(true) - }) - - it('returns xml + knownPous (including stub) + parsed manifest on success', () => { - mockXmlGenerator.mockReturnValue({ ok: true, message: 'XML generated', data: '' }) - const result = prepareXmlForLibraryBuild(makeLibraryProject(), VALID_MANIFEST_JSON) - expect('xml' in result).toBe(true) - if (!('xml' in result)) return - expect(result.xml).toBe('') + // `error` is the union discriminant — its absence means success. + expect('error' in result).toBe(false) + if ('error' in result) return expect(result.manifest.name).toBe('demo_lib') + // The stubbed project carries the library's POUs plus the + // synthesised `main` program the transpiler requires. + expect(result.projectData.pous.map((p) => p.data.name)).toEqual(['TankController', STUB.STUB_PROGRAM_NAME]) + // POUs from the project + the stub program const names = result.knownPous.map((p) => p.name) expect(names).toEqual(['TankController', STUB.STUB_PROGRAM_NAME]) @@ -337,7 +314,6 @@ describe('prepareXmlForLibraryBuild', () => { }) it('maps each POU type to the correct splitter kind', () => { - mockXmlGenerator.mockReturnValue({ ok: true, data: '' }) const project = makeLibraryProject({ pous: [ { @@ -364,7 +340,7 @@ describe('prepareXmlForLibraryBuild', () => { ], }) const result = prepareXmlForLibraryBuild(project, VALID_MANIFEST_JSON) - if (!('knownPous' in result)) throw new Error('expected success') + if ('error' in result) throw new Error('expected success') const byName = Object.fromEntries(result.knownPous.map((p) => [p.name, p.kind])) expect(byName).toEqual({ Add2: 'FUNCTION', Tank: 'FUNCTION_BLOCK', main: 'PROGRAM' }) }) diff --git a/src/backend/shared/library/__tests__/library-build-orchestrator.test.ts b/src/backend/shared/library/__tests__/library-build-orchestrator.test.ts index 0e2073c8c..4823c0a5a 100644 --- a/src/backend/shared/library/__tests__/library-build-orchestrator.test.ts +++ b/src/backend/shared/library/__tests__/library-build-orchestrator.test.ts @@ -14,8 +14,13 @@ */ import type { LibraryBuildPort, VerifyCompileArgs } from '../../../../middleware/shared/ports/library-build-port' +import type { TranspileToStArgs, TranspileToStResult } from '../../../../middleware/shared/ports/compiler-platform-port' import type { PLCProjectData } from '../../types/PLC/open-plc' +// ST the fake `transpileToSt` port method emits. Fixed content so the +// verification-cache tests can precompute the harness MD5 off it. +const FAKE_PROGRAM_ST = 'PROGRAM main\n(* transpiled *)\nEND_PROGRAM\n' + // --------------------------------------------------------------------------- // Mocks for the inner shared helpers // --------------------------------------------------------------------------- @@ -48,6 +53,8 @@ interface PortHarness { missing: string[] verifyResult: { success: boolean; message?: string } verifyCalls: VerifyCompileArgs[] + transpileResult: TranspileToStResult + transpileCalls: TranspileToStArgs[] /** Programmable error for whichever method the test wants to fail. */ throwOn: Partial> } @@ -61,6 +68,8 @@ function makePort(): PortHarness { missing: [], verifyResult: { success: true }, verifyCalls: [], + transpileResult: { ok: true, programSt: FAKE_PROGRAM_ST }, + transpileCalls: [], throwOn: {}, } harness.port = { @@ -70,9 +79,12 @@ function makePort(): PortHarness { // distinguishable. return `md5-${input.length}-${input.charCodeAt(0) ?? 0}` }, - async transpileXmlToSt({ xml }) { - if (harness.throwOn.transpileXmlToSt) throw harness.throwOn.transpileXmlToSt - return { ok: true, programSt: `PROGRAM main\n(* from xml: ${xml.length} bytes *)\nEND_PROGRAM\n` } + async transpileToSt(args: TranspileToStArgs, log) { + if (harness.throwOn.transpileToSt) throw harness.throwOn.transpileToSt + harness.transpileCalls.push(args) + // Exercise the orchestrator's log-forwarding lambda. + log('transpiler: parsing project IR', 'info') + return harness.transpileResult }, async readBuildFile(_projectPath: string, relPath: string) { if (harness.throwOn.readBuildFile) throw harness.throwOn.readBuildFile @@ -122,7 +134,7 @@ beforeEach(() => { mockComposeVerify.mockClear() mockPrepareXml.mockReturnValue({ - xml: '...', + projectData: projectDataEmpty(), knownPous: [], manifest: { name: 'lib', version: '0.1.0', namespace: 'lib', extra: {} }, }) @@ -155,22 +167,23 @@ describe('runLibraryBuildPipeline', () => { expect(harness.files.get('build/lib.stlib')).toMatch(/^\{[\s\S]+\}\n$/) // Verification cache persisted with the MD5 the orchestrator computed. expect(harness.files.has('build/.verify-cache-library.json')).toBe(true) - // Intermediates (plc.xml, program.st) live in memory only — the - // orchestrator does NOT persist them. See the path-constants - // comment in library-build-orchestrator.ts for the rationale. - expect(harness.files.has('build/library/src/plc.xml')).toBe(false) + // Intermediates (program.st) live in memory only — the ST is + // produced in-process by `transpileToSt` and never persisted. + // See the path-constants comment in library-build-orchestrator.ts. expect(harness.files.has('build/library/src/program.st')).toBe(false) // Stage messages flow through in order. expect(events.map((e) => e.message)).toEqual( expect.arrayContaining([ 'Starting library build...', 'Manifest OK — building "lib" v0.1.0.', - 'Compiling file plc.xml', + 'Transpiling project to Structured Text', 'Verifying with OpenPLC Simulator (avr-gcc)...', 'Compiling library archive...', 'Library built successfully: build/lib.stlib', ]), ) + // The stubbed projectData from Stage 1 is what the transpiler sees. + expect(harness.transpileCalls).toHaveLength(1) }) it('does not call deleteBuildSubtree (intermediates are no longer persisted)', async () => { @@ -247,7 +260,7 @@ describe('runLibraryBuildPipeline', () => { expect(aux.dependencyRefs).toEqual([{ name: 'oscat-basic', version: '1.0.0' }]) }) - it('aborts before xml2st when the project enables an unresolved library', async () => { + it('aborts before the strucpp compile when the project enables an unresolved library', async () => { const harness = makePort() harness.missing = ['ghost-lib'] const { events, emit } = captureEvents() @@ -277,9 +290,8 @@ describe('runLibraryBuildPipeline', () => { const harness = makePort() // Pre-seed the cache. computeMd5 in the harness is deterministic // off program.st length + first char; the orchestrator's value - // will match this when the same xml2st output replays. - const programSt = `PROGRAM main\n(* from xml: 24 bytes *)\nEND_PROGRAM\n` - const expectedMd5 = `md5-${programSt.length}-${programSt.charCodeAt(0)}` + // will match this when the same transpiler output replays. + const expectedMd5 = `md5-${FAKE_PROGRAM_ST.length}-${FAKE_PROGRAM_ST.charCodeAt(0)}` harness.files.set('build/.verify-cache-library.json', JSON.stringify({ md5: expectedMd5, success: true })) const { events, emit } = captureEvents() @@ -300,8 +312,7 @@ describe('runLibraryBuildPipeline', () => { it('cleanBuild forces a fresh verification regardless of cache', async () => { const harness = makePort() - const programSt = `PROGRAM main\n(* from xml: 24 bytes *)\nEND_PROGRAM\n` - const expectedMd5 = `md5-${programSt.length}-${programSt.charCodeAt(0)}` + const expectedMd5 = `md5-${FAKE_PROGRAM_ST.length}-${FAKE_PROGRAM_ST.charCodeAt(0)}` harness.files.set('build/.verify-cache-library.json', JSON.stringify({ md5: expectedMd5, success: true })) const { emit } = captureEvents() @@ -319,6 +330,52 @@ describe('runLibraryBuildPipeline', () => { expect(harness.verifyCalls).toHaveLength(1) }) + it('runs a fresh verification when the cache read throws', async () => { + const harness = makePort() + // Throw only on the cache read; the manifest read (Stage 0) must + // still succeed so we reach the cache-consult path. + const realRead = harness.port.readBuildFile.bind(harness.port) + harness.port.readBuildFile = async (projectPath, relPath) => { + if (relPath === 'build/.verify-cache-library.json') throw new Error('cache read blew up') + return realRead(projectPath, relPath) + } + const { emit } = captureEvents() + + await runLibraryBuildPipeline( + { + projectPath: '/project', + projectData: projectDataEmpty(), + verifyProjectData: projectDataEmpty(), + cleanBuild: false, + }, + harness.port, + emit, + ) + + // Cache read failed → treated as a miss → fresh verification runs. + expect(harness.verifyCalls).toHaveLength(1) + }) + + it('runs a fresh verification when the cached file is malformed JSON', async () => { + const harness = makePort() + harness.files.set('build/.verify-cache-library.json', '{ not valid json') + const { emit } = captureEvents() + + await runLibraryBuildPipeline( + { + projectPath: '/project', + projectData: projectDataEmpty(), + verifyProjectData: projectDataEmpty(), + cleanBuild: false, + }, + harness.port, + emit, + ) + + // Malformed cache → fall through to a real verification run. + expect(harness.verifyCalls).toHaveLength(1) + }) + it('surfaces a verification failure as a warning but still emits the .stlib', async () => { const harness = makePort() harness.verifyResult = { success: false, message: 'AVR ran out of flash' } @@ -418,4 +475,151 @@ describe('runLibraryBuildPipeline', () => { expect(mockLibraryBuild).not.toHaveBeenCalled() expect(harness.verifyCalls).toHaveLength(0) }) + + it('fails when reading library.json throws an IO error', async () => { + const harness = makePort() + harness.throwOn.readBuildFile = new Error('disk on fire') + const { emit } = captureEvents() + + const result = await runLibraryBuildPipeline( + { + projectPath: '/project', + projectData: projectDataEmpty(), + verifyProjectData: projectDataEmpty(), + cleanBuild: false, + }, + harness.port, + emit, + ) + + expect(result.success).toBe(false) + expect(result.error).toMatch(/Could not read library\.json: disk on fire/) + expect(mockPrepareXml).not.toHaveBeenCalled() + }) + + it('fails when the transpiler reports an error', async () => { + const harness = makePort() + harness.transpileResult = { + ok: false, + errors: [{ message: 'unexpected token in POU body', line: 1, column: 1, severity: 'error' }], + } + const { emit } = captureEvents() + + const result = await runLibraryBuildPipeline( + { + projectPath: '/project', + projectData: projectDataEmpty(), + verifyProjectData: projectDataEmpty(), + cleanBuild: false, + }, + harness.port, + emit, + ) + + expect(result.success).toBe(false) + expect(result.error).toMatch(/transpile-from-json failed: unexpected token in POU body/) + expect(result.libraryName).toBe('lib') + expect(mockLibraryBuild).not.toHaveBeenCalled() + expect(harness.verifyCalls).toHaveLength(0) + }) + + it('falls back to a generic message when the transpiler returns ok=true but no ST', async () => { + const harness = makePort() + // ok=true with an undefined programSt still short-circuits, and + // with no `errors[]` the orchestrator uses its default message. + harness.transpileResult = { ok: true, programSt: undefined } + const { emit } = captureEvents() + + const result = await runLibraryBuildPipeline( + { + projectPath: '/project', + projectData: projectDataEmpty(), + verifyProjectData: projectDataEmpty(), + cleanBuild: false, + }, + harness.port, + emit, + ) + + expect(result.success).toBe(false) + expect(result.error).toMatch(/transpile-from-json failed: transpile-from-json failed/) + }) + + it('treats a thrown verifyCompile as a failed (advisory) verification', async () => { + const harness = makePort() + // A non-Error throwable exercises the `String(error)` fallback in + // `formatError`. + harness.port.verifyCompile = async () => { + throw 'avr-gcc segfaulted' + } + const { events, emit } = captureEvents() + + const result = await runLibraryBuildPipeline( + { + projectPath: '/project', + projectData: projectDataEmpty(), + verifyProjectData: projectDataEmpty(), + cleanBuild: false, + }, + harness.port, + emit, + ) + + // Verification failures are advisory — the build still succeeds. + expect(result.success).toBe(true) + expect(result.verification?.success).toBe(false) + expect(result.verification?.message).toBe('avr-gcc segfaulted') + expect(events.some((e) => e.level === 'warning' && /Verification reported issues/.test(e.message))).toBe(true) + }) + + it('warns but still ships the .stlib when the verification cache cannot be written', async () => { + const harness = makePort() + // Fail only the cache write; the .stlib write happens later and + // must still succeed. + const realWrite = harness.port.writeBuildFile.bind(harness.port) + harness.port.writeBuildFile = async (projectPath, relPath, content) => { + if (relPath === 'build/.verify-cache-library.json') throw new Error('cache dir read-only') + return realWrite(projectPath, relPath, content) + } + const { events, emit } = captureEvents() + + const result = await runLibraryBuildPipeline( + { + projectPath: '/project', + projectData: projectDataEmpty(), + verifyProjectData: projectDataEmpty(), + cleanBuild: false, + }, + harness.port, + emit, + ) + + expect(result.success).toBe(true) + expect(harness.files.has('build/lib.stlib')).toBe(true) + expect(events.some((e) => e.level === 'warning' && /Could not write verification cache/.test(e.message))).toBe(true) + }) + + it('fails when the .stlib archive cannot be written', async () => { + const harness = makePort() + harness.port.writeBuildFile = async (_projectPath, relPath) => { + if (relPath === 'build/lib.stlib') throw new Error('out of disk space') + // Let the cache write succeed. + } + const { emit } = captureEvents() + + const result = await runLibraryBuildPipeline( + { + projectPath: '/project', + projectData: projectDataEmpty(), + verifyProjectData: projectDataEmpty(), + cleanBuild: false, + }, + harness.port, + emit, + ) + + expect(result.success).toBe(false) + expect(result.error).toMatch(/Could not write lib\.stlib: out of disk space/) + expect(result.libraryName).toBe('lib') + }) }) diff --git a/src/backend/shared/styles/globals.css b/src/backend/shared/styles/globals.css index f8b4bad6f..affb8c9ba 100644 --- a/src/backend/shared/styles/globals.css +++ b/src/backend/shared/styles/globals.css @@ -224,33 +224,18 @@ } } +/* Sidebar scrollbar: hidden from layout so the icon column stays centered in + the 56px rail. Native scrollbars/gutters reserve asymmetric space on one + edge (6px in Chromium, the full ~16px native width in Safari regardless of + the ::-webkit-scrollbar width) and shift every icon off-center. The rail + still scrolls via wheel/trackpad/keyboard. */ .sidebar-scroll { overflow-y: auto; - scrollbar-gutter: stable; - direction: rtl; -} - -.sidebar-scroll > * { - direction: ltr; + scrollbar-width: none; } .sidebar-scroll::-webkit-scrollbar { - width: 6px; - background-color: transparent; -} - -.sidebar-scroll::-webkit-scrollbar-thumb { - background-color: transparent; - border-radius: 4px; - transition: background-color 0.2s; -} - -.sidebar-scroll:hover::-webkit-scrollbar-thumb { - background-color: rgba(180, 208, 254, 0.3); -} - -.sidebar-scroll:hover::-webkit-scrollbar-thumb:hover { - background-color: rgba(180, 208, 254, 0.6); + display: none; } .apexcharts-yaxis-label tspan, @@ -442,21 +427,6 @@ width: 32px !important; height: 32px !important; } -/* Left-align the button COLUMNS (the container divs) so the squares hug the - left and aren't clipped by the scrollbar gutter — but DON'T touch the - buttons' own `items-center`, or the icon stops being centred inside each. */ -.nineties .w-14.bg-brand-dark .sidebar-scroll, -.nineties .w-14.bg-brand-dark div[class*='items-center'] { - align-items: flex-start !important; -} -.nineties .w-14.bg-brand-dark .sidebar-scroll { - padding-left: 2px !important; -} -/* Keep the rail's own scrollbar thin — the global chunky 16px bar otherwise - eats most of the 56px rail and crops the 42px buttons. */ -.nineties .w-14.bg-brand-dark .sidebar-scroll::-webkit-scrollbar { - width: 6px !important; -} /* Darken stroke-based custom rail icons (e.g. the Exit/back arrow, a pale brand-blue that disappeared on silver). */ .nineties .w-14.bg-brand-dark svg:not([class*='lucide']):not(.retro-icon) { diff --git a/src/backend/shared/types/PLC/open-plc.ts b/src/backend/shared/types/PLC/open-plc.ts index 15884479b..4a5fa1b57 100644 --- a/src/backend/shared/types/PLC/open-plc.ts +++ b/src/backend/shared/types/PLC/open-plc.ts @@ -140,15 +140,15 @@ const PLCVariableSchema = z.object({ value: z.string(), }), ]), + /** + * The variable's binding. Single-field model: holds EITHER an alias name + * (bound to a producer channel) OR a literal IEC address the user typed + * (`%QX0.0`). Empty = unlocated. Alias→address resolution happens at + * compile time; a manual literal is honoured verbatim. Legacy projects + * that carried a separate `alias` field are migrated on load (the alias + * name is folded into `location`). + */ location: z.string(), - /** Stable alias name the variable is bound to, when present. Looked - * up in the alias registry to refresh `location` whenever the - * underlying producer reassigns the address. Variable cells show - * `alias` when set, falling back to the raw `location` otherwise. - * When the alias goes missing from the registry, the variable is - * "orphaned" — last-known `location` is kept, the cell flags it - * for the user. */ - alias: z.string().optional(), initialValue: z.string().or(z.null()).optional(), documentation: z.string(), debug: z.boolean().optional(), diff --git a/src/backend/shared/utils/__tests__/parse-project-files.test.ts b/src/backend/shared/utils/__tests__/parse-project-files.test.ts index 1926d91de..c93a423f8 100644 --- a/src/backend/shared/utils/__tests__/parse-project-files.test.ts +++ b/src/backend/shared/utils/__tests__/parse-project-files.test.ts @@ -73,6 +73,68 @@ describe('parseProjectFiles — basic', () => { }) }) +// --------------------------------------------------------------------------- +// Legacy alias → single-field location migration (foldLegacyVariableAliases) +// --------------------------------------------------------------------------- + +describe('parseProjectFiles — legacy alias migration', () => { + it('folds a JSON POU variable with a non-empty alias into its location', () => { + // Old two-field model: an alias-bound variable stored the resolved + // %address in `location` and the alias name in `alias`. The single- + // field model keeps only the alias name in `location`. + const legacyPou = JSON.stringify({ + type: 'program', + data: { + name: 'Legacy', + variables: [ + { + name: 'sensor', + class: 'local', + type: { definition: 'base-type', value: 'BOOL' }, + location: '%IX0.0', + alias: 'flow_sensor', + documentation: '', + }, + ], + body: { language: 'st', value: '' }, + documentation: '', + }, + }) + const pouFiles: RawProjectFile[] = [{ relativePath: 'pous/programs/Legacy.json', content: legacyPou }] + const result = parseProjectFiles('/p', makeProjectJson(), makeDeviceConfig(), makePinMapping(), pouFiles, [], []) + const variable = result.projectData.pous[0].interface?.variables[0] + // Alias name folded into location; the `alias` field is gone. + expect(variable?.location).toBe('flow_sensor') + expect('alias' in (variable as unknown as Record)).toBe(false) + }) + + it('leaves a manual (empty-alias) JSON POU variable location untouched', () => { + const legacyPou = JSON.stringify({ + type: 'program', + data: { + name: 'Manual', + variables: [ + { + name: 'coil', + class: 'local', + type: { definition: 'base-type', value: 'BOOL' }, + location: '%QX0.0', + alias: '', + documentation: '', + }, + ], + body: { language: 'st', value: '' }, + documentation: '', + }, + }) + const pouFiles: RawProjectFile[] = [{ relativePath: 'pous/programs/Manual.json', content: legacyPou }] + const result = parseProjectFiles('/p', makeProjectJson(), makeDeviceConfig(), makePinMapping(), pouFiles, [], []) + const variable = result.projectData.pous[0].interface?.variables[0] + // Empty alias is not a binding — the manual %address is preserved. + expect(variable?.location).toBe('%QX0.0') + }) +}) + // --------------------------------------------------------------------------- // Line 79 — detectPouTypeFromPath: function-block and function detection // --------------------------------------------------------------------------- diff --git a/src/backend/shared/utils/parse-project-files.ts b/src/backend/shared/utils/parse-project-files.ts index 494923d67..646885ec5 100644 --- a/src/backend/shared/utils/parse-project-files.ts +++ b/src/backend/shared/utils/parse-project-files.ts @@ -222,6 +222,41 @@ function createFallbackPou(content: string, language: string, pouType: string, p * On parse failure, falls back to createFallbackPou which preserves * documentation, raw variable text, and body content. */ +/** + * Migrate legacy variables from the two-field (`location` + `alias`) model to + * the single-field model, where `location` holds the binding itself — the + * alias name for an alias-bound variable, a literal `%addr` for a manual one. + * + * Any object that carries BOTH a string `location` and a non-empty string + * `alias` is a legacy alias-bound PLCVariable: its alias name is folded into + * `location` and the `alias` field dropped. Objects with `alias` but no + * `location` (producer channels: pins, VPP entries, Modbus points, EtherCAT + * mappings) keep their alias untouched. Manual variables (empty alias) keep + * their literal `location`. + * + * Generic, idempotent deep walk: projects already in the single-field form + * (no `alias` on variables) pass through unchanged, so it is safe to run on + * every load. + */ +function foldLegacyVariableAliases(value: unknown): unknown { + if (Array.isArray(value)) return value.map(foldLegacyVariableAliases) + if (value && typeof value === 'object') { + const obj = value as Record + const isLegacyAliasBound = typeof obj.location === 'string' && typeof obj.alias === 'string' && obj.alias.length > 0 + const out: Record = {} + for (const [key, child] of Object.entries(obj)) { + if (isLegacyAliasBound && key === 'alias') continue // fold away + if (isLegacyAliasBound && key === 'location') { + out.location = obj.alias as string + continue + } + out[key] = foldLegacyVariableAliases(child) + } + return out + } + return value +} + function parsePouFile(file: RawProjectFile): (PLCPou & { variablesText?: string }) | null { const ext = file.relativePath.split('.').pop()?.toLowerCase() /* istanbul ignore if -- defensive: parseProjectFiles upstream only forwards files whose @@ -233,7 +268,7 @@ function parsePouFile(file: RawProjectFile): (PLCPou & { variablesText?: string // Legacy JSON format if (ext === 'json') { try { - const parsed = JSON.parse(file.content) as unknown + const parsed = foldLegacyVariableAliases(JSON.parse(file.content)) // JSON POUs may be in the old discriminated union format: { type, data } if (parsed && typeof parsed === 'object' && 'type' in parsed && 'data' in parsed) { const ipcPou = parsed as { type: string; data: Record } @@ -352,7 +387,7 @@ export function parseProjectFiles( // Parse and Zod-validate project.json (matches old backend safeParseProjectFile behavior) let project: { meta?: { name?: string; type?: string }; data?: Record } try { - const raw = projectJson ? (JSON.parse(projectJson) as unknown) : null + const raw = projectJson ? foldLegacyVariableAliases(JSON.parse(projectJson)) : null if (raw) { const result = PLCProjectSchema.safeParse(raw) if (result.success) { diff --git a/src/frontend/components/_atoms/location-warning-glyph/index.tsx b/src/frontend/components/_atoms/location-warning-glyph/index.tsx new file mode 100644 index 000000000..81ffe0d68 --- /dev/null +++ b/src/frontend/components/_atoms/location-warning-glyph/index.tsx @@ -0,0 +1,36 @@ +import { Tooltip, TooltipContent, TooltipProvider, TooltipTrigger } from '../tooltip' + +/** + * Amber warning triangle shown in a variable's location cell, with an + * explanatory tooltip rendered through the app's shared Radix `Tooltip` + * (same look as the sidebar / HAL help-icon tooltips). Used for both the + * orphaned-alias and the manual-address-conflict warnings. + * + * `pointer-events-auto` re-enables hover on the glyph: the display-mode cell + * that hosts it is `pointer-events-none`, so the trigger would otherwise never + * see the pointer. + */ +function LocationWarningGlyph({ label, tooltip }: { label: string; tooltip: string }) { + return ( + + + + + + + + + {tooltip} + + + + ) +} + +export { LocationWarningGlyph } diff --git a/src/frontend/components/_features/[workspace]/editor/device/configuration/board.tsx b/src/frontend/components/_features/[workspace]/editor/device/configuration/board.tsx index 6a88e64e5..ae528d485 100644 --- a/src/frontend/components/_features/[workspace]/editor/device/configuration/board.tsx +++ b/src/frontend/components/_features/[workspace]/editor/device/configuration/board.tsx @@ -173,15 +173,10 @@ const Board = memo(function () { handleDeviceValueAtFirstRender() }, []) - // Sync alias-bound variables whenever the target changes. Producers - // gate by capability, so switching boards activates / deactivates - // entire I/O sources and the variables bound to their aliases may - // need to refresh or orphan. The effect fires after setDeviceBoard - // commits, regardless of which code path triggered the switch - // (regular pick, Python-warning confirm, V4-features-warning confirm). - useEffect(() => { - useOpenPLCStore.getState().projectActions.syncVariableAliases() - }, [deviceBoard]) + // A target switch no longer needs to touch program variables: their + // `location` holds a stable alias name (resolved at compile) or a literal + // address — neither changes with the board. Producer address recompaction + // for the new target is handled by `setDeviceBoard → recalculateIecAddresses`. useEffect(() => { scrollToSelectedOption(deviceSelectRef, deviceSelectIsOpen) diff --git a/src/frontend/components/_features/[workspace]/editor/device/configuration/components/pin-mapping-table.tsx b/src/frontend/components/_features/[workspace]/editor/device/configuration/components/pin-mapping-table.tsx index bcdb5b45e..3206b61a9 100644 --- a/src/frontend/components/_features/[workspace]/editor/device/configuration/components/pin-mapping-table.tsx +++ b/src/frontend/components/_features/[workspace]/editor/device/configuration/components/pin-mapping-table.tsx @@ -85,10 +85,10 @@ const PinMappingTable = ({ pins, selectedRowId, handleRowClick }: PinMappingTabl return { ok: false, title: 'Alias already in use', message: 'Pin alias collides with another producer.' } } - // Phase 2 — cascade rename onto bound variables BEFORE - // mutating the pin, so the subsequent `syncVariableAliases()` - // sees variables pointing at the new alias and refreshes - // locations rather than orphaning them. + // Cascade the rename onto bound variables: any variable whose + // `location` holds the old alias name follows to the new one, so it + // stays located (resolved to the address at compile time) rather than + // orphaning. const oldAlias = currentPin?.alias ?? '' if (oldAlias) { useOpenPLCStore.getState().projectActions.renameAlias(oldAlias, value) @@ -98,12 +98,10 @@ const PinMappingTable = ({ pins, selectedRowId, handleRowClick }: PinMappingTabl const res = updatePin({ [columnId as keyof DevicePin]: value, }) - // Pin alias / address edits are producer mutations — refresh - // variables bound to the affected addresses so the table - // reflects the new bindings without a save/reload. - if (res?.ok && (columnId === 'alias' || columnId === 'address')) { - useOpenPLCStore.getState().projectActions.syncVariableAliases() - } + // No variable re-sync needed: variables bound to this pin hold its alias + // NAME (resolved at compile time), and the rename above already cascaded + // to them via `renameAlias`. Manual literal locations are honoured + // verbatim and are the user's responsibility. return res } diff --git a/src/frontend/components/_features/[workspace]/editor/device/configuration/vendor-screen/layouts/io-table-layout.tsx b/src/frontend/components/_features/[workspace]/editor/device/configuration/vendor-screen/layouts/io-table-layout.tsx index 998b4fd85..fc1b1dc9f 100644 --- a/src/frontend/components/_features/[workspace]/editor/device/configuration/vendor-screen/layouts/io-table-layout.tsx +++ b/src/frontend/components/_features/[workspace]/editor/device/configuration/vendor-screen/layouts/io-table-layout.tsx @@ -10,6 +10,7 @@ import { nextFreeAddress, validateAliasEdit, } from '@root/middleware/shared/utils/iec-address' +import { vppMemoryKey } from '@root/middleware/shared/utils/iec-address/registry' import { resolveTargetCapabilities } from '@root/middleware/shared/utils/target-capabilities' import { useCallback, useEffect, useMemo, useRef, useState } from 'react' @@ -20,6 +21,33 @@ type IoTableLayoutProps = { moduleSystem: ModuleSystem } +/** + * Alias cell with local state so the value commits on blur (focus change), + * not on every keystroke. Committing per keystroke fires the alias-rename + * cascade for every intermediate string while editing — e.g. clearing "flow" + * would cascade through "flo", "fl", "f" onto bound variables. Committing on + * blur means a single rename (old → final) reaches the store, and clearing the + * field to empty leaves bound variables orphaned rather than partially renamed. + */ +function AliasInputCell({ value, onCommit }: { value: string; onCommit: (next: string) => void }) { + const [local, setLocal] = useState(value) + useEffect(() => { + setLocal(value) + }, [value]) + return ( + setLocal(e.target.value)} + onBlur={() => { + if (local !== value) onCommit(local) + }} + placeholder='Alias...' + className='h-[26px] w-full rounded border border-neutral-100 bg-white px-2 font-caption text-cp-sm text-neutral-850 outline-none placeholder:text-neutral-400 focus:border-brand-medium-dark dark:border-neutral-800 dark:bg-neutral-950 dark:text-neutral-300 dark:placeholder:text-neutral-600' + /> + ) +} + function IoTableLayout({ section, moduleSystem }: IoTableLayoutProps) { // See `getSectionPersistenceKey` in ../index.tsx — the single // source of truth for the per-section storage key. Falls back to @@ -142,11 +170,13 @@ function IoTableLayout({ section, moduleSystem }: IoTableLayoutProps) { } } - setEntries(newEntries) + // Write the freshly-derived channel structure, then let the central + // registry own the FINAL addresses (VPP + Modbus packed together, + // aliases restored from the session memory) and pull its result back + // into local state so the table renders the compacted addresses. setVendorScreenData(persistenceKey, { entries: newEntries }) - // Producer mutation: addresses were just re-allocated for every - // VPP-active slot. Refresh variables bound to those aliases. - useOpenPLCStore.getState().projectActions.syncVariableAliases() + useOpenPLCStore.getState().projectActions.recalculateIecAddresses() + setEntries(getStoreState().storedMapping?.entries ?? newEntries) // eslint-disable-next-line react-hooks/exhaustive-deps }, [slots, formatSelectionKey]) @@ -184,10 +214,9 @@ function IoTableLayout({ section, moduleSystem }: IoTableLayoutProps) { return } - // Phase 2 — cascade rename onto bound variables BEFORE writing - // so the subsequent `syncVariableAliases()` sees variables - // pointing at the new alias and takes the refresh path instead - // of orphan. + // Cascade the rename onto bound variables: any variable whose + // `location` holds the old alias name follows to the new one, so it + // stays located (resolved at compile time) rather than orphaning. const oldAlias = target.alias ?? '' if (oldAlias) { useOpenPLCStore.getState().projectActions.renameAlias(oldAlias, alias) @@ -197,8 +226,13 @@ function IoTableLayout({ section, moduleSystem }: IoTableLayoutProps) { updated[index] = { ...updated[index], alias } setEntries(updated) setVendorScreenData(persistenceKey, { entries: updated }) - // Refresh variables against any allocator-driven address shifts. - useOpenPLCStore.getState().projectActions.syncVariableAliases() + // Record in the session alias-memory so the alias returns if this module + // is removed and re-added on the same slot within the session. + useOpenPLCStore + .getState() + .projectActions.rememberChannelAlias(vppMemoryKey(target.moduleId ?? '', target.slot, target.channelName), alias) + // Variables bound to this channel hold its alias NAME (resolved at + // compile); the `renameAlias` above already cascaded any rename to them. } const groups = useMemo(() => { @@ -328,12 +362,9 @@ function IoTableLayout({ section, moduleSystem }: IoTableLayoutProps) { {entry.iecAddress} - handleAliasChange(entry.globalIndex, e.target.value)} - placeholder='Alias...' - className='h-[26px] w-full rounded border border-neutral-100 bg-white px-2 font-caption text-cp-sm text-neutral-850 outline-none placeholder:text-neutral-400 focus:border-brand-medium-dark dark:border-neutral-800 dark:bg-neutral-950 dark:text-neutral-300 dark:placeholder:text-neutral-600' + handleAliasChange(entry.globalIndex, next)} /> diff --git a/src/frontend/components/_features/[workspace]/editor/device/configuration/vendor-screen/layouts/module-slots-layout.tsx b/src/frontend/components/_features/[workspace]/editor/device/configuration/vendor-screen/layouts/module-slots-layout.tsx index c81b292a4..36008314e 100644 --- a/src/frontend/components/_features/[workspace]/editor/device/configuration/vendor-screen/layouts/module-slots-layout.tsx +++ b/src/frontend/components/_features/[workspace]/editor/device/configuration/vendor-screen/layouts/module-slots-layout.tsx @@ -23,6 +23,7 @@ import { nextFreeAddress, validateAliasEdit, } from '@root/middleware/shared/utils/iec-address' +import { vppMemoryKey } from '@root/middleware/shared/utils/iec-address/registry' import { resolveTargetCapabilities } from '@root/middleware/shared/utils/target-capabilities' import { useCallback, useEffect, useMemo, useRef, useState } from 'react' @@ -87,6 +88,33 @@ type ConfigScreenDefinition = { // Walk a screen definition and return every field that contributes // to the configuration form. The screen JSON is a vendor artifact — // be tolerant of missing/oddly-shaped fragments rather than crash. +/** + * Alias cell with local state so the value commits on blur (focus change), + * not on every keystroke. Committing per keystroke fires the alias-rename + * cascade for every intermediate string while editing — e.g. clearing "flow" + * would cascade through "flo", "fl", "f" onto bound variables. Committing on + * blur means a single rename (old → final) reaches the store, and clearing the + * field to empty leaves bound variables orphaned rather than partially renamed. + */ +function AliasInputCell({ value, onCommit }: { value: string; onCommit: (next: string) => void }) { + const [local, setLocal] = useState(value) + useEffect(() => { + setLocal(value) + }, [value]) + return ( + setLocal(e.target.value)} + onBlur={() => { + if (local !== value) onCommit(local) + }} + placeholder='Alias...' + className='h-[26px] w-full rounded border border-neutral-100 bg-white px-2 font-caption text-cp-sm text-neutral-850 outline-none placeholder:text-neutral-400 focus:border-brand-medium-dark dark:border-neutral-800 dark:bg-neutral-950 dark:text-neutral-300 dark:placeholder:text-neutral-600' + /> + ) +} + function collectConfigFields(def: ConfigScreenDefinition | undefined | null): ConfigFieldDef[] { if (!def?.sections) return [] const out: ConfigFieldDef[] = [] @@ -467,10 +495,12 @@ function ModuleSlotsLayout({ section, moduleSystem }: ModuleSlotsLayoutProps) { } } + // Write the derived channel structure, then let the central registry own + // the final addresses (VPP + Modbus packed together, aliases restored + // from the session memory) and reconcile variables. This layout renders + // from the store, so the registry's write-back propagates automatically. setVendorScreenData('io-mapping', { entries: newEntries }) - // Producer mutation: every VPP slot just had its addresses - // re-allocated. Sync variables that were bound to those aliases. - useOpenPLCStore.getState().projectActions.syncVariableAliases() + useOpenPLCStore.getState().projectActions.recalculateIecAddresses() // eslint-disable-next-line react-hooks/exhaustive-deps }, [slots, formatSelectionKey]) @@ -644,10 +674,10 @@ function ModuleSlotsLayout({ section, moduleSystem }: ModuleSlotsLayoutProps) { // from the live state (including the entry being edited, scoped // to the active board's capabilities) and reject the edit if the // new alias is already claimed by a different channel. Without - // this gate, the pool's silent first-wins reservation would - // cause every variable that the user later binds to the losing - // entry to collapse to the winner's address through - // `syncVariableAliases`'s refresh path. + // this gate, the pool's silent first-wins reservation would make + // the losing entry's alias unresolvable, so every variable the + // user later binds to it would silently become unlocated at + // compile time. const boardInfo = state.deviceAvailableOptions.availableBoards.get( state.deviceDefinitions.configuration.deviceBoard ?? '', ) @@ -673,21 +703,25 @@ function ModuleSlotsLayout({ section, moduleSystem }: ModuleSlotsLayoutProps) { return } - // Phase 2 — cascade rename onto bound variables BEFORE writing - // the new entries, so the subsequent `syncVariableAliases()` - // call sees variables already pointing at the new alias name and - // takes the refresh path (location follows alias) instead of the - // orphan path (location cleared, warning glyph rendered). - const oldAlias = currentEntries.find((e) => e.slot === slot && e.channelName === channelName)?.alias ?? '' + // Cascade the rename onto bound variables: any variable whose + // `location` holds the old alias name follows to the new one + // (location follows alias), keeping it located instead of orphaning + // it (location cleared, warning glyph rendered). + const targetEntry = currentEntries.find((e) => e.slot === slot && e.channelName === channelName) + const oldAlias = targetEntry?.alias ?? '' if (oldAlias) { useOpenPLCStore.getState().projectActions.renameAlias(oldAlias, alias) } const entries = currentEntries.map((e) => (e.slot === slot && e.channelName === channelName ? { ...e, alias } : e)) setVendorScreenData('io-mapping', { entries }) - // Refresh variables bound to the (now-renamed) alias against - // any address shifts produced by the change. - useOpenPLCStore.getState().projectActions.syncVariableAliases() + // Record in the session alias-memory so the alias returns if this module + // is removed and re-added on the same slot within the session. + useOpenPLCStore + .getState() + .projectActions.rememberChannelAlias(vppMemoryKey(targetEntry?.moduleId ?? '', slot, channelName), alias) + // Variables bound to this channel hold its alias NAME (resolved at + // compile); the `renameAlias` above already cascaded any rename to them. } /** @@ -986,12 +1020,9 @@ function ModuleSlotsLayout({ section, moduleSystem }: ModuleSlotsLayoutProps) { {entry.iecAddress} - handleAliasChange(entry.slot, entry.channelName, e.target.value)} - placeholder='Alias...' - className='h-[26px] w-full rounded border border-neutral-100 bg-white px-2 font-caption text-cp-sm text-neutral-850 outline-none placeholder:text-neutral-400 focus:border-brand-medium-dark dark:border-neutral-800 dark:bg-neutral-950 dark:text-neutral-300 dark:placeholder:text-neutral-600' + handleAliasChange(entry.slot, entry.channelName, next)} /> diff --git a/src/frontend/components/_features/[workspace]/editor/device/ethercat/ethercat-device-editor.tsx b/src/frontend/components/_features/[workspace]/editor/device/ethercat/ethercat-device-editor.tsx index 36f60b4d4..2877a856e 100644 --- a/src/frontend/components/_features/[workspace]/editor/device/ethercat/ethercat-device-editor.tsx +++ b/src/frontend/components/_features/[workspace]/editor/device/ethercat/ethercat-device-editor.tsx @@ -145,12 +145,9 @@ const EtherCATDeviceEditor = ({ busName: propBusName, deviceId: propDeviceId }: const syncDevicesToStore = useCallback( (devices: ConfiguredEtherCATDevice[]) => { + // `updateEthercatConfig` reallocates addresses through the central + // registry and cascades any alias rename onto bound variables' names. projectActions.updateEthercatConfig(busName, { masterConfig, devices }) - // Producer mutation: any change to channelMappings or aliases - // may move addresses or attach/detach aliases. Refresh the - // variables bound to those aliases so the table reflects the - // new bindings without waiting for save/reload. - projectActions.syncVariableAliases() // Mark the slave file dirty (same pattern as other file types) const { sharedWorkspaceActions } = useOpenPLCStore.getState() if (deviceName) { diff --git a/src/frontend/components/_features/[workspace]/editor/device/remote-device/index.tsx b/src/frontend/components/_features/[workspace]/editor/device/remote-device/index.tsx index 44f087fa1..3000d5585 100644 --- a/src/frontend/components/_features/[workspace]/editor/device/remote-device/index.tsx +++ b/src/frontend/components/_features/[workspace]/editor/device/remote-device/index.tsx @@ -1,10 +1,10 @@ +import { Pencil1Icon, TrashIcon } from '@radix-ui/react-icons' import type { ModbusIOGroup, ModbusIOPoint } from '@root/middleware/shared/ports/types' import { useRuntime } from '@root/middleware/shared/providers/platform-context' import { useCallback, useEffect, useMemo, useState } from 'react' import { v4 as uuidv4 } from 'uuid' import { ArrowIcon } from '../../../../../../assets/icons/interface/Arrow' -import { MinusIcon } from '../../../../../../assets/icons/interface/Minus' import { PlusIcon } from '../../../../../../assets/icons/interface/Plus' import { useOpenPLCStore } from '../../../../../../store' import { cn } from '../../../../../../utils/cn' @@ -348,21 +348,12 @@ type IOGroupRowProps = { ioGroup: ModbusIOGroup isExpanded: boolean onToggleExpand: () => void - isSelected: boolean - onSelect: () => void onEdit: () => void + onDelete: () => void onUpdateAlias: (ioPointId: string, alias: string) => void } -const IOGroupRow = ({ - ioGroup, - isExpanded, - onToggleExpand, - isSelected, - onSelect, - onEdit, - onUpdateAlias, -}: IOGroupRowProps) => { +const IOGroupRow = ({ ioGroup, isExpanded, onToggleExpand, onEdit, onDelete, onUpdateAlias }: IOGroupRowProps) => { const firstIOPoint = ioGroup.ioPoints?.[0] const groupType = firstIOPoint?.type || '-' const groupAddress = firstIOPoint?.iecLocation || '-' @@ -370,22 +361,9 @@ const IOGroupRow = ({ return ( <> - + - + + + {isExpanded && (ioGroup.ioPoints ?? []).map((ioPoint: ModbusIOPoint, index: number) => ( @@ -446,6 +446,7 @@ const IOPointRow = ({ ioPoint, offset, onUpdateAlias }: IOPointRowProps) => { className='h-6 w-full rounded border border-neutral-200 bg-white px-1 text-xs text-neutral-700 outline-none focus:border-brand dark:border-neutral-700 dark:bg-neutral-900 dark:text-neutral-300' /> + ) } @@ -490,7 +491,6 @@ const RemoteDeviceEditor = () => { // UI state const [expandedGroups, setExpandedGroups] = useState>(new Set()) - const [selectedGroupId, setSelectedGroupId] = useState(null) const [isModalOpen, setIsModalOpen] = useState(false) const [editingGroup, setEditingGroup] = useState(null) @@ -737,13 +737,13 @@ const RemoteDeviceEditor = () => { [deviceName, projectActions, editingGroup, sharedWorkspaceActions], ) - const handleDeleteIOGroup = useCallback(() => { - if (selectedGroupId) { - projectActions.deleteIOGroup(deviceName, selectedGroupId) - setSelectedGroupId(null) + const handleDeleteIOGroup = useCallback( + (groupId: string) => { + projectActions.deleteIOGroup(deviceName, groupId) sharedWorkspaceActions.handleFileAndWorkspaceSavedState(deviceName) - } - }, [deviceName, selectedGroupId, projectActions, sharedWorkspaceActions]) + }, + [deviceName, projectActions, sharedWorkspaceActions], + ) const handleUpdateAlias = useCallback( (ioGroupId: string, ioPointId: string, alias: string) => { @@ -951,13 +951,6 @@ const RemoteDeviceEditor = () => { icon: , id: 'add-io-group-button', }, - { - ariaLabel: 'Remove IO Group', - onClick: handleDeleteIOGroup, - disabled: !selectedGroupId, - icon: , - id: 'remove-io-group-button', - }, ]} buttonProps={{ className: @@ -983,18 +976,21 @@ const RemoteDeviceEditor = () => { Offset - + Function Code Alias + + Actions + {ioGroups.length === 0 ? ( - + No IO groups configured. Click the + button to add one. @@ -1005,9 +1001,8 @@ const RemoteDeviceEditor = () => { ioGroup={ioGroup} isExpanded={expandedGroups.has(ioGroup.id)} onToggleExpand={() => handleToggleExpand(ioGroup.id)} - isSelected={selectedGroupId === ioGroup.id} - onSelect={() => setSelectedGroupId(ioGroup.id)} onEdit={() => handleOpenEditModal(ioGroup.id)} + onDelete={() => handleDeleteIOGroup(ioGroup.id)} onUpdateAlias={(ioPointId, alias) => handleUpdateAlias(ioGroup.id, ioPointId, alias)} /> )) diff --git a/src/frontend/components/_features/[workspace]/editor/monaco/index.tsx b/src/frontend/components/_features/[workspace]/editor/monaco/index.tsx index b8b939be7..285456ca4 100644 --- a/src/frontend/components/_features/[workspace]/editor/monaco/index.tsx +++ b/src/frontend/components/_features/[workspace]/editor/monaco/index.tsx @@ -1245,6 +1245,17 @@ void loop() minimap: { enabled: false }, dropIntoEditor: { enabled: true }, readOnly: isDebuggerVisible, + // Force Monaco's classic hidden-