feat: one device connection over serial or Modbus TCP, with run/stop

[marconetsf]

Segregates the connection work that the VPP-licensing branch was carrying as an
implicit dependency, so it can land on development on its own. No licensable
VPP code is in here, and nothing in it requires a change to openplc-packages:
it works with the VPPs published today.

Why draft

Paired with openplc-web #641,
which mirrors the 56 shared-surface files byte-identical (0 of 56 differ). The two
have to merge together: either one alone leaves the other repo's development
carrying a different surface, which is the drift the sync gate exists to catch.

Marking both ready at the same time.

Shared-surface files mirrored (56)

Everything under src/frontend/, src/middleware/shared/, src/backend/shared/
and src/__architecture__/ in this diff. git diff --name-only development | grep -E '^src/(frontend|middleware/shared|backend/shared|__architecture__)/'

What lands

Always-on debugger. The baremetal firmware keeps the serial debug protocol
compiled in unconditionally, so there is always a way in — even with Modbus
disabled entirely. Previously that configuration reported "Modbus Required" and
the board was unreachable.

Modbus layer split. ModbusSlave becomes twelve per-entity modules
(modbus_config/crc/debug/frame/pdu/registers/serial/tcp/types) with an
ARCHITECTURE.md. Modbus RTU can run on a secondary UART while the debugger
keeps the default port.

One device connection. DeviceLinkManager owns THE connection: it takes the
ordered candidates resolved from the board's debug spec, tries them in order, and
keeps the first that both opens and verifies. Every command — debug reads and
writes, run/stop, md5, the status poll — borrows that one client; none of them can
open a connection.

That is what fixes run/stop over Modbus TCP: the command path used to recognise
only an RTU client as reusable, so a tcp stop opened a transient SECOND socket,
which an Arduino Modbus TCP server (one client at a time) never answers.

Run/stop. FC 0x4b rides the same link. Reads come from the status frame
(FC 0x46), which already carries the run/stop state and the mode-switch
position — so a switch flipped by hand shows up within one poll interval, with no
second timer and no extra traffic. A RUN request is refused, not queued, while the
switch reads STOP.

Session structure. Two channel slots (control + debug), lazily opened, so a
debug session shares the link rather than replacing it. Runtime v3/v4 come into
the same session; the runtime-v4 WebSocket keeps its own client, being a different
protocol to a different target.

Field-tested, and what it took

Validated on an ESP8266 NodeMCU at 9600 baud: flash, connect, run/stop, debug
session. Four defects surfaced doing that, each fixed with a regression test:

Symptom	Cause
.text1 will not fit in region iram1_0_seg — would not build	The precompile named archive objects foo.o. esp8266's linker sends code to flash by matching *.cpp.o; unmatched code falls into a catch-all mapped into the 32 KB IRAM. Measured 7387 of 32768 bytes of ordinary code running from IRAM. Pre-existing and board-independent; surfaced here because the run/stop glue grew the largest offender
"No Firmware Detected" on a healthy board	DEBUG_BAUD was read from the serial screen section alone. A VPP published today has no such section — it configures the baud on the RTU section — so the firmware listened at 115200 while the editor dialled the screen's rate
Debug session died every ~10s	The liveness read queued behind the debug poll on the one serial link and timed out; two timeouts entered recovery and tore down a session whose own reads were succeeding. A successful command IS liveness evidence, so the poll now skips its round trip
~8 identical log lines per second	The channel-use trace fired on every acquisition. Now logged once per distinct command+channel+endpoint

Also added, since a wrong baud is the one misconfiguration that looks like healthy
silence: Connect tries the configured rate first, then sweeps a short list of
fallbacks. Guesses are appended AFTER everything the project declared, get a short
probe budget, and the patient budget stays with the last configured endpoint —
where a just-flashed board still needs it.

Two things in here that are not features

• validate:arch resolved SRC_ROOT via URL.pathname, producing C:\C:\… on
  win32 — the check could not scan the tree at all. Pre-existing on development;
  without it the gate does not run on this branch.
• The object-naming fix above. Pre-existing, and independent of any board.

Compatibility with published VPPs

Verified against the real com.openplc.espressif manifest from
openplc-packages@development (no serialPorts, no defaultSerial, only the
legacy modbus_rtu/modbus_tcp screen sections): the generated defines are
correct for RTU on the default port, RTU on a secondary port, TCP static and
TCP Wi-Fi/DHCP; and candidate resolution works with RTU on, TCP on, both on, and
neither on — the last being the case the always-on debugger exists for.

The editor's manifest schema is untouched, and there are no references to the
split screen files. serialPorts / defaultSerial / optionsRef are all
optional with fallbacks.

Not in here

No license blob, no on-device license storage, no activation, no FC 0x48–0x4C
licensing set, no isLicensable/licenseStore capabilities, no purchase flow.
The device screen confirms a held link and nothing more.

Two commits about re-homing the Modbus screen under Servers and migrating
project data into new serial/network sections were deliberately left out —
they belong with the split-screen work, and the migration would have stripped
fields that no published VPP renders.

Checks

tsc --noEmit clean · validate:arch 0 violations · eslint 0 errors ·
prettier --check clean · jest 5611 passing.

Six failures remain in two suites (stats-table, board-info-resolver) and are
pre-existing — reproduced identically on origin/development. They are
environmental: a pt-BR locale formatting 1.234 where 1,234 is asserted, and
POSIX path assertions on win32.

🤖 Generated with Claude Code

Summary by CodeRabbit

• New Features

  • Added persistent device connections with automatic probing, fallback, recovery, and status reporting.
  • Added PLC Run/Stop controls, runtime status, board identity, and firmware version reporting.
  • Added clearer serial-port labels using address, board, and manufacturer information.
  • Added dynamic form options based on connected board details.
  • Added connection controls for runtime and serial targets, including firmware-flash prompts.
  • Improved debugger polling across supported connection types.

• Bug Fixes

  • Improved board validation, networking, and serial-interface handling.
  • Fixed precompiled library placement for ESP8266 builds.
  • Added clearer errors for unsupported or unavailable board configurations.

[coderabbitai]

Walkthrough

This PR adds managed device sessions with probing, recovery, persistent connection state, Modbus status and PLC control, unified IPC contracts, frontend connection flows, dynamic VPP options, and compiler updates.

Changes

Compiler, protocol, and configuration

Layer / File(s)	Summary
Compiler validation and VPP defines src/__architecture__/validate.ts, src/backend/editor/compiler/..., src/backend/shared/compile/...	Architecture validation now handles multiline imports and Windows paths. Precompiled objects retain .cpp.o names. Compiler defines resolve debugger serial settings and migrated VPP network settings.
Modbus status and PLC control src/backend/shared/debug/..., src/backend/editor/modbus/..., src/backend/shared/simulator/...	Shared PDUs and transport contracts support board identity, runtime status, firmware version, and RUN/STOP control across RTU, TCP, and simulator transports.

Device lifecycle and connection resolution

Layer / File(s)	Summary
Device probing and session recovery src/backend/editor/hardware/...	The session manager opens ordered candidates, verifies links, shares channels, polls liveness, performs bounded recovery, and reports connection status.
Candidate resolution and transport contracts src/backend/shared/hardware/..., src/middleware/shared/ports/..., src/frontend/services/...	Candidate resolution supports transport filtering, RTU fallback, deferred prompts, runtime debug channels, and persistent device-session contracts.

Application integration

Layer / File(s)	Summary
IPC and middleware lifecycle src/main/modules/ipc/..., src/middleware/adapters/...	IPC and adapters now use managed device sessions for connection, runtime sessions, debug channels, PLC control, serial handoff, and status subscriptions.
Frontend state and controls src/frontend/hooks/..., src/frontend/store/..., src/frontend/components/..., src/frontend/utils/...	Frontend state tracks device connections, PLC state, switch position, and debug media. Connection controls, serial labels, dynamic VPP options, flash events, and unified PLC dialogs were added.

Estimated code review effort: 5 (Critical) | ~120 minutes

Sequence Diagram(s)

sequenceDiagram
  participant WorkspaceUI
  participant DeviceAdapter
  participant MainIPC
  participant DeviceSessionManager
  participant ModbusTransport
  WorkspaceUI->>DeviceAdapter: connect candidates
  DeviceAdapter->>MainIPC: deviceConnect(candidates)
  MainIPC->>DeviceSessionManager: open candidates
  DeviceSessionManager->>ModbusTransport: connect and probe
  ModbusTransport-->>DeviceSessionManager: connection result
  DeviceSessionManager-->>MainIPC: connection status
  MainIPC-->>DeviceAdapter: result and events
  WorkspaceUI->>DeviceAdapter: set PLC state
  DeviceAdapter->>MainIPC: debuggerPlcControl(run or stop)
  MainIPC->>DeviceSessionManager: use active client
  DeviceSessionManager->>ModbusTransport: send PLC control PDU
  ModbusTransport-->>WorkspaceUI: PLC control result

Loading

Possibly related PRs

• Autonomy-Logic/openplc-editor#947: Both changes modify serial-port merging and descriptor handling.
• Autonomy-Logic/openplc-editor#936: Both changes modify architecture validation rules.
• Autonomy-Logic/openplc-editor#886: Both changes modify compiler and precompiled-object handling.

Suggested labels: feature

Suggested reviewers: thiagoralves, dcoutinho1328

Poem

A rabbit watches links connect,
While PLC states reflect.
Ports keep descriptors clear,
Recovery paths persevere.
RUN and STOP now share one flow.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 71.79% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly summarizes the primary change: one shared device connection over serial or Modbus TCP with run/stop support.
Description check	✅ Passed	The description thoroughly explains the scope, architecture, compatibility, validation results, known failures, and licensing exclusions.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

✨ Finishing Touches 💡 1

📝 Generate docstrings 💡

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch feat/baremetal-connection

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[coderabbitai]

Actionable comments posted: 18

Note

Due to the large number of review comments, Critical, Major severity comments were prioritized as inline comments.

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

src/middleware/shared/ports/debugger-port.ts (1)

62-67: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win

Remove the stale @param config from the doc comment.

Line 67 no longer accepts a connection config. The @param config line at Line 65 documents a parameter that does not exist, and editors will show it in the tooltip for every implementer.

🐛 Proposed fix

   /**
    * Verify that the running program matches the expected MD5 hash.
    * Used to detect program mismatch before starting a debug session.
-   * `@param` config — Connection target used for the verification request.
+   * `@param` expectedMd5 — Digest the compiled program is expected to have.
    */
   verifyMd5(expectedMd5: string): Promise<Md5VerifyResult>

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/middleware/shared/ports/debugger-port.ts` around lines 62 - 67, Remove
the stale `@param` config documentation from the verifyMd5 method comment, leaving
documentation only for the existing expectedMd5 parameter and the method’s
purpose.

🟡 Minor comments (12)

src/backend/shared/debug/__tests__/modbus-pdu-plc-control.test.ts-22-36 (1)

22-36: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Both uniqueness tests always pass and cannot detect a collision.

others is built by filtering out every numeric value equal to the target. A duplicate member sharing that value is filtered out too. expect(others).not.toContain(...) is therefore true by construction. The same defect applies to the REFUSED_BY_SWITCH test at lines 30-36.

These tests guard a wire-protocol contract, so the guard should actually fail on a collision. Count how many members hold the value instead.

🐛 Proposed fix that fails on a real collision

   it('claims 0x4b, clear of every other debug function code', () => {
     expect(ModbusFunctionCode.PLC_SET_STATE).toBe(0x4b)
-    const others = Object.values(ModbusFunctionCode).filter(
-      (value): value is number => typeof value === 'number' && value !== ModbusFunctionCode.PLC_SET_STATE,
-    )
-    expect(others).not.toContain(ModbusFunctionCode.PLC_SET_STATE)
+    const codes = Object.values(ModbusFunctionCode).filter((value): value is number => typeof value === 'number')
+    expect(codes.filter((value) => value === ModbusFunctionCode.PLC_SET_STATE)).toHaveLength(1)
+    // Nothing else in the enum shares a value either.
+    expect(new Set(codes).size).toBe(codes.length)
   })
 
   it('claims 0x86 for REFUSED_BY_SWITCH, clear of every other status code', () => {
     expect(ModbusDebugResponse.REFUSED_BY_SWITCH).toBe(0x86)
-    const others = Object.values(ModbusDebugResponse).filter(
-      (value): value is number => typeof value === 'number' && value !== ModbusDebugResponse.REFUSED_BY_SWITCH,
-    )
-    expect(others).not.toContain(ModbusDebugResponse.REFUSED_BY_SWITCH)
+    const codes = Object.values(ModbusDebugResponse).filter((value): value is number => typeof value === 'number')
+    expect(codes.filter((value) => value === ModbusDebugResponse.REFUSED_BY_SWITCH)).toHaveLength(1)
+    expect(new Set(codes).size).toBe(codes.length)
   })

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/backend/shared/debug/__tests__/modbus-pdu-plc-control.test.ts` around
lines 22 - 36, Fix both uniqueness tests in the PLC_SET_STATE and
REFUSED_BY_SWITCH cases so they count all enum members whose numeric value
equals the target, without filtering the target value out first. Assert that
each count is exactly one, preserving the existing wire-value assertions.

src/main/modules/ipc/main.ts-1901-1918 (1)

1901-1918: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Preserve the Runtime v4 debug transport when the session connects.

handleOpenRuntimeSession stores params.debug.connectionType, but handleDebuggerConnect later replaces it with getLink()?.transport ?? 'tcp'. A REST-controlled Runtime v4 session has no control link (getLink() returns null) and should keep the debug channel type, such as websocket, instead of defaulting to tcp.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/main/modules/ipc/main.ts` around lines 1901 - 1918, Update
handleDebuggerConnect so it preserves the debug connection type established by
handleOpenRuntimeSession when the session has no control link, rather than
defaulting to tcp. Reuse the stored params.debug.connectionType for Runtime v4
sessions while retaining the existing getLink transport behavior where a link is
available.

src/middleware/shared/utils/target-capabilities/types.ts-96-101 (1)

96-101: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win

Use the shipped Modbus run/stop function code in the doc comment.

arduino-cli targets use FC 0x4b for PLC_SET_STATE; update this comment so the capability contract matches src/backend/shared/debug/modbus-pdu.ts and src/backend/shared/simulator/types.ts. Status state is read with FC 0x46.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/middleware/shared/utils/target-capabilities/types.ts` around lines 96 -
101, Update the doc comment for plcStateControl to identify the shipped
arduino-cli PLC_SET_STATE Modbus function code as FC 0x4b, and mention FC 0x46
for reading status state. Leave the capability behavior and surrounding target
descriptions unchanged.

src/middleware/shared/ports/device-port.ts-161-172 (1)

161-172: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win

Reattach the connection-status doc block to onConnectionStatus.

Lines 161-165 document onConnectionStatus, but Lines 166-170 insert a second doc block and the onLinkLog? declaration between them. Only the nearest preceding block attaches, so onLinkLog receives the link-log text and onConnectionStatus at Line 172 receives no documentation in tooltips. Move the first block down to onConnectionStatus.

🐛 Proposed fix

-  /**
-   * Subscribe to live serial-link status pushed by the main process (liveness
-   * failure, upload/debug handoff). Returns an unsubscribe function. Editor:
-   * `device:connection-status` IPC event. Web: no-op.
-   */
   /**
    * Subscribe to the device connection's diagnostic trace. Returns an unsubscribe
    * function. Editor: `device:link-log`. Web: no-op.
    */
   onLinkLog?(callback: (message: string) => void): () => void
 
+  /**
+   * Subscribe to live device-link status pushed by the main process (liveness
+   * failure, upload/debug handoff). Returns an unsubscribe function. Editor:
+   * `device:connection-status` IPC event. Web: no-op.
+   */
   onConnectionStatus(callback: (payload: DeviceConnectionStatusPayload) => void): () => void

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/middleware/shared/ports/device-port.ts` around lines 161 - 172, Move the
connection-status documentation block so it immediately precedes
onConnectionStatus, leaving the link-log documentation directly above
onLinkLog?. Preserve both declarations and ensure each tooltip describes the
correct subscription.

src/middleware/adapters/editor/device-adapter.ts-42-64 (1)

42-64: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win

Handle IPC failures before returning bridge result shapes.

deviceConnect() and deviceDisconnect() currently resolve only when window.bridge.* resolves, but an IPC failure still rejects. Most call sites wrap the call and report failure as { status: 'error' } or { success: false }, but useDeviceConnect.disconnect() and runtime-session opening also call these methods and do not catch rejections. Return the adapter’s declared result shapes on bridge failure, or ensure every caller catches before surface UX.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/middleware/adapters/editor/device-adapter.ts` around lines 42 - 64,
Update the device adapter methods connect, openRuntimeSession, and disconnect to
catch rejected window.bridge calls and return their declared failure result
shapes instead of propagating IPC rejections. Preserve successful bridge
results, and map failures to the appropriate DeviceConnectResult or { success:
false } response so callers remain safe without requiring additional handling.

src/__architecture__/validate.ts-201-206 (1)

201-206: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Calculate the line from the import/export statement boundary.

The pattern matches before import or export, including leading newlines and blank lines. Use the offset of the import or export keyword before counting lines so imports after blank lines report the correct source line. Add a regression test with leading blank lines.

Proposed fix

   while ((match = pattern.exec(source)) !== null) {
     const path = match[1] ?? match[2]
     if (!path) continue
-    results.push({ path, line: source.slice(0, match.index).split('\n').length })
+    const statementOffset = match.index + match[0].search(/\b(?:import|export)\b/)
+    results.push({ path, line: source.slice(0, statementOffset).split('\n').length })
   }

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/__architecture__/validate.ts` around lines 201 - 206, Update the line
calculation in the import/export scanning loop using the match offset of the
actual import or export keyword, rather than match.index, so leading newlines
and blank lines are excluded. Preserve path extraction and result collection,
and add a regression test covering an import preceded by blank lines.

src/frontend/services/device-link-resolution.ts-269-304 (1)

269-304: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win

Correct the stale doc comment.

The doc block states that this function "Uses the SINGLE-channel resolver, not the candidate one", and then describes pointing the candidate resolver at a websocket channel as a failure. The implementation calls resolveDeviceLinkCandidates (Line 300), and the inline comment on Line 296 states the opposite. Keep the inline explanation and remove the contradicting paragraph, so a later reader does not change the call to match the doc.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/frontend/services/device-link-resolution.ts` around lines 269 - 304,
Update the doc block for resolveRuntimeDebugChannel by removing the stale
paragraph claiming it uses a single-channel resolver and describing candidate
resolution as a failure. Keep the implementation calling
resolveDeviceLinkCandidates and preserve the accurate inline explanation below
it.

src/frontend/utils/vpp/field-options.ts-31-33 (1)

31-33: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Validate the option shape more strictly.

isFieldOption accepts any object that has a value key. A manifest entry such as { value: 1 } or { value: 'x' } without label passes, and the select then renders an item with an empty label and a non-string value. Require a string value and fall back to value when label is absent.

🛡️ Proposed change

 function isFieldOption(value: unknown): value is FieldOption {
-  return typeof value === 'string' || (typeof value === 'object' && value !== null && 'value' in value)
+  if (typeof value === 'string') return true
+  if (typeof value !== 'object' || value === null || !('value' in value)) return false
+  const candidate = value as { value: unknown; label?: unknown }
+  return typeof candidate.value === 'string' && (candidate.label === undefined || typeof candidate.label === 'string')
 }

If label stays optional, make it optional on FieldOption and let the caller display label ?? value.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/frontend/utils/vpp/field-options.ts` around lines 31 - 33, Update
isFieldOption to accept object options only when value is a string, while
preserving string options. Ensure FieldOption treats label as optional and
update the select rendering path to display label ?? value when label is absent,
retaining the string value for option values.

src/frontend/components/_features/[workspace]/editor/device/configuration/board.tsx-30-37 (1)

30-37: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Remove the duplicate connected indicators.

DeviceConnectButton already renders ● Connected. Both callers also render DeviceConnectedIndicator, so the UI shows two connected confirmations.

Remove the local indicator and its two children.

Also applies to: 637-643, 721-723

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In
`@src/frontend/components/_features/`[workspace]/editor/device/configuration/board.tsx
around lines 30 - 37, Remove the DeviceConnectedIndicator function and both of
its usages from the two parent render paths in the board configuration
component. Keep DeviceConnectButton’s existing connected display unchanged so
each connection state has only one confirmation.

src/frontend/hooks/use-device-connect.ts-65-89 (1)

65-89: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

A cancelled address prompt shows a "No Response" dialog for an attempt that never happened.

result starts as { status: 'no-response' }. If the first pass produced no candidates (only awaitingInput) and the user then cancels the address prompt, prompted is null, result stays no-response, and tried is empty. The hook then shows "Could not reach the device on this device." This contradicts the stated rule at Line 51 that a cancel is the user's answer and must not stack another dialog.

Track whether any candidate was actually attempted and return when none was.

🐛 Proposed fix

     if (result.status !== 'connected-with-firmware' && silent.awaitingInput.length > 0) {
       const prompted = await resolveDeviceLinkWithUx(deviceBoard, boardInfo, {
         onlyChannels: silent.awaitingInput,
       })
       if (prompted && prompted.candidates.length > 0) {
         tried.push(...prompted.candidates.map((candidate) => describeDebugEndpoint(candidate.config)))
         result = await device.connect(prompted.candidates.map((candidate) => candidate.config))
       }
     }
 
+    // Nothing was attempted: the only channel needed input and the user declined.
+    // The prompt was their answer, so do not report a failed attempt on top of it.
+    if (tried.length === 0) return
+
     const endpoints = tried.join(' or ') || 'this device'

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/frontend/hooks/use-device-connect.ts` around lines 65 - 89, Track whether
any connection candidate was attempted in the device-connect flow, including
both silent and prompted passes. Before the no-response dialog block, return
without opening another dialog when no candidate was attempted, covering
cancellation of the address prompt; preserve the existing error dialog for
attempts that actually ran.

src/frontend/hooks/use-device-connection-monitor.ts-45-56 (1)

45-56: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win

A late ipAddress leaves the runtime without a session.

The effect re-runs only for device, connectionStatus, and jwtToken. It reads runtimeConnection.ipAddress through getState(), so the value is not reactive. If the store records connectionStatus: 'connected' before the address, the effect logs the warning at Line 65 and never opens a session. Every later command then reports "not connected", which is the failure mode the comment at Line 58 describes. Select ipAddress and add it to the dependency array.

🔧 Proposed fix

   const connectionStatus = useOpenPLCStore((state) => state.runtimeConnection.connectionStatus)
   const jwtToken = useOpenPLCStore((state) => state.runtimeConnection.jwtToken)
+  const ipAddress = useOpenPLCStore((state) => state.runtimeConnection.ipAddress)

-  }, [device, connectionStatus, jwtToken])
+  }, [device, connectionStatus, jwtToken, ipAddress])

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/frontend/hooks/use-device-connection-monitor.ts` around lines 45 - 56,
Update the effect around the runtime connection setup to select
`runtimeConnection.ipAddress` reactively from `useOpenPLCStore` and include that
selected value in the `useEffect` dependency array. Use the reactive `ipAddress`
in the existing session-opening flow so a late address reruns the effect and
establishes the session.

src/frontend/hooks/use-device-connection-monitor.ts-79-88 (1)

79-88: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win

Handle rejection on the session IPC calls.

void device.openRuntimeSession(...).then(...) handles only a resolved result. If the IPC call rejects, the promise becomes an unhandled rejection and the user sees nothing. Line 49 has the same gap. The coding guidelines require every promise to be awaited or to handle rejection explicitly.

🔧 Proposed fix

-    void device.openRuntimeSession({ address, debug: debugChannel }).then((result) => {
-      if (!result.success) {
-        store.consoleActions.addLog({
-          id: crypto.randomUUID(),
-          level: 'error',
-          message: `[connection] could not open the runtime session: ${result.error ?? 'unknown error'}`,
-        })
-      }
-    })
+    void device
+      .openRuntimeSession({ address, debug: debugChannel })
+      .then((result) => {
+        if (!result.success) {
+          store.consoleActions.addLog({
+            id: crypto.randomUUID(),
+            level: 'error',
+            message: `[connection] could not open the runtime session: ${result.error ?? 'unknown error'}`,
+          })
+        }
+      })
+      .catch((error: unknown) => {
+        store.consoleActions.addLog({
+          id: crypto.randomUUID(),
+          level: 'error',
+          message: `[connection] could not open the runtime session: ${error instanceof Error ? error.message : String(error)}`,
+        })
+      })

Apply the same .catch to device.closeRuntimeSession?.() at Line 49.

As per coding guidelines: "Do not allow floating promises; await them or handle rejection explicitly."

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/frontend/hooks/use-device-connection-monitor.ts` around lines 79 - 88,
Handle rejected promises in both runtime session IPC calls: add explicit
rejection handling to the openRuntimeSession flow near the existing result check
and to closeRuntimeSession?.() at the earlier cleanup path. Log each failure
through store.consoleActions.addLog with error-level context, ensuring no
promise remains unhandled while preserving existing success and result-error
behavior.

Source: Coding guidelines

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@src/backend/editor/compiler/compiler-module.ts`:
- Around line 2666-2667: Validate the complete Modbus screen subsection before
constructing the state passed to the compile pipeline, replacing the unchecked
casts in the surrounding compiler flow. Use the existing Zod schema or type
guard for VppModbusScreenState to validate serial, network, modbus_rtu, and
modbus_tcp together, and handle validation failures by stopping or returning
through the established compiler error path instead of passing malformed data
onward.

In `@src/backend/editor/hardware/__tests__/device-session-manager.test.ts`:
- Line 44: Remove all forbidden as unknown as casts in the device-session
manager test harness, including asTransport and the casts near probe/verify.
Prefer making FakeClient implement DeviceModbusTransport with stub members so
transport arguments can be used directly, or maintain a
Map<DeviceModbusTransport, FakeClient> and resolve fakes by identity; preserve
existing test behavior without introducing other type assertions.

In `@src/backend/editor/hardware/device-session-manager.ts`:
- Around line 362-436: Add a monotonically increasing session-generation guard
shared by open(), tick(), and recovery operations so each async operation
captures its generation before awaiting. In open(), invalidate and disconnect
any client being replaced, then discard stale tryCandidate results and never
assign this.client, startPolling(), or emit connected for a superseded
generation; close any stale client returned by the probe. Apply the same
generation checks to tick()/recovery, clearing the old client before probing so
stale operations cannot restore or overwrite session state.
- Around line 660-669: In the recovered branch of the reopen-result handling,
replace both non-null assertions on reopened with an explicit local binding
after confirming it is non-null, then use that narrowed value for client,
candidate, transport, and descriptor access.

In `@src/backend/editor/hardware/device-transport-factory.ts`:
- Around line 80-90: The TCP branch of buildDeviceModbusTransport currently
ignores numeric-string ports and falls back to MODBUS_TCP_PORT. Accept string
port values by parsing them as integers, validate the resulting port against the
supported TCP port range, and use the validated value when constructing
ModbusTcpClient while preserving the default for absent ports.

In `@src/backend/shared/compile/steps/modbus-defines.ts`:
- Around line 200-202: Update the default-port branch of the baud selection
expression in the compile step to check rtu.baud_rate before rtu.rtu_baud_rate,
matching resolveDebugBaud’s precedence while retaining the existing serial and
RTU defaults.

In `@src/backend/shared/hardware/__tests__/connect-resolve-regression.test.ts`:
- Around line 321-329: Replace the malformed `as unknown as DebugSpec` fixture
in the `reports unsupported when the board declares nothing reachable` test with
a directly type-checked `DebugSpec` literal whose `channels` value is empty,
preserving coverage of the `!channels?.length` branch and the existing
unsupported assertions.

In `@src/backend/shared/simulator/modbus-rtu-client.ts`:
- Around line 579-592: Add unit tests for ModbusRtuClient.setPlcState in the
shared simulator test suite, without relying on the skipped firmware-backed
plc-control-e2e suite. Cover successful parsing, supported refusal results such
as REFUSED_BY_SWITCH, responses shorter than 8 bytes, Error rejections, and
non-Error rejections, mocking sendRequest and validating the returned
PlcControlResult for each case.

In
`@src/frontend/components/_features/`[workspace]/editor/device/configuration/board.tsx:
- Around line 381-386: Wrap runtime.clearCredentials() in a try/finally block so
device.closeRuntimeSession?.() always executes, including when credential
cleanup rejects. Move the disconnected renderer state updates until cleanup
completes, while preserving the existing token reset and session-close behavior.
- Around line 82-90: Update the modbusTcpConfigured selector to validate
vendorScreenData at runtime before accessing modbus_tcp.enabled, replacing the
unchecked Record type assertion with an existing Zod schema or type guard. Only
treat the value as configured when the validated nested data explicitly contains
enabled === true.

In `@src/frontend/components/_molecules/device-connect-button/index.tsx`:
- Around line 9-12: Update the onConnect and onDisconnect props in the
device-connect button to accept synchronous or asynchronous callbacks using void
| Promise<void>. In the onClick flow, invoke the selected callback through an
async-aware handler and catch or report any rejected promise so connection and
teardown failures are not unhandled.

In `@src/frontend/components/_organisms/workspace-activity-bar/default.tsx`:
- Around line 359-384: Move the reconnect logic currently guarded by
serialWasReleased && result.success into shared completion handling that
executes for both successful and failed uploads, including exceptions from
compiler.compileProgram. Preserve the serialWasReleased guard, silent candidate
resolution, and best-effort device.connect behavior, and remove the success-only
block from its current location.

In `@src/frontend/services/__tests__/device-link-resolution.test.ts`:
- Around line 20-33: Remove both as unknown as casts in the test fixtures: type
mockUseOpenPLCStore with an explicit callable type that includes getState, and
update boardWith to accept DebuggerTransport[] while returning a fully populated
BoardInfo literal containing compiler, core, preview, specs, debug, and
capabilities fields. Import the required DebuggerTransport type and preserve the
existing fixture behavior without type assertions.

In `@src/frontend/store/slices/device/slice.ts`:
- Line 53: Update the clearDeviceDefinitions and clearRuntimeConnection reset
actions to set runtimeConnection.switchPosition to null, ensuring every
connection reset clears the previous target’s switch position.

In `@src/frontend/utils/vpp/field-options.ts`:
- Around line 35-40: Update resolveFieldOptions in
src/frontend/utils/vpp/field-options.ts#L35-L40 to accept context with an opaque
object board value, change lookupPath’s context parameter to unknown, and remove
the Record<string, unknown> assertion while preserving its existing object
narrowing. In
src/frontend/components/_features/[workspace]/editor/device/configuration/vendor-screen/layouts/form-layout.tsx#L174-L176,
pass currentBoardInfo directly in { board: currentBoardInfo } without a type
assertion.

In `@src/main/modules/ipc/main.ts`:
- Around line 1699-1700: Update the return value around getMd5Hash in the
channel probe flow to explicitly populate targetMd5 from probe.md5 before or
alongside spreading probe. Preserve the existing success and match fields and
ensure verifyResult.targetMd5 contains the probed hash.
- Around line 690-698: Add the snake_case switch_position field to the parsed
/api/status response type alongside switchPosition, then map or normalize it
into the IPC switchPosition value before the gating logic uses it. Preserve
support for runtimes that provide the existing camelCase field and treat both
absent fields as undefined.

In `@src/middleware/shared/ports/debugger-port.ts`:
- Line 31: Move the PlcControlResult contract out of backend/shared/debug/types
into the shared ports layer, declaring it in debugger-port.ts or a nearby
./types module. Update debugger-port.ts, debugger-adapter.ts, and the preload
bridge to use the shared definition, and make backend/shared/debug/types
re-export it so existing backend consumers remain compatible without the port
layer importing backend code.

---

Outside diff comments:
In `@src/middleware/shared/ports/debugger-port.ts`:
- Around line 62-67: Remove the stale `@param` config documentation from the
verifyMd5 method comment, leaving documentation only for the existing
expectedMd5 parameter and the method’s purpose.

---

Minor comments:
In `@src/__architecture__/validate.ts`:
- Around line 201-206: Update the line calculation in the import/export scanning
loop using the match offset of the actual import or export keyword, rather than
match.index, so leading newlines and blank lines are excluded. Preserve path
extraction and result collection, and add a regression test covering an import
preceded by blank lines.

In `@src/backend/shared/debug/__tests__/modbus-pdu-plc-control.test.ts`:
- Around line 22-36: Fix both uniqueness tests in the PLC_SET_STATE and
REFUSED_BY_SWITCH cases so they count all enum members whose numeric value
equals the target, without filtering the target value out first. Assert that
each count is exactly one, preserving the existing wire-value assertions.

In
`@src/frontend/components/_features/`[workspace]/editor/device/configuration/board.tsx:
- Around line 30-37: Remove the DeviceConnectedIndicator function and both of
its usages from the two parent render paths in the board configuration
component. Keep DeviceConnectButton’s existing connected display unchanged so
each connection state has only one confirmation.

In `@src/frontend/hooks/use-device-connect.ts`:
- Around line 65-89: Track whether any connection candidate was attempted in the
device-connect flow, including both silent and prompted passes. Before the
no-response dialog block, return without opening another dialog when no
candidate was attempted, covering cancellation of the address prompt; preserve
the existing error dialog for attempts that actually ran.

In `@src/frontend/hooks/use-device-connection-monitor.ts`:
- Around line 45-56: Update the effect around the runtime connection setup to
select `runtimeConnection.ipAddress` reactively from `useOpenPLCStore` and
include that selected value in the `useEffect` dependency array. Use the
reactive `ipAddress` in the existing session-opening flow so a late address
reruns the effect and establishes the session.
- Around line 79-88: Handle rejected promises in both runtime session IPC calls:
add explicit rejection handling to the openRuntimeSession flow near the existing
result check and to closeRuntimeSession?.() at the earlier cleanup path. Log
each failure through store.consoleActions.addLog with error-level context,
ensuring no promise remains unhandled while preserving existing success and
result-error behavior.

In `@src/frontend/services/device-link-resolution.ts`:
- Around line 269-304: Update the doc block for resolveRuntimeDebugChannel by
removing the stale paragraph claiming it uses a single-channel resolver and
describing candidate resolution as a failure. Keep the implementation calling
resolveDeviceLinkCandidates and preserve the accurate inline explanation below
it.

In `@src/frontend/utils/vpp/field-options.ts`:
- Around line 31-33: Update isFieldOption to accept object options only when
value is a string, while preserving string options. Ensure FieldOption treats
label as optional and update the select rendering path to display label ?? value
when label is absent, retaining the string value for option values.

In `@src/main/modules/ipc/main.ts`:
- Around line 1901-1918: Update handleDebuggerConnect so it preserves the debug
connection type established by handleOpenRuntimeSession when the session has no
control link, rather than defaulting to tcp. Reuse the stored
params.debug.connectionType for Runtime v4 sessions while retaining the existing
getLink transport behavior where a link is available.

In `@src/middleware/adapters/editor/device-adapter.ts`:
- Around line 42-64: Update the device adapter methods connect,
openRuntimeSession, and disconnect to catch rejected window.bridge calls and
return their declared failure result shapes instead of propagating IPC
rejections. Preserve successful bridge results, and map failures to the
appropriate DeviceConnectResult or { success: false } response so callers remain
safe without requiring additional handling.

In `@src/middleware/shared/ports/device-port.ts`:
- Around line 161-172: Move the connection-status documentation block so it
immediately precedes onConnectionStatus, leaving the link-log documentation
directly above onLinkLog?. Preserve both declarations and ensure each tooltip
describes the correct subscription.

In `@src/middleware/shared/utils/target-capabilities/types.ts`:
- Around line 96-101: Update the doc comment for plcStateControl to identify the
shipped arduino-cli PLC_SET_STATE Modbus function code as FC 0x4b, and mention
FC 0x46 for reading status state. Leave the capability behavior and surrounding
target descriptions unchanged.

---

Nitpick comments:
In `@src/backend/editor/hardware/__tests__/device-session-manager.test.ts`:
- Around line 236-241: Move the jest.restoreAllMocks() call from the test body
into the existing afterEach setup so Date.now is restored even when assertions
fail. Remove the in-test restore call after h.manager.tick(), while preserving
the existing manager cleanup.
- Around line 655-676: Rename the second acquireDebugChannel holder in the
“closes its own channel only when the last holder lets go” test from “license
check” to “status poll”, and update the corresponding releaseDebugChannel call
to use the same holder id.
- Around line 126-154: Update the test using one harness instead of separate h
and h2 instances: create the harness with the verify override, clear and
populate that harness’s ports set with COM5, and use the same harness manager,
clients, and port-presence state throughout the test. Remove the unused h
instance and serialPortPresent indirection while preserving the existing
baud-attempt assertions.

In `@src/backend/editor/hardware/device-transport-factory.ts`:
- Around line 37-42: Replace the any-derived type of
DeviceTransportOptions.virtualSerialPort with an explicit serial-port interface.
Define and reuse a shared SerialPortLike type between
ModbusRtuClientOptions.serialPort and the simulator transport so
options.virtualSerialPort remains strongly typed throughout the device transport
factory.

In `@src/backend/editor/modbus/modbus-client.ts`:
- Around line 373-436: Update getBoardId to use the existing buildTcpFrame(pdu)
helper for request construction and transaction ID generation, replacing its
inline transaction, MBAP, and unit-ID setup. Preserve the subsequent
sendTcpRequest call and transaction-ID validation using the helper’s returned
request and transactionId.
- Around line 27-41: Replace the local ModbusFunctionCode and
ModbusDebugResponse enum definitions in the modbus client with imports from the
shared simulator types module. Re-export the imported enums if existing
consumers rely on this module’s exports, while preserving the PLC_SET_STATE and
REFUSED_BY_SWITCH values through the shared definitions.

In `@src/backend/shared/debug/__tests__/modbus-pdu.test.ts`:
- Around line 206-225: Add a test case alongside “parses running / tick / uptime
on success” that builds a 12-byte successful status frame by appending a
switch-position byte, invokes parseGetStatusResponse, and asserts the returned
switchPosition value while preserving the existing parsed fields. This must
exercise the data.length >= 12 branch and maintain full coverage for the parser.

In `@src/backend/shared/hardware/debug-spec.ts`:
- Around line 121-126: Update the CHANNEL_TRANSPORT declaration to use a partial
record so unmapped channel kinds are typed as possibly undefined, preserving the
existing filter behavior that excludes channels without a configured transport.
- Around line 412-422: Deduplicate the transport values used by the outer loop
in the eligibility calculation so repeated entries in options.transports cannot
push the same channel index more than once. Update the loop around eligible and
preserve the existing included, CHANNEL_TRANSPORT, and enabledWhen checks.

In `@src/backend/shared/simulator/__tests__/debug-e2e.test.ts`:
- Line 100: Remove the non-null assertions from the debug result tests: in
src/backend/shared/simulator/__tests__/debug-e2e.test.ts lines 100-100, assert
and narrow both tick values before comparing them; at lines 115-115, narrow
result.boardId before reading its length. In
src/backend/shared/simulator/__tests__/modbus-rtu-client.test.ts lines 700-700
and 712-712, bind each result.boardId to a local and explicitly narrow it before
using Array.from or asserting the empty-id case.

In `@src/backend/shared/simulator/__tests__/plc-control-e2e.test.ts`:
- Around line 196-230: The tests 'boots STOPPED when the switch reads STOP, and
reports the position' and 'refuses a RUN request while the switch reads STOP'
have a timing race: the switch HAL reads STOP until 3s uptime, but connect()
resolves at 2.5s, leaving only 500ms margin. Emulator overhead can push
execution past that window and find the switch already transitioned to RUN.
Update the test that calls setPlcState to first verify the actual observed
switchPosition before asserting about the refusal behavior, or make the refusal
assertions conditional on confirming switchPosition is still STOP. This ensures
the test does not fail when timing causes the switch to transition between test
setup and assertion.

In
`@src/frontend/components/_molecules/device-connect-button/__tests__/device-connect-button.test.tsx`:
- Around line 45-67: Remove the HTMLButtonElement type assertions in the tests
for “is inert while connecting,” “explains itself when something blocks
connecting,” and “stays live when nothing blocks it.” Use the generic form of
screen.getByRole<HTMLButtonElement>(...) or the appropriate DOM matcher such as
toBeDisabled(), while preserving the existing assertions and behavior.

In `@src/frontend/components/_organisms/modals/runtime-connection-lost-modal.tsx`:
- Line 13: Replace the type assertion on modalData with a type guard that
validates the payload as an object containing optional string label and body
fields, then use the guard’s narrowed result while preserving undefined for
invalid or missing data. Define the guard near the modal data access and apply
it to modals['runtime-connection-lost']?.data.

In `@src/frontend/components/_organisms/workspace-activity-bar/default.tsx`:
- Around line 391-407: Update the dependency array for handleBuild to include
the debuggerPort and device values read in its body, and remove runtime because
it is no longer referenced there. Keep all other existing dependencies
unchanged.
- Around line 529-530: Update handlePlcControl to remove both non-const type
assertions: add or reuse a board type exposing optional
stateControl.modeSwitch.label for typed access to switchLabel, and define an
explicit return type for the PLC status mapping so the mapped literal is
inferred without casting to NonNullable<RuntimeConnection['plcStatus']>.
Preserve the existing behavior and values.

In `@src/frontend/hooks/__tests__/use-device-connect.test.ts`:
- Around line 20-22: Extract a shared typed mock helper for the callable
useOpenPLCStore shape with its getState property, then use it in both
use-device-connect.test.ts:20-22 and
use-device-connection-monitor.test.ts:24-25. Remove both local as unknown as
casts while preserving selector and state behavior; the helper should be the
only implementation change needed at these sites.

In `@src/frontend/hooks/__tests__/use-device-plc-state.test.ts`:
- Line 47: Replace the non-null assertions on `pushed` in the affected tests
with a small helper that validates the hook subscribed, throws a clear error
when it did not, and returns the callable push function. Update each test to
invoke the helper as `push(...)` while preserving the existing payloads and
assertions.

In `@src/frontend/hooks/__tests__/use-runtime-polling.test.ts`:
- Line 8: Add assertions in the runtime polling hook tests using
mockSetPlcSwitchPosition to verify successful polls pass
statusResult.switchPosition ?? null and clearConnectionState passes null,
including the default-to-null case.

In `@src/frontend/services/device-link-resolution.ts`:
- Around line 96-98: Replace the type assertion in the screens initialization
within the device-link resolution flow with boundary validation for
cfg.vendorScreenData. Add or reuse a type guard or Zod schema that verifies the
nested record shape, use the validated value when valid, and fall back to an
empty record when absent or invalid.

In `@src/frontend/utils/device-connect-events.ts`:
- Around line 11-23: Add a focused unit test for requestDeviceFlash and
onDeviceFlashRequest that registers a handler, dispatches the flash request
event, verifies the handler is called, and invokes the returned unsubscribe
function. Ensure the test exercises the real module implementations rather than
mocks and cleans up the subscription.

In `@src/main/modules/ipc/renderer.ts`:
- Around line 413-424: Update debuggerPlcControl in the renderer IPC API to
return the shared PlcControlResult type instead of its duplicated inline object
shape. Add the type-only import from the existing debug types module and
preserve the current action parameter and IPC invocation.
- Around line 352-357: Import the existing CommunicationPort type alongside the
current type imports in renderer.ts, then replace the duplicated inline port
object types in getAvailableCommunicationPorts and refreshCommunicationPorts
with CommunicationPort[].

In `@src/middleware/adapters/editor/__tests__/debugger-adapter.test.ts`:
- Around line 215-229: Update both setPlcState tests in the setPlcState describe
block to be async and await adapter.setPlcState('STOPPED') and
adapter.setPlcState('RUNNING') instead of discarding the promises with void.
Keep the existing debuggerPlcControl assertions unchanged.
- Around line 78-93: Remove the three redundant connection-type tests around
adapter.connect—“supports simulator connection type,” “supports websocket
connection type with JWT,” and “supports RTU connection type with serial
params”—from the debugger adapter test suite. Keep the existing equivalent
connect test and do not add replacement coverage, since connect() accepts no
configuration to distinguish these types.
- Around line 235-254: In the verifyMd5 tests, rename “uses different configs
per call” to describe verifying different MD5 values per call, and remove the
unused simConfig binding. Also remove the DebugConnectionConfig import if no
other tests in the file reference it.

In `@src/middleware/adapters/editor/__tests__/device-adapter.test.ts`:
- Around line 76-100: Extend the device-adapter test suite with cases for
openRuntimeSession, closeRuntimeSession, releaseSerialPort, onLinkLog, and
onPlcState, verifying each delegates to the corresponding window.bridge method
and returns or exposes its result correctly. For releaseSerialPort, explicitly
assert the adapter returns result.released, and for the subscription methods
verify the callback delegation and returned unsubscribe function to achieve full
coverage.

In `@src/middleware/shared/ports/device-port.ts`:
- Around line 174-183: Define and export a shared DevicePlcStatePayload in
src/middleware/shared/ports/device-port.ts#L174-183 with descriptor instead of
port, optionally narrowing plcState to 0 | 1 | 2 and switchPosition to 0 | 1,
and use it in onPlcState. In
src/middleware/adapters/editor/device-adapter.ts#L74-76, import and use this
type; in src/main/modules/ipc/renderer.ts#L500-507, import it and use it for
both the callback parameter and listener payload.
- Around line 113-159: Update the DevicePort interface signatures for connect
and releaseSerialPort to align with their documented web no-op behavior, either
by making them optional or by documenting concrete web return values such as
connect returning failure; preserve the required contract only if web
implementations provide those returns. Also revise disconnect’s comment to
describe closing the held device connection rather than only a serial link.

🪄 Autofix

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 4bf7f039-2878-4afe-944a-7d168815f987

📥 Commits

Reviewing files that changed from the base of the PR and between bfa9f16 and 1c8ee64.

⛔ Files ignored due to path filters (24)

• resources/sources/Baremetal/ARCHITECTURE.md is excluded by !resources/**
• resources/sources/Baremetal/Baremetal.ino is excluded by !resources/**
• resources/sources/Baremetal/ModbusSlave.cpp is excluded by !resources/**
• resources/sources/Baremetal/ModbusSlave.h is excluded by !resources/**
• resources/sources/Baremetal/modbus_config.h is excluded by !resources/**
• resources/sources/Baremetal/modbus_crc.cpp is excluded by !resources/**
• resources/sources/Baremetal/modbus_crc.h is excluded by !resources/**
• resources/sources/Baremetal/modbus_debug.cpp is excluded by !resources/**
• resources/sources/Baremetal/modbus_debug.h is excluded by !resources/**
• resources/sources/Baremetal/modbus_frame.cpp is excluded by !resources/**
• resources/sources/Baremetal/modbus_frame.h is excluded by !resources/**
• resources/sources/Baremetal/modbus_pdu.cpp is excluded by !resources/**
• resources/sources/Baremetal/modbus_pdu.h is excluded by !resources/**
• resources/sources/Baremetal/modbus_registers.cpp is excluded by !resources/**
• resources/sources/Baremetal/modbus_registers.h is excluded by !resources/**
• resources/sources/Baremetal/modbus_serial.cpp is excluded by !resources/**
• resources/sources/Baremetal/modbus_serial.h is excluded by !resources/**
• resources/sources/Baremetal/modbus_tcp.cpp is excluded by !resources/**
• resources/sources/Baremetal/modbus_tcp.h is excluded by !resources/**
• resources/sources/Baremetal/modbus_types.h is excluded by !resources/**
• resources/sources/Baremetal/openplc_version.h is excluded by !resources/**
• resources/sources/arduino/arduino_runtime_glue.cpp is excluded by !resources/**
• resources/sources/arduino/arduino_runtime_glue.h is excluded by !resources/**
• resources/sources/arduino/openplc.h is excluded by !resources/**

📒 Files selected for processing (77)

• src/__architecture__/validate.ts
• src/backend/editor/compiler/__tests__/handle-precompile-user-lib.test.ts
• src/backend/editor/compiler/compiler-module.ts
• src/backend/editor/hardware/__tests__/device-link-policy.test.ts
• src/backend/editor/hardware/__tests__/device-probe.test.ts
• src/backend/editor/hardware/__tests__/device-session-manager.test.ts
• src/backend/editor/hardware/__tests__/serial-port-list.test.ts
• src/backend/editor/hardware/device-link-policy.ts
• src/backend/editor/hardware/device-probe.ts
• src/backend/editor/hardware/device-session-manager.ts
• src/backend/editor/hardware/device-transport-factory.ts
• src/backend/editor/hardware/hardware-module.ts
• src/backend/editor/hardware/serial-port-list.ts
• src/backend/editor/hardware/types.ts
• src/backend/editor/modbus/modbus-client.ts
• src/backend/editor/modbus/modbus-rtu-client.ts
• src/backend/shared/compile/__tests__/generate-defines.test.ts
• src/backend/shared/compile/__tests__/modbus-defines.test.ts
• src/backend/shared/compile/steps/generate-defines.ts
• src/backend/shared/compile/steps/modbus-defines.ts
• src/backend/shared/debug/__tests__/modbus-pdu-plc-control.test.ts
• src/backend/shared/debug/__tests__/modbus-pdu.test.ts
• src/backend/shared/debug/modbus-pdu.ts
• src/backend/shared/debug/types.ts
• src/backend/shared/debug/websocket-debug-transport.ts
• src/backend/shared/hardware/__tests__/connect-resolve-regression.test.ts
• src/backend/shared/hardware/__tests__/debug-spec.test.ts
• src/backend/shared/hardware/debug-spec.ts
• src/backend/shared/simulator/__tests__/debug-e2e.test.ts
• src/backend/shared/simulator/__tests__/modbus-rtu-client.test.ts
• src/backend/shared/simulator/__tests__/plc-control-e2e.test.ts
• src/backend/shared/simulator/modbus-rtu-client.ts
• src/backend/shared/simulator/types.ts
• src/frontend/components/_features/[workspace]/editor/device/configuration/board.tsx
• src/frontend/components/_features/[workspace]/editor/device/configuration/vendor-screen/layouts/form-layout.tsx
• src/frontend/components/_molecules/device-connect-button/__tests__/device-connect-button.test.tsx
• src/frontend/components/_molecules/device-connect-button/index.tsx
• src/frontend/components/_organisms/modals/confirm-device-switch-modal.tsx
• src/frontend/components/_organisms/modals/runtime-connection-lost-modal.tsx
• src/frontend/components/_organisms/workspace-activity-bar/default.tsx
• src/frontend/components/_templates/app-layout.tsx
• src/frontend/hooks/__tests__/use-device-connect.test.ts
• src/frontend/hooks/__tests__/use-device-connection-monitor.test.ts
• src/frontend/hooks/__tests__/use-device-plc-state.test.ts
• src/frontend/hooks/__tests__/use-runtime-polling.test.ts
• src/frontend/hooks/use-device-connect.ts
• src/frontend/hooks/use-device-connection-monitor.ts
• src/frontend/hooks/use-device-plc-state.ts
• src/frontend/hooks/use-runtime-polling.ts
• src/frontend/hooks/useDebugSession.ts
• src/frontend/screens/workspace-screen.tsx
• src/frontend/services/__tests__/device-link-resolution.test.ts
• src/frontend/services/device-link-resolution.ts
• src/frontend/store/__tests__/device-slice.test.ts
• src/frontend/store/__tests__/device-types.test.ts
• src/frontend/store/slices/device/index.ts
• src/frontend/store/slices/device/slice.ts
• src/frontend/store/slices/device/types.ts
• src/frontend/utils/__tests__/serial-port-label.test.ts
• src/frontend/utils/device-connect-events.ts
• src/frontend/utils/serial-port-label.ts
• src/frontend/utils/vpp/__tests__/field-options.test.ts
• src/frontend/utils/vpp/field-options.ts
• src/main/modules/ipc/main.ts
• src/main/modules/ipc/renderer.ts
• src/middleware/adapters/editor/__tests__/debugger-adapter.test.ts
• src/middleware/adapters/editor/__tests__/device-adapter.test.ts
• src/middleware/adapters/editor/debugger-adapter.ts
• src/middleware/adapters/editor/device-adapter.ts
• src/middleware/shared/ports/debugger-port.ts
• src/middleware/shared/ports/device-port.ts
• src/middleware/shared/ports/runtime-port.ts
• src/middleware/shared/ports/types.ts
• src/middleware/shared/utils/debug-endpoint.ts
• src/middleware/shared/utils/target-capabilities/presets.ts
• src/middleware/shared/utils/target-capabilities/resolve.ts
• src/middleware/shared/utils/target-capabilities/types.ts

[JulioSergioFS]

Review — openplc-editor #980

Verdict

Strong work, and close to ready on its own merits.

DeviceSessionManager / DeviceLinkPolicy is a clean single-ownership design: one held
link, two channel slots that collapse to the same client when one medium serves both roles,
and the counting for down/back/lost isolated in a class that can be tested without a cable.
That structure is what actually fixes run/stop over Modbus TCP — the transient second socket
an Arduino TCP server never answers.

Verified rather than taken on trust:

Check	Result
src/backend/editor/hardware/__tests__ + src/backend/shared/hardware/__tests__	✅ 8 suites, 159 tests passing
FC 0x46 status frame offsets, TS ↔ firmware	✅ agree byte-for-byte (see below)
FC 0x4b run/stop response layout	✅ [FC][status][plc_state][switch], matches plcSetState
DeviceLinkPolicy reopen accounting	✅ onReopenResult(true) always returns recovered, so no client leaks on the success path

The status-frame check in full, because a mismatch here is exactly the class of field bug
this PR exists to fix. Firmware writes mb_frame[3] = state, [4..7] = tick BE,
[8..11] = uptime BE, [12] = switch, mb_frame_len = 13 — so 12 PDU bytes once the slave
byte is stripped. parseGetStatusResponse reads running at 2, readU32BE(data, 3),
readU32BE(data, 7), and switch at 11, guarding < 11 and gating the switch on >= 12.
Correct, including the "11 bytes on older firmware" back-compat case.

The two non-feature fixes are correct and well justified: fileURLToPath for the C:\C:\…
win32 SRC_ROOT, and .cpp.o object naming for the esp8266 linker script (the IRAM
overflow diagnosis — 7387 bytes of ordinary code in a 32 KB segment — is the kind of root
cause that saves the next person a day). Two smaller additions in compiler-module.ts are
also good calls: adding ArduinoUniqueID to the always-installed libs (FC 0x48 needs it,
and the include isn't behind a gate), and turning a missing HAL into a hard error naming the
board and path instead of an undefined-reference wall several hundred log lines later.

---

1. Blocking for the pair, not for this diff

Web #641 cannot ship as written: the shared UI this PR introduces gates every debug
session and Run/Stop on deviceConnection.status === 'connected', and nothing on web ever
publishes that status. Full detail in PR-641-web-review.md.

Relevant here only for where the fix lands. The recommended fix — the web DevicePort
publishing a real session status — is entirely inside src/middleware/adapters/web/ and
needs no change to this PR, which is why it is the better of the two routes.

The alternative route would touch this PR, because two of the implicated files are
byte-identical shared surface:

• src/frontend/components/_organisms/workspace-activity-bar/default.tsx — the gate itself,
  if it is made capability-aware instead of status-aware.
• src/frontend/hooks/useDebugSession.ts:189 — the ?? 'simulator' fallback (see §3).

Per the sync gate, either change has to originate here and be mirrored. Worth agreeing on
the route before either PR moves.

---

2. Should fix — resolveDebugBaud and generateModbusDefines disagree

Both live in src/backend/shared/compile/steps/modbus-defines.ts and are documented to
mirror each other, but they read modbus_rtu.baud_rate differently when RTU is on the
default port. Executed against this branch:

legacy: RTU on default, rtu_baud_rate 9600
   MBSERIAL_BAUD=9600    DEBUG_BAUD=9600   shared=true
phase2 field on default port: baud_rate 9600
   MBSERIAL_BAUD=115200  DEBUG_BAUD=9600   shared=true   <-- MISMATCH
phase2 both: baud_rate 9600 + rtu_baud_rate 19200
   MBSERIAL_BAUD=19200   DEBUG_BAUD=9600   shared=true   <-- MISMATCH
serial section wins
   MBSERIAL_BAUD=57600   DEBUG_BAUD=57600  shared=true

Two macros describing the same physical port, emitted into the same defines.h, disagreeing.

generateModbusDefines looks right per the field's own documented meaning — "baud for RTU
on a secondary port. On the default port the Serial section's baud is used" — so
resolveDebugBaud is the one that should not consult rtu.baud_rate when
iface === defaultSerial.

Latent today: no published VPP carries the Phase-2 serial/network sections, and the
migration commits were deliberately left out of this PR. But it lands the moment the
split-screen work does, and it is the same "No Firmware Detected on a healthy board" class
this PR just spent a commit fixing.

3. Should fix — ?? 'simulator' is the wrong default for an unknown medium

src/frontend/hooks/useDebugSession.ts:189:

wsActions.setDebugConnectionType(
  useOpenPLCStore.getState().deviceConnection.debugTransport ?? "simulator",
);

Harmless in the editor, where the manager always publishes debugTransport. But 'simulator'
is a meaningful value, not a neutral one: in useDebugPolling it selects RTU framing —
19-variable batches and a 50 ms cadence — and short-circuits the usesHttpFallback branch.
Defaulting an unknown medium to the tightest cadence in the system is the wrong direction to
fail in.

The sibling call in default.tsx:708 already guards correctly (if (activeTransport)), so
this is also an inconsistency between two call sites doing the same job.

This is what breaks web today (§1), but it is worth fixing on its own merits regardless of
which route the pair takes.

---

Minor

• MBSERIAL_SHARES_DEBUG_SERIAL is emitted but never read. No #ifdef tests it anywhere
  in the firmware — only a comment in Baremetal.ino:171 mentions it. The generator's own
  comment ("tell the firmware to begin the port once") implies it does something. Either
  gate on it or drop it; a define that looks load-bearing and isn't will mislead the next
  reader.

• debug-spec.ts — the TCP-only justification is factually wrong. The comment on the new
  rtu fallback claims it is skipped for TCP-only builds "matching the firmware which does NOT
  bring up the serial debugger in that configuration." Baremetal.ino:176-181 does exactly
  that: #elif defined(DEBUGGER_ENABLED) → DEBUG_IFACE.begin(DEBUG_BAUD) +
  modbus.slaveid = DEBUG_SLAVE. The behaviour is fine — resolveDeviceLinkCandidates
  exempts rtu unconditionally, which is right precisely because the firmware does bring it up
  — but the stated reason is backwards, and it is the kind of comment that justifies the wrong
  change later.

• device-link-resolution.ts — resolveRuntimeDebugChannel docblock contradicts itself.
  It opens "Uses the SINGLE-channel resolver, not the candidate one", then the code calls
  resolveDeviceLinkCandidates and the next paragraph says "The SAME resolver Connect uses."
  The first paragraph is stale.

• modbus_debug.cpp — debugGetStatus header comment stale. Still reads
  [FC, STATUS, running:u8, tick:u32 BE, uptime_ms:u32 BE] (no switch byte) and "running
  is always 1 on baremetal (the PLC scan is unconditional)" — both contradicted by the
  function body eight lines down, which writes runtime_get_plc_state() and appends
  mb_frame[12].

• extractImports line attribution. The new whole-source regex uses an unbounded lazy
  [\s\S]*? between import|export and from. A non-import export above the import block
  matches forward to a later import's path and reports it at the export's line. No import is
  missed (the lazy quantifier stops at the first from '…'), so this is cosmetic — violation
  reports can point at the wrong line. The multi-line fix itself is a clear improvement and
  worth having; it found a real pre-existing violation in the activity bar.

• DeviceSessionManager.acquireDebugChannel — concurrent-open leak. Two callers arriving
  while debugClientHeld === null both create() + await connect(); the second assignment
  overwrites the first, leaking a connected channel. Wants an in-flight promise. Narrow window,
  and I did not find a caller that hits it today.

• open() has no re-entrancy guard. A second open() while the first is awaiting
  tryCandidate calls close({ silent: true }), and the first then assigns this.client
  afterwards. Needs a double-click on Connect to reach; noting it since the module's whole
  premise is single ownership.

---

Notes, not objections

• declareLost calls close({ silent: true }), which clears candidates — so a serial device
  unplugged and replugged needs a manual reconnect rather than being picked up. Consistent with
  the documented gone → fail-now design (nothing to reset, nothing to wait for); flagging
  only so it reads as a conscious choice.
• noteTraffic is correctly called on success only, in both call sites (getVariablesList and
  setVariable). That invariant is what the whole liveness shortcut rests on, so it is worth
  stating that it holds.
• The run/stop command path and the status poll do not call noteTraffic. Only a missed
  optimisation, not a defect.

[coderabbitai]

Actionable comments posted: 1

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

src/main/modules/ipc/main.ts (1)

1891-1908: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win

Do not let a status read reject a verified candidate.

verifyDeviceCandidate awaits pushPlcState() after classifyDeviceLink returns connected-with-firmware, but DeviceSessionManager.tryCandidate treats any verify rejection as { ok: false }. A rejected status read can close an otherwise usable candidate. Wrap pushPlcState so failed reads only skip the initial push.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/main/modules/ipc/main.ts` around lines 1891 - 1908, The pushPlcState call
used by verifyDeviceCandidate must not propagate status-read failures and reject
an otherwise verified connected-with-firmware candidate. Catch failures around
the initial push after classifyDeviceLink succeeds, allowing verification to
continue while only skipping the PLC state update; keep normal successful push
behavior unchanged.

🧹 Nitpick comments (5)

src/frontend/components/_organisms/workspace-activity-bar/default.tsx (2)

537-538: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Replace the stateControl cast with a type guard.

Line 537 casts boardInfo to an inline shape to read stateControl.modeSwitch.label. That value comes from a VPP manifest, which is external data. The coding guidelines forbid type assertions and require a Zod schema or a type guard at the boundary. Declare stateControl on the board-info type, or narrow it with a guard before reading the label.

♻️ Proposed change

-    const switchLabel = (boardInfo as { stateControl?: { modeSwitch?: { label?: string } } } | undefined)?.stateControl
-      ?.modeSwitch?.label
+    const switchLabel = readModeSwitchLabel(boardInfo)

Add the guard outside this callback:

function readModeSwitchLabel(boardInfo: unknown): string | undefined {
  if (typeof boardInfo !== 'object' || boardInfo === null) return undefined
  const { stateControl } = boardInfo as Record<string, unknown>
  if (typeof stateControl !== 'object' || stateControl === null) return undefined
  const { modeSwitch } = stateControl as Record<string, unknown>
  if (typeof modeSwitch !== 'object' || modeSwitch === null) return undefined
  const { label } = modeSwitch as Record<string, unknown>
  return typeof label === 'string' ? label : undefined
}

A Zod schema for the manifest stateControl block is the cleaner option if one already exists for the surrounding board type.

As per coding guidelines: "Do not use type assertions, except as const" and "Validate external data at boundaries, including IPC payloads, project files, and downloaded-binary metadata, using Zod schemas or type guards instead of casts."

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/frontend/components/_organisms/workspace-activity-bar/default.tsx` around
lines 537 - 538, Replace the inline cast used to read the label in the
switch-label logic with boundary validation. Prefer an existing Zod schema for
the board manifest; otherwise add a type guard or helper such as
readModeSwitchLabel that safely narrows unknown stateControl, modeSwitch, and
label values, then use it at the callback call site without type assertions.

Source: Coding guidelines

---

591-599: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Replace the state-code assertion with named PlcRuntimeState values.

The ternary already produces 'RUNNING' | 'ERROR' | 'STOPPED'; add PlcRuntimeState to the imports/branch mapping and pass the status without as NonNullable<RuntimeConnection['plcStatus']> so this path avoids a type assertion and aligns 1/2 with PlcRuntimeState.RUNNING/PlcRuntimeState.ERROR.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/frontend/components/_organisms/workspace-activity-bar/default.tsx` around
lines 591 - 599, Update the result.state mapping in the workspace activity bar
to use the imported PlcRuntimeState.RUNNING and PlcRuntimeState.ERROR named
values, with STOPPED for other states. Remove the as
NonNullable<RuntimeConnection['plcStatus']> assertion and pass the resulting
PlcRuntimeState-compatible status directly to setPlcRuntimeStatus.

Source: Coding guidelines

src/frontend/utils/__tests__/device-connect-events.test.ts (1)

34-46: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Unsubscribe in a cleanup hook so one failure does not cascade.

Each test unsubscribes on its success path only. The assertions at Lines 42-43 run before unsubFirst() and unsubSecond() at Lines 44-45. If Line 42 fails, both listeners survive into the following tests and break their call-count assertions, which hides the original failure. Collect the unsubscribe functions and release them in an afterEach.

♻️ Proposed change

 describe('device flash-request bridge', () => {
+  const cleanups: Array<() => void> = []
+  const subscribe = (handler: () => void): void => {
+    cleanups.push(onDeviceFlashRequest(handler))
+  }
+
+  afterEach(() => {
+    while (cleanups.length > 0) cleanups.pop()?.()
+  })

Then replace the per-test onDeviceFlashRequest(...) calls with subscribe(...) and drop the manual unsubscribe() calls, except in the tests that assert unsubscribe behavior.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/frontend/utils/__tests__/device-connect-events.test.ts` around lines 34 -
46, Update the device-connect event tests to register cleanup callbacks in an
afterEach hook, using a shared subscription collection and subscribe helper for
onDeviceFlashRequest listeners. Replace per-test manual unsubscribe calls with
this cleanup, while retaining explicit unsubscribe calls only in tests that
verify unsubscribe behavior.

src/frontend/hooks/use-device-connect.ts (1)

57-61: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Reuse the device-port connect result type.

Import DeviceConnectResult and type result with it instead of duplicating the connect-status union. Replace the current branches with an exhaustive switch (result.status) and add a default: never case so future device.connect status values fail type checking instead of falling through.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/frontend/hooks/use-device-connect.ts` around lines 57 - 61, In
use-device-connect, import and reuse the existing DeviceConnectResult type for
result instead of duplicating its status union. Replace the result-status
branching with an exhaustive switch on result.status, including a default never
case so newly added device.connect statuses fail type checking rather than
falling through.

Source: Coding guidelines

src/frontend/hooks/__tests__/use-device-connect.test.ts (1)

12-17: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Type the mocked store state instead of casting it.

mockState.deviceConnection is declared as unknown, so currentStatus uses a forbidden type assertion. Give deviceConnection an explicit type, such as DeviceConnection, and read .status directly.

currentStatus appears only once, so there is no duplicate declaration issue.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/frontend/hooks/__tests__/use-device-connect.test.ts` around lines 12 -
17, Type mockState.deviceConnection explicitly with the appropriate
DeviceConnection type in the mocked store state, then update currentStatus to
access .status directly without a type assertion. Keep
mockSetDeviceConnectionStatus and the existing status-reading behavior
unchanged.

Source: Coding guidelines

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In
`@src/backend/editor/hardware/__tests__/device-session-channel-lifetime.test.ts`:
- Line 65: Replace the unsafe mock casts with contract-preserving typed test
doubles: in
src/backend/editor/hardware/__tests__/device-session-channel-lifetime.test.ts at
lines 65, 96, and 116, build a typed DeviceDebugChannel fixture, and at line 140
build a typed DeviceModbusTransport fixture. In
src/middleware/adapters/editor/__tests__/debugger-adapter.test.ts at lines 233
and 249, use jest.mocked(...) for bridge methods instead of as jest.Mock. In
src/middleware/adapters/editor/__tests__/device-adapter.test.ts at line 137, use
a typed Jest mock for window.bridge.deviceReleaseSerialPort.

---

Outside diff comments:
In `@src/main/modules/ipc/main.ts`:
- Around line 1891-1908: The pushPlcState call used by verifyDeviceCandidate
must not propagate status-read failures and reject an otherwise verified
connected-with-firmware candidate. Catch failures around the initial push after
classifyDeviceLink succeeds, allowing verification to continue while only
skipping the PLC state update; keep normal successful push behavior unchanged.

---

Nitpick comments:
In `@src/frontend/components/_organisms/workspace-activity-bar/default.tsx`:
- Around line 537-538: Replace the inline cast used to read the label in the
switch-label logic with boundary validation. Prefer an existing Zod schema for
the board manifest; otherwise add a type guard or helper such as
readModeSwitchLabel that safely narrows unknown stateControl, modeSwitch, and
label values, then use it at the callback call site without type assertions.
- Around line 591-599: Update the result.state mapping in the workspace activity
bar to use the imported PlcRuntimeState.RUNNING and PlcRuntimeState.ERROR named
values, with STOPPED for other states. Remove the as
NonNullable<RuntimeConnection['plcStatus']> assertion and pass the resulting
PlcRuntimeState-compatible status directly to setPlcRuntimeStatus.

In `@src/frontend/hooks/__tests__/use-device-connect.test.ts`:
- Around line 12-17: Type mockState.deviceConnection explicitly with the
appropriate DeviceConnection type in the mocked store state, then update
currentStatus to access .status directly without a type assertion. Keep
mockSetDeviceConnectionStatus and the existing status-reading behavior
unchanged.

In `@src/frontend/hooks/use-device-connect.ts`:
- Around line 57-61: In use-device-connect, import and reuse the existing
DeviceConnectResult type for result instead of duplicating its status union.
Replace the result-status branching with an exhaustive switch on result.status,
including a default never case so newly added device.connect statuses fail type
checking rather than falling through.

In `@src/frontend/utils/__tests__/device-connect-events.test.ts`:
- Around line 34-46: Update the device-connect event tests to register cleanup
callbacks in an afterEach hook, using a shared subscription collection and
subscribe helper for onDeviceFlashRequest listeners. Replace per-test manual
unsubscribe calls with this cleanup, while retaining explicit unsubscribe calls
only in tests that verify unsubscribe behavior.

🪄 Autofix

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 43aa918d-cd1f-4d01-9143-ce81bd2423e4

📥 Commits

Reviewing files that changed from the base of the PR and between 1c8ee64 and 87001fd.

⛔ Files ignored due to path filters (1)

• resources/sources/Baremetal/modbus_config.h is excluded by !resources/**

📒 Files selected for processing (20)

• src/backend/editor/compiler/compiler-module.spec.ts
• src/backend/editor/hardware/__tests__/device-session-channel-lifetime.test.ts
• src/backend/editor/hardware/device-session-manager.ts
• src/backend/shared/compile/__tests__/generate-defines.test.ts
• src/backend/shared/compile/__tests__/modbus-defines.test.ts
• src/backend/shared/compile/steps/generate-defines.ts
• src/backend/shared/compile/steps/modbus-defines.ts
• src/backend/shared/debug/__tests__/modbus-pdu-plc-control.test.ts
• src/backend/shared/hardware/__tests__/connect-resolve-regression.test.ts
• src/backend/shared/simulator/__tests__/modbus-rtu-client.test.ts
• src/frontend/components/_organisms/workspace-activity-bar/default.tsx
• src/frontend/hooks/__tests__/use-device-connect.test.ts
• src/frontend/hooks/use-device-connect.ts
• src/frontend/store/__tests__/device-slice.test.ts
• src/frontend/utils/__tests__/device-connect-events.test.ts
• src/main/modules/ipc/main.ts
• src/middleware/adapters/editor/__tests__/debugger-adapter.test.ts
• src/middleware/adapters/editor/__tests__/device-adapter.test.ts
• src/middleware/shared/utils/target-capabilities/presets.ts
• src/middleware/shared/utils/target-capabilities/types.ts

🚧 Files skipped from review as they are similar to previous changes (5)

• src/middleware/shared/utils/target-capabilities/types.ts
• src/backend/shared/compile/steps/generate-defines.ts
• src/backend/shared/debug/tests/modbus-pdu-plc-control.test.ts
• src/backend/shared/compile/tests/generate-defines.test.ts
• src/backend/editor/hardware/device-session-manager.ts

[coderabbitai]

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@src/frontend/hooks/useDebugPolling.ts`:
- Around line 396-418: Update the polling setup in the useEffect and its
interval callback to share one guarded poll starter that checks isPollingRef
before invoking getVariablesList. Use this same starter for the immediate poll
after debugMedium changes and subsequent interval ticks, preserving serialized
updates to batchOffsetRef and batchSizeRef.

🪄 Autofix

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 20a98658-0138-48ce-8d85-d60db18caa43

📥 Commits

Reviewing files that changed from the base of the PR and between 87001fd and 58bdcda.

📒 Files selected for processing (18)

• src/backend/editor/hardware/device-session-manager.ts
• src/backend/shared/debug/index.ts
• src/backend/shared/debug/types.ts
• src/frontend/components/_organisms/workspace-activity-bar/default.tsx
• src/frontend/hooks/__tests__/debug-medium-profile.test.ts
• src/frontend/hooks/useDebugPolling.ts
• src/frontend/hooks/useDebugSession.ts
• src/frontend/store/__tests__/webrtc-slice.test.ts
• src/frontend/store/__tests__/workspace-slice.test.ts
• src/frontend/store/slices/device/types.ts
• src/frontend/store/slices/webrtc/index.ts
• src/frontend/store/slices/webrtc/slice.ts
• src/frontend/store/slices/webrtc/types.ts
• src/frontend/store/slices/workspace/slice.ts
• src/frontend/store/slices/workspace/types.ts
• src/middleware/shared/ports/device-port.ts
• src/middleware/shared/ports/platform-capabilities.ts
• src/middleware/shared/ports/types.ts

💤 Files with no reviewable changes (4)

• src/frontend/store/tests/workspace-slice.test.ts
• src/frontend/store/slices/workspace/slice.ts
• src/frontend/store/slices/workspace/types.ts
• src/frontend/components/_organisms/workspace-activity-bar/default.tsx

🚧 Files skipped from review as they are similar to previous changes (5)

• src/backend/editor/hardware/device-session-manager.ts
• src/backend/shared/debug/types.ts
• src/middleware/shared/ports/device-port.ts
• src/frontend/store/slices/device/types.ts
• src/frontend/hooks/useDebugSession.ts

[thiagoralves]

Not all issues reported were valid. Fixed all valid issues and other minor issues as consequence of the fixes.

[coderabbitai]

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (8)

src/main/modules/ipc/main.ts (8)

2703-2724: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win

Stop the simulator when session opening throws.

loadAndRun starts the emulator before deviceSession.open. The !opened.ok branch stops it, but the catch branch only returns an error. If session opening rejects, the emulator remains running without a managed session.

Close the session and stop the simulator in the exception path.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/main/modules/ipc/main.ts` around lines 2703 - 2724, Update
handleSimulatorLoadFirmware’s catch path to close the device session and stop
the simulator when deviceSession.open or subsequent setup throws. Preserve the
existing error response from getErrorMessage(error), and ensure cleanup occurs
before returning the failure result.

---

1746-1760: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

Reject invalid PLC actions at the IPC boundary.

The 'run' | 'stop' annotation is erased at runtime. Any value other than 'run' selects the STOPPED branch, so malformed IPC data can stop the PLC. Validate action before REST routing and before calculating target.

Proposed fix

 handleDebuggerPlcControl = async (_event: IpcMainInvokeEvent, action: 'run' | 'stop'): Promise<PlcControlResult> => {
+  if (action !== 'run' && action !== 'stop') {
+    return { success: false, error: 'Invalid PLC action' }
+  }

As per coding guidelines: “Validate external data at boundaries, including IPC payloads, project files, and downloaded-binary metadata, using Zod schemas or type guards instead of casts.”

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/main/modules/ipc/main.ts` around lines 1746 - 1760, Validate the runtime
value of action at the start of handleDebuggerPlcControl, before trace logging,
REST routing, or target calculation, using the project’s established Zod schema
or type-guard approach. Reject any value other than "run" or "stop" with the
existing PlcControlResult error shape, while preserving the current valid-action
behavior.

Source: Coding guidelines

---

2365-2379: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick win

Validate candidates before opening a device link.

handleDeviceConnect trusts the renderer argument as DebugConnectionConfig[]. Type annotations do not validate IPC data. A non-array or malformed object can make toDeviceLinkCandidates throw or pass invalid transport parameters to the device factory.

Parse the payload with a Zod schema or an explicit type guard before iterating it.

As per coding guidelines: “Validate external data at boundaries, including IPC payloads, project files, and downloaded-binary metadata, using Zod schemas or type guards instead of casts.”

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/main/modules/ipc/main.ts` around lines 2365 - 2379, Validate the
renderer-supplied payload at the start of handleDeviceConnect before accessing
candidates.length, mapping it, or calling toDeviceLinkCandidates. Use the
project’s existing Zod schema or an explicit type guard to confirm the value is
an array of well-formed DebugConnectionConfig objects, and return the
established invalid-input outcome for malformed data instead of opening a device
link.

Source: Coding guidelines

---

2074-2087: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick win

Bind the runtime session to the authenticated runtime.

handleOpenRuntimeSession stores params.address from IPC without checking it against runtimeIp. Later REST control calls use that stored address with the current bearer token. A malformed or compromised renderer can route the token to another host.

Reject a session address that differs from the login address, or authenticate separately for the target. Validate the complete params payload before creating the session.

As per coding guidelines: “Validate external data at boundaries, including IPC payloads, project files, and downloaded-binary metadata, using Zod schemas or type guards instead of casts.”

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/main/modules/ipc/main.ts` around lines 2074 - 2087, Update
handleOpenRuntimeSession to validate the complete IPC params payload with the
established Zod schema or type-guard approach before use, including debug
connection fields and address. Reject any address that does not match the
authenticated runtimeIp, and only call deviceSession.openRestSession and update
debuggerConnectionType after validation succeeds.

Source: Coding guidelines

---

2203-2210: 🚀 Performance & Scalability | 🟡 Minor | ⚡ Quick win

Reuse the status frame during liveness probing.

Line 2206 reads status, then Line 2209 calls pushPlcState with an interval of 0. That helper performs another getStatus call. This doubles FC 0x46 traffic and can queue a second request on a single-client serial or TCP link.

Pass the first status result to the push helper, or emit it directly.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/main/modules/ipc/main.ts` around lines 2203 - 2210, Update
probeDeviceLink to reuse the status result returned by client.getStatus when
invoking pushPlcState, avoiding a second getStatus request. Extend or use the
helper’s existing status-frame input as needed while preserving the current
failure check, plcStatePushedAt reset, and successful return behavior.

---

2403-2408: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick win

Reset debugger state when the device session closes.

handleDebuggerGetVariablesList treats a non-null debuggerConnectionType as an active debugger. handleDeviceDisconnect closes deviceSession but does not clear this field. A disconnect during debugging can therefore keep the poll loop active and return channel errors instead of the intentional disconnected state.

   this.deviceSession.close()
+  this.debuggerConnectionType = null
   this.deviceLinkProbe = null

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/main/modules/ipc/main.ts` around lines 2403 - 2408, Update
handleDeviceDisconnect to clear debuggerConnectionType when closing the device
session, alongside resetting deviceLinkProbe and tracedChannelUses, so debugger
polling observes the disconnected state.

---

2103-2122: 🔒 Security & Privacy | 🟠 Major | 🏗️ Heavy lift

Keep certificate verification enabled for runtime WebSocket sessions.

rejectUnauthorized: false accepts any certificate for the WebSocket that carries the JWT. A network attacker can impersonate the runtime and capture the token.

Use a trusted CA or explicit per-device certificate or fingerprint approval. Do not disable verification globally.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/main/modules/ipc/main.ts` around lines 2103 - 2122, Update
toDebugCandidate’s websocket transport creation to keep TLS certificate
verification enabled instead of setting rejectUnauthorized to false. Use the
existing trusted CA or explicit per-device certificate/fingerprint approval
mechanism, without disabling verification globally, while preserving the current
host, port, and token handling.

---

709-731: 🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift

Validate runtime responses before using the declared shapes.

JSON.parse(data) as ... and the later raw as ... checks only silence TypeScript. They do not validate status, task fields, plugin_stats, or switchPosition. Malformed /api/status data can reach the renderer as a valid result. The same pattern in restStartPlc can turn a non-string status into an exception path before .trim().

Parse into unknown, then validate with a Zod schema or an explicit type guard.

As per coding guidelines: “Validate external data at boundaries, including IPC payloads, project files, and downloaded-binary metadata, using Zod schemas or type guards instead of casts.” Also, “Do not use type assertions, except as const.”

Also applies to: 1796-1802

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/main/modules/ipc/main.ts` around lines 709 - 731, Replace the asserted
JSON parsing in the /api/status response callback with parsing into unknown
followed by a Zod schema or explicit type guard that validates status,
timing_stats task fields and plugin_stats, and switchPosition before
constructing the renderer result. Apply the same boundary validation in
restStartPlc so status is confirmed to be a string before trim() is called.
Remove the raw as-casts and only pass validated data onward.

Source: Coding guidelines

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Outside diff comments:
In `@src/main/modules/ipc/main.ts`:
- Around line 2703-2724: Update handleSimulatorLoadFirmware’s catch path to
close the device session and stop the simulator when deviceSession.open or
subsequent setup throws. Preserve the existing error response from
getErrorMessage(error), and ensure cleanup occurs before returning the failure
result.
- Around line 1746-1760: Validate the runtime value of action at the start of
handleDebuggerPlcControl, before trace logging, REST routing, or target
calculation, using the project’s established Zod schema or type-guard approach.
Reject any value other than "run" or "stop" with the existing PlcControlResult
error shape, while preserving the current valid-action behavior.
- Around line 2365-2379: Validate the renderer-supplied payload at the start of
handleDeviceConnect before accessing candidates.length, mapping it, or calling
toDeviceLinkCandidates. Use the project’s existing Zod schema or an explicit
type guard to confirm the value is an array of well-formed DebugConnectionConfig
objects, and return the established invalid-input outcome for malformed data
instead of opening a device link.
- Around line 2074-2087: Update handleOpenRuntimeSession to validate the
complete IPC params payload with the established Zod schema or type-guard
approach before use, including debug connection fields and address. Reject any
address that does not match the authenticated runtimeIp, and only call
deviceSession.openRestSession and update debuggerConnectionType after validation
succeeds.
- Around line 2203-2210: Update probeDeviceLink to reuse the status result
returned by client.getStatus when invoking pushPlcState, avoiding a second
getStatus request. Extend or use the helper’s existing status-frame input as
needed while preserving the current failure check, plcStatePushedAt reset, and
successful return behavior.
- Around line 2403-2408: Update handleDeviceDisconnect to clear
debuggerConnectionType when closing the device session, alongside resetting
deviceLinkProbe and tracedChannelUses, so debugger polling observes the
disconnected state.
- Around line 2103-2122: Update toDebugCandidate’s websocket transport creation
to keep TLS certificate verification enabled instead of setting
rejectUnauthorized to false. Use the existing trusted CA or explicit per-device
certificate/fingerprint approval mechanism, without disabling verification
globally, while preserving the current host, port, and token handling.
- Around line 709-731: Replace the asserted JSON parsing in the /api/status
response callback with parsing into unknown followed by a Zod schema or explicit
type guard that validates status, timing_stats task fields and plugin_stats, and
switchPosition before constructing the renderer result. Apply the same boundary
validation in restStartPlc so status is confirmed to be a string before trim()
is called. Remove the raw as-casts and only pass validated data onward.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 83ac08e2-2588-4eb2-8c2f-bd1572e679b9

📥 Commits

Reviewing files that changed from the base of the PR and between 58bdcda and 7fd4fd4.

📒 Files selected for processing (1)

• src/main/modules/ipc/main.ts