From 7efb3e680d530df3162e44061ab6c468a3dca0ec Mon Sep 17 00:00:00 2001 From: ttbombadil Date: Mon, 5 Oct 2026 08:35:05 +0200 Subject: [PATCH] fix(simulation): invalidate obsolete REST compile-to-start ownership --- client/src/hooks/use-compile-and-run.ts | 83 +++++- client/src/hooks/use-compile-controller.ts | 14 +- client/src/hooks/use-simulation-controller.ts | 11 +- client/src/hooks/useArduinoSimulatorPage.tsx | 12 +- .../use-compile-and-run.ownership.test.tsx | 240 ++++++++++++++++++ 5 files changed, 345 insertions(+), 15 deletions(-) create mode 100644 tests/client/hooks/use-compile-and-run.ownership.test.tsx diff --git a/client/src/hooks/use-compile-and-run.ts b/client/src/hooks/use-compile-and-run.ts index b31c97d7a..fc4f9c14f 100644 --- a/client/src/hooks/use-compile-and-run.ts +++ b/client/src/hooks/use-compile-and-run.ts @@ -1,4 +1,4 @@ -import { useCallback, useEffect, type MutableRefObject, type RefObject } from "react"; +import { useCallback, useEffect, useLayoutEffect, useRef, type MutableRefObject, type RefObject } from "react"; import { type UseMutationResult } from "@tanstack/react-query"; import { Logger } from "@shared/logger"; import type { IOPinRecord, OutputLine, ParserMessage } from "@shared/schema"; @@ -173,10 +173,41 @@ interface UseCompileAndRunResult { startSimulation: () => void; startSimulationRef: MutableRefObject<(() => void) | null>; suppressAutoStopOnce: () => void; + /** Call before replacing the workspace or externally loading code. */ + invalidatePendingStart: () => void; } export function useCompileAndRun(params: CompileAndRunParams): UseCompileAndRunResult { const controllerState = useSimulatorControllerState(); + const startGeneration = useRef(0); + const compileOwners = useRef(new WeakMap()); + const pendingCompileStart = useRef(false); + const resetTimer = useRef | null>(null); + const invalidatePendingStart = useCallback(() => { + startGeneration.current++; + if (pendingCompileStart.current) { + controllerState.setCompilationStatus("ready"); + controllerState.setArduinoCliStatus("idle"); + pendingCompileStart.current = false; + } + if (resetTimer.current !== null) clearTimeout(resetTimer.current); + resetTimer.current = null; + }, [controllerState.setCompilationStatus, controllerState.setArduinoCliStatus]); + const isCompileResultCurrent = useCallback((payload: CompileConfig) => { + const owner = compileOwners.current.get(payload); + return owner === undefined || owner === startGeneration.current; + }, []); + + // Workspace identity excludes editor contents and the selected tab: ordinary + // navigation/edits keep the captured compile snapshot semantics. + const workspaceIdentity = JSON.stringify([ + params.tabs.map((tab) => [tab.id, tab.path ?? tab.name]), + params.sourceProject?.entryFile, + ]); + useLayoutEffect(() => { + invalidatePendingStart(); + }, [workspaceIdentity, invalidatePendingStart]); + useEffect(() => invalidatePendingStart, [invalidatePendingStart]); // ------------------------------------------------------------ // UI Feedback Adapter (extrahiert für Schritt 1 von Phase 2.1) @@ -212,6 +243,7 @@ export function useCompileAndRun(params: CompileAndRunParams): UseCompileAndRunR } = useCompileController({ ...controllerState, capabilities: params.capabilities, + isCompileResultCurrent, // Callbacks setParserMessages: params.setParserMessages, setParserPanelDismissed: params.setParserPanelDismissed, @@ -248,6 +280,7 @@ export function useCompileAndRun(params: CompileAndRunParams): UseCompileAndRunR }); const simulation = useSimulationController({ + onStartOrStop: invalidatePendingStart, code: params.code, hasCompilationErrors, isModified: params.isModified, @@ -278,6 +311,8 @@ export function useCompileAndRun(params: CompileAndRunParams): UseCompileAndRunR }, [simulation.setCompiledCode]); const handleCompileAndStart = useCallback(() => { + invalidatePendingStart(); + const generation = startGeneration.current; if ( params.capabilities && (!params.capabilities.canCompile || !params.capabilities.canSimulate) @@ -315,8 +350,12 @@ export function useCompileAndRun(params: CompileAndRunParams): UseCompileAndRunR // Compile with custom handlers for compile + start flow const compilePayload = { code: mainSketchCode, headers, ...(entryFile ? { entryFile } : {}) }; + compileOwners.current.set(compilePayload, generation); + pendingCompileStart.current = true; compileMutation.mutate(compilePayload, { onSuccess: (data) => { + if (generation !== startGeneration.current) return; + pendingCompileStart.current = false; logger.info(`[CLIENT] Compile response: ${JSON.stringify(data, null, 2)}`); if (data.success) { @@ -334,15 +373,18 @@ export function useCompileAndRun(params: CompileAndRunParams): UseCompileAndRunR } }, onError: () => { + if (generation !== startGeneration.current) return; + pendingCompileStart.current = false; setCompilationStatus("error"); simulation.setSimulationStatus("idle"); uiFeedback.showCompilationFailedWithErrorsToast(); scheduleCliIdle(setArduinoCliStatus); }, }); - }, [params, clearOutputs, compileMutation, simulation, uiFeedback]); + }, [params, clearOutputs, compileMutation, simulation, uiFeedback, invalidatePendingStart]); const handleReset = useCallback(() => { + invalidatePendingStart(); if (params.capabilities && !params.capabilities.canSimulate) return; if (!params.ensureBackendConnected("Reset simulation")) return; if (simulation.simulationStatus === "running") simulation.handleStop(); @@ -351,8 +393,10 @@ export function useCompileAndRun(params: CompileAndRunParams): UseCompileAndRunR uiFeedback.showResettingToast(); - setTimeout(() => { - handleCompileAndStart(); + const generation = startGeneration.current; + resetTimer.current = setTimeout(() => { + resetTimer.current = null; + if (generation === startGeneration.current) handleCompileAndStart(); }, 100); }, [ clearOutputs, @@ -361,6 +405,7 @@ export function useCompileAndRun(params: CompileAndRunParams): UseCompileAndRunR params.resetPinUI, simulation, uiFeedback, + invalidatePendingStart, ]); return { @@ -377,7 +422,10 @@ export function useCompileAndRun(params: CompileAndRunParams): UseCompileAndRunR cliOutput, setCliOutput, compileMutation, - handleCompile, + handleCompile: () => { + invalidatePendingStart(); + handleCompile(); + }, handleCompileAndStart, handleClearCompilationOutput, clearOutputs, @@ -390,8 +438,28 @@ export function useCompileAndRun(params: CompileAndRunParams): UseCompileAndRunR setSimulationTimeout: simulation.setSimulationTimeout, dockerGccPhase: controllerState.dockerGccPhase, setDockerGccPhase: controllerState.setDockerGccPhase, - startMutation: simulation.startMutation, - stopMutation: simulation.stopMutation, + startMutation: { + ...simulation.startMutation, + mutate: (...args) => { + invalidatePendingStart(); + simulation.startMutation.mutate(...args); + }, + mutateAsync: (...args) => { + invalidatePendingStart(); + return simulation.startMutation.mutateAsync(...args); + }, + }, + stopMutation: { + ...simulation.stopMutation, + mutate: (...args) => { + invalidatePendingStart(); + simulation.stopMutation.mutate(...args); + }, + mutateAsync: (...args) => { + invalidatePendingStart(); + return simulation.stopMutation.mutateAsync(...args); + }, + }, pauseMutation: simulation.pauseMutation, resumeMutation: simulation.resumeMutation, handleStart: simulation.handleStart, @@ -403,5 +471,6 @@ export function useCompileAndRun(params: CompileAndRunParams): UseCompileAndRunR startSimulation: simulation.startSimulation, startSimulationRef: simulation.startSimulationRef, suppressAutoStopOnce: simulation.suppressAutoStopOnce, + invalidatePendingStart, }; } diff --git a/client/src/hooks/use-compile-controller.ts b/client/src/hooks/use-compile-controller.ts index c1f1aec6a..2c77f926a 100644 --- a/client/src/hooks/use-compile-controller.ts +++ b/client/src/hooks/use-compile-controller.ts @@ -38,6 +38,8 @@ export type CompilationErrors = CompilerError[] | string | undefined; export interface UseCompileControllerParams { readonly capabilities?: ServerCapabilities; + /** Ignore completions whose compile-to-start owner was invalidated. */ + isCompileResultCurrent?: (payload: CompileConfig) => boolean; // State compilationStatus: CompilationStatus; setCompilationStatus: SetState; @@ -105,8 +107,10 @@ export function useCompileController(params: UseCompileControllerParams): UseCom const compileMutation = useMutation({ mutationFn: async (payload: CompileConfig): Promise => { - params.setArduinoCliStatus("compiling"); - params.setLastCompilationResult(null); + if (params.isCompileResultCurrent?.(payload) !== false) { + params.setArduinoCliStatus("compiling"); + params.setLastCompilationResult(null); + } params.uiFeedback.logCompileRequest(payload.code.length); const response = await apiRequest("POST", "/api/compile", payload); const ct = (response.headers.get("content-type") || "").toLowerCase(); @@ -126,14 +130,16 @@ export function useCompileController(params: UseCompileControllerParams): UseCom const txt = await response.text(); return { success: false, errors: txt, raw: txt }; }, - onSuccess: (data) => { + onSuccess: (data, payload) => { + if (params.isCompileResultCurrent?.(payload) === false) return; if (data.success) { handleCompileSuccess(data); } else { handleCompileError(data); } }, - onError: (error: unknown) => { + onError: (error: unknown, payload) => { + if (params.isCompileResultCurrent?.(payload) === false) return; params.setArduinoCliStatus("error"); params.uiFeedback.triggerCompileErrorGlitch(); if (params.isBackendUnreachableError(error)) { diff --git a/client/src/hooks/use-simulation-controller.ts b/client/src/hooks/use-simulation-controller.ts index fe52a93dc..d434a7bc1 100644 --- a/client/src/hooks/use-simulation-controller.ts +++ b/client/src/hooks/use-simulation-controller.ts @@ -13,6 +13,8 @@ const logger = new Logger("useSimulationController"); export type SimulationControllerParams = { readonly capabilities?: ServerCapabilities; + /** Synchronously relinquish pending REST compile-to-start ownership. */ + onStartOrStop?: () => void; code: string; hasCompilationErrors: boolean; isModified?: boolean; @@ -162,8 +164,9 @@ export function useSimulationController( }); const startSimulation = useCallback(() => { + params.onStartOrStop?.(); if (canSimulate) startMutation.mutate(); - }, [canSimulate, startMutation]); + }, [canSimulate, startMutation, params.onStartOrStop]); const setCompiledCode = useCallback((code: string) => { compiledCodeRef.current = code; }, []); @@ -174,15 +177,17 @@ export function useSimulationController( compiledEntryFileRef.current = entryFile; }, []); const handleStart = useCallback(() => { + params.onStartOrStop?.(); if (!canSimulate) return; if (!params.ensureBackendConnected("Simulation starten")) return; startSimulation(); - }, [canSimulate, params.ensureBackendConnected, startSimulation]); + }, [canSimulate, params.ensureBackendConnected, startSimulation, params.onStartOrStop]); const handleStop = useCallback(() => { + params.onStartOrStop?.(); if (!canSimulate) return; if (!params.ensureBackendConnected("Simulation stoppen")) return; stopMutation.mutate(); - }, [canSimulate, params.ensureBackendConnected, stopMutation]); + }, [canSimulate, params.ensureBackendConnected, stopMutation, params.onStartOrStop]); const handlePause = useCallback(() => { if (!canSimulate) return; if (!params.ensureBackendConnected("Simulation pausieren")) return; diff --git a/client/src/hooks/useArduinoSimulatorPage.tsx b/client/src/hooks/useArduinoSimulatorPage.tsx index 1af7d8c05..a13961611 100644 --- a/client/src/hooks/useArduinoSimulatorPage.tsx +++ b/client/src/hooks/useArduinoSimulatorPage.tsx @@ -288,6 +288,7 @@ export function useArduinoSimulatorPage() { handleResume: controllerHandleResume, handleReset: controllerHandleReset, suppressAutoStopOnce, + invalidatePendingStart, } = useCompileAndRun({ editorRef, tabs, @@ -365,6 +366,7 @@ export function useArduinoSimulatorPage() { // Use centralized output panel hook for all output-related state and callbacks const onReplaceAllFiles = useCallback(() => { + invalidatePendingStart(); if (simulationStatus === "running") { sendMessage({ type: "stop_simulation" }); } @@ -378,6 +380,7 @@ export function useArduinoSimulatorPage() { setSimulationStatus("idle"); setHasCompiledOnce(false); }, [ + invalidatePendingStart, simulationStatus, sendMessage, clearOutputs, @@ -391,6 +394,7 @@ export function useArduinoSimulatorPage() { ]); const onLoadExample = useCallback(() => { + invalidatePendingStart(); if (simulationStatus === "running") { sendMessage({ type: "stop_simulation" }); } @@ -409,6 +413,7 @@ export function useArduinoSimulatorPage() { setSimulationStatus("idle"); setHasCompiledOnce(false); }, [ + invalidatePendingStart, simulationStatus, sendMessage, clearOutputs, @@ -734,6 +739,11 @@ export function useArduinoSimulatorPage() { const externalAllowedOrigin = globalThis.location.ancestorOrigins?.[0] ?? globalThis.location.origin; + const loadExternalCode = useCallback((nextCode: string) => { + invalidatePendingStart(); + setCode(nextCode); + }, [invalidatePendingStart, setCode]); + const { pendingExternalStart } = useSimulatorExternalControl({ allowedOrigin: externalAllowedOrigin, backendReachable, @@ -742,7 +752,7 @@ export function useArduinoSimulatorPage() { handleStop, handlePause, handleResume, - setCode, + setCode: loadExternalCode, setSimulationStatus, sendMessage, pinStates, diff --git a/tests/client/hooks/use-compile-and-run.ownership.test.tsx b/tests/client/hooks/use-compile-and-run.ownership.test.tsx new file mode 100644 index 000000000..81e36072f --- /dev/null +++ b/tests/client/hooks/use-compile-and-run.ownership.test.tsx @@ -0,0 +1,240 @@ +import { beforeEach, describe, expect, it, vi } from "vitest"; +import { act, renderHook, waitFor } from "@testing-library/react"; +import type { ReactNode } from "react"; +import { QueryClient, QueryClientProvider } from "@tanstack/react-query"; +import { useCompileAndRun, type CompileAndRunParams } from "../../../client/src/hooks/use-compile-and-run"; +import { apiRequest } from "../../../client/src/lib/queryClient"; + +vi.mock("@/lib/queryClient", () => ({ apiRequest: vi.fn() })); + +function deferredResponse() { + let resolve!: (response: Response) => void; + let reject!: (error: Error) => void; + const promise = new Promise((res, rej) => { resolve = res; reject = rej; }); + return { + promise, + reject, + complete: (success = true) => resolve(new Response(JSON.stringify({ + success, output: "compiled", ...(success ? {} : { errors: "old compile error" }), + }), { headers: { "content-type": "application/json" } })), + }; +} + +function setup() { + const requests = [deferredResponse(), deferredResponse()]; + vi.mocked(apiRequest).mockImplementationOnce(() => requests[0].promise) + .mockImplementationOnce(() => requests[1].promise); + const params: CompileAndRunParams = { + editorRef: { current: null }, + tabs: [{ id: "first", name: "sketch.ino", content: "void setup() {} void loop() {}" }], + activeTabId: "first", code: "void setup() {} void loop() {}", + setSerialOutput: vi.fn(), clearSerialOutput: vi.fn(), setParserMessages: vi.fn(), + setParserPanelDismissed: vi.fn(), resetPinUI: vi.fn(), setIoRegistry: vi.fn(), + setIsModified: vi.fn(), setDebugMessages: vi.fn(), addDebugMessage: vi.fn(), + ensureBackendConnected: vi.fn(() => true), isBackendUnreachableError: vi.fn(() => false), + triggerErrorGlitch: vi.fn(), toast: vi.fn(), sendMessage: vi.fn(), + sendMessageImmediate: vi.fn(() => true), serialEventQueueRef: { current: [] }, + pendingPinConflicts: [], setPendingPinConflicts: vi.fn(), + }; + const client = new QueryClient({ defaultOptions: { mutations: { retry: false } } }); + const wrapper = ({ children }: { children: ReactNode }) => + {children}; + const hook = renderHook((p) => useCompileAndRun(p), { initialProps: params, wrapper }); + const start = async () => { + await act(async () => { hook.result.current.handleCompileAndStart(); }); + }; + const complete = async (index = 0, success = true) => { + await act(async () => { requests[index].complete(success); }); + await waitFor(() => expect(client.isMutating()).toBe(0)); + }; + const starts = () => vi.mocked(params.sendMessageImmediate!).mock.calls + .map(([message]) => message).filter((message) => message.type === "start_simulation"); + return { ...hook, params, requests, client, start, complete, starts }; +} + +describe("REST compile-to-start ownership", () => { + beforeEach(() => vi.resetAllMocks()); + + it.each([true, false])("does not start after Stop (backend reachable: %s)", async (reachable) => { + const fixture = setup(); + await fixture.start(); + vi.mocked(fixture.params.ensureBackendConnected).mockReturnValue(reachable); + act(() => fixture.result.current.handleStop()); + await fixture.complete(); + expect(fixture.starts()).toEqual([]); + expect(fixture.params.setIsModified).not.toHaveBeenCalledWith(false); + }); + + it("does not start after the project is replaced", async () => { + const fixture = setup(); + await fixture.start(); + fixture.rerender({ ...fixture.params, + tabs: [{ id: "replacement", name: "other.ino", content: "void setup() {} void loop() { delay(1); }" }], + activeTabId: "replacement", code: "void setup() {} void loop() { delay(1); }", + }); + await fixture.complete(); + expect(fixture.starts()).toEqual([]); + }); + + it("only starts the latest of two compile requests completed out of order", async () => { + const fixture = setup(); + await fixture.start(); + await fixture.start(); + await act(async () => { fixture.requests[1].complete(); }); + await waitFor(() => expect(fixture.starts()).toHaveLength(1)); + await fixture.complete(0); + expect(fixture.starts()).toHaveLength(1); + }); + + it("does not start again after a newer standalone Start", async () => { + const fixture = setup(); + await fixture.start(); + await act(async () => { fixture.result.current.handleStart(); }); + await fixture.complete(); + expect(fixture.starts()).toHaveLength(1); + }); + + it("relinquishes ownership when the exposed Stop mutation is used directly", async () => { + const fixture = setup(); + await fixture.start(); + act(() => fixture.result.current.stopMutation.mutate()); + await fixture.complete(); + expect(fixture.starts()).toEqual([]); + }); + + it("relinquishes ownership when the exposed Start mutation is used directly", async () => { + const fixture = setup(); + await fixture.start(); + await act(async () => { await fixture.result.current.startMutation.mutateAsync(); }); + await fixture.complete(); + expect(fixture.starts()).toHaveLength(1); + }); + + it("does not let an old failing compile stop the newer running simulation", async () => { + const fixture = setup(); + await fixture.start(); + await fixture.start(); + await act(async () => { fixture.requests[1].complete(); }); + await waitFor(() => expect(fixture.result.current.simulationStatus).toBe("running")); + await fixture.complete(0, false); + expect(fixture.result.current.simulationStatus).toBe("running"); + expect(fixture.result.current.hasCompilationErrors).toBe(false); + expect(fixture.params.sendMessage).not.toHaveBeenCalledWith({ type: "stop_simulation" }); + }); + + it("lets standalone Compile supersede an older compile-to-start result", async () => { + const fixture = setup(); + await fixture.start(); + await act(async () => { fixture.result.current.handleCompile(); }); + await act(async () => { fixture.requests[1].complete(); }); + await fixture.complete(0, false); + expect(fixture.starts()).toEqual([]); + expect(fixture.result.current.hasCompilationErrors).toBe(false); + expect(fixture.result.current.arduinoCliStatus).toBe("success"); + }); + + it("does not start after unmount", async () => { + const fixture = setup(); + await fixture.start(); + fixture.unmount(); + await fixture.complete(); + expect(fixture.starts()).toEqual([]); + }); + + it.each(["offline Stop", "external code replacement"])("releases compile indicators after %s", async (cancellation) => { + const fixture = setup(); + await fixture.start(); + expect(fixture.result.current.arduinoCliStatus).toBe("compiling"); + act(() => { + if (cancellation === "offline Stop") { + vi.mocked(fixture.params.ensureBackendConnected).mockReturnValue(false); + fixture.result.current.handleStop(); + } else { + fixture.result.current.invalidatePendingStart(); + } + }); + expect(fixture.result.current.compilationStatus).toBe("ready"); + expect(fixture.result.current.arduinoCliStatus).toBe("idle"); + await fixture.complete(); + expect(fixture.result.current.compilationStatus).toBe("ready"); + expect(fixture.result.current.arduinoCliStatus).toBe("idle"); + }); + + it("does not restore a busy indicator when Stop precedes mutation execution", async () => { + const fixture = setup(); + await act(async () => { + fixture.result.current.handleCompileAndStart(); + fixture.result.current.handleStop(); + }); + expect(fixture.result.current.compilationStatus).toBe("ready"); + expect(fixture.result.current.arduinoCliStatus).toBe("idle"); + await fixture.complete(); + expect(fixture.starts()).toEqual([]); + expect(fixture.result.current.arduinoCliStatus).toBe("idle"); + }); + + it("does not restart when Stop cancels a delayed Reset", async () => { + const fixture = setup(); + vi.useFakeTimers(); + try { + act(() => { + fixture.result.current.handleReset(); + fixture.result.current.handleStop(); + }); + await act(async () => { await vi.advanceTimersByTimeAsync(100); }); + expect(apiRequest).not.toHaveBeenCalled(); + expect(fixture.starts()).toEqual([]); + } finally { + vi.useRealTimers(); + } + }); + + it("allows a new compile-to-start requested immediately after Stop", async () => { + const fixture = setup(); + await fixture.start(); + await act(async () => { + fixture.result.current.handleStop(); + fixture.result.current.handleCompileAndStart(); + }); + await act(async () => { fixture.requests[0].complete(); }); + await fixture.complete(1); + expect(fixture.starts()).toHaveLength(1); + }); + + it("keeps the captured source when ordinary editor contents change", async () => { + const fixture = setup(); + await fixture.start(); + fixture.rerender({ ...fixture.params, code: "edited while compiling" }); + await fixture.complete(); + expect(fixture.starts()).toEqual([{ + type: "start_simulation", timeout: 60, code: fixture.params.tabs[0].content, + }]); + }); + + it("does not mistake navigation within the same project for replacement", async () => { + const fixture = setup(); + const tabs = [...fixture.params.tabs, { id: "header", name: "pins.h", content: "#define PIN 13" }]; + fixture.rerender({ ...fixture.params, tabs }); + await fixture.start(); + fixture.rerender({ ...fixture.params, tabs: tabs.map((tab) => ({ ...tab })), + activeTabId: "header", code: "#define PIN 13", + }); + await fixture.complete(); + expect(fixture.starts()).toHaveLength(1); + expect(fixture.starts()[0]).toMatchObject({ headers: [{ name: "pins.h", content: "#define PIN 13" }] }); + }); + + it("ignores an old transport error after the replacement compile succeeds", async () => { + const fixture = setup(); + await fixture.start(); + await fixture.start(); + await act(async () => { fixture.requests[1].complete(); }); + await waitFor(() => expect(fixture.result.current.simulationStatus).toBe("running")); + vi.mocked(fixture.params.toast).mockClear(); + await act(async () => { fixture.requests[0].reject(new Error("old connection error")); }); + await waitFor(() => expect(fixture.client.isMutating()).toBe(0)); + expect(fixture.result.current.simulationStatus).toBe("running"); + expect(fixture.result.current.arduinoCliStatus).toBe("success"); + expect(fixture.params.toast).not.toHaveBeenCalled(); + }); +});