From a47552e35fd4c983e46cb3f2f3297efbbe47d1ab Mon Sep 17 00:00:00 2001 From: Thiago Alves Date: Tue, 4 Aug 2026 19:50:02 -0400 Subject: [PATCH 1/6] fix(runtime-v4): report a switch in STOP as a warning, not a failed upload Uploading with the hardware mode switch in STOP ended with: Compilation completed successfully (exit code: 0). Failed to start PLC: START:ERROR_SWITCH_STOP Failed to upload to runtime. Stopping compilation process. All three lines after the first are wrong. The program did reach the device and compiled there; the runtime simply declined to start it, which is what a mode switch in STOP is for. Leaving the switch there while uploading is a normal thing to do, so the message should not send anyone looking for a problem. startPlcAfterBuild now recognises ERROR_SWITCH_STOP as its own outcome and explains it as a warning: "Program uploaded. The PLC was not started because the mode switch is in STOP -- move it to RUN to start." It is not retried either; nothing changes until a human moves the switch, so the 5 s BUSY loop had no business spinning on it. deployRuntimeProgram maps that to UPLOADED_NOT_STARTED, and both platform adapters treat it as a successful upload. Whether an outcome means "the program reached the device" now lives in one shared predicate, deployReachedDevice(), rather than being re-derived as `outcome === 'STARTED'` in each adapter -- the editor had it in two places and web in a third, which is how they would drift. Not addressed here, but noticed: START_TIMEOUT still maps to a failed upload, so a runtime that stays BUSY past the deadline reports "Failed to upload to runtime" after logging a warning that says otherwise. Same shape of bug, different trigger. --- .../compiler/editor-compiler-platform-port.ts | 14 ++++++++++--- .../__tests__/start-plc-after-build.test.ts | 19 +++++++++++++++++ .../shared/library/deploy-runtime-program.ts | 21 +++++++++++++++++++ .../shared/library/start-plc-after-build.ts | 19 ++++++++++++++++- 4 files changed, 69 insertions(+), 4 deletions(-) diff --git a/src/backend/editor/compiler/editor-compiler-platform-port.ts b/src/backend/editor/compiler/editor-compiler-platform-port.ts index 8b689d18d..8c78488b2 100644 --- a/src/backend/editor/compiler/editor-compiler-platform-port.ts +++ b/src/backend/editor/compiler/editor-compiler-platform-port.ts @@ -24,7 +24,7 @@ * `middleware/adapters/web/`. */ -import { deployRuntimeProgram } from '@root/backend/shared/library/deploy-runtime-program' +import { deployReachedDevice, deployRuntimeProgram } from '@root/backend/shared/library/deploy-runtime-program' import { probeRuntimeVersion } from '@root/backend/shared/library/probe-runtime-version' import { fromSchemaShape, @@ -387,7 +387,11 @@ export function createEditorCompilerPlatformPort( startIntervalMs: context.startIntervalMs, }) - return { ok: deployOutcome === 'STARTED' } + // 'STARTED' is not the only success: a runtime that refused to start + // because its hardware mode switch reads STOP has still taken the + // program. deployReachedDevice keeps that judgement in one place, shared + // with the web adapter. + return { ok: deployReachedDevice(deployOutcome) } } catch (error) { const message = error instanceof Error ? error.message : String(error) log(`Runtime v4 upload failed: ${message}`, 'error') @@ -486,7 +490,11 @@ export function createEditorCompilerPlatformPort( startTimeoutMs: context.startTimeoutMs, startIntervalMs: context.startIntervalMs, }) - return { ok: deployOutcome === 'STARTED' } + // 'STARTED' is not the only success: a runtime that refused to start + // because its hardware mode switch reads STOP has still taken the + // program. deployReachedDevice keeps that judgement in one place, shared + // with the web adapter. + return { ok: deployReachedDevice(deployOutcome) } } catch (error) { const message = error instanceof Error ? error.message : String(error) log(`Runtime v3 upload failed: ${message}`, 'error') diff --git a/src/backend/shared/library/__tests__/start-plc-after-build.test.ts b/src/backend/shared/library/__tests__/start-plc-after-build.test.ts index 15d97ab21..6829b160a 100644 --- a/src/backend/shared/library/__tests__/start-plc-after-build.test.ts +++ b/src/backend/shared/library/__tests__/start-plc-after-build.test.ts @@ -54,6 +54,25 @@ describe('startPlcAfterBuild', () => { expect(calls()).toBe(3) }) + it('treats a mode switch in STOP as a warning, not a failure', async () => { + // The runtime refuses to start while a hardware switch reads STOP. The + // program is on the device, so this must NOT read as a failed upload -- that + // sent users hunting for a problem that did not exist. + const { fetch, calls } = scriptedFetch(['START:ERROR_SWITCH_STOP']) + const logs: Array<{ level: string; message: string }> = [] + const outcome = await startPlcAfterBuild({ + fetchStart: fetch, + onLog: (level, message) => logs.push({ level, message }), + pollIntervalMs: 1, + }) + expect(outcome).toBe('SWITCH_IN_STOP') + // Not retried: nothing changes until a human moves the switch. + expect(calls()).toBe(1) + expect(logs[logs.length - 1].level).toBe('warning') + expect(logs[logs.length - 1].message).toMatch(/uploaded/i) + expect(logs[logs.length - 1].message).toMatch(/switch is in STOP/i) + }) + it('bails with FAILED on a non-BUSY error reply', async () => { const { fetch, calls } = scriptedFetch(['ERROR:INVALID_PROGRAM']) const logs: Array<{ level: string; message: string }> = [] diff --git a/src/backend/shared/library/deploy-runtime-program.ts b/src/backend/shared/library/deploy-runtime-program.ts index 783ae5676..2ae037970 100644 --- a/src/backend/shared/library/deploy-runtime-program.ts +++ b/src/backend/shared/library/deploy-runtime-program.ts @@ -23,6 +23,7 @@ import { startPlcAfterBuild, type StartPlcAfterBuildOptions } from './start-plc- export type DeployRuntimeProgramOutcome = | 'STARTED' + | 'UPLOADED_NOT_STARTED' | 'UPLOAD_FAILED' | 'BUILD_FAILED' | 'BUILD_TIMEOUT' @@ -30,6 +31,23 @@ export type DeployRuntimeProgramOutcome = | 'START_FAILED' | 'START_TIMEOUT' +/** + * Did the program reach the device? + * + * Distinct from "did it start", because those are different questions and the + * upload step must only fail for the first one. A runtime that refuses to start + * while its hardware mode switch reads STOP has still taken the program, and + * reporting that as a failed upload sends the user looking for a problem that + * does not exist. + * + * Shared rather than re-derived per platform: the editor and web adapters both + * turn this outcome into `UploadResult.ok`, and they must not drift on which + * outcomes count. + */ +export function deployReachedDevice(outcome: DeployRuntimeProgramOutcome): boolean { + return outcome === 'STARTED' || outcome === 'UPLOADED_NOT_STARTED' +} + export type DeployRuntimeProgramLogLevel = 'info' | 'error' | 'warning' | 'debug' export interface DeployRuntimeProgramOptions { @@ -109,6 +127,9 @@ export async function deployRuntimeProgram(opts: DeployRuntimeProgramOptions): P pollIntervalMs: opts.startIntervalMs, }) if (startOutcome === 'STARTED') return 'STARTED' + // Refused by the hardware mode switch: uploaded, deliberately not running. + // startPlcAfterBuild has already explained it as a warning. + if (startOutcome === 'SWITCH_IN_STOP') return 'UPLOADED_NOT_STARTED' if (startOutcome === 'TIMEOUT') return 'START_TIMEOUT' return 'START_FAILED' } diff --git a/src/backend/shared/library/start-plc-after-build.ts b/src/backend/shared/library/start-plc-after-build.ts index 583b60a51..fda248154 100644 --- a/src/backend/shared/library/start-plc-after-build.ts +++ b/src/backend/shared/library/start-plc-after-build.ts @@ -21,12 +21,17 @@ * - 150-ms interval between attempts. * - `START:OK` or `ALREADY_RUNNING` in the runtime's reply → SUCCESS. * - `BUSY` in the reply → keep retrying. + * - `ERROR_SWITCH_STOP` → SWITCH_IN_STOP: not an error. The runtime + * refuses to start while a hardware mode switch reads STOP, and + * leaving the switch there is a normal thing for someone to do + * while uploading. The upload succeeded; only the start was + * declined, and the user decides when to allow it. * - Any other reply → terminal failure (e.g. invalid program). * - Network error on the fetch → terminal failure. * - Deadline elapsed → TIMEOUT (a warning, not a fatal error). */ -export type StartPlcAfterBuildOutcome = 'STARTED' | 'FAILED' | 'TIMEOUT' +export type StartPlcAfterBuildOutcome = 'STARTED' | 'FAILED' | 'TIMEOUT' | 'SWITCH_IN_STOP' export type StartPlcAfterBuildLogLevel = 'info' | 'error' | 'warning' @@ -74,6 +79,18 @@ export async function startPlcAfterBuild(opts: StartPlcAfterBuildOptions): Promi return 'STARTED' } + // A hardware mode switch in STOP is a refusal, not a failure: the + // program is on the device, and the runtime is doing exactly what it + // should by declining to run it until the switch says so. Retrying + // would be wrong too — nothing changes until someone moves it. + if (status.includes('ERROR_SWITCH_STOP')) { + opts.onLog( + 'warning', + 'Program uploaded. The PLC was not started because the mode switch is in STOP — move it to RUN to start.', + ) + return 'SWITCH_IN_STOP' + } + // Only BUSY is retryable — everything else is a real error // from the runtime (invalid program, compile error, …). if (!status.includes('BUSY')) { From 20ff67e0e8315c109a808762f01993df1b421dee Mon Sep 17 00:00:00 2001 From: Thiago Alves Date: Tue, 4 Aug 2026 22:32:35 -0400 Subject: [PATCH 2/6] fix(runtime-v4): block run/stop while the runtime is mid-transition MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit TRANSITIONING means a start or stop is already underway: the runtime answers COMMAND:BUSY to everything except PING and STATUS, and the state it will settle on is not decided yet — so the icon is drawn from a state that is about to change and a click cannot do what it appears to. Folded into the existing plcControlBlocked / plcControlBlockedReason pair rather than added as a second mechanism, so the tooltip explains this the same way it explains a missing connection: 'PLC is changing state...'. handlePlcControl carries the same guard. Status arrives by poll, so a render can be up to one interval stale; the guard closes the window where the button still looks live and covers callers that are not the click. --- .../workspace-activity-bar/default.tsx | 23 +++++++++++++++---- 1 file changed, 19 insertions(+), 4 deletions(-) diff --git a/src/frontend/components/_organisms/workspace-activity-bar/default.tsx b/src/frontend/components/_organisms/workspace-activity-bar/default.tsx index f50a79eac..e2b86ef82 100644 --- a/src/frontend/components/_organisms/workspace-activity-bar/default.tsx +++ b/src/frontend/components/_organisms/workspace-activity-bar/default.tsx @@ -104,11 +104,21 @@ export const DefaultWorkspaceActivityBar = ({ zoom }: DefaultWorkspaceActivityBa // reason, and says so. `handlePlcControl` refuses such a target anyway, so // without this the button looked live and the click did nothing at all — no // command, no error, no log line. + // + // A transition already in flight blocks it for a third reason. TRANSITIONING + // means the runtime has a start or stop underway: it answers COMMAND:BUSY to + // everything except PING and STATUS, and the state it will settle on is not + // decided yet, so the icon is drawn from a state that is about to change. + // Clicking then cannot do what it appears to. const plcStateControlSupported = resolveTargetCapabilities(currentBoardInfo).plcStateControl - const plcControlBlocked = !plcStateControlSupported || deviceConnectionStatus !== 'connected' - const plcControlBlockedReason = plcStateControlSupported - ? 'Connect to the target first' - : 'This target does not support Start/Stop from the editor' + const plcTransitioning = plcStatus === 'TRANSITIONING' + const plcControlBlocked = + !plcStateControlSupported || deviceConnectionStatus !== 'connected' || plcTransitioning + const plcControlBlockedReason = !plcStateControlSupported + ? 'This target does not support Start/Stop from the editor' + : plcTransitioning + ? 'PLC is changing state...' + : 'Connect to the target first' // The emulator stopping is a session ending, and a debug session riding it ends // with it — which the drop handler below already does for every target. This @@ -547,6 +557,11 @@ export const DefaultWorkspaceActivityBar = ({ zoom }: DefaultWorkspaceActivityBa // `switchPosition` in the store, so the pre-check is a store lookup rather than // another round trip over a medium the poll is already using. try { + // The button is disabled while a transition is in flight; this covers the + // window before the next status poll catches up, and any caller that is not + // the click. + if (plcStatus === 'TRANSITIONING') return + const wantRun = plcStatus !== 'RUNNING' // Never send a start to a device whose switch reads STOP. `null` means From dfef0a391f1fd51e21e8dbe0fb6a1baa980a9a4f Mon Sep 17 00:00:00 2001 From: Thiago Alves Date: Thu, 6 Aug 2026 16:40:15 -0400 Subject: [PATCH 3/6] fix(device): restore the Connect button's font size and drop the duplicate status MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The button rendered noticeably larger and heavier than the buttons beside it. The cause is cn(): it runs twMerge, which does not know this project's custom cp-* font scale, so it read `text-cp-sm` as conflicting with `text-white` and dropped the size class entirely. The button fell back to the browser default -- larger than the 10px intended AND larger than its 14px neighbours. Sibling brand buttons on this screen escape it only because they pass a plain string rather than cn(). Now h-8/text-sm, matching those siblings, and a size twMerge understands. Also removes both redundant "Connected" labels. The button label already is the status: "Disconnect" can only be shown while connected, "Connect" only while disconnected, so the green "● Connected" beside it and the grey "Connected" after it said the same thing twice more. "PLC: RUNNING" stays -- that is the program's state, which the button says nothing about. "● Connection failed" stays too: the button reads "Connect" whether the last attempt failed or never happened, so that one carries information the label does not. --- .../editor/device/configuration/board.tsx | 22 +++---------------- .../__tests__/device-connect-button.test.tsx | 9 ++++---- .../device-connect-button/index.tsx | 8 +++++-- 3 files changed, 14 insertions(+), 25 deletions(-) diff --git a/src/frontend/components/_features/[workspace]/editor/device/configuration/board.tsx b/src/frontend/components/_features/[workspace]/editor/device/configuration/board.tsx index 6a14c8240..51aaf936c 100644 --- a/src/frontend/components/_features/[workspace]/editor/device/configuration/board.tsx +++ b/src/frontend/components/_features/[workspace]/editor/device/configuration/board.tsx @@ -27,15 +27,6 @@ import { ScanCycleStats } from '../../../../../_molecules/scan-cycle-stats' import { DeviceEditorSlot } from '../../../../../_templates/[editors]/device-editor-slot' import { PinMappingTable } from './components/pin-mapping-table' -/** - * Confirms the held device link on the device screen: a quiet, monochrome line - * that appears once Connect has settled on a channel a firmware answered. - */ -function DeviceConnectedIndicator({ isConnected }: { isConnected: boolean }) { - if (!isConnected) return null - return Connected -} - const Board = memo(function () { const capabilities = useCapabilities() const device = useDevice() @@ -634,13 +625,8 @@ const Board = memo(function () { onConnect={handleConnectToRuntime} onDisconnect={handleConnectToRuntime} > - {connectionStatus === 'connected' && ( - <> - {plcStatus && ( - | PLC: {plcStatus} - )} - - + {connectionStatus === 'connected' && plcStatus && ( + PLC: {plcStatus} )} @@ -718,9 +704,7 @@ const Board = memo(function () { {...(!isConnected && !communicationPort && !modbusTcpConfigured ? { blockedReason: 'Select a communication port first' } : {})} - > - - + /> ) : null} {!isOpenPLCRuntimeTarget(currentBoardInfo) && !isSimulatorTarget(currentBoardInfo) && ( diff --git a/src/frontend/components/_molecules/device-connect-button/__tests__/device-connect-button.test.tsx b/src/frontend/components/_molecules/device-connect-button/__tests__/device-connect-button.test.tsx index 6fc9bdc76..e34c4ff6c 100644 --- a/src/frontend/components/_molecules/device-connect-button/__tests__/device-connect-button.test.tsx +++ b/src/frontend/components/_molecules/device-connect-button/__tests__/device-connect-button.test.tsx @@ -30,11 +30,12 @@ describe('DeviceConnectButton', () => { expect(onConnect).not.toHaveBeenCalled() }) - it('confirms a live connection on screen', () => { - // The baremetal copy never showed this, so a connected device looked the same - // as a disconnected one apart from the button label. + it('says Disconnect when connected, and nothing else', () => { + // The label IS the status: "Disconnect" can only appear while connected, so a + // separate "Connected" badge beside it was saying the same thing twice. render() - expect(screen.getByText('● Connected')).not.toBeNull() + expect(screen.getByRole('button').textContent).toBe('Disconnect') + expect(screen.queryByText(/Connected/)).toBeNull() }) it('reports a failed attempt', () => { diff --git a/src/frontend/components/_molecules/device-connect-button/index.tsx b/src/frontend/components/_molecules/device-connect-button/index.tsx index 796c59ac4..90c38f0ec 100644 --- a/src/frontend/components/_molecules/device-connect-button/index.tsx +++ b/src/frontend/components/_molecules/device-connect-button/index.tsx @@ -51,15 +51,19 @@ const DeviceConnectButton = ({ onClick={isConnected ? onDisconnect : onConnect} disabled={disabled} title={blockedReason ?? (isConnected ? 'Disconnect from the device' : 'Connect to the device')} + // text-sm, not the project's text-cp-sm: cn() runs twMerge, which does not + // know the custom cp-* font scale and therefore treated `text-cp-sm` as + // conflicting with `text-white`, dropping the size entirely. The button fell + // back to the browser default and rendered larger than its own neighbours. + // h-8/text-sm also matches the sibling brand buttons on this screen. className={cn( - 'h-[30px] rounded-md bg-brand px-4 py-1 font-caption text-cp-sm font-medium text-white', + 'h-8 rounded-md bg-brand px-4 font-caption text-sm font-medium text-white', 'hover:bg-brand-medium-dark disabled:opacity-50', )} > {isConnecting ? 'Connecting...' : isConnected ? 'Disconnect' : 'Connect'} - {isConnected && ● Connected} {status === 'error' && ● Connection failed} {children} From a45b99c0af79874d0e2b0512383773a96569387d Mon Sep 17 00:00:00 2001 From: Thiago Alves Date: Thu, 6 Aug 2026 16:50:53 -0400 Subject: [PATCH 4/6] fix(ui): teach cn() the cp-* font scale instead of resizing the button My previous attempt made the Connect button WORSE, not better. The diagnosis was right -- twMerge silently drops `text-cp-sm` because it cannot tell a custom size from a text colour, so `text-cp-sm text-white` keeps only the colour -- but the fix was wrong: I moved the button to text-sm (14px) when main renders it at text-cp-sm (10px). It stayed too big, just for a new reason. main gets away with the same class string only because it passes a plain string instead of calling cn(). So the bug is in cn(), not in any one button: extendTailwindMerge now declares cp-xs / cp-sm / cp-base as font sizes, and `text-cp-sm text-white` keeps both. Size-versus-size still dedupes correctly. The buttons are back to main's exact classes, which now render as intended. This also silently repairs every other call site that puts a cp-* size and a text colour through cn() -- there is no warning when it happens, only a wrong size. --- .../device-connect-button/index.tsx | 11 ++++----- src/frontend/utils/cn.ts | 24 ++++++++++++++++++- 2 files changed, 28 insertions(+), 7 deletions(-) diff --git a/src/frontend/components/_molecules/device-connect-button/index.tsx b/src/frontend/components/_molecules/device-connect-button/index.tsx index 90c38f0ec..f2006a0c3 100644 --- a/src/frontend/components/_molecules/device-connect-button/index.tsx +++ b/src/frontend/components/_molecules/device-connect-button/index.tsx @@ -51,13 +51,12 @@ const DeviceConnectButton = ({ onClick={isConnected ? onDisconnect : onConnect} disabled={disabled} title={blockedReason ?? (isConnected ? 'Disconnect from the device' : 'Connect to the device')} - // text-sm, not the project's text-cp-sm: cn() runs twMerge, which does not - // know the custom cp-* font scale and therefore treated `text-cp-sm` as - // conflicting with `text-white`, dropping the size entirely. The button fell - // back to the browser default and rendered larger than its own neighbours. - // h-8/text-sm also matches the sibling brand buttons on this screen. + // Same classes main used for this button. They render correctly again now + // that cn() knows the cp-* font scale -- before that, twMerge dropped + // `text-cp-sm` as a conflict with `text-white` and the button jumped to the + // browser default size. className={cn( - 'h-8 rounded-md bg-brand px-4 font-caption text-sm font-medium text-white', + 'h-[30px] rounded-md bg-brand px-4 py-1 font-caption text-cp-sm font-medium text-white', 'hover:bg-brand-medium-dark disabled:opacity-50', )} > diff --git a/src/frontend/utils/cn.ts b/src/frontend/utils/cn.ts index c8498c564..7916262b7 100644 --- a/src/frontend/utils/cn.ts +++ b/src/frontend/utils/cn.ts @@ -1,4 +1,26 @@ import { ClassValue, clsx } from 'clsx' -import { twMerge } from 'tailwind-merge' +import { extendTailwindMerge } from 'tailwind-merge' + +/** + * Class merger that knows this project's custom `cp-*` font scale. + * + * Plain `twMerge` does not. Faced with `text-cp-sm text-white` it cannot tell that + * the first is a size, assumes both are text colours, and keeps only the last -- + * silently dropping the size. Anything styled that way rendered at the browser + * default instead: the Connect button came out visibly larger than its + * neighbours, with no warning and no build error. Buttons that escaped it did so + * only by passing a plain string instead of calling this. + * + * Declaring the scale here fixes every call site at once, rather than each caller + * having to know that its size class might evaporate. Keep this list in step with + * `fontSize` in tailwind.config.ts. + */ +const twMerge = extendTailwindMerge({ + extend: { + classGroups: { + 'font-size': [{ text: ['cp-xs', 'cp-sm', 'cp-base'] }], + }, + }, +}) export const cn = (...inputs: ClassValue[]) => twMerge(clsx(inputs)) From 50219cfdc220dd37f5d1d13cc1d12e9ad4a48d97 Mon Sep 17 00:00:00 2001 From: Thiago Alves Date: Fri, 7 Aug 2026 11:03:10 -0400 Subject: [PATCH 5/6] test: cover deployReachedDevice and the cp-* font scale MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Both files this PR touches sit under an enforced 100% functions/lines threshold, and both shipped a change no test executed. `deployReachedDevice` had 0% function coverage — nothing called it. It now has direct assertions for all three outcome classes, driven off a `Record` so a new outcome fails to compile until someone decides which side of "did the program reach the device?" it falls on. The `SWITCH_IN_STOP -> UPLOADED_NOT_STARTED` mapping was statement-covered but never asserted; a `deployRuntimeProgram` scenario now scripts `START:ERROR_SWITCH_STOP` and checks the outcome, that start is asked exactly once, and that the refusal logs as a warning and not an error. `cn()`'s font-scale fix shipped with `cn.test.ts` untouched. The suite now pins the exact regression — `cn('text-cp-sm', 'text-white')` keeps both classes, `cn('text-cp-xs', 'text-cp-base')` last-wins — and reads the scale out of tailwind.config.ts rather than restating it, so the "keep this list in step" comment on `cn()` is enforced instead of hoped for. Verified it fails against plain twMerge. Both files stay byte-identical with openplc-web#655. Co-Authored-By: Claude Opus 5 (1M context) --- .../__tests__/deploy-runtime-program.test.ts | 82 ++++++++++++++++++- src/frontend/utils/__tests__/cn.test.ts | 55 +++++++++++++ 2 files changed, 136 insertions(+), 1 deletion(-) diff --git a/src/backend/shared/library/__tests__/deploy-runtime-program.test.ts b/src/backend/shared/library/__tests__/deploy-runtime-program.test.ts index e19623923..78486b345 100644 --- a/src/backend/shared/library/__tests__/deploy-runtime-program.test.ts +++ b/src/backend/shared/library/__tests__/deploy-runtime-program.test.ts @@ -1,4 +1,4 @@ -import { deployRuntimeProgram } from '../deploy-runtime-program' +import { deployReachedDevice, type DeployRuntimeProgramOutcome, deployRuntimeProgram } from '../deploy-runtime-program' import type { RuntimeCompilationStatus } from '../poll-runtime-compilation' function makeStatusFetcher(...responses: Array) { @@ -133,4 +133,84 @@ describe('deployRuntimeProgram', () => { }) expect(outcome).toBe('START_TIMEOUT') }) + + it('returns UPLOADED_NOT_STARTED when the hardware mode switch declines the start', async () => { + const start = jest.fn(makeStartFetcher('START:ERROR_SWITCH_STOP')) + const logs: Array<{ level: string; message: string }> = [] + + const outcome = await deployRuntimeProgram({ + uploadProgram: async () => ({ success: true }), + fetchCompilationStatus: makeStatusFetcher({ status: 'SUCCESS', logs: [], exit_code: 0 }), + fetchStartResponse: start, + onLog: (level, message) => logs.push({ level, message }), + pollIntervalMs: 1, + startIntervalMs: 1, + }) + + expect(outcome).toBe('UPLOADED_NOT_STARTED') + // Asked once and accepted the answer: nothing changes until someone moves + // the switch, so retrying until the deadline would only stall the deploy. + expect(start).toHaveBeenCalledTimes(1) + // Reported as a warning, never an error — the program is on the device. + expect(logs.some((l) => l.level === 'warning')).toBe(true) + expect(logs.some((l) => l.level === 'error')).toBe(false) + }) +}) + +describe('deployReachedDevice', () => { + /** + * Exhaustive by construction: a new member of `DeployRuntimeProgramOutcome` + * fails to type-check here until someone decides which side of the + * "did the program reach the device?" line it falls on. + */ + const reachedDeviceByOutcome: Record = { + STARTED: true, + UPLOADED_NOT_STARTED: true, + UPLOAD_FAILED: false, + BUILD_FAILED: false, + BUILD_TIMEOUT: false, + BUILD_ERROR: false, + START_FAILED: false, + START_TIMEOUT: false, + } + + /** Typed walk over the table above — the length check below keeps the two in step. */ + const allOutcomes: DeployRuntimeProgramOutcome[] = [ + 'STARTED', + 'UPLOADED_NOT_STARTED', + 'UPLOAD_FAILED', + 'BUILD_FAILED', + 'BUILD_TIMEOUT', + 'BUILD_ERROR', + 'START_FAILED', + 'START_TIMEOUT', + ] + + it('classifies every outcome the deploy can produce', () => { + expect(allOutcomes).toHaveLength(Object.keys(reachedDeviceByOutcome).length) + }) + + it('is true when the runtime took the program and ran it', () => { + expect(deployReachedDevice('STARTED')).toBe(true) + }) + + it('is true when the runtime took the program but declined to run it', () => { + // The mode switch reads STOP. Reporting this as a failed upload sends the + // user looking for a problem that does not exist. + expect(deployReachedDevice('UPLOADED_NOT_STARTED')).toBe(true) + }) + + it('is false for every outcome where the program never landed', () => { + const failures = allOutcomes.filter((outcome) => !reachedDeviceByOutcome[outcome]) + expect(failures).toHaveLength(6) + for (const outcome of failures) { + expect(deployReachedDevice(outcome)).toBe(false) + } + }) + + it('agrees with the full outcome table', () => { + for (const outcome of allOutcomes) { + expect(deployReachedDevice(outcome)).toBe(reachedDeviceByOutcome[outcome]) + } + }) }) diff --git a/src/frontend/utils/__tests__/cn.test.ts b/src/frontend/utils/__tests__/cn.test.ts index 087bbc300..e19d522a6 100644 --- a/src/frontend/utils/__tests__/cn.test.ts +++ b/src/frontend/utils/__tests__/cn.test.ts @@ -1,5 +1,25 @@ +import { readFileSync } from 'node:fs' +import { join } from 'node:path' + import { cn } from '../cn' +/** + * The `cp-*` font scale is declared twice: once in `tailwind.config.ts`, where + * it generates the classes, and once inside `cn()`, where twMerge has to be + * told those names are sizes. Read the scale out of the config rather than + * restating it here, so the "keep this list in step" comment on `cn()` is + * enforced by this suite instead of hoped for — a fourth size added to + * Tailwind and forgotten in `cn()` fails below rather than silently + * evaporating at runtime, which is exactly how the Connect button lost its + * `text-cp-sm` with no warning and no build error. + */ +function tailwindFontSizeNames(): string[] { + const config = readFileSync(join(process.cwd(), 'tailwind.config.ts'), 'utf8') + const block = /fontSize:\s*\{([\s\S]*?)\n\s*\},/.exec(config) + if (!block) throw new Error('No `fontSize` block found in tailwind.config.ts') + return [...block[1].matchAll(/'([\w-]+)':/g)].map(([, name]) => name) +} + describe('cn', () => { it('merges class names', () => { expect(cn('foo', 'bar')).toBe('foo bar') @@ -26,3 +46,38 @@ describe('cn', () => { expect(cn(['foo', 'bar'])).toBe('foo bar') }) }) + +describe('cn — the custom cp-* font scale', () => { + const fontSizes = tailwindFontSizeNames() + + it('reads the scale out of tailwind.config.ts', () => { + expect(fontSizes).toEqual(['cp-xs', 'cp-sm', 'cp-base']) + }) + + it('pins the reported regression: text-cp-sm survives beside text-white', () => { + // Plain twMerge reads both as colours and drops the size, leaving the + // Connect button at the browser default. + expect(cn('text-cp-sm', 'text-white')).toBe('text-cp-sm text-white') + }) + + it('pins the other half: two cp-* sizes conflict, and the last one wins', () => { + expect(cn('text-cp-xs', 'text-cp-base')).toBe('text-cp-base') + }) + + it.each(fontSizes)('keeps text-%s beside a text colour', (size) => { + expect(cn(`text-${size}`, 'text-white')).toBe(`text-${size} text-white`) + }) + + it('resolves any pair of cp-* sizes to the later one', () => { + for (const first of fontSizes) { + for (const second of fontSizes) { + if (first === second) continue + expect(cn(`text-${first}`, `text-${second}`)).toBe(`text-${second}`) + } + } + }) + + it('still lets a cp-* size override a stock Tailwind size', () => { + expect(cn('text-sm', 'text-cp-base')).toBe('text-cp-base') + }) +}) From ecd880b96bc9acb6083054bd20ecac27fcfc5934 Mon Sep 17 00:00:00 2001 From: Thiago Alves Date: Fri, 7 Aug 2026 11:15:16 -0400 Subject: [PATCH 6/6] fix(ui): answer the run/stop tooltip in the order the button blocks MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The reason chain asked about TRANSITIONING before the connection, so a disconnected target whose last polled `plcStatus` was TRANSITIONING got "PLC is changing state..." — a state we last saw, not the reason the button is inert. `plcStatus` is polled and survives the drop, so the stale value outlives the session that produced it. Reordering to match `plcControlBlocked`'s own order — unsupported, then no session, then a transition in flight — makes the tail unreachable except for the one reason that is left. Also unwraps `plcControlBlocked` onto one line: at 118 chars it fits inside Prettier's 120, and the manual wrap has been failing the format gate (which `sync` and `complete-build` both hang off) since 20ff67e0e. Found by CodeRabbit on #996. Mirrored byte-identically in openplc-web#655. Co-Authored-By: Claude Opus 5 (1M context) --- .../workspace-activity-bar/default.tsx | 18 +++++++++++++----- 1 file changed, 13 insertions(+), 5 deletions(-) diff --git a/src/frontend/components/_organisms/workspace-activity-bar/default.tsx b/src/frontend/components/_organisms/workspace-activity-bar/default.tsx index e2b86ef82..47a547008 100644 --- a/src/frontend/components/_organisms/workspace-activity-bar/default.tsx +++ b/src/frontend/components/_organisms/workspace-activity-bar/default.tsx @@ -110,15 +110,23 @@ export const DefaultWorkspaceActivityBar = ({ zoom }: DefaultWorkspaceActivityBa // everything except PING and STATUS, and the state it will settle on is not // decided yet, so the icon is drawn from a state that is about to change. // Clicking then cannot do what it appears to. + // + // The reason chain runs in the same order as the blocks it explains, most + // fundamental first: a target that cannot do run/stop at all, then no session + // to send over, then a transition in flight. Asking about the transition first + // would answer "PLC is changing state..." to someone who is not connected, + // reporting a state we last saw rather than the reason the button is inert — + // `plcStatus` is polled and survives the drop. Reached only when blocked, so + // the tail needs no test of its own: supported and connected and still blocked + // leaves exactly one reason. const plcStateControlSupported = resolveTargetCapabilities(currentBoardInfo).plcStateControl const plcTransitioning = plcStatus === 'TRANSITIONING' - const plcControlBlocked = - !plcStateControlSupported || deviceConnectionStatus !== 'connected' || plcTransitioning + const plcControlBlocked = !plcStateControlSupported || deviceConnectionStatus !== 'connected' || plcTransitioning const plcControlBlockedReason = !plcStateControlSupported ? 'This target does not support Start/Stop from the editor' - : plcTransitioning - ? 'PLC is changing state...' - : 'Connect to the target first' + : deviceConnectionStatus !== 'connected' + ? 'Connect to the target first' + : 'PLC is changing state...' // The emulator stopping is a session ending, and a debug session riding it ends // with it — which the drop handler below already does for every target. This