From 40e01cc843aaeb92b8bca3e3acfb9f96404369d1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marcone=20Ten=C3=B3rio=20da=20Silva=20Filho?= Date: Wed, 12 Aug 2026 15:49:18 +0200 Subject: [PATCH 1/5] feat(licensing): watch for the purchase after buy() and write the licence automatically MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Buying happens in an external browser tab, and until now nothing brought the result back: the editor kept showing "Not licensed" until the user guessed they should click "Check again". The purchase flow ended in a dead end at its most important moment. buy() now opens the purchase page AND starts a purchase watch (deviceLicense.awaitingPurchase). While it runs, the existing refresh() executes every 20s — one Modbus read frame plus one HTTP round-trip — and on the first tick after the completion webhook lands, that same refresh() activates the licence and WRITES the blob to the device, no click required. The watch ends on the first licensed report (whoever produced it: the poll, a manual re-check, the connect flow), after a 30-tick / 10-minute budget, on "Stop waiting", or when the board is switched / disconnected (clearDeviceLicense). Badge behaviour while waiting: - reads "Waiting for purchase…" steadily — it outranks the isChecking flicker each tick would otherwise cause; - withdraws "Buy licence" — offering it mid-wait invites a double buy; - offers "Stop waiting". Ticks that would overlap a call still in flight (slow device, 30s HTTP timeout) are skipped instead of stacking a second call on the same link. The interval calls refresh through a ref: its identity follows the device port, and an interval keyed on it would reset the tick budget on every change. Companion change (autonomy-edge, EDGE-593): the /buy page now polls the same truth and shows "License issued" only when the webhook actually landed it. Verified: 7 new hook tests (watch start, no-URL no-watch, per-tick refresh, overlap skip, licensed ends it, tick budget, cancel), 6 new badge tests, 4 new slice tests; device-slice 127/127, badge 26/26, tsc --noEmit clean, eslint 0 errors (the 4 unbound-method warnings in use-device-license.ts pre-date this change, same count). Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_015uUH3ZL5ehreMUf2dtanWD --- .../__tests__/device-license-status.test.tsx | 69 +++++++- .../editor/device/configuration/board.tsx | 2 + .../components/device-license-status.tsx | 58 ++++++- .../__tests__/use-device-license.test.ts | 159 ++++++++++++++++++ src/frontend/hooks/use-device-license.ts | 77 ++++++++- .../store/__tests__/device-slice.test.ts | 51 +++++- src/frontend/store/slices/device/slice.ts | 12 ++ src/frontend/store/slices/device/types.ts | 14 ++ 8 files changed, 422 insertions(+), 20 deletions(-) create mode 100644 src/frontend/hooks/__tests__/use-device-license.test.ts diff --git a/src/frontend/components/_features/[workspace]/editor/device/configuration/__tests__/device-license-status.test.tsx b/src/frontend/components/_features/[workspace]/editor/device/configuration/__tests__/device-license-status.test.tsx index 6c36ce0a6..285fdf964 100644 --- a/src/frontend/components/_features/[workspace]/editor/device/configuration/__tests__/device-license-status.test.tsx +++ b/src/frontend/components/_features/[workspace]/editor/device/configuration/__tests__/device-license-status.test.tsx @@ -6,20 +6,26 @@ import { DeviceLicenseStatus } from '../components/device-license-status' const DEVICE_ID = '659a3520540f803625ddc34081e893d3' const BUY_URL = `https://edge.example.com/buy?vppId=com.openplc.espressif-licensed&deviceId=${DEVICE_ID}` -function setup(report: DeviceLicenseReport | null, overrides: { isChecking?: boolean; buyUrl?: string | null } = {}) { +function setup( + report: DeviceLicenseReport | null, + overrides: { isChecking?: boolean; buyUrl?: string | null; awaitingPurchase?: boolean } = {}, +) { const onBuy = jest.fn() const onRecheck = jest.fn() + const onCancelPurchaseWatch = jest.fn() render( , ) - return { onBuy, onRecheck } + return { onBuy, onRecheck, onCancelPurchaseWatch } } function expand() { @@ -32,7 +38,15 @@ describe('DeviceLicenseStatus', () => { // Connect. A placeholder badge would invite the user to read meaning into a // check that never happened. const { container } = render( - , + , ) expect(container.firstChild).toBeNull() }) @@ -85,8 +99,10 @@ describe('DeviceLicenseStatus', () => { report={{ deviceId: DEVICE_ID, outcome }} isChecking={false} buyUrl={BUY_URL} + awaitingPurchase={false} onBuy={jest.fn()} onRecheck={jest.fn()} + onCancelPurchaseWatch={jest.fn()} />, ) expect(container.textContent ?? '').not.toMatch(/full mode|unlocked|demo mode/i) @@ -104,8 +120,10 @@ describe('DeviceLicenseStatus', () => { report={{ deviceId: DEVICE_ID, outcome: { state: 'licensed', how: 'already-stored' } }} isChecking={false} buyUrl={BUY_URL} + awaitingPurchase={false} onBuy={jest.fn()} onRecheck={jest.fn()} + onCancelPurchaseWatch={jest.fn()} />, ) @@ -211,4 +229,49 @@ describe('DeviceLicenseStatus', () => { expect(screen.getByText(/not the same as having no licence/)).toBeTruthy() }) }) + + describe('purchase watch', () => { + const UNLICENSED: DeviceLicenseReport = { + deviceId: DEVICE_ID, + outcome: { state: 'unlicensed', entitlementChecked: true }, + } + + it('reads "Waiting for purchase…" while the watch runs', () => { + setup(UNLICENSED, { awaitingPurchase: true }) + expect(screen.getByText('Waiting for purchase…')).toBeTruthy() + }) + + it('outranks the periodic check tick — the badge must not flap between two labels', () => { + // Every poll tick flips isChecking on and off; alternating + // "Waiting…"/"Checking…" reads as flapping when it is one continuous wait. + setup(UNLICENSED, { awaitingPurchase: true, isChecking: true }) + expect(screen.getByText('Waiting for purchase…')).toBeTruthy() + expect(screen.queryByText('Checking licence…')).toBeNull() + }) + + it('withdraws the purchase button while waiting — offering it again invites a double buy', () => { + setup(UNLICENSED, { awaitingPurchase: true }) + expand() + expect(screen.queryByRole('button', { name: 'Buy licence' })).toBeNull() + }) + + it('offers to stop waiting, and only then', () => { + const { onCancelPurchaseWatch } = setup(UNLICENSED, { awaitingPurchase: true }) + expand() + fireEvent.click(screen.getByRole('button', { name: 'Stop waiting' })) + expect(onCancelPurchaseWatch).toHaveBeenCalledTimes(1) + }) + + it('shows no stop button when no watch is running', () => { + setup(UNLICENSED) + expand() + expect(screen.queryByRole('button', { name: 'Stop waiting' })).toBeNull() + }) + + it('explains that the editor will write the licence by itself', () => { + setup(UNLICENSED, { awaitingPurchase: true }) + expand() + expect(screen.getByText(/write the licence to this device by itself/)).toBeTruthy() + }) + }) }) 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 f3aa8982c..c7aecec26 100644 --- a/src/frontend/components/_features/[workspace]/editor/device/configuration/board.tsx +++ b/src/frontend/components/_features/[workspace]/editor/device/configuration/board.tsx @@ -720,8 +720,10 @@ const Board = memo(function () { report={licensing.report} isChecking={licensing.isChecking} buyUrl={licensing.buyUrl} + awaitingPurchase={licensing.awaitingPurchase} onBuy={() => void licensing.buy()} onRecheck={() => void licensing.refresh()} + onCancelPurchaseWatch={licensing.cancelPurchaseWatch} /> ) : null} diff --git a/src/frontend/components/_features/[workspace]/editor/device/configuration/components/device-license-status.tsx b/src/frontend/components/_features/[workspace]/editor/device/configuration/components/device-license-status.tsx index 94602b16f..65ba7c140 100644 --- a/src/frontend/components/_features/[workspace]/editor/device/configuration/components/device-license-status.tsx +++ b/src/frontend/components/_features/[workspace]/editor/device/configuration/components/device-license-status.tsx @@ -75,8 +75,15 @@ export interface DeviceLicenseStatusProps { isChecking: boolean /** Null when no valid purchase link can be built; the button is then hidden. */ buyUrl: string | null + /** + * True while the purchase watch runs — `buy` opened the external purchase + * page and the editor is polling so it can write the licence on its own. + * The badge then reads "Waiting for purchase…" and the panel can stop it. + */ + awaitingPurchase: boolean onBuy: () => void onRecheck: () => void + onCancelPurchaseWatch: () => void } /** The label + icon for an outcome. One place, so no branch can drift. */ @@ -131,7 +138,15 @@ function describeOutcome(report: DeviceLicenseReport): { } } -export function DeviceLicenseStatus({ report, isChecking, buyUrl, onBuy, onRecheck }: DeviceLicenseStatusProps) { +export function DeviceLicenseStatus({ + report, + isChecking, + buyUrl, + awaitingPurchase, + onBuy, + onRecheck, + onCancelPurchaseWatch, +}: DeviceLicenseStatusProps) { const [copied, setCopied] = useState(false) // Nothing has run: every non-licensable board stays here, and so does a @@ -148,10 +163,21 @@ export function DeviceLicenseStatus({ report, isChecking, buyUrl, onBuy, onReche const { label, Icon, negative, detail } = describeOutcome(report) const deviceId = report.deviceId + // The watch outranks the tick: while it runs, every periodic refresh flips + // `isChecking` on and off, and a badge alternating "Waiting…"/"Checking…" + // reads as flapping when it is one continuous wait. + const badgeLabel = awaitingPurchase ? 'Waiting for purchase…' : isChecking ? 'Checking licence…' : label + // The purchase button appears ONLY where buying is the honest next step: the // backend was asked and reported no entitlement. On `check-failed` or an - // unchecked `unlicensed` it would be a guess, and a costly one. - const offerPurchase = !!buyUrl && report.outcome.state === 'unlicensed' && report.outcome.entitlementChecked === true + // unchecked `unlicensed` it would be a guess, and a costly one. While the + // watch runs the step was already taken — offering it again mid-wait invites + // a double purchase. + const offerPurchase = + !!buyUrl && + !awaitingPurchase && + report.outcome.state === 'unlicensed' && + report.outcome.entitlementChecked === true return ( // Radix Popover, PORTALLED. The details used to be a conditional
in the @@ -171,9 +197,13 @@ export function DeviceLicenseStatus({ report, isChecking, buyUrl, onBuy, onReche : 'text-neutral-600 hover:text-neutral-950 dark:text-neutral-400 dark:hover:text-white', )} > - {isChecking ? : } - - {isChecking ? 'Checking licence…' : label} + {awaitingPurchase || isChecking ? : } + + {badgeLabel} @@ -226,6 +256,13 @@ export function DeviceLicenseStatus({ report, isChecking, buyUrl, onBuy, onReche
) : null} + {awaitingPurchase ? ( +

+ Waiting for the purchase to complete. The editor checks periodically and will write the licence to this + device by itself — you can keep working meanwhile. +

+ ) : null} +
+ ) : null}
diff --git a/src/frontend/hooks/__tests__/use-device-license.test.ts b/src/frontend/hooks/__tests__/use-device-license.test.ts new file mode 100644 index 000000000..7804f0b93 --- /dev/null +++ b/src/frontend/hooks/__tests__/use-device-license.test.ts @@ -0,0 +1,159 @@ +import { act, renderHook } from '@testing-library/react' + +// `mock*`-prefixed refs are hoisted into the jest.mock factories below. + +const DEVICE_ID = '659a3520540f803625ddc34081e893d3' +const UNLICENSED = { deviceId: DEVICE_ID, outcome: { state: 'unlicensed', entitlementChecked: true } } +const LICENSED = { deviceId: DEVICE_ID, outcome: { state: 'licensed', how: 'activated' } } + +const mockStartLicenseCheck = jest.fn(() => { + ;(mockState.deviceLicense as { phase: string }).phase = 'checking' +}) +/** Write-through, like the real action: the poll's overlap guard reads it back. */ +const mockSetLicenseReport = jest.fn((report: unknown) => { + const lic = mockState.deviceLicense as { phase: string; report: unknown } + lic.phase = 'done' + lic.report = report +}) +const mockSetAwaitingPurchase = jest.fn((awaiting: boolean) => { + ;(mockState.deviceLicense as { awaitingPurchase: boolean }).awaitingPurchase = awaiting +}) + +const mockState: Record = { + deviceLicense: { phase: 'done', report: UNLICENSED, awaitingPurchase: false }, + deviceActions: { + startDeviceLicenseCheck: mockStartLicenseCheck, + setDeviceLicenseReport: mockSetLicenseReport, + setAwaitingPurchase: mockSetAwaitingPurchase, + }, +} + +type Selector = (s: typeof mockState) => T +const mockUseOpenPLCStore = ((selector?: Selector) => + selector ? selector(mockState) : mockState) as unknown as jest.Mock & { getState: () => typeof mockState } +mockUseOpenPLCStore.getState = () => mockState + +const mockReadLicense = jest.fn().mockResolvedValue(UNLICENSED) +const mockRefreshLicense = jest.fn().mockResolvedValue(UNLICENSED) +const mockOpenExternalLink = jest.fn().mockResolvedValue({ success: true }) + +jest.mock('../../store', () => ({ useOpenPLCStore: mockUseOpenPLCStore })) +jest.mock('@root/middleware/shared/providers/platform-context', () => ({ + useDevice: () => ({ readLicense: mockReadLicense, refreshLicense: mockRefreshLicense }), + useSystem: () => ({ + getEdgeFrontendUrl: () => 'https://edge.example.com', + openExternalLink: mockOpenExternalLink, + }), +})) +jest.mock('@root/middleware/shared/utils/licensing', () => ({ + resolveLicensingTarget: () => ({ licensable: true, packageId: 'com.openplc.industrialshields' }), +})) + +import type { BoardInfo } from '@root/middleware/shared/ports/types' + +import { useDeviceLicense } from '../use-device-license' + +const BOARD = { name: 'ESP32 PLC 21' } as unknown as BoardInfo + +const POLL_MS = 20_000 +const MAX_TICKS = 30 + +function setLicenseState(patch: Partial<{ phase: string; report: unknown; awaitingPurchase: boolean }>) { + Object.assign(mockState.deviceLicense as object, patch) +} + +describe('useDeviceLicense — purchase watch', () => { + beforeEach(() => { + jest.useFakeTimers() + setLicenseState({ phase: 'done', report: UNLICENSED, awaitingPurchase: false }) + }) + + afterEach(() => { + jest.useRealTimers() + jest.clearAllMocks() + }) + + it('buy() opens the device-bound page and starts the watch', async () => { + const { result } = renderHook(() => useDeviceLicense(BOARD)) + + await act(() => result.current.buy(DEVICE_ID)) + + expect(mockOpenExternalLink).toHaveBeenCalledWith(expect.stringContaining(DEVICE_ID)) + expect(mockOpenExternalLink).toHaveBeenCalledWith(expect.stringContaining('com.openplc.industrialshields')) + expect(mockSetAwaitingPurchase).toHaveBeenCalledWith(true) + }) + + it('does NOT start a watch when no purchase page could be opened', async () => { + // No deviceId anywhere → urlFor yields null → nothing opened, nothing to watch. + setLicenseState({ report: null }) + const { result } = renderHook(() => useDeviceLicense(BOARD)) + + await act(() => result.current.buy()) + + expect(mockOpenExternalLink).not.toHaveBeenCalled() + expect(mockSetAwaitingPurchase).not.toHaveBeenCalled() + }) + + it('refreshes on every tick while the watch runs — the write happens inside refresh', async () => { + setLicenseState({ awaitingPurchase: true }) + renderHook(() => useDeviceLicense(BOARD)) + + await act(async () => { + jest.advanceTimersByTime(POLL_MS) + }) + expect(mockRefreshLicense).toHaveBeenCalledTimes(1) + + // The tick landed an unlicensed report (webhook not done): keep going. + await act(async () => { + jest.advanceTimersByTime(POLL_MS) + }) + expect(mockRefreshLicense).toHaveBeenCalledTimes(2) + }) + + it('skips a tick that would overlap a call still in flight', async () => { + setLicenseState({ awaitingPurchase: true, phase: 'checking' }) + renderHook(() => useDeviceLicense(BOARD)) + + await act(async () => { + jest.advanceTimersByTime(POLL_MS) + }) + + expect(mockRefreshLicense).not.toHaveBeenCalled() + }) + + it('ends the watch when a licensed report lands, whoever produced it', () => { + setLicenseState({ awaitingPurchase: true }) + const { rerender } = renderHook(() => useDeviceLicense(BOARD)) + + // A manual "Check again" (or the poll) landed the licence. + setLicenseState({ report: LICENSED }) + rerender() + + expect(mockSetAwaitingPurchase).toHaveBeenCalledWith(false) + }) + + it('gives up after the tick budget instead of polling a forgotten tab forever', async () => { + setLicenseState({ awaitingPurchase: true }) + renderHook(() => useDeviceLicense(BOARD)) + + for (let i = 0; i < MAX_TICKS + 3; i++) { + // eslint-disable-next-line no-await-in-loop -- each tick must settle before the next + await act(async () => { + jest.advanceTimersByTime(POLL_MS) + }) + } + + // 30 refreshes, then the budget closes the watch; the extra ticks refresh nothing. + expect(mockRefreshLicense).toHaveBeenCalledTimes(MAX_TICKS) + expect(mockSetAwaitingPurchase).toHaveBeenCalledWith(false) + }) + + it('cancelPurchaseWatch stops the watch on request', () => { + setLicenseState({ awaitingPurchase: true }) + const { result } = renderHook(() => useDeviceLicense(BOARD)) + + act(() => result.current.cancelPurchaseWatch()) + + expect(mockSetAwaitingPurchase).toHaveBeenCalledWith(false) + }) +}) diff --git a/src/frontend/hooks/use-device-license.ts b/src/frontend/hooks/use-device-license.ts index 88e30a9b2..56e88aca6 100644 --- a/src/frontend/hooks/use-device-license.ts +++ b/src/frontend/hooks/use-device-license.ts @@ -1,12 +1,16 @@ /** * useDeviceLicense — the renderer side of the VPP licensing flow. * - * Owns the four things the UI needs and nothing else: + * Owns the five things the UI needs and nothing else: * - whether licensing applies to this board at all (`isLicensable`); * - the last landed report, from the store; * - `check()` (read + verify, local) and `refresh()` (full flow, may reach the * network and write); - * - `buy()`, which opens the device-bound purchase page. + * - `buy()`, which opens the device-bound purchase page and starts the + * purchase watch; + * - the purchase watch itself: while `awaitingPurchase`, `refresh()` runs on an + * interval so the licence bought in the external browser is activated and + * WRITTEN to the device without the user having to click anything. * * Deliberately NOT folded into `useDeviceConnect`: that hook is about resolving * and holding a link, this one about what the device is entitled to run. The only @@ -16,11 +20,21 @@ import type { DeviceLicenseReport } from '@root/middleware/shared/ports/device-p import type { BoardInfo } from '@root/middleware/shared/ports/types' import { useDevice, useSystem } from '@root/middleware/shared/providers/platform-context' import { resolveLicensingTarget } from '@root/middleware/shared/utils/licensing' -import { useCallback, useMemo } from 'react' +import { useCallback, useEffect, useMemo, useRef } from 'react' import { useOpenPLCStore } from '../store' import { buildLicenseBuyUrl } from '../utils/license-buy-url' +/** + * Purchase-watch cadence. Each tick is one Modbus read frame plus, while the + * backend still answers "no purchase", one cheap HTTP round-trip — light enough + * to repeat, heavy enough not to hammer a public rate-limited route. 30 ticks + * of 20s = a 10-minute window, generous for a checkout without leaving a + * forgotten tab polling forever. + */ +const PURCHASE_POLL_INTERVAL_MS = 20_000 +const PURCHASE_POLL_MAX_TICKS = 30 + export interface UseDeviceLicenseResult { /** Whether the selected board's VPP participates in licensing at all. When * false every other member here is inert and the UI shows nothing. */ @@ -58,6 +72,13 @@ export interface UseDeviceLicenseResult { * doing nothing on the one path where it matters most. */ buy: (deviceId?: string) => Promise + /** + * True while the purchase watch is running — from `buy()` until a licensed + * report lands, the 10-minute window closes, or `cancelPurchaseWatch`. + */ + awaitingPurchase: boolean + /** Stop the purchase watch without waiting for it to conclude. */ + cancelPurchaseWatch: () => void } export function useDeviceLicense(boardInfo: BoardInfo | undefined): UseDeviceLicenseResult { @@ -65,8 +86,10 @@ export function useDeviceLicense(boardInfo: BoardInfo | undefined): UseDeviceLic const system = useSystem() const startCheck = useOpenPLCStore((s) => s.deviceActions.startDeviceLicenseCheck) const setReport = useOpenPLCStore((s) => s.deviceActions.setDeviceLicenseReport) + const setAwaitingPurchase = useOpenPLCStore((s) => s.deviceActions.setAwaitingPurchase) const phase = useOpenPLCStore((s) => s.deviceLicense.phase) const report = useOpenPLCStore((s) => s.deviceLicense.report) + const awaitingPurchase = useOpenPLCStore((s) => s.deviceLicense.awaitingPurchase) const target = useMemo(() => resolveLicensingTarget(boardInfo), [boardInfo]) @@ -114,6 +137,46 @@ export function useDeviceLicense(boardInfo: BoardInfo | undefined): UseDeviceLic const check = useCallback(() => run('check'), [run]) const refresh = useCallback(() => run('refresh'), [run]) + const cancelPurchaseWatch = useCallback(() => setAwaitingPurchase(false), [setAwaitingPurchase]) + + // End the watch the moment a licensed report lands, whoever produced it — + // the poll below, a manual "Check again", the connect flow. Watching the + // REPORT rather than the poll's own return value is what lets all of those + // paths conclude the purchase. + useEffect(() => { + if (awaitingPurchase && report?.outcome.state === 'licensed') { + setAwaitingPurchase(false) + } + }, [awaitingPurchase, report, setAwaitingPurchase]) + + // The purchase watch. One `refresh()` per tick: read the device, ask the + // backend and — on the first tick after the completion webhook lands — write + // the blob to the device and read it back. `refresh` reports its own + // failures as `check-failed` reports, so a flaky tick shows in the badge + // instead of silently killing the watch. + // + // Called through a ref: `refresh`'s identity follows the device port and the + // board target, and an interval keyed on it would be torn down and rebuilt + // on every such change, resetting the tick budget each time. + const refreshRef = useRef(refresh) + refreshRef.current = refresh + useEffect(() => { + if (!awaitingPurchase) return + let ticks = 0 + const timer = setInterval(() => { + ticks += 1 + if (ticks > PURCHASE_POLL_MAX_TICKS) { + setAwaitingPurchase(false) + return + } + // A tick that would overlap an in-flight call (slow device, 30s HTTP + // timeout) skips instead of stacking a second one on the same link. + if (useOpenPLCStore.getState().deviceLicense.phase === 'checking') return + void refreshRef.current() + }, PURCHASE_POLL_INTERVAL_MS) + return () => clearInterval(timer) + }, [awaitingPurchase, setAwaitingPurchase]) + /** * Build the purchase link for a given device id. * @@ -141,8 +204,12 @@ export function useDeviceLicense(boardInfo: BoardInfo | undefined): UseDeviceLic const url = urlFor(deviceId ?? report?.deviceId) if (!url) return await system.openExternalLink(url) + // The purchase now lives in an external browser tab; start watching for + // its completion so the licence is activated and written to the device + // without the user having to come back and click anything. + setAwaitingPurchase(true) }, - [report?.deviceId, system, urlFor], + [report?.deviceId, setAwaitingPurchase, system, urlFor], ) return { @@ -157,5 +224,7 @@ export function useDeviceLicense(boardInfo: BoardInfo | undefined): UseDeviceLic refresh, buyUrl, buy, + awaitingPurchase, + cancelPurchaseWatch, } } diff --git a/src/frontend/store/__tests__/device-slice.test.ts b/src/frontend/store/__tests__/device-slice.test.ts index 92b842c4f..ae283ad3a 100644 --- a/src/frontend/store/__tests__/device-slice.test.ts +++ b/src/frontend/store/__tests__/device-slice.test.ts @@ -279,13 +279,13 @@ describe('createDeviceSlice', () => { it('starts idle with nothing known', () => { // The state every non-licensable board stays in: nothing runs, so nothing is // known, and the UI shows no licensing affordance at all. - expect(makeStore().getState().deviceLicense).toEqual({ phase: 'idle', report: null }) + expect(makeStore().getState().deviceLicense).toEqual({ phase: 'idle', report: null, awaitingPurchase: false }) }) it('startDeviceLicenseCheck marks the call in flight', () => { const store = makeStore() store.getState().deviceActions.startDeviceLicenseCheck() - expect(store.getState().deviceLicense).toEqual({ phase: 'checking', report: null }) + expect(store.getState().deviceLicense).toEqual({ phase: 'checking', report: null, awaitingPurchase: false }) }) it('startDeviceLicenseCheck KEEPS the last report instead of blanking it', () => { @@ -295,14 +295,14 @@ describe('createDeviceSlice', () => { const store = makeStore() store.getState().deviceActions.setDeviceLicenseReport(LICENSED) store.getState().deviceActions.startDeviceLicenseCheck() - expect(store.getState().deviceLicense).toEqual({ phase: 'checking', report: LICENSED }) + expect(store.getState().deviceLicense).toEqual({ phase: 'checking', report: LICENSED, awaitingPurchase: false }) }) it('setDeviceLicenseReport lands the report and settles the phase', () => { const store = makeStore() store.getState().deviceActions.startDeviceLicenseCheck() store.getState().deviceActions.setDeviceLicenseReport(LICENSED) - expect(store.getState().deviceLicense).toEqual({ phase: 'done', report: LICENSED }) + expect(store.getState().deviceLicense).toEqual({ phase: 'done', report: LICENSED, awaitingPurchase: false }) }) it('preserves the outcome union verbatim, including the entitlement distinction', () => { @@ -346,7 +346,44 @@ describe('createDeviceSlice', () => { const store = makeStore() store.getState().deviceActions.setDeviceLicenseReport(LICENSED) store.getState().deviceActions.clearDeviceLicense() - expect(store.getState().deviceLicense).toEqual({ phase: 'idle', report: null }) + expect(store.getState().deviceLicense).toEqual({ phase: 'idle', report: null, awaitingPurchase: false }) + }) + + it('setAwaitingPurchase opens and closes the purchase-watch window', () => { + const store = makeStore() + store.getState().deviceActions.setAwaitingPurchase(true) + expect(store.getState().deviceLicense.awaitingPurchase).toBe(true) + store.getState().deviceActions.setAwaitingPurchase(false) + expect(store.getState().deviceLicense.awaitingPurchase).toBe(false) + }) + + it('setAwaitingPurchase leaves the report and phase alone', () => { + // The watch is ORTHOGONAL to what is known: opening it must not blank the + // last report (the badge would flicker) nor fake an in-flight phase. + const store = makeStore() + store.getState().deviceActions.setDeviceLicenseReport(LICENSED) + store.getState().deviceActions.setAwaitingPurchase(true) + expect(store.getState().deviceLicense).toEqual({ phase: 'done', report: LICENSED, awaitingPurchase: true }) + }) + + it('landing a report does NOT end the watch by itself — the poll effect owns that', () => { + // An unlicensed report mid-wait is the EXPECTED state (webhook not done + // yet); if the store ended the watch on every landing, the first poll tick + // would kill the watch it serves. + const store = makeStore() + store.getState().deviceActions.setAwaitingPurchase(true) + store.getState().deviceActions.setDeviceLicenseReport({ + deviceId: '659a3520540f803625ddc34081e893d3', + outcome: { state: 'unlicensed', entitlementChecked: true }, + }) + expect(store.getState().deviceLicense.awaitingPurchase).toBe(true) + }) + + it('clearDeviceLicense ends the watch — a new board/disconnect makes it moot', () => { + const store = makeStore() + store.getState().deviceActions.setAwaitingPurchase(true) + store.getState().deviceActions.clearDeviceLicense() + expect(store.getState().deviceLicense.awaitingPurchase).toBe(false) }) it('clearDeviceBoard change drops the licence — it was verified against the OLD board', () => { @@ -362,7 +399,7 @@ describe('createDeviceSlice', () => { store.getState().deviceActions.setDeviceBoard('Raspberry Pi 4') - expect(store.getState().deviceLicense).toEqual({ phase: 'idle', report: null }) + expect(store.getState().deviceLicense).toEqual({ phase: 'idle', report: null, awaitingPurchase: false }) }) it('leaves the licence alone when setDeviceBoard is called with the same board', () => { @@ -383,7 +420,7 @@ describe('createDeviceSlice', () => { const store = makeStore() store.getState().deviceActions.setDeviceLicenseReport(LICENSED) store.getState().deviceActions.clearDeviceDefinitions() - expect(store.getState().deviceLicense).toEqual({ phase: 'idle', report: null }) + expect(store.getState().deviceLicense).toEqual({ phase: 'idle', report: null, awaitingPurchase: false }) }) }) diff --git a/src/frontend/store/slices/device/slice.ts b/src/frontend/store/slices/device/slice.ts index 25553bba4..cc66d16b7 100644 --- a/src/frontend/store/slices/device/slice.ts +++ b/src/frontend/store/slices/device/slice.ts @@ -70,6 +70,7 @@ const createDeviceSlice: StateCreator = (s deviceLicense: { phase: 'idle', report: null, + awaitingPurchase: false, }, deviceActions: { @@ -602,11 +603,22 @@ const createDeviceSlice: StateCreator = (s }), ) }, + setAwaitingPurchase: (awaiting): void => { + setState( + produce(({ deviceLicense }: DeviceSlice) => { + deviceLicense.awaitingPurchase = awaiting + }), + ) + }, clearDeviceLicense: (): void => { setState( produce(({ deviceLicense }: DeviceSlice) => { deviceLicense.phase = 'idle' deviceLicense.report = null + // A new board / disconnect ends any purchase watch: the poll effect + // keys off this flag, and a watch for a device that is no longer + // there would keep hitting it over a dead link. + deviceLicense.awaitingPurchase = false }), ) }, diff --git a/src/frontend/store/slices/device/types.ts b/src/frontend/store/slices/device/types.ts index 4123852ea..f516ebe43 100644 --- a/src/frontend/store/slices/device/types.ts +++ b/src/frontend/store/slices/device/types.ts @@ -145,6 +145,14 @@ export type DeviceLicenseInfo = { * it needs `node:crypto` — and which feeds the copy button and the buy link). */ report: DeviceLicenseReport | null + /** + * True from `buy()` opening the purchase page until the poll that watches for + * the completed purchase lands a licensed report, times out, or the user + * cancels. Drives the "Waiting for purchase…" affordance and the poll effect + * in `useDeviceLicense` — the purchase happens in an external browser, so + * polling is the only feedback channel the editor has. + */ + awaitingPurchase: boolean } // --------------------------------------------------------------------------- @@ -254,6 +262,12 @@ export type DeviceActions = { startDeviceLicenseCheck: () => void /** Land a finished licensing call: `phase='done'`, store the report. */ setDeviceLicenseReport: (report: DeviceLicenseReport) => void + /** + * Toggle the awaiting-purchase window (see `DeviceLicenseInfo.awaitingPurchase`). + * Deliberately dumb: the poll effect in `useDeviceLicense` owns WHEN it ends + * (licensed report, timeout, cancel) — the store only records that it is open. + */ + setAwaitingPurchase: (awaiting: boolean) => void /** Reset licensing to `idle`/null — on disconnect, board change, project close. */ clearDeviceLicense: () => void setVendorScreenData: (persistenceKey: string, data: unknown) => void From c00d186249c492f9f2b5ad500303b0cf6e686976 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marcone=20Ten=C3=B3rio=20da=20Silva=20Filho?= Date: Wed, 12 Aug 2026 15:59:41 +0200 Subject: [PATCH 2/5] style(licensing): run the repo prettier over the licence badge MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The format check runs prettier --check over src/**; the multiline className in the waiting-state badge was hand-wrapped differently than prettier wants it. No behavioural change — the badge suite still passes 26/26. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_015uUH3ZL5ehreMUf2dtanWD --- .../configuration/components/device-license-status.tsx | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/src/frontend/components/_features/[workspace]/editor/device/configuration/components/device-license-status.tsx b/src/frontend/components/_features/[workspace]/editor/device/configuration/components/device-license-status.tsx index 65ba7c140..d776ccb46 100644 --- a/src/frontend/components/_features/[workspace]/editor/device/configuration/components/device-license-status.tsx +++ b/src/frontend/components/_features/[workspace]/editor/device/configuration/components/device-license-status.tsx @@ -174,10 +174,7 @@ export function DeviceLicenseStatus({ // watch runs the step was already taken — offering it again mid-wait invites // a double purchase. const offerPurchase = - !!buyUrl && - !awaitingPurchase && - report.outcome.state === 'unlicensed' && - report.outcome.entitlementChecked === true + !!buyUrl && !awaitingPurchase && report.outcome.state === 'unlicensed' && report.outcome.entitlementChecked === true return ( // Radix Popover, PORTALLED. The details used to be a conditional
in the From 2d97f478554f491769fc57ae43bd99ce093e53a5 Mon Sep 17 00:00:00 2001 From: Thiago Alves Date: Wed, 12 Aug 2026 12:32:09 -0400 Subject: [PATCH 3/5] fix(simulator): stop the emulator on Stop, and close its session on every stop path MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two fixes, one ownership rule: the emulator and the session it serves start and end together. 1. Stop actually stops the emulator (shared surface) The Stop button ended the debug session, logged "Simulator stopped." and left the emulator running. `stopSession()` deliberately does not stop it (a debug session is a consumer of the emulator, not its owner — see its docstring), and this button, which owns that job, never made the call. The avr8js loop kept re-scheduling itself and burning a core for the rest of the session, unreachable from the UI because the button had already flipped back to "Start". `workspace-activity-bar/default.tsx` is byte-identical with openplc-web and carries the same change there; the two must land together for the Shared Surface Sync gate. 2. Every stop path closes the session (main process) Stops were scattered, and two of them closed nothing: - `handleWindowReload` stopped the emulator but left the session open, so main went on holding a simulator session the reloaded renderer knew nothing about. - `handleSimulatorLoadFirmware`'s catch did neither. `loadAndRun` marks the emulator running before it finishes wiring, so a throw after that point leaked a running emulator with no session — and no button to reach it, because the renderer never learned it had started. All six sites now route through one `stopSimulator()` choke point that closes the session first, then stops the emulator. The load-failure path is the parity fix for openplc-web, where the worker already cleans up and reports 'stopped' when `loadAndRun` throws. Adds `simulator-session.handler.test.ts`: 5 cases over ordering, the non-simulator link left alone, the reload path and the throw path. The two covering new behaviour fail without this change. Co-Authored-By: Claude Opus 5 (1M context) --- .../workspace-activity-bar/default.tsx | 18 ++- .../simulator-session.handler.test.ts | 152 ++++++++++++++++++ src/main/modules/ipc/main.ts | 37 ++++- 3 files changed, 198 insertions(+), 9 deletions(-) create mode 100644 src/main/modules/ipc/__tests__/simulator-session.handler.test.ts diff --git a/src/frontend/components/_organisms/workspace-activity-bar/default.tsx b/src/frontend/components/_organisms/workspace-activity-bar/default.tsx index 47a547008..c41914962 100644 --- a/src/frontend/components/_organisms/workspace-activity-bar/default.tsx +++ b/src/frontend/components/_organisms/workspace-activity-bar/default.tsx @@ -636,7 +636,23 @@ export const DefaultWorkspaceActivityBar = ({ zoom }: DefaultWorkspaceActivityBa const handleSimulatorControl = useCallback(async (): Promise => { try { if (simulatorRunning) { + // Two things end here, in this order. + // + // The debug session goes first: it is a CONSUMER of the emulator, so it + // has to let go of the transport before the thing on the other end + // disappears. await debugSession.stopSession() + + // Then the emulator itself — which is this button's job and nothing + // else's. `stopSession()` deliberately does not do it (see its + // docstring: a debug session is not the owner of the thing it talks + // to), so with this call missing "Stop" ended the debug session, logged + // "Simulator stopped." and left the avr8js loop running: it kept + // re-scheduling itself and burning a core for the rest of the session, + // with no way to reach it from the UI because the button had already + // flipped back to "Start". + await simulator.stop() + setSimulatorRunning(false) addLog({ id: crypto.randomUUID(), level: 'info', message: 'Simulator stopped.' }) } else { @@ -649,7 +665,7 @@ export const DefaultWorkspaceActivityBar = ({ zoom }: DefaultWorkspaceActivityBa pendingSimulatorDebugRef.current = false addLog({ id: crypto.randomUUID(), level: 'error', message: `Simulator control error: ${getErrorMessage(error)}` }) } - }, [debugSession, simulatorRunning, addLog]) + }, [debugSession, simulator, simulatorRunning, addLog]) // --------------------------------------------------------------------------- // MD5 verification — runs after debug compilation for non-simulator diff --git a/src/main/modules/ipc/__tests__/simulator-session.handler.test.ts b/src/main/modules/ipc/__tests__/simulator-session.handler.test.ts new file mode 100644 index 000000000..d827c573b --- /dev/null +++ b/src/main/modules/ipc/__tests__/simulator-session.handler.test.ts @@ -0,0 +1,152 @@ +/** + * The simulator's session lifecycle, in the main process. + * + * What is worth testing HERE is the ownership rule rather than the emulator + * itself: starting the emulator opens this target's session and stopping it + * closes it, every stop path routes through the one choke point, and the order + * is session-then-emulator so the debug client is dropped before the thing it + * talks to disappears. + * + * The paths that used to leak — a window reload and a start that threw — are the + * reason this file exists: both left a running emulator behind, and the reload + * also left main holding a session the reloaded renderer knew nothing about. + */ + +import MainProcessBridge from '../main' + +jest.mock('electron', () => ({ + app: { getPath: jest.fn(() => '/tmp'), quit: jest.fn() }, + dialog: {}, + nativeTheme: { shouldUseDarkColors: false, themeSource: 'system' }, + shell: { openExternal: jest.fn() }, +})) + +jest.mock('@root/backend/editor/ethercat', () => ({ ESIService: jest.fn() })) +jest.mock('@root/backend/editor/library-manager/desktop-catalog-transport', () => ({ + createDesktopCatalogTransport: jest.fn(() => ({})), +})) +jest.mock('@root/backend/editor/utils/runtime-https-config', () => ({ getRuntimeHttpsOptions: jest.fn(() => ({})) })) +jest.mock('@root/backend/shared/ethercat/esi-parser-main', () => ({ parseESIDeviceFull: jest.fn() })) +jest.mock('@root/backend/shared/library/public-catalog-client', () => ({ listPublicLibraries: jest.fn() })) +jest.mock('../../../../backend/editor/library-manager', () => ({ + LibraryManagerModule: jest.fn(() => ({ loadEnabledArchives: jest.fn(() => ({ archives: [], missing: [] })) })), +})) +jest.mock('../../../../backend/editor/package-manager', () => ({ PackageManagerModule: jest.fn(() => ({})) })) +jest.mock('../../../../backend/editor/services', () => ({ + logger: { error: jest.fn(), info: jest.fn(), warn: jest.fn() }, +})) +jest.mock('../../../../backend/editor/utils', () => ({ getOpenProjectPath: jest.fn(), getProjectPath: jest.fn() })) + +const simulatorModule = { loadAndRun: jest.fn(), stop: jest.fn(), isRunning: jest.fn(() => true) } +jest.mock('../../../../backend/shared/simulator/simulator-module', () => ({ + SimulatorModule: jest.fn(() => simulatorModule), +})) + +type Bridge = MainProcessBridge + +function createBridge(): Bridge { + return new MainProcessBridge({ + ipcMain: {}, + mainWindow: { + isDestroyed: jest.fn(() => false), + isMaximized: jest.fn(() => false), + webContents: { reload: jest.fn(), send: jest.fn() }, + }, + projectService: {}, + store: { get: jest.fn(() => undefined) }, + menuBuilder: {}, + pouService: {}, + compilerModule: {}, + hardwareModule: { isSerialPortPresent: jest.fn(() => true) }, + } as never) +} + +/** + * Pretend the session manager holds a link of the given transport, and record + * whether it is closed. Returns the recorder for the closing side. + */ +function holdLink(bridge: Bridge, transport: 'simulator' | 'serial') { + const session = (bridge as unknown as { deviceSession: { getLink: () => unknown; close: () => void } }).deviceSession + jest.spyOn(session, 'getLink').mockReturnValue({ transport }) + return jest.spyOn(session, 'close').mockImplementation(() => undefined) +} + +beforeEach(() => { + simulatorModule.loadAndRun.mockReset() + simulatorModule.stop.mockReset() + simulatorModule.isRunning.mockReset().mockReturnValue(true) +}) + +afterEach(() => { + jest.restoreAllMocks() +}) + +describe('simulator:stop', () => { + it('closes the session and stops the emulator', async () => { + const bridge = createBridge() + const close = holdLink(bridge, 'simulator') + + await expect(bridge.handleSimulatorStop({} as never)).resolves.toEqual({ success: true }) + + expect(close).toHaveBeenCalledTimes(1) + expect(simulatorModule.stop).toHaveBeenCalledTimes(1) + }) + + it('closes the session BEFORE stopping the emulator', async () => { + const bridge = createBridge() + const order: string[] = [] + const session = (bridge as unknown as { deviceSession: { getLink: () => unknown; close: () => void } }) + .deviceSession + jest.spyOn(session, 'getLink').mockReturnValue({ transport: 'simulator' }) + jest.spyOn(session, 'close').mockImplementation(() => { + order.push('session-closed') + }) + simulatorModule.stop.mockImplementation(() => order.push('emulator-stopped')) + + await bridge.handleSimulatorStop({} as never) + + expect(order).toEqual(['session-closed', 'emulator-stopped']) + }) + + it('leaves a non-simulator session alone', async () => { + const bridge = createBridge() + const close = holdLink(bridge, 'serial') + + await bridge.handleSimulatorStop({} as never) + + // A cabled board's link is not the emulator's to close, but the emulator + // still stops. + expect(close).not.toHaveBeenCalled() + expect(simulatorModule.stop).toHaveBeenCalledTimes(1) + }) +}) + +describe('window:reload', () => { + it('takes the session down with the emulator', () => { + const bridge = createBridge() + const close = holdLink(bridge, 'simulator') + + bridge.handleWindowReload() + + // The reload resets the renderer's store to 'disconnected'; a session left + // open here would be one only the main process still believes in. + expect(close).toHaveBeenCalledTimes(1) + expect(simulatorModule.stop).toHaveBeenCalledTimes(1) + }) +}) + +describe('simulator:load-firmware', () => { + it('stops the emulator and closes the session when the start throws', async () => { + const bridge = createBridge() + const close = holdLink(bridge, 'simulator') + + // An unreadable hex path fails inside the handler's try, standing in for any + // throw during start — `loadAndRun` marks the emulator running before it + // finishes wiring, so a throw after that point used to leak one. + const result = await bridge.handleSimulatorLoadFirmware({} as never, '/nonexistent/simulator.hex') + + expect(result.success).toBe(false) + expect(simulatorModule.stop).toHaveBeenCalledTimes(1) + expect(close).toHaveBeenCalledTimes(1) + }) +}) diff --git a/src/main/modules/ipc/main.ts b/src/main/modules/ipc/main.ts index 387d3c5ff..c2f047871 100644 --- a/src/main/modules/ipc/main.ts +++ b/src/main/modules/ipc/main.ts @@ -1487,7 +1487,7 @@ class MainProcessBridge implements MainIpcModule { } } handleAppQuit = () => { - this.simulatorModule.stop() + this.stopSimulator() if (this.mainWindow) { this.mainWindow.destroy() } @@ -1579,7 +1579,10 @@ class MainProcessBridge implements MainIpcModule { } } handleWindowReload = () => { - this.simulatorModule.stop() + // The reload wipes the renderer's store back to 'disconnected', so the + // session has to go with the emulator — otherwise main keeps holding an open + // simulator session that the reloaded UI has no idea about. + this.stopSimulator() this.mainWindow?.webContents.reload() } handleWindowRebuildMenu = () => { @@ -2573,8 +2576,7 @@ class MainProcessBridge implements MainIpcModule { /** Stops the simulator and notifies the renderer so it can update UI state. */ private stopSimulatorAndNotify(): void { if (this.simulatorModule.isRunning()) { - this.closeSimulatorSession() - this.simulatorModule.stop() + this.stopSimulator() this.mainWindow?.webContents.send('simulator:stopped') } } @@ -2825,26 +2827,45 @@ class MainProcessBridge implements MainIpcModule { this.toDeviceLinkCandidates([{ connectionType: 'simulator', connectionParams: {} }]), ) if (!opened.ok) { - this.simulatorModule.stop() + this.stopSimulator() const reason = opened.attempts.map((attempt) => attempt.error).join('; ') return { success: false, error: reason || 'The simulator did not answer its debug protocol' } } this.debuggerConnectionType = 'simulator' return { success: true } } catch (error) { + // A start that threw part-way still leaves state behind: `loadAndRun` + // marks the emulator running before it finishes wiring, so a throw after + // that point leaked a running emulator with no session — and no button to + // reach it, because the renderer never learned it had started. Web ends up + // in the right place through its worker, which cleans up and reports + // 'stopped' when `loadAndRun` throws; this is the editor's counterpart. + this.stopSimulator() return { success: false, error: getErrorMessage(error) } } } /** * Stop the emulator entirely — the simulator's Stop button means "stop the - * simulator", not "stop the program it is running". The session closes first so - * the client is dropped before the thing it talks to disappears. + * simulator", not "stop the program it is running". */ handleSimulatorStop = (_event: IpcMainInvokeEvent): Promise<{ success: boolean }> => { + this.stopSimulator() + return Promise.resolve({ success: true }) + } + + /** + * The one way the emulator stops: session first, then the emulator, so the + * debug client is dropped before the thing it talks to disappears. + * + * Every stop path routes through here on purpose. The session is a consumer of + * the emulator, so an emulator that goes away while its session stays open + * leaves the renderer gated on a session whose target no longer exists — which + * a window reload and a failed start both used to do. + */ + private stopSimulator(): void { this.closeSimulatorSession() this.simulatorModule.stop() - return Promise.resolve({ success: true }) } /** Close the session if it is the simulator's. No-op for any other target. */ From eccb27767e72419ed0cce1a2727b59326dc62ff4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marcone=20Ten=C3=B3rio=20da=20Silva=20Filho?= Date: Wed, 12 Aug 2026 18:53:02 +0200 Subject: [PATCH 4/5] fix(licensing): harden the purchase watch per review Review fixes on the post-purchase watch: - reset the watch on EVERY licence reset path: a real board change and project close previously left it polling (and potentially writing a licence) against the next board's package id. One resetDeviceLicense helper now serves all three reset paths so a fourth cannot drift. - replace the 30-tick budget with an absolute deadline persisted in the store (awaitingPurchaseUntil, PURCHASE_WATCH_WINDOW_MS): a remount resumes the SAME window, an overlap-skipped tick costs nothing, and the state is inspectable. - let a check-failed report outrank the "Waiting for purchase..." badge label so a dead link cannot hide behind a calm wait for ten minutes; the failure label holds steady across poll ticks. - start the watch only when openExternalLink actually opened the page, and fire the first check immediately instead of at t+20s. - give the watch a single owner (the board screen passes ownsWatch) so the useDeviceConnect instance no longer double-polls the same flag. - product-neutral panel copy: "OpenPLC checks periodically...". - slice tests arm the watch before asserting the resets, so the two reset assertions can actually fail (verified by mutation). Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_015uUH3ZL5ehreMUf2dtanWD --- .../__tests__/device-license-status.test.tsx | 27 +++- .../editor/device/configuration/board.tsx | 6 +- .../components/device-license-status.tsx | 30 +++-- .../__tests__/use-device-license.test.ts | 121 ++++++++++++++---- src/frontend/hooks/use-device-license.ts | 81 ++++++++---- .../store/__tests__/device-slice.test.ts | 81 +++++++++--- src/frontend/store/slices/device/slice.ts | 41 ++++-- src/frontend/store/slices/device/types.ts | 34 +++-- 8 files changed, 321 insertions(+), 100 deletions(-) diff --git a/src/frontend/components/_features/[workspace]/editor/device/configuration/__tests__/device-license-status.test.tsx b/src/frontend/components/_features/[workspace]/editor/device/configuration/__tests__/device-license-status.test.tsx index 285fdf964..0903e8a1c 100644 --- a/src/frontend/components/_features/[workspace]/editor/device/configuration/__tests__/device-license-status.test.tsx +++ b/src/frontend/components/_features/[workspace]/editor/device/configuration/__tests__/device-license-status.test.tsx @@ -249,6 +249,28 @@ describe('DeviceLicenseStatus', () => { expect(screen.queryByText('Checking licence…')).toBeNull() }) + it('does NOT outrank a failed check — a dead link must not hide behind a calm wait', () => { + // If the link goes bad mid-wait, every tick lands a check-failed report; + // reading "Waiting for purchase…" over that would mask a real failure for + // the remaining minutes of the window. The failure label wins, and the + // watch keeps running underneath. + setup({ outcome: { state: 'check-failed', error: 'Request timeout' } }, { awaitingPurchase: true }) + expect(screen.getByText('Licence check failed')).toBeTruthy() + expect(screen.queryByText('Waiting for purchase…')).toBeNull() + }) + + it('holds the failure label steady across poll ticks while waiting', () => { + // Mid-wait ticks still flip isChecking; with a check-failed report on + // record the badge must not flap to "Checking…" nor back to "Waiting…". + setup( + { outcome: { state: 'check-failed', error: 'Request timeout' } }, + { awaitingPurchase: true, isChecking: true }, + ) + expect(screen.getByText('Licence check failed')).toBeTruthy() + expect(screen.queryByText('Checking licence…')).toBeNull() + expect(screen.queryByText('Waiting for purchase…')).toBeNull() + }) + it('withdraws the purchase button while waiting — offering it again invites a double buy', () => { setup(UNLICENSED, { awaitingPurchase: true }) expand() @@ -268,9 +290,12 @@ describe('DeviceLicenseStatus', () => { expect(screen.queryByRole('button', { name: 'Stop waiting' })).toBeNull() }) - it('explains that the editor will write the licence by itself', () => { + it('explains that OpenPLC will write the licence by itself', () => { setup(UNLICENSED, { awaitingPurchase: true }) expand() + // The subject is deliberately product-neutral ("OpenPLC", not "the + // editor"): the same bytes render in the desktop editor and the web IDE. + expect(screen.getByText(/OpenPLC checks periodically/)).toBeTruthy() expect(screen.getByText(/write the licence to this device by itself/)).toBeTruthy() }) }) 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 c7aecec26..a96e485aa 100644 --- a/src/frontend/components/_features/[workspace]/editor/device/configuration/board.tsx +++ b/src/frontend/components/_features/[workspace]/editor/device/configuration/board.tsx @@ -65,8 +65,10 @@ const Board = memo(function () { } = useDeviceConnect(currentBoardInfo) // VPP licensing. Inert for every board whose VPP is not sold licensed, which is - // every built-in board — `isLicensable` gates the whole affordance. - const licensing = useDeviceLicense(currentBoardInfo) + // every built-in board — `isLicensable` gates the whole affordance. This mount + // OWNS the purchase watch: `useDeviceConnect` above holds a second instance of + // the hook, and without a single owner both would poll on every watch tick. + const licensing = useDeviceLicense(currentBoardInfo, { ownsWatch: true }) // Whether this target exposes the GPIO pin-mapping table. Arduino boards // enable it via their preset; runtime-v4 GPIO boards (e.g. the Raspberry diff --git a/src/frontend/components/_features/[workspace]/editor/device/configuration/components/device-license-status.tsx b/src/frontend/components/_features/[workspace]/editor/device/configuration/components/device-license-status.tsx index d776ccb46..e70fc3198 100644 --- a/src/frontend/components/_features/[workspace]/editor/device/configuration/components/device-license-status.tsx +++ b/src/frontend/components/_features/[workspace]/editor/device/configuration/components/device-license-status.tsx @@ -77,8 +77,9 @@ export interface DeviceLicenseStatusProps { buyUrl: string | null /** * True while the purchase watch runs — `buy` opened the external purchase - * page and the editor is polling so it can write the licence on its own. - * The badge then reads "Waiting for purchase…" and the panel can stop it. + * page and OpenPLC is polling so it can write the licence on its own. The + * badge then reads "Waiting for purchase…" (unless the last report is a + * check-failed, which outranks the wait) and the panel can stop it. */ awaitingPurchase: boolean onBuy: () => void @@ -163,10 +164,21 @@ export function DeviceLicenseStatus({ const { label, Icon, negative, detail } = describeOutcome(report) const deviceId = report.deviceId - // The watch outranks the tick: while it runs, every periodic refresh flips - // `isChecking` on and off, and a badge alternating "Waiting…"/"Checking…" - // reads as flapping when it is one continuous wait. - const badgeLabel = awaitingPurchase ? 'Waiting for purchase…' : isChecking ? 'Checking licence…' : label + // The watch outranks the tick, but never a FAILURE. While the watch runs, + // every periodic refresh flips `isChecking` on and off, and a badge + // alternating "Waiting…"/"Checking…" reads as flapping when it is one + // continuous wait — so the waiting label absorbs the ticks. A check-failed + // report is different: it means the ticks currently cannot see the device, + // and a calm "Waiting for purchase…" over that would hide a dead link for + // up to ten minutes. The failure label (and its normal styling) wins, held + // steady across ticks; the watch keeps running underneath. + const checkFailed = report.outcome.state === 'check-failed' + const showWaiting = awaitingPurchase && !checkFailed + const badgeLabel = showWaiting + ? 'Waiting for purchase…' + : isChecking && !awaitingPurchase + ? 'Checking licence…' + : label // The purchase button appears ONLY where buying is the honest next step: the // backend was asked and reported no entitlement. On `check-failed` or an @@ -194,10 +206,10 @@ export function DeviceLicenseStatus({ : 'text-neutral-600 hover:text-neutral-950 dark:text-neutral-400 dark:hover:text-white', )} > - {awaitingPurchase || isChecking ? : } + {showWaiting || isChecking ? : } {badgeLabel} @@ -255,7 +267,7 @@ export function DeviceLicenseStatus({ {awaitingPurchase ? (

- Waiting for the purchase to complete. The editor checks periodically and will write the licence to this + Waiting for the purchase to complete. OpenPLC checks periodically and will write the licence to this device by itself — you can keep working meanwhile.

) : null} diff --git a/src/frontend/hooks/__tests__/use-device-license.test.ts b/src/frontend/hooks/__tests__/use-device-license.test.ts index 7804f0b93..bb4943bd4 100644 --- a/src/frontend/hooks/__tests__/use-device-license.test.ts +++ b/src/frontend/hooks/__tests__/use-device-license.test.ts @@ -15,12 +15,15 @@ const mockSetLicenseReport = jest.fn((report: unknown) => { lic.phase = 'done' lic.report = report }) +/** Write-through, like the real action: stamps the absolute deadline the poll reads back. */ const mockSetAwaitingPurchase = jest.fn((awaiting: boolean) => { - ;(mockState.deviceLicense as { awaitingPurchase: boolean }).awaitingPurchase = awaiting + ;(mockState.deviceLicense as { awaitingPurchaseUntil: number | null }).awaitingPurchaseUntil = awaiting + ? Date.now() + PURCHASE_WATCH_WINDOW_MS + : null }) const mockState: Record = { - deviceLicense: { phase: 'done', report: UNLICENSED, awaitingPurchase: false }, + deviceLicense: { phase: 'done', report: UNLICENSED, awaitingPurchaseUntil: null }, deviceActions: { startDeviceLicenseCheck: mockStartLicenseCheck, setDeviceLicenseReport: mockSetLicenseReport, @@ -51,21 +54,37 @@ jest.mock('@root/middleware/shared/utils/licensing', () => ({ import type { BoardInfo } from '@root/middleware/shared/ports/types' +import { PURCHASE_WATCH_WINDOW_MS } from '../../store/slices/device/types' import { useDeviceLicense } from '../use-device-license' const BOARD = { name: 'ESP32 PLC 21' } as unknown as BoardInfo const POLL_MS = 20_000 -const MAX_TICKS = 30 -function setLicenseState(patch: Partial<{ phase: string; report: unknown; awaitingPurchase: boolean }>) { +function setLicenseState(patch: Partial<{ phase: string; report: unknown; awaitingPurchaseUntil: number | null }>) { Object.assign(mockState.deviceLicense as object, patch) } +/** Open the watch window the way the real action does: deadline = now + window. */ +function openPurchaseWindow(remainingMs: number = PURCHASE_WATCH_WINDOW_MS) { + setLicenseState({ awaitingPurchaseUntil: Date.now() + remainingMs }) +} + +/** + * Mount the one instance that owns the watch (the board screen's), then settle + * the immediate first tick so each subsequent timer advance starts from a + * landed report instead of tripping the overlap guard on its own leftovers. + */ +async function mountOwner() { + const utils = renderHook(() => useDeviceLicense(BOARD, { ownsWatch: true })) + await act(async () => {}) + return utils +} + describe('useDeviceLicense — purchase watch', () => { beforeEach(() => { jest.useFakeTimers() - setLicenseState({ phase: 'done', report: UNLICENSED, awaitingPurchase: false }) + setLicenseState({ phase: 'done', report: UNLICENSED, awaitingPurchaseUntil: null }) }) afterEach(() => { @@ -94,25 +113,46 @@ describe('useDeviceLicense — purchase watch', () => { expect(mockSetAwaitingPurchase).not.toHaveBeenCalled() }) - it('refreshes on every tick while the watch runs — the write happens inside refresh', async () => { - setLicenseState({ awaitingPurchase: true }) - renderHook(() => useDeviceLicense(BOARD)) + it('does NOT start a watch when the platform failed to open the page', async () => { + // The link call reports failure: no browser opened, so there is no purchase + // to wait for — and the Buy button must stay offered instead. + mockOpenExternalLink.mockResolvedValueOnce({ success: false }) + const { result } = renderHook(() => useDeviceLicense(BOARD)) + + await act(() => result.current.buy(DEVICE_ID)) + + expect(mockOpenExternalLink).toHaveBeenCalledTimes(1) + expect(mockSetAwaitingPurchase).not.toHaveBeenCalled() + }) + + it('checks immediately when the watch opens — a checkout that already completed must not wait 20s', async () => { + openPurchaseWindow() + await mountOwner() + + expect(mockRefreshLicense).toHaveBeenCalledTimes(1) + }) + + it('keeps refreshing on the poll cadence — the write happens inside refresh', async () => { + openPurchaseWindow() + await mountOwner() await act(async () => { jest.advanceTimersByTime(POLL_MS) }) - expect(mockRefreshLicense).toHaveBeenCalledTimes(1) + // The immediate tick plus the first interval tick. Each landed an + // unlicensed report (webhook not done yet): keep going. + expect(mockRefreshLicense).toHaveBeenCalledTimes(2) - // The tick landed an unlicensed report (webhook not done): keep going. await act(async () => { jest.advanceTimersByTime(POLL_MS) }) - expect(mockRefreshLicense).toHaveBeenCalledTimes(2) + expect(mockRefreshLicense).toHaveBeenCalledTimes(3) }) - it('skips a tick that would overlap a call still in flight', async () => { - setLicenseState({ awaitingPurchase: true, phase: 'checking' }) - renderHook(() => useDeviceLicense(BOARD)) + it('skips any tick that would overlap a call still in flight, including the first', async () => { + openPurchaseWindow() + setLicenseState({ phase: 'checking' }) + await mountOwner() await act(async () => { jest.advanceTimersByTime(POLL_MS) @@ -121,8 +161,26 @@ describe('useDeviceLicense — purchase watch', () => { expect(mockRefreshLicense).not.toHaveBeenCalled() }) + it('never polls from an instance that does not own the watch', async () => { + // The hook is mounted twice per screen (the board screen's own instance and + // the one inside useDeviceConnect). Only the owner runs the interval — + // otherwise every tick would fire once per instance on the same link. + openPurchaseWindow() + renderHook(() => useDeviceLicense(BOARD)) + await act(async () => {}) + + for (let i = 0; i < 3; i++) { + // eslint-disable-next-line no-await-in-loop -- each tick must settle before the next + await act(async () => { + jest.advanceTimersByTime(POLL_MS) + }) + } + + expect(mockRefreshLicense).not.toHaveBeenCalled() + }) + it('ends the watch when a licensed report lands, whoever produced it', () => { - setLicenseState({ awaitingPurchase: true }) + openPurchaseWindow() const { rerender } = renderHook(() => useDeviceLicense(BOARD)) // A manual "Check again" (or the poll) landed the licence. @@ -132,24 +190,43 @@ describe('useDeviceLicense — purchase watch', () => { expect(mockSetAwaitingPurchase).toHaveBeenCalledWith(false) }) - it('gives up after the tick budget instead of polling a forgotten tab forever', async () => { - setLicenseState({ awaitingPurchase: true }) - renderHook(() => useDeviceLicense(BOARD)) + it('gives up when the 10-minute window closes instead of polling a forgotten tab forever', async () => { + openPurchaseWindow() + await mountOwner() - for (let i = 0; i < MAX_TICKS + 3; i++) { + const windowTicks = PURCHASE_WATCH_WINDOW_MS / POLL_MS + for (let i = 0; i < windowTicks + 3; i++) { // eslint-disable-next-line no-await-in-loop -- each tick must settle before the next await act(async () => { jest.advanceTimersByTime(POLL_MS) }) } - // 30 refreshes, then the budget closes the watch; the extra ticks refresh nothing. - expect(mockRefreshLicense).toHaveBeenCalledTimes(MAX_TICKS) + // The immediate tick plus every interval tick strictly inside the window + // refreshed; the tick AT the deadline closed the watch instead, and the + // extra ticks refreshed nothing. + expect(mockRefreshLicense).toHaveBeenCalledTimes(windowTicks) + expect(mockSetAwaitingPurchase).toHaveBeenCalledWith(false) + }) + + it('resumes the SAME window after a remount — the deadline is absolute, not a per-mount budget', async () => { + // The deadline lives in the store. Unmount the owner, let the wall clock + // pass the deadline, remount: the first tick must close the watch rather + // than grant a fresh ten minutes to a stale checkout. + openPurchaseWindow(30_000) + const first = await mountOwner() + expect(mockRefreshLicense).toHaveBeenCalledTimes(1) + first.unmount() + + jest.setSystemTime(Date.now() + 40_000) + await mountOwner() + + expect(mockRefreshLicense).toHaveBeenCalledTimes(1) expect(mockSetAwaitingPurchase).toHaveBeenCalledWith(false) }) it('cancelPurchaseWatch stops the watch on request', () => { - setLicenseState({ awaitingPurchase: true }) + openPurchaseWindow() const { result } = renderHook(() => useDeviceLicense(BOARD)) act(() => result.current.cancelPurchaseWatch()) diff --git a/src/frontend/hooks/use-device-license.ts b/src/frontend/hooks/use-device-license.ts index 56e88aca6..9a2133c73 100644 --- a/src/frontend/hooks/use-device-license.ts +++ b/src/frontend/hooks/use-device-license.ts @@ -10,7 +10,8 @@ * purchase watch; * - the purchase watch itself: while `awaitingPurchase`, `refresh()` runs on an * interval so the licence bought in the external browser is activated and - * WRITTEN to the device without the user having to click anything. + * WRITTEN to the device without the user having to click anything. The + * interval runs in exactly ONE hook instance — see `UseDeviceLicenseOptions`. * * Deliberately NOT folded into `useDeviceConnect`: that hook is about resolving * and holding a link, this one about what the device is entitled to run. The only @@ -28,12 +29,27 @@ import { buildLicenseBuyUrl } from '../utils/license-buy-url' /** * Purchase-watch cadence. Each tick is one Modbus read frame plus, while the * backend still answers "no purchase", one cheap HTTP round-trip — light enough - * to repeat, heavy enough not to hammer a public rate-limited route. 30 ticks - * of 20s = a 10-minute window, generous for a checkout without leaving a - * forgotten tab polling forever. + * to repeat, heavy enough not to hammer a public rate-limited route. How LONG + * the watch runs is not counted in ticks: the window is the absolute deadline + * stamped into `deviceLicense.awaitingPurchaseUntil` by `setAwaitingPurchase` + * (see `PURCHASE_WATCH_WINDOW_MS` in the device slice types), so a remount + * cannot renew it and a skipped tick spends none of it. */ const PURCHASE_POLL_INTERVAL_MS = 20_000 -const PURCHASE_POLL_MAX_TICKS = 30 + +export interface UseDeviceLicenseOptions { + /** + * Whether THIS instance runs the purchase-watch interval. The hook is mounted + * more than once over the same store state — the board screen mounts one for + * the badge and `useDeviceConnect` mounts another for the connect flow — and + * if every instance ran the poll effect, each tick would fire once per + * instance against the same device link. Exactly one mount per screen owns + * the watch (the board screen passes true); every other instance keeps the + * default `false` and can still start, observe and cancel the watch, since + * those only touch store state. + */ + ownsWatch?: boolean +} export interface UseDeviceLicenseResult { /** Whether the selected board's VPP participates in licensing at all. When @@ -75,13 +91,18 @@ export interface UseDeviceLicenseResult { /** * True while the purchase watch is running — from `buy()` until a licensed * report lands, the 10-minute window closes, or `cancelPurchaseWatch`. + * Derived from the deadline in the store, so every instance agrees. */ awaitingPurchase: boolean /** Stop the purchase watch without waiting for it to conclude. */ cancelPurchaseWatch: () => void } -export function useDeviceLicense(boardInfo: BoardInfo | undefined): UseDeviceLicenseResult { +export function useDeviceLicense( + boardInfo: BoardInfo | undefined, + opts?: UseDeviceLicenseOptions, +): UseDeviceLicenseResult { + const ownsWatch = opts?.ownsWatch ?? false const device = useDevice() const system = useSystem() const startCheck = useOpenPLCStore((s) => s.deviceActions.startDeviceLicenseCheck) @@ -89,7 +110,8 @@ export function useDeviceLicense(boardInfo: BoardInfo | undefined): UseDeviceLic const setAwaitingPurchase = useOpenPLCStore((s) => s.deviceActions.setAwaitingPurchase) const phase = useOpenPLCStore((s) => s.deviceLicense.phase) const report = useOpenPLCStore((s) => s.deviceLicense.report) - const awaitingPurchase = useOpenPLCStore((s) => s.deviceLicense.awaitingPurchase) + const awaitingPurchaseUntil = useOpenPLCStore((s) => s.deviceLicense.awaitingPurchaseUntil) + const awaitingPurchase = awaitingPurchaseUntil !== null const target = useMemo(() => resolveLicensingTarget(boardInfo), [boardInfo]) @@ -149,33 +171,42 @@ export function useDeviceLicense(boardInfo: BoardInfo | undefined): UseDeviceLic } }, [awaitingPurchase, report, setAwaitingPurchase]) - // The purchase watch. One `refresh()` per tick: read the device, ask the - // backend and — on the first tick after the completion webhook lands — write - // the blob to the device and read it back. `refresh` reports its own - // failures as `check-failed` reports, so a flaky tick shows in the badge - // instead of silently killing the watch. + // The purchase watch — mounted only by the instance that owns it. One + // `refresh()` per tick: read the device, ask the backend and — on the first + // tick after the completion webhook lands — write the blob to the device and + // read it back. `refresh` reports its own failures as `check-failed` reports, + // and the badge lets a check-failed report outrank the waiting label, so a + // flaky tick shows in the badge instead of silently killing the watch. // // Called through a ref: `refresh`'s identity follows the device port and the // board target, and an interval keyed on it would be torn down and rebuilt - // on every such change, resetting the tick budget each time. + // on every such change. The 10-minute window is immune to such churn either + // way — it is the absolute deadline in the store, read back at every tick. const refreshRef = useRef(refresh) refreshRef.current = refresh useEffect(() => { - if (!awaitingPurchase) return - let ticks = 0 - const timer = setInterval(() => { - ticks += 1 - if (ticks > PURCHASE_POLL_MAX_TICKS) { + if (!ownsWatch || !awaitingPurchase) return + const tick = () => { + const { phase, awaitingPurchaseUntil: until } = useOpenPLCStore.getState().deviceLicense + if (until === null || Date.now() >= until) { + // The window closed (or the watch was cancelled between ticks): stop + // instead of polling a forgotten checkout forever. setAwaitingPurchase(false) return } // A tick that would overlap an in-flight call (slow device, 30s HTTP - // timeout) skips instead of stacking a second one on the same link. - if (useOpenPLCStore.getState().deviceLicense.phase === 'checking') return + // timeout) skips instead of stacking a second one on the same link — + // at no cost to the window, since the deadline above is wall-clock. + if (phase === 'checking') return void refreshRef.current() - }, PURCHASE_POLL_INTERVAL_MS) + } + // First check right away rather than at t+20s: in the common case the + // webhook already landed while the user finished checkout, and the licence + // should be on the device the moment they switch back from the browser. + tick() + const timer = setInterval(tick, PURCHASE_POLL_INTERVAL_MS) return () => clearInterval(timer) - }, [awaitingPurchase, setAwaitingPurchase]) + }, [ownsWatch, awaitingPurchase, setAwaitingPurchase]) /** * Build the purchase link for a given device id. @@ -203,7 +234,11 @@ export function useDeviceLicense(boardInfo: BoardInfo | undefined): UseDeviceLic // this hook's closure still sees the previous one. const url = urlFor(deviceId ?? report?.deviceId) if (!url) return - await system.openExternalLink(url) + const { success } = await system.openExternalLink(url) + // No browser opened means no purchase to watch for. Leave the state + // untouched so the Buy button stays offered, instead of trading it for + // a ten-minute wait on a page nobody is looking at. + if (!success) return // The purchase now lives in an external browser tab; start watching for // its completion so the licence is activated and written to the device // without the user having to come back and click anything. diff --git a/src/frontend/store/__tests__/device-slice.test.ts b/src/frontend/store/__tests__/device-slice.test.ts index ae283ad3a..e61cd321c 100644 --- a/src/frontend/store/__tests__/device-slice.test.ts +++ b/src/frontend/store/__tests__/device-slice.test.ts @@ -5,6 +5,7 @@ import type { BoardInfo, CommunicationPort, DevicePin, TimingStats } from '../.. import { createConsoleSlice } from '../slices/console' import { createDeviceSlice, DeviceSlice } from '../slices/device' import { defaultDeviceConfiguration } from '../slices/device/data/types' +import { PURCHASE_WATCH_WINDOW_MS } from '../slices/device/types' import * as pinsValidation from '../slices/device/validation/pins' import { createEditorSlice } from '../slices/editor' import { createLibrarySlice } from '../slices/library' @@ -279,13 +280,21 @@ describe('createDeviceSlice', () => { it('starts idle with nothing known', () => { // The state every non-licensable board stays in: nothing runs, so nothing is // known, and the UI shows no licensing affordance at all. - expect(makeStore().getState().deviceLicense).toEqual({ phase: 'idle', report: null, awaitingPurchase: false }) + expect(makeStore().getState().deviceLicense).toEqual({ + phase: 'idle', + report: null, + awaitingPurchaseUntil: null, + }) }) it('startDeviceLicenseCheck marks the call in flight', () => { const store = makeStore() store.getState().deviceActions.startDeviceLicenseCheck() - expect(store.getState().deviceLicense).toEqual({ phase: 'checking', report: null, awaitingPurchase: false }) + expect(store.getState().deviceLicense).toEqual({ + phase: 'checking', + report: null, + awaitingPurchaseUntil: null, + }) }) it('startDeviceLicenseCheck KEEPS the last report instead of blanking it', () => { @@ -295,14 +304,22 @@ describe('createDeviceSlice', () => { const store = makeStore() store.getState().deviceActions.setDeviceLicenseReport(LICENSED) store.getState().deviceActions.startDeviceLicenseCheck() - expect(store.getState().deviceLicense).toEqual({ phase: 'checking', report: LICENSED, awaitingPurchase: false }) + expect(store.getState().deviceLicense).toEqual({ + phase: 'checking', + report: LICENSED, + awaitingPurchaseUntil: null, + }) }) it('setDeviceLicenseReport lands the report and settles the phase', () => { const store = makeStore() store.getState().deviceActions.startDeviceLicenseCheck() store.getState().deviceActions.setDeviceLicenseReport(LICENSED) - expect(store.getState().deviceLicense).toEqual({ phase: 'done', report: LICENSED, awaitingPurchase: false }) + expect(store.getState().deviceLicense).toEqual({ + phase: 'done', + report: LICENSED, + awaitingPurchaseUntil: null, + }) }) it('preserves the outcome union verbatim, including the entitlement distinction', () => { @@ -346,15 +363,24 @@ describe('createDeviceSlice', () => { const store = makeStore() store.getState().deviceActions.setDeviceLicenseReport(LICENSED) store.getState().deviceActions.clearDeviceLicense() - expect(store.getState().deviceLicense).toEqual({ phase: 'idle', report: null, awaitingPurchase: false }) + expect(store.getState().deviceLicense).toEqual({ phase: 'idle', report: null, awaitingPurchaseUntil: null }) }) - it('setAwaitingPurchase opens and closes the purchase-watch window', () => { - const store = makeStore() - store.getState().deviceActions.setAwaitingPurchase(true) - expect(store.getState().deviceLicense.awaitingPurchase).toBe(true) - store.getState().deviceActions.setAwaitingPurchase(false) - expect(store.getState().deviceLicense.awaitingPurchase).toBe(false) + it('setAwaitingPurchase stamps the absolute deadline and clears it', () => { + // The window is a wall-clock deadline recorded in the store — not a tick + // counter in the poll effect — so a remounted effect resumes the SAME + // window, a skipped tick costs nothing, and the state is inspectable. + vi.useFakeTimers() + try { + vi.setSystemTime(1_755_000_000_000) + const store = makeStore() + store.getState().deviceActions.setAwaitingPurchase(true) + expect(store.getState().deviceLicense.awaitingPurchaseUntil).toBe(1_755_000_000_000 + PURCHASE_WATCH_WINDOW_MS) + store.getState().deviceActions.setAwaitingPurchase(false) + expect(store.getState().deviceLicense.awaitingPurchaseUntil).toBeNull() + } finally { + vi.useRealTimers() + } }) it('setAwaitingPurchase leaves the report and phase alone', () => { @@ -363,7 +389,11 @@ describe('createDeviceSlice', () => { const store = makeStore() store.getState().deviceActions.setDeviceLicenseReport(LICENSED) store.getState().deviceActions.setAwaitingPurchase(true) - expect(store.getState().deviceLicense).toEqual({ phase: 'done', report: LICENSED, awaitingPurchase: true }) + expect(store.getState().deviceLicense).toEqual({ + phase: 'done', + report: LICENSED, + awaitingPurchaseUntil: expect.any(Number), + }) }) it('landing a report does NOT end the watch by itself — the poll effect owns that', () => { @@ -376,51 +406,60 @@ describe('createDeviceSlice', () => { deviceId: '659a3520540f803625ddc34081e893d3', outcome: { state: 'unlicensed', entitlementChecked: true }, }) - expect(store.getState().deviceLicense.awaitingPurchase).toBe(true) + expect(store.getState().deviceLicense.awaitingPurchaseUntil).not.toBeNull() }) it('clearDeviceLicense ends the watch — a new board/disconnect makes it moot', () => { const store = makeStore() store.getState().deviceActions.setAwaitingPurchase(true) store.getState().deviceActions.clearDeviceLicense() - expect(store.getState().deviceLicense.awaitingPurchase).toBe(false) + expect(store.getState().deviceLicense.awaitingPurchaseUntil).toBeNull() }) - it('clearDeviceBoard change drops the licence — it was verified against the OLD board', () => { + it('a board change drops the licence AND the purchase watch — both were bound to the OLD board', () => { // setDeviceBoard already wipes everything else that is board-specific // (platform options, the pin-table row, vendor-screen data). A licence // report is just as board-specific: it was verified against the previous // board's deviceId and its VPP's productId. Kept across a switch, the badge // asserts possession for hardware that is no longer selected, and the buy - // link is built from the NEW package id and the OLD device id. + // link is built from the NEW package id and the OLD device id. The watch is + // OPENED before the switch so the null below asserts the reset, not the + // initial state — a live watch surviving the switch would keep polling (and + // could write a licence) against the new board's package id. const store = makeStore() store.getState().deviceActions.setDeviceBoard('ESP8266 NodeMCU') store.getState().deviceActions.setDeviceLicenseReport(LICENSED) + store.getState().deviceActions.setAwaitingPurchase(true) store.getState().deviceActions.setDeviceBoard('Raspberry Pi 4') - expect(store.getState().deviceLicense).toEqual({ phase: 'idle', report: null, awaitingPurchase: false }) + expect(store.getState().deviceLicense).toEqual({ phase: 'idle', report: null, awaitingPurchaseUntil: null }) }) it('leaves the licence alone when setDeviceBoard is called with the same board', () => { // The device screen re-sets the board on several paths; only an actual - // change invalidates the licence. + // change invalidates the licence — or ends a running purchase watch. const store = makeStore() store.getState().deviceActions.setDeviceBoard('ESP8266 NodeMCU') store.getState().deviceActions.setDeviceLicenseReport(LICENSED) + store.getState().deviceActions.setAwaitingPurchase(true) store.getState().deviceActions.setDeviceBoard('ESP8266 NodeMCU') expect(store.getState().deviceLicense.report).toEqual(LICENSED) + expect(store.getState().deviceLicense.awaitingPurchaseUntil).not.toBeNull() }) - it('clearDeviceDefinitions clears licensing — a new project may select another board', () => { + it('clearDeviceDefinitions clears licensing and the watch — a new project may select another board', () => { // A "Licensed" badge carried across a project close would be an assertion - // about hardware that is not even connected. + // about hardware that is not even connected — and a purchase watch carried + // across it would keep polling for a purchase nobody is making. The watch + // is opened first so the null below asserts the reset, not the default. const store = makeStore() store.getState().deviceActions.setDeviceLicenseReport(LICENSED) + store.getState().deviceActions.setAwaitingPurchase(true) store.getState().deviceActions.clearDeviceDefinitions() - expect(store.getState().deviceLicense).toEqual({ phase: 'idle', report: null, awaitingPurchase: false }) + expect(store.getState().deviceLicense).toEqual({ phase: 'idle', report: null, awaitingPurchaseUntil: null }) }) }) diff --git a/src/frontend/store/slices/device/slice.ts b/src/frontend/store/slices/device/slice.ts index cc66d16b7..529d4e95c 100644 --- a/src/frontend/store/slices/device/slice.ts +++ b/src/frontend/store/slices/device/slice.ts @@ -3,7 +3,8 @@ import { StateCreator } from 'zustand' import type { DeviceConfiguration, DevicePin } from '../../../../middleware/shared/ports/types' import { defaultDeviceConfiguration } from './data/types' -import type { DeviceSlice, DeviceSliceRoot, PinUpdateResponse } from './types' +import type { DeviceLicenseInfo, DeviceSlice, DeviceSliceRoot, PinUpdateResponse } from './types' +import { PURCHASE_WATCH_WINDOW_MS } from './types' import { checkIfPinAliasIsValid, checkIfPinIsValid, @@ -31,6 +32,24 @@ function getActivePinsDraft(draft: DeviceSlice): DevicePin[] { return draft.deviceDefinitions.pinMapping.pinsByBoard[board] } +/** + * Reset licensing to its initial state on an Immer draft — THE one way to drop + * a licence. Three paths must do it (`clearDeviceLicense`, an actual board + * change in `setDeviceBoard`, `clearDeviceDefinitions`), and when each inlined + * its own reset they drifted: two of them forgot the purchase watch, whose poll + * then outlived the board it was started for and kept running `refresh()` — + * including its licence WRITE — against whatever board came next. Route any + * future reset path through here so it cannot drift the same way. + */ +function resetDeviceLicense(deviceLicense: DeviceLicenseInfo): void { + deviceLicense.phase = 'idle' + deviceLicense.report = null + // Ends any purchase watch too: the poll effect keys off this deadline, and a + // watch without its board would poll (and could write a licence to) hardware + // the user never asked about. + deviceLicense.awaitingPurchaseUntil = null +} + const createDeviceSlice: StateCreator = (setState, getState) => ({ deviceAvailableOptions: { availableBoards: new Map(), @@ -70,7 +89,7 @@ const createDeviceSlice: StateCreator = (s deviceLicense: { phase: 'idle', report: null, - awaitingPurchase: false, + awaitingPurchaseUntil: null, }, deviceActions: { @@ -149,8 +168,7 @@ const createDeviceSlice: StateCreator = (s // select a different board entirely, and a "Licensed" badge carried over // from the previous one would be an assertion about hardware that is not // even connected. - deviceLicense.phase = 'idle' - deviceLicense.report = null + resetDeviceLicense(deviceLicense) }), ) }, @@ -414,8 +432,7 @@ const createDeviceSlice: StateCreator = (s // asserts possession for hardware that is no longer selected, and // the buy link gets built from the NEW package id paired with the // OLD device id — binding a purchase to the wrong board. - deviceLicense.phase = 'idle' - deviceLicense.report = null + resetDeviceLicense(deviceLicense) } deviceDefinitions.configuration.deviceBoard = deviceBoard }), @@ -606,19 +623,17 @@ const createDeviceSlice: StateCreator = (s setAwaitingPurchase: (awaiting): void => { setState( produce(({ deviceLicense }: DeviceSlice) => { - deviceLicense.awaitingPurchase = awaiting + // The window is an absolute wall-clock deadline stamped here, not a + // counter kept by the poll effect — so a remounted effect resumes + // the SAME window and a skipped overlap tick spends none of it. + deviceLicense.awaitingPurchaseUntil = awaiting ? Date.now() + PURCHASE_WATCH_WINDOW_MS : null }), ) }, clearDeviceLicense: (): void => { setState( produce(({ deviceLicense }: DeviceSlice) => { - deviceLicense.phase = 'idle' - deviceLicense.report = null - // A new board / disconnect ends any purchase watch: the poll effect - // keys off this flag, and a watch for a device that is no longer - // there would keep hitting it over a dead link. - deviceLicense.awaitingPurchase = false + resetDeviceLicense(deviceLicense) }), ) }, diff --git a/src/frontend/store/slices/device/types.ts b/src/frontend/store/slices/device/types.ts index f516ebe43..4f5b2309c 100644 --- a/src/frontend/store/slices/device/types.ts +++ b/src/frontend/store/slices/device/types.ts @@ -123,6 +123,14 @@ export type DeviceConnection = { // VPP licensing // --------------------------------------------------------------------------- +/** + * How long the purchase watch stays open after `buy()` opens the external + * purchase page: 10 minutes, generous for a checkout without leaving a + * forgotten watch polling a public rate-limited route forever. Stamped into + * `DeviceLicenseInfo.awaitingPurchaseUntil` by `setAwaitingPurchase(true)`. + */ +export const PURCHASE_WATCH_WINDOW_MS = 10 * 60_000 + /** * What the UI knows about the connected device's VPP license. * @@ -146,13 +154,19 @@ export type DeviceLicenseInfo = { */ report: DeviceLicenseReport | null /** - * True from `buy()` opening the purchase page until the poll that watches for - * the completed purchase lands a licensed report, times out, or the user - * cancels. Drives the "Waiting for purchase…" affordance and the poll effect - * in `useDeviceLicense` — the purchase happens in an external browser, so - * polling is the only feedback channel the editor has. + * Wall-clock deadline (epoch ms) of the purchase watch, or null when no watch + * is running. Non-null from `buy()` opening the purchase page until the poll + * that watches for the completed purchase lands a licensed report, the + * deadline passes, or the user cancels. Drives the "Waiting for purchase…" + * affordance and the poll effect in `useDeviceLicense` — the purchase happens + * in an external browser, so polling is the only feedback channel there is. + * + * An absolute deadline rather than a tick counter on purpose: the poll effect + * can be torn down and remounted without renewing the window, a tick skipped + * to avoid overlapping an in-flight call costs none of the budget, and the + * state stays inspectable ("waiting until T", not an opaque count). */ - awaitingPurchase: boolean + awaitingPurchaseUntil: number | null } // --------------------------------------------------------------------------- @@ -263,9 +277,11 @@ export type DeviceActions = { /** Land a finished licensing call: `phase='done'`, store the report. */ setDeviceLicenseReport: (report: DeviceLicenseReport) => void /** - * Toggle the awaiting-purchase window (see `DeviceLicenseInfo.awaitingPurchase`). - * Deliberately dumb: the poll effect in `useDeviceLicense` owns WHEN it ends - * (licensed report, timeout, cancel) — the store only records that it is open. + * Open (true) or close (false) the purchase-watch window (see + * `DeviceLicenseInfo.awaitingPurchaseUntil`). Opening stamps the absolute + * deadline `now + PURCHASE_WATCH_WINDOW_MS`; closing nulls it. Otherwise + * deliberately dumb: the poll effect in `useDeviceLicense` owns WHEN it ends + * (licensed report, deadline, cancel) — the store only records the window. */ setAwaitingPurchase: (awaiting: boolean) => void /** Reset licensing to `idle`/null — on disconnect, board change, project close. */ From 885b7da21e73ab3ebb6474e5141cecda9ba18123 Mon Sep 17 00:00:00 2001 From: Thiago Alves Date: Wed, 12 Aug 2026 14:14:39 -0400 Subject: [PATCH 5/5] fix(review): unconditional emulator stop, and test the leak the comment claims MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Addresses the review on #1009 / #670. Stop the emulator from a `finally` (shared surface, mirrors openplc-web). Sequencing `simulator.stop()` after a plain `await debugSession.stopSession()` left the emulator running whenever the teardown rejected — control jumped to the catch, which only logs. Reproduced on web by injecting a rejecting `debugger.disconnect`: emulator still running, debug panel up on frozen values, `simulatorRunning` stuck true, every retry failing identically. Nothing settled, which is worse than the bug this branch fixes. The nearest trigger is a throwing `onDisconnected` subscriber, which this repo's debugger adapter re-invokes inside its own catch — the second throw escapes. Clear `debugSessionRidesDeviceRef` on the manual Stop path (shared surface). `stopSession()` hides the debugger first, so the drop handler's `isDebuggerVisible` gate returns before it reaches the reset. Fix the load-firmware test so it exercises the leak its comment describes. It threw on `fs.readFile`, which runs BEFORE `loadAndRun` — so the emulator was never marked running and the assertion passed only because `stopSimulator()` is unconditional. It would have stayed green if the real post-`loadAndRun` leak regressed. `fs/promises` is now stubbed so the read succeeds and `loadAndRun` throws, putting the throw where the leak was; the read-failure case is kept as a separate test, since the catch cannot tell the two apart. Both fail with the `stopSimulator()` call removed. 303 suites / 6404 tests pass. The modbus-rtu-client flake and the jest worker teardown warning both reproduce on a clean `development`. Co-Authored-By: Claude Opus 5 (1M context) --- .../workspace-activity-bar/default.tsx | 43 +++++++++++++------ .../simulator-session.handler.test.ts | 39 +++++++++++++++-- 2 files changed, 66 insertions(+), 16 deletions(-) diff --git a/src/frontend/components/_organisms/workspace-activity-bar/default.tsx b/src/frontend/components/_organisms/workspace-activity-bar/default.tsx index c41914962..1966a169e 100644 --- a/src/frontend/components/_organisms/workspace-activity-bar/default.tsx +++ b/src/frontend/components/_organisms/workspace-activity-bar/default.tsx @@ -636,22 +636,41 @@ export const DefaultWorkspaceActivityBar = ({ zoom }: DefaultWorkspaceActivityBa const handleSimulatorControl = useCallback(async (): Promise => { try { if (simulatorRunning) { - // Two things end here, in this order. + // Two things end here, in this order, and the emulator's end is not + // conditional on the debug session's. // // The debug session goes first: it is a CONSUMER of the emulator, so it // has to let go of the transport before the thing on the other end // disappears. - await debugSession.stopSession() - - // Then the emulator itself — which is this button's job and nothing - // else's. `stopSession()` deliberately does not do it (see its - // docstring: a debug session is not the owner of the thing it talks - // to), so with this call missing "Stop" ended the debug session, logged - // "Simulator stopped." and left the avr8js loop running: it kept - // re-scheduling itself and burning a core for the rest of the session, - // with no way to reach it from the UI because the button had already - // flipped back to "Start". - await simulator.stop() + // + // The emulator goes second, from a `finally`, because stopping it is + // this button's job and nothing else's. `stopSession()` deliberately + // does not do it (see its docstring: a debug session is not the owner of + // the thing it talks to), so with this call missing "Stop" ended the + // debug session, logged "Simulator stopped." and left the avr8js loop + // running — re-scheduling itself and burning a core for the rest of the + // session, unreachable because the button had flipped back to "Start". + // + // Sequencing it after a plain `await` reintroduced the same leak on the + // error path: anything that rejects inside the teardown (today only a + // throwing `onDisconnected` subscriber, which is why nothing hits it + // yet) skipped straight to the catch below, which only logs. The + // emulator kept running, `simulatorRunning` stayed true, and every + // retry failed identically — worse than the original bug, because + // nothing settled at all. `finally` keeps the order and drops the + // condition. + try { + await debugSession.stopSession() + } finally { + await simulator.stop() + } + + // The session this debug session was riding is gone, so the claim that it + // rides one goes with it. The drop handler cannot clear it on this path: + // `stopSession()` has already hidden the debugger, so the handler's + // `isDebuggerVisible` gate returns before it reaches the reset — leaving + // the ref stale-`true` for a session that no longer exists. + debugSessionRidesDeviceRef.current = false setSimulatorRunning(false) addLog({ id: crypto.randomUUID(), level: 'info', message: 'Simulator stopped.' }) diff --git a/src/main/modules/ipc/__tests__/simulator-session.handler.test.ts b/src/main/modules/ipc/__tests__/simulator-session.handler.test.ts index d827c573b..33452a8e8 100644 --- a/src/main/modules/ipc/__tests__/simulator-session.handler.test.ts +++ b/src/main/modules/ipc/__tests__/simulator-session.handler.test.ts @@ -42,6 +42,13 @@ jest.mock('../../../../backend/shared/simulator/simulator-module', () => ({ SimulatorModule: jest.fn(() => simulatorModule), })) +// The handler reads the hex off disk before it starts anything. Stubbing the read +// to SUCCEED is what lets a test put the throw where the leak actually was — +// after `loadAndRun` has marked the emulator running. A failing read throws +// before that and proves nothing about it. +const readFile = jest.fn, [string, string?]>() +jest.mock('fs/promises', () => ({ readFile: (...args: [string, string?]) => readFile(...args) })) + type Bridge = MainProcessBridge function createBridge(): Bridge { @@ -75,6 +82,7 @@ beforeEach(() => { simulatorModule.loadAndRun.mockReset() simulatorModule.stop.mockReset() simulatorModule.isRunning.mockReset().mockReturnValue(true) + readFile.mockReset().mockResolvedValue(':00000001FF') }) afterEach(() => { @@ -136,15 +144,38 @@ describe('window:reload', () => { }) describe('simulator:load-firmware', () => { - it('stops the emulator and closes the session when the start throws', async () => { + it('stops the emulator and closes the session when the start throws AFTER it is running', async () => { + const bridge = createBridge() + const close = holdLink(bridge, 'simulator') + + // The leak this covers: `loadAndRun` marks the emulator running before it + // finishes wiring, so a throw from that point on left one running with no + // session behind it. The read must succeed for the throw to land there — + // throwing on the read instead would exercise a path where the emulator was + // never started, and would stay green even if this leak came back. + simulatorModule.loadAndRun.mockImplementation(() => { + throw new Error('avr8js refused the hex') + }) + + const result = await bridge.handleSimulatorLoadFirmware({} as never, '/tmp/simulator.hex') + + expect(readFile).toHaveBeenCalledTimes(1) + expect(simulatorModule.loadAndRun).toHaveBeenCalledTimes(1) + expect(result).toEqual({ success: false, error: 'avr8js refused the hex' }) + expect(simulatorModule.stop).toHaveBeenCalledTimes(1) + expect(close).toHaveBeenCalledTimes(1) + }) + + it('stops the emulator and closes the session when the read fails before it starts', async () => { const bridge = createBridge() const close = holdLink(bridge, 'simulator') + readFile.mockRejectedValue(new Error('ENOENT')) - // An unreadable hex path fails inside the handler's try, standing in for any - // throw during start — `loadAndRun` marks the emulator running before it - // finishes wiring, so a throw after that point used to leak one. const result = await bridge.handleSimulatorLoadFirmware({} as never, '/nonexistent/simulator.hex') + // Nothing was started, so there is nothing to leak — but the cleanup still + // has to be harmless, because the catch cannot tell the two apart. + expect(simulatorModule.loadAndRun).not.toHaveBeenCalled() expect(result.success).toBe(false) expect(simulatorModule.stop).toHaveBeenCalledTimes(1) expect(close).toHaveBeenCalledTimes(1)