diff --git a/src/frontend/components/_templates/accelerator-handler.tsx b/src/frontend/components/_templates/accelerator-handler.tsx index 1d5136a1e..a28bd40d5 100644 --- a/src/frontend/components/_templates/accelerator-handler.tsx +++ b/src/frontend/components/_templates/accelerator-handler.tsx @@ -288,21 +288,31 @@ const AcceleratorHandler = () => { /** * Undo / Redo */ + // An invalid graphical body blocks history changes outright, so say so rather + // than letting the shortcut look broken (DOPE-495). + const notifyStaleBody = useCallback((pouName: string) => { + toast({ + title: 'History unavailable', + description: `The graphical body of "${pouName}" is invalid, so its history was not changed.`, + variant: 'fail', + }) + }, []) + useEffect(() => { const unsub = accelerator.onUndo(() => { if (!meta?.name) return - undo(meta.name) + if (!undo(meta.name)) notifyStaleBody(meta.name) }) return unsub - }, [meta.name, isMonacoFocused, accelerator, undo]) + }, [meta.name, isMonacoFocused, accelerator, undo, notifyStaleBody]) useEffect(() => { const unsub = accelerator.onRedo(() => { if (!meta?.name) return - redo(meta.name) + if (!redo(meta.name)) notifyStaleBody(meta.name) }) return unsub - }, [meta.name, isMonacoFocused, accelerator, redo]) + }, [meta.name, isMonacoFocused, accelerator, redo, notifyStaleBody]) /** * Quit app (Ctrl+Q on Windows/Linux) diff --git a/src/frontend/hooks/use-pou-snapshot.ts b/src/frontend/hooks/use-pou-snapshot.ts index 1ba92a930..e7f715e90 100644 --- a/src/frontend/hooks/use-pou-snapshot.ts +++ b/src/frontend/hooks/use-pou-snapshot.ts @@ -24,8 +24,9 @@ export function usePouSnapshot() { const captureAndPush = useCallback( (pouName: string) => { // A debounced graphical write-back may still be pending — flush it so - // the snapshot can't pair a stale body with a fresh flow. - flushFlowWriteBacks(useOpenPLCStore.getState, pouName) + // the snapshot can't pair a stale body with a fresh flow. A failed flush + // leaves the body stale, so there is nothing coherent to capture. + if (flushFlowWriteBacks(useOpenPLCStore.getState, pouName).length > 0) return const { project, ladderFlows, fbdFlows } = useOpenPLCStore.getState() const pou = project.data.pous.find((p) => p.name === pouName) if (!pou) return diff --git a/src/frontend/services/__tests__/save-actions.test.ts b/src/frontend/services/__tests__/save-actions.test.ts index dac639efe..f1f67823a 100644 --- a/src/frontend/services/__tests__/save-actions.test.ts +++ b/src/frontend/services/__tests__/save-actions.test.ts @@ -1,22 +1,162 @@ /** * save-actions.ts test file * - * All exported functions (executeSaveProject, executeSaveFile, executeSaveActiveFile) - * depend on `openPLCStoreBase.getState()` to read the Zustand store and on - * `projectPort` (an async interface) to persist data. They also call `toast()` - * which mutates module-level state. - * - * Because these functions are NOT pure (they read/mutate external state), - * they cannot be tested without mocking. The pure helpers they delegate to - * (sanitizePou, collectDebugVariables, serializePouToText, etc.) are covered - * by their own dedicated test suites. - * - * SKIPPED: requires jest.mock / vi.mock for openPLCStoreBase and projectPort. + * The pure helpers these functions delegate to (sanitizePou, + * collectDebugVariables, serializePouToText, …) are covered by their own + * suites. The cases below drive the real store singleton to pin the DOPE-495 + * contract: a graphical flow that fails schema validation keeps a stale + * `pou.body.value`, so it must never be reported as saved. */ +import type { PlatformCapabilities } from '../../../middleware/shared/ports/platform-capabilities' +import type { ProjectPort } from '../../../middleware/shared/ports/project-port' +import { openPLCStoreBase } from '../../store' +import type { LadderFlowType } from '../../store/slices/ladder' +import { getMemoryState } from '../../utils/toast' +import { executeSaveFile, executeSaveProject } from '../save-actions' + +const capabilities = { isNativeApplication: true } as PlatformCapabilities + +const lastToast = () => getMemoryState().toasts[0] + +function makeProjectPort(): ProjectPort { + return { + saveProject: vi.fn().mockResolvedValue({ success: true }), + saveFile: vi.fn().mockResolvedValue({ success: true }), + } as unknown as ProjectPort +} + +function createLadderPou(name: string) { + const state = openPLCStoreBase.getState() + state.pouActions.create({ type: 'program', name, language: 'ld' }) + state.ladderFlowActions.startLadderRung({ + editorName: name, + rungId: `rung_${name}_1`, + defaultBounds: [300, 100], + reactFlowViewport: [300, 100], + }) + state.ladderFlowActions.setFlowUpdated({ editorName: name, updated: true }) +} + +/** Drop `defaultBounds` / `reactFlowViewport` so the flow fails the zod guard. */ +function corruptFlow(name: string) { + const flow = openPLCStoreBase.getState().ladderFlows.find((f) => f.name === name) + openPLCStoreBase.getState().ladderFlowActions.addLadderFlow({ + name, + updated: true, + rungs: (flow?.rungs ?? []).map((rung) => ({ id: rung.id, comment: '', nodes: [], edges: [] })), + } as unknown as LadderFlowType) + openPLCStoreBase.getState().ladderFlowActions.setFlowUpdated({ editorName: name, updated: true }) +} + +const flowUpdated = (name: string) => openPLCStoreBase.getState().ladderFlows.find((f) => f.name === name)?.updated +const fileSaved = (name: string) => openPLCStoreBase.getState().files[name]?.saved + describe('save-actions', () => { - it('is skipped because all exported functions require store mocking', () => { - // Intentionally empty — see file-level comment. - expect(true).toBe(true) + let warn: ReturnType + + beforeEach(() => { + warn = vi.spyOn(console, 'warn').mockImplementation(() => {}) + openPLCStoreBase.getState().ladderFlowActions.clearLadderFlows() + }) + + afterEach(() => { + warn.mockRestore() + }) + + describe('executeSaveProject', () => { + it('reports success and clears the updated flag for a valid flow', async () => { + createLadderPou('ValidPou') + + const result = await executeSaveProject(makeProjectPort(), capabilities) + + expect(result.success).toBe(true) + expect(flowUpdated('ValidPou')).toBe(false) + }) + + it('does not report a POU as saved when its flow fails validation', async () => { + createLadderPou('BrokenPou') + corruptFlow('BrokenPou') + + const result = await executeSaveProject(makeProjectPort(), capabilities) + + expect(result.success).toBe(false) + // Keeping `updated` set is what lets a later edit retry the write-back. + expect(flowUpdated('BrokenPou')).toBe(true) + expect(fileSaved('BrokenPou')).toBe(false) + expect(lastToast()).toMatchObject({ title: 'Some changes were not saved', variant: 'fail' }) + }) + + it('still saves the valid POUs alongside a failing one', async () => { + createLadderPou('GoodPou') + createLadderPou('BadPou') + corruptFlow('BadPou') + + const projectPort = makeProjectPort() + const result = await executeSaveProject(projectPort, capabilities) + + expect(result.success).toBe(false) + expect(projectPort.saveProject).toHaveBeenCalled() + expect(flowUpdated('GoodPou')).toBe(false) + expect(fileSaved('GoodPou')).toBe(true) + }) + + it('stops blocking saves once the POU behind an invalid flow is deleted', async () => { + createLadderPou('Doomed') + corruptFlow('Doomed') + createLadderPou('Healthy') + + expect((await executeSaveProject(makeProjectPort(), capabilities)).success).toBe(false) + + // Deleting the POU is the user's only escape hatch, and it leaves the + // flow behind — the save must stop reporting it. + openPLCStoreBase.getState().pouActions.delete('Doomed') + + const result = await executeSaveProject(makeProjectPort(), capabilities) + + expect(result.success).toBe(true) + expect(fileSaved('Healthy')).toBe(true) + }) + }) + + describe('executeSaveFile', () => { + it('refuses to write the stale body of a failing flow', async () => { + createLadderPou('BrokenFile') + corruptFlow('BrokenFile') + + const projectPort = makeProjectPort() + const result = await executeSaveFile('BrokenFile', projectPort, capabilities) + + expect(result.success).toBe(false) + expect(projectPort.saveFile).not.toHaveBeenCalled() + expect(flowUpdated('BrokenFile')).toBe(true) + }) + + it('writes a valid flow normally', async () => { + createLadderPou('ValidFile') + + const projectPort = makeProjectPort() + const result = await executeSaveFile('ValidFile', projectPort, capabilities) + + expect(result.success).toBe(true) + expect(projectPort.saveFile).toHaveBeenCalled() + expect(flowUpdated('ValidFile')).toBe(false) + }) + + it('leaves an unrelated failing POU untouched', async () => { + createLadderPou('TargetFile') + createLadderPou('Unrelated') + corruptFlow('Unrelated') + + const projectPort = makeProjectPort() + const result = await executeSaveFile('TargetFile', projectPort, capabilities) + + expect(result.success).toBe(true) + expect(projectPort.saveFile).toHaveBeenCalled() + // The flush is scoped to the target, so the unrelated flow is never + // validated and never warns. + expect(warn).not.toHaveBeenCalled() + expect(flowUpdated('Unrelated')).toBe(true) + }) }) }) diff --git a/src/frontend/services/save-actions.ts b/src/frontend/services/save-actions.ts index 5daf9e5e7..6e852ddf0 100644 --- a/src/frontend/services/save-actions.ts +++ b/src/frontend/services/save-actions.ts @@ -324,8 +324,9 @@ export async function executeSaveProject( ): Promise<{ success: boolean }> { // Run any pending debounced graphical write-backs before reading state: // a save landing inside the debounce window must serialize the fresh - // POU bodies, not the pre-edit ones. - flushFlowWriteBacks(openPLCStoreBase.getState) + // POU bodies, not the pre-edit ones. Flows that fail validation keep a + // stale body, so they must not be reported as saved (DOPE-495). + const staleFlows = flushFlowWriteBacks(openPLCStoreBase.getState) const state = openPLCStoreBase.getState() // Persist gate. Every save path — Ctrl+S, File → Save, auto-save after // a rename/delete, the AI panel — funnels through here. When the viewer @@ -339,7 +340,7 @@ export async function executeSaveProject( } const { project, pendingDeletions } = state const { setEditingState } = state.workspaceActions - const { setAllToSaved } = state.fileActions + const { setAllToSaved, updateFile } = state.fileActions const { markAllSaved } = state.snapshotActions const deletionsBeforeSave = [...pendingDeletions] @@ -438,22 +439,38 @@ export async function executeSaveProject( deleted: deletionsBeforeSave, }) + const isStale = new Set(staleFlows) + state.projectActions.clearPendingDeletions() - setEditingState('saved') + setEditingState(staleFlows.length > 0 ? 'unsaved' : 'saved') setAllToSaved() - markAllSaved() + markAllSaved(staleFlows) - // Reset graphical flow state: clear selections and updated flags + // Reset graphical flow state: clear selections and updated flags. + // A stale flow keeps `updated` set and its file dirty — clearing them + // would strand the in-memory edit with no way back to disk. for (const flow of state.ladderFlows) { state.ladderFlowActions.clearSelections({ editorName: flow.name }) + if (isStale.has(flow.name)) continue state.ladderFlowActions.setFlowUpdated({ editorName: flow.name, updated: false }) } for (const flow of state.fbdFlows) { state.fbdFlowActions.clearSelections({ editorName: flow.name }) + if (isStale.has(flow.name)) continue state.fbdFlowActions.setFlowUpdated({ editorName: flow.name, updated: false }) } + // Must stay after `setAllToSaved()` above, which marks every file saved. + for (const name of staleFlows) { + updateFile({ name, saved: false }) + } - if (!capabilities.isNativeApplication) { + if (staleFlows.length > 0) { + toast({ + title: 'Some changes were not saved', + description: `The graphical body of ${staleFlows.join(', ')} is invalid and could not be written to disk. Every other file was saved.`, + variant: 'fail', + }) + } else if (!capabilities.isNativeApplication) { toast({ title: 'Changes saved!', description: 'The project was saved successfully!', @@ -468,7 +485,9 @@ export async function executeSaveProject( variant: 'fail', }) } - return { success: res.success } + // A stale flow means the user's graphical edit never reached disk, so + // callers that gate on the save (build, close-project) must not proceed. + return { success: res.success && staleFlows.length === 0 } } catch { setEditingState('unsaved') toast({ @@ -496,8 +515,9 @@ export async function executeSaveFile( projectPort: ProjectPort, capabilities: PlatformCapabilities, ): Promise<{ success: boolean }> { - // See executeSaveProject — same pending write-back flush requirement. - flushFlowWriteBacks(openPLCStoreBase.getState) + // See executeSaveProject — same pending write-back flush requirement, scoped + // to the target so a single-file save doesn't touch unrelated POUs. + const staleFlows = flushFlowWriteBacks(openPLCStoreBase.getState, fileName) const state = openPLCStoreBase.getState() // See executeSaveProject for rationale — same persist gate. if (!state.workspace.canEdit) { @@ -524,6 +544,12 @@ export async function executeSaveFile( return { success: false } } + // Writing the stale body would overwrite disk with pre-edit content and + // then report success — abort instead (DOPE-495). + if (staleFlows.includes(fileName)) { + return fail(`The graphical body of "${fileName}" is invalid, so the file was not written to disk.`) + } + try { // Use the same canonical serializer as the full-project save path so // both flows agree on what bytes hit disk. For POUs and JSON files this diff --git a/src/frontend/store/__tests__/flow-writeback.test.ts b/src/frontend/store/__tests__/flow-writeback.test.ts index 649595c8c..36c5d60e0 100644 --- a/src/frontend/store/__tests__/flow-writeback.test.ts +++ b/src/frontend/store/__tests__/flow-writeback.test.ts @@ -173,11 +173,13 @@ describe('flow write-back scheduler', () => { } as unknown as LadderFlowType) store.getState().ladderFlowActions.setFlowUpdated({ editorName: 'Main', updated: true }) + const warn = vi.spyOn(console, 'warn').mockImplementation(() => {}) scheduleFlowWriteBack(getState, 'Main', 'ld') vi.advanceTimersByTime(FLOW_WRITEBACK_DEBOUNCE_MS) expect(ladderBody('Main')?.rungs).toHaveLength(0) expect(store.getState().ladderFlows.find((f) => f.name === 'Main')?.updated).toBe(true) + warn.mockRestore() }) }) @@ -210,13 +212,67 @@ describe('flow write-back scheduler', () => { expect(ladderBody('B')?.rungs).toHaveLength(1) }) - it('is a no-op when nothing is pending', () => { + it('leaves the other language untouched when scoped to one POU', () => { makeDirtyLadderPou('Main') + store.getState().pouActions.create({ type: 'program', name: 'FbdMain', language: 'fbd' }) + store.getState().fbdFlowActions.startFBDRung({ editorName: 'FbdMain' }) + store.getState().fbdFlowActions.setFlowUpdated({ editorName: 'FbdMain', updated: true }) + + expect(flushFlowWriteBacks(getState, 'Main')).toEqual([]) + expect(store.getState().fbdFlows.find((f) => f.name === 'FbdMain')?.updated).toBe(true) + }) + + it('writes back an updated flow that has no pending timer', () => { + makeDirtyLadderPou('Main') + + expect(flushFlowWriteBacks(getState)).toEqual([]) + expect(ladderBody('Main')?.rungs).toHaveLength(1) + expect(store.getState().ladderFlows.find((f) => f.name === 'Main')?.updated).toBe(false) + }) + + it('is a no-op when no flow is marked updated', () => { + makeDirtyLadderPou('Main') + flushFlowWriteBacks(getState) const bodyBefore = ladderBody('Main') + expect(flushFlowWriteBacks(getState)).toEqual([]) + expect(ladderBody('Main')).toBe(bodyBefore) + }) + + it('reports the POU and leaves the body stale when the flow fails validation', () => { + makeDirtyLadderPou('Main') flushFlowWriteBacks(getState) + const bodyBefore = ladderBody('Main') + + const warn = vi.spyOn(console, 'warn').mockImplementation(() => {}) + const flow = store.getState().ladderFlows.find((f) => f.name === 'Main') + store.getState().ladderFlowActions.setRungs({ + editorName: 'Main', + // `defaultBounds` is required by the schema — dropping it makes the flow invalid. + rungs: (flow?.rungs ?? []).map(({ defaultBounds: _defaultBounds, ...rung }) => rung) as never, + }) + expect(flushFlowWriteBacks(getState)).toEqual(['Main']) expect(ladderBody('Main')).toBe(bodyBefore) + expect(store.getState().ladderFlows.find((f) => f.name === 'Main')?.updated).toBe(true) + expect(warn).toHaveBeenCalled() + warn.mockRestore() + }) + + it('reports a failing FBD flow', () => { + store.getState().pouActions.create({ type: 'program', name: 'FbdMain', language: 'fbd' }) + store.getState().fbdFlowActions.startFBDRung({ editorName: 'FbdMain' }) + + const warn = vi.spyOn(console, 'warn').mockImplementation(() => {}) + // `nodes` is required by the schema — dropping it makes the flow invalid. + store.getState().fbdFlowActions.setRung({ + editorName: 'FbdMain', + rung: { comment: '', edges: [], selectedNodes: [] } as never, + }) + + expect(flushFlowWriteBacks(getState)).toEqual(['FbdMain']) + expect(warn).toHaveBeenCalled() + warn.mockRestore() }) }) diff --git a/src/frontend/store/__tests__/shared-slice.test.ts b/src/frontend/store/__tests__/shared-slice.test.ts index 0b7720d91..87c7f22c0 100644 --- a/src/frontend/store/__tests__/shared-slice.test.ts +++ b/src/frontend/store/__tests__/shared-slice.test.ts @@ -8,6 +8,7 @@ import { createEditorSlice } from '../slices/editor/slice' import { createFBDFlowSlice } from '../slices/fbd/slice' import { createFileSlice } from '../slices/file/slice' import { createHistorySlice } from '../slices/history/slice' +import type { LadderFlowType } from '../slices/ladder' import { createLadderFlowSlice } from '../slices/ladder/slice' import { createLibrarySlice } from '../slices/library/slice' import { createModalSlice } from '../slices/modal/slice' @@ -1104,6 +1105,34 @@ describe('createSharedSlice', () => { expect(store.getState().undoRedo['Main'].past).toHaveLength(0) }) + it('does nothing when the POU flow fails its write-back', () => { + const warn = vi.spyOn(console, 'warn').mockImplementation(() => {}) + store.getState().pouActions.create({ type: 'program', name: 'Graphical', language: 'ld' }) + store.getState().snapshotActions.pushToHistory('Graphical', snapshot1) + // `rungs` entries without `defaultBounds` fail the ladder schema, so the + // body stays stale and a snapshot here would pair it with a fresh flow. + store.getState().ladderFlowActions.addLadderFlow({ + name: 'Graphical', + updated: true, + rungs: [{ id: 'r1', comment: '', nodes: [], edges: [] }], + } as unknown as LadderFlowType) + store.getState().ladderFlowActions.setFlowUpdated({ editorName: 'Graphical', updated: true }) + + // `false` is what drives the "History unavailable" toast in the UI. + expect(store.getState().snapshotActions.undo('Graphical')).toBe(false) + expect(store.getState().snapshotActions.redo('Graphical')).toBe(false) + + expect(store.getState().undoRedo['Graphical'].past).toHaveLength(1) + expect(store.getState().undoRedo['Graphical'].future).toHaveLength(0) + warn.mockRestore() + }) + + it('reports success when there is simply nothing to undo', () => { + expect(store.getState().snapshotActions.undo('Main')).toBe(true) + store.getState().snapshotActions.pushToHistory('Main', snapshot1) + expect(store.getState().snapshotActions.undo('Main')).toBe(true) + }) + it('undo falls back to empty array when POU has no interface', () => { // Manually strip the POU interface to test the ?? [] fallback const pous = store.getState().project.data.pous.map((p) => { @@ -2158,6 +2187,16 @@ describe('createSharedSlice', () => { expect(store.getState().undoRedo['P1'].savedAtDepth).toBe(1) expect(store.getState().undoRedo['P2'].savedAtDepth).toBe(2) }) + + it('skips the excluded POUs', () => { + store.getState().snapshotActions.pushToHistory('P1', { variables: [], body: 'v1' }) + store.getState().snapshotActions.pushToHistory('P2', { variables: [], body: 'v1' }) + + store.getState().snapshotActions.markAllSaved(['P2']) + + expect(store.getState().undoRedo['P1'].savedAtDepth).toBe(1) + expect(store.getState().undoRedo['P2'].savedAtDepth).toBe(0) + }) }) // ----------------------------------------------------------------------- diff --git a/src/frontend/store/slices/shared/flow-writeback.ts b/src/frontend/store/slices/shared/flow-writeback.ts index 07e3cd491..b439232b0 100644 --- a/src/frontend/store/slices/shared/flow-writeback.ts +++ b/src/frontend/store/slices/shared/flow-writeback.ts @@ -34,13 +34,14 @@ type GetWriteBackState = () => SharedRootState const pendingWriteBacks = new Map }>() -function runWriteBack(getState: GetWriteBackState, pouName: string, language: FlowLanguage): void { +/** @returns `false` when the flow is invalid and `pou.body.value` is left stale. */ +function runWriteBack(getState: GetWriteBackState, pouName: string, language: FlowLanguage): boolean { const state = getState() const flow = language === 'ld' ? state.ladderFlows.find((f) => f.name === pouName) : state.fbdFlows.find((f) => f.name === pouName) - if (!flow?.updated) return + if (!flow?.updated) return true // Validate with zod but persist the raw object (minus the transient // `updated` flag). Using the parsed result would silently strip every @@ -48,13 +49,20 @@ function runWriteBack(getState: GetWriteBackState, pouName: string, language: Fl // byte-drifting the serialized POU vs. the loaded disk copy — phantom // "Modified" entries in Source Control (see DOPE-477). const schema = language === 'ld' ? zodLadderFlowSchema : zodFBDFlowSchema - if (!schema.safeParse(flow).success) return + const validation = schema.safeParse(flow) + if (!validation.success) { + console.warn(`[flow-writeback] "${pouName}" (${language}) failed validation — body left stale`, { + issues: validation.error.issues, + }) + return false + } const { updated: _updated, ...flowBody } = flow state.projectActions.updatePou({ name: pouName, content: { language, value: flowBody } }) const flowActions = language === 'ld' ? state.ladderFlowActions : state.fbdFlowActions flowActions.setFlowUpdated({ editorName: pouName, updated: false }) + return true } /** @@ -78,14 +86,36 @@ export function scheduleFlowWriteBack(getState: GetWriteBackState, pouName: stri pendingWriteBacks.set(pouName, { language, timer }) } -/** Run pending write-backs immediately — all of them, or a single POU's. */ -export function flushFlowWriteBacks(getState: GetWriteBackState, pouName?: string): void { - for (const [name, pending] of [...pendingWriteBacks]) { - if (pouName !== undefined && name !== pouName) continue - clearTimeout(pending.timer) - pendingWriteBacks.delete(name) - runWriteBack(getState, name, pending.language) +/** + * Run pending write-backs immediately — all of them, or a single POU's. + * + * Every `updated` flow is written back, not just the ones with a live timer: + * a timer that already fired and failed validation leaves no pending entry + * behind, so a pending-only sweep would report success for a POU whose body + * is still stale (DOPE-495). + * + * @returns names of POUs whose body could not be updated. + */ +export function flushFlowWriteBacks(getState: GetWriteBackState, pouName?: string): readonly string[] { + cancelFlowWriteBacks(pouName) + + const state = getState() + // Deleting a POU leaves its flow behind, so an invalid one would be reported + // stale on every future save — blocking saves and builds for good. It has no + // body left to write back, so it isn't a write-back failure. + const livePous = new Set(state.project.data.pous.map((pou) => pou.name)) + const failed: string[] = [] + for (const flow of state.ladderFlows) { + if (pouName !== undefined && flow.name !== pouName) continue + if (!livePous.has(flow.name)) continue + if (flow.updated && !runWriteBack(getState, flow.name, 'ld')) failed.push(flow.name) + } + for (const flow of state.fbdFlows) { + if (pouName !== undefined && flow.name !== pouName) continue + if (!livePous.has(flow.name)) continue + if (flow.updated && !runWriteBack(getState, flow.name, 'fbd')) failed.push(flow.name) } + return failed } /** Drop pending write-backs without running them (project open). */ diff --git a/src/frontend/store/slices/shared/slice.ts b/src/frontend/store/slices/shared/slice.ts index 016c920bb..cc2b78116 100644 --- a/src/frontend/store/slices/shared/slice.ts +++ b/src/frontend/store/slices/shared/slice.ts @@ -979,10 +979,11 @@ const createSharedSlice: StateCreator = (s ) }, - markAllSaved: () => { + markAllSaved: (except) => { setState( produce((state: SharedRootState) => { - for (const history of Object.values(state.undoRedo)) { + for (const [pouName, history] of Object.entries(state.undoRedo)) { + if (except?.includes(pouName)) continue history.savedAtDepth = history.past.length } }), @@ -992,14 +993,16 @@ const createSharedSlice: StateCreator = (s undo: (pouName) => { // A debounced graphical write-back may still be pending — flush it so // the redo snapshot below can't pair a stale body with a fresh flow. - flushFlowWriteBacks(getState, pouName) + // A failed flush leaves the body stale, and capturing it would restore + // the file to "saved" over content that never reached disk (DOPE-495). + if (flushFlowWriteBacks(getState, pouName).length > 0) return false const state = getState() const history = state.undoRedo[pouName] - if (!history || history.past.length === 0) return + if (!history || history.past.length === 0) return true const snapshot = history.past[history.past.length - 1] const pou = state.project.data.pous.find((p) => p.name === pouName) - if (!pou) return + if (!pou) return true // Save current state to future. Plain references — the store is // immer-managed (frozen, copy-on-write), so later edits can never @@ -1045,18 +1048,19 @@ const createSharedSlice: StateCreator = (s if (afterUndo?.savedAtDepth !== null && afterUndo?.savedAtDepth === afterUndo?.past.length) { getState().fileActions.updateFile({ name: pouName, saved: true }) } + return true }, redo: (pouName) => { // See undo — same pending write-back consistency requirement. - flushFlowWriteBacks(getState, pouName) + if (flushFlowWriteBacks(getState, pouName).length > 0) return false const state = getState() const history = state.undoRedo[pouName] - if (!history || history.future.length === 0) return + if (!history || history.future.length === 0) return true const snapshot = history.future[history.future.length - 1] const pou = state.project.data.pous.find((p) => p.name === pouName) - if (!pou) return + if (!pou) return true // Save current state to past. Plain references — see undo. const currentSnapshot: PouHistorySnapshot = { @@ -1100,6 +1104,7 @@ const createSharedSlice: StateCreator = (s if (afterRedo?.savedAtDepth !== null && afterRedo?.savedAtDepth === afterRedo?.past.length) { getState().fileActions.updateFile({ name: pouName, saved: true }) } + return true }, }, }) diff --git a/src/frontend/store/slices/shared/types.ts b/src/frontend/store/slices/shared/types.ts index ba3efb36a..5025c4129 100644 --- a/src/frontend/store/slices/shared/types.ts +++ b/src/frontend/store/slices/shared/types.ts @@ -117,9 +117,11 @@ export type EtherCATDeviceActions = { export type SnapshotActions = { pushToHistory: (pouName: string, snapshot: PouHistorySnapshot) => void markSaved: (pouName: string) => void - markAllSaved: () => void - undo: (pouName: string) => void - redo: (pouName: string) => void + markAllSaved: (except?: readonly string[]) => void + /** @returns `false` when the POU's graphical body is stale, so history was left untouched. */ + undo: (pouName: string) => boolean + /** @returns `false` when the POU's graphical body is stale, so history was left untouched. */ + redo: (pouName: string) => boolean } export type OpenProjectResponseData = {