From b9e53329b2c1bf7e69844aaaa34340d2f28072f4 Mon Sep 17 00:00:00 2001 From: stevenmunn312-tech Date: Thu, 29 Oct 2026 22:11:56 +1030 Subject: [PATCH] fix workspace history, recovery, and build cancellation Co-Authored-By: Codex --- backend/firmware.py | 3 +- backend/tests/test_build_cancellation.py | 151 +++++++++++++ backend/tests/test_engine.py | 5 +- backend/toolchain.py | 144 ++++++++---- src/App.tsx | 9 +- src/components/Recover/RecoverPopup.tsx | 5 +- src/state/__tests__/graphStore.test.ts | 1 + src/state/__tests__/workspaceHistory.test.ts | 116 ++++++++++ src/state/graphStore.ts | 221 ++++++++++++------- 9 files changed, 528 insertions(+), 127 deletions(-) create mode 100644 backend/tests/test_build_cancellation.py create mode 100644 src/state/__tests__/workspaceHistory.test.ts diff --git a/backend/firmware.py b/backend/firmware.py index f4d9e76d4..bab14eb25 100644 --- a/backend/firmware.py +++ b/backend/firmware.py @@ -808,7 +808,7 @@ def _upload_result_lines(rc, phase, port): # didn't compile" sends the user to inspect a graph that is fine. yield ("\n*** DID NOT RUN *** Another build was already using the build " "directory. Nothing was compiled or sent to the board.\n") - elif toolchain._build_was_cancelled(): + elif rc == toolchain._BUILD_CANCELLED: pass # already said so, in _run_phase, before the exit code elif rc != 0 and phase == "compile": yield (f"\n*** BUILD FAILED (exit code {rc}) *** The sketch didn't compile, so " @@ -1446,6 +1446,7 @@ def cancel_build(): @router.post("/api/compile-check") +@toolchain._background_build def compile_check(payload: dict = Body(...)): """Compile-only capacity check for the live controller-capacity meter: runs the same compile a real Upload would (no port, so nothing is flashed) and diff --git a/backend/tests/test_build_cancellation.py b/backend/tests/test_build_cancellation.py new file mode 100644 index 000000000..5179d4494 --- /dev/null +++ b/backend/tests/test_build_cancellation.py @@ -0,0 +1,151 @@ +"""Cancellation belongs to one upload, including across streaming worker hops.""" +from concurrent.futures import ThreadPoolExecutor + +import pytest + +import firmware +import toolchain + + +def drain(generator): + lines = [] + while True: + try: + lines.append(next(generator)) + except StopIteration as result: + return "".join(lines), result.value + + +@pytest.fixture +def processes(monkeypatch): + created = [] + + class Process: + def __init__(self, args, **kwargs): + self.args = args + self.stdout = self + self.returncode = None + self.stopped = False + created.append(self) + + def poll(self): + return self.returncode + + def wait(self): + # Some tools can exit successfully while cancellation is in flight. + self.returncode = 0 + return 0 + + def stop(proc): + proc.stopped = True + proc.returncode = 0 + + monkeypatch.setattr(toolchain.subprocess, "Popen", Process) + monkeypatch.setattr(toolchain, "_stop_build_process", stop) + monkeypatch.setattr(toolchain, "_iter_stream_lines", lambda stream: iter(["compiler alive\n"])) + yield created + assert toolchain._active_builds == [] + + +def start(generator): + while next(generator) != "compiler alive\n": + pass + + +@pytest.mark.parametrize("finish_check_first", [True, False]) +def test_overlapping_capacity_check_does_not_own_upload_cancellation(tmp_path, processes, finish_check_first): + upload_dir = tmp_path / "upload" + check_dir = tmp_path / "check" + for directory in (upload_dir, check_dir): + directory.mkdir() + (directory / "sketch.ino").write_text("void setup() {}") + upload = firmware._compile_upload("Upload", upload_dir, "arduino:avr:uno", "COM7") + check = firmware._compile_upload("Capacity check", check_dir, "arduino:avr:uno", "") + try: + start(upload) + toolchain._background_build(lambda: start(check))() + assert toolchain._cancel_active_build() is True + assert processes[0].stopped is True + assert processes[1].stopped is False + assert toolchain._build_was_cancelled() is False + # Resume on another worker, as Starlette does for streamed responses. + with ThreadPoolExecutor(max_workers=1) as worker: + if finish_check_first: + check_log, check_result = worker.submit(drain, check).result() + upload_log, upload_result = worker.submit(drain, upload).result() + else: + upload_log, upload_result = worker.submit(drain, upload).result() + check_log, check_result = worker.submit(drain, check).result() + assert check_result == (0, "compile") + assert "CANCELLED" not in check_log + assert upload_result == (toolchain._BUILD_CANCELLED, "compile") + assert "CANCELLED" in upload_log + assert len(processes) == 2 # neither run starts an upload process + finally: + upload.close() + check.close() + + +def test_cancel_between_compile_and_upload_never_starts_flash(tmp_path, processes): + (tmp_path / "sketch.ino").write_text("void setup() {}") + upload = firmware._compile_upload("Upload", tmp_path, "arduino:avr:uno", "COM7") + try: + for line in upload: + if "[compiled]" in line: + break + assert toolchain._cancel_active_build() is True + log, (rc, phase) = drain(upload) + assert rc == toolchain._BUILD_CANCELLED + assert phase == "upload" + assert "CANCELLED" in log + assert len(processes) == 1 + assert "Upload complete" not in "".join(firmware._upload_result_lines(rc, phase, "COM7")) + finally: + upload.close() + + +def test_capacity_check_alone_cannot_be_cancelled(processes): + check = toolchain._run_phase("Capacity check", ["compiler"]) + try: + toolchain._background_build(lambda: start(check))() + assert toolchain._cancel_active_build() is False + assert drain(check)[1] == 0 + assert not processes[0].stopped + finally: + check.close() + + +def test_closing_abandoned_stream_stops_process_and_unregisters_run(processes): + build = toolchain._run_phase("Upload", ["compiler"]) + start(build) + build.close() + assert processes[0].stopped + assert toolchain._cancel_active_build() is False + + +def test_cancel_during_process_creation_stops_new_process(monkeypatch, processes): + create = toolchain.subprocess.Popen + + def create_and_cancel(*args, **kwargs): + proc = create(*args, **kwargs) + assert toolchain._cancel_active_build() is True + return proc + + monkeypatch.setattr(toolchain.subprocess, "Popen", create_and_cancel) + log, rc = drain(toolchain._run_phase("Upload", ["compiler"])) + assert rc == toolchain._BUILD_CANCELLED + assert "CANCELLED" in log + assert processes[0].stopped + + +def test_cancel_after_phase_header_does_not_spawn_process(processes): + build = toolchain._run_phase("Upload", ["compiler"]) + try: + assert "=== Upload ===" in next(build) + assert toolchain._cancel_active_build() is True + log, rc = drain(build) + assert rc == toolchain._BUILD_CANCELLED + assert "CANCELLED" in log + assert processes == [] + finally: + build.close() diff --git a/backend/tests/test_engine.py b/backend/tests/test_engine.py index 5afa21e71..02ff770db 100644 --- a/backend/tests/test_engine.py +++ b/backend/tests/test_engine.py @@ -1124,7 +1124,6 @@ def test_flash_size_is_read_off_the_request_or_ignored(): def test_cancelling_with_nothing_running_is_not_an_error(): # The UI offers Cancel whenever an upload is busy, and a build that finished # a moment earlier is not a failure worth reporting. - toolchain._register_build(None) assert toolchain._cancel_active_build() is False @@ -1135,13 +1134,11 @@ def test_a_cancelled_build_is_not_reported_as_a_failed_one(monkeypatch): sending someone to hunt for a compile error in a graph they never doubted is the worst possible answer. """ - monkeypatch.setattr(toolchain, "_build_cancelled", True) - lines = "".join(firmware._upload_result_lines(130, "compile", "COM7")) + lines = "".join(firmware._upload_result_lines(toolchain._BUILD_CANCELLED, "compile", "COM7")) assert "BUILD FAILED" not in lines def test_a_real_failure_still_says_so(monkeypatch): - monkeypatch.setattr(toolchain, "_build_cancelled", False) lines = "".join(firmware._upload_result_lines(1, "compile", "COM7")) assert "BUILD FAILED" in lines diff --git a/backend/toolchain.py b/backend/toolchain.py index d2ba217e3..9b5c42405 100644 --- a/backend/toolchain.py +++ b/backend/toolchain.py @@ -1701,27 +1701,42 @@ def _write_fbuild_main(ino: str, files: dict[str, str] | None = None) -> None: # Killing the process makes `_run_phase` return normally, so the generator # unwinds through its own `finally`: lock released, stashed libraries restored. _active_build_lock = threading.Lock() -_active_build_proc: subprocess.Popen | None = None -_build_cancelled = False +_BUILD_CANCELLED = -130 +_build_context = threading.local() +_active_builds: list[_BuildRun] = [] -def _register_build(proc: subprocess.Popen | None) -> None: - global _active_build_proc - with _active_build_lock: - _active_build_proc = proc +class _BuildRun: + def __init__(self, cancellable=True): + self.cancellable = cancellable + self.cancelled = False + self.proc = None def _build_was_cancelled() -> bool: - with _active_build_lock: - return _build_cancelled + run = getattr(_build_context, "run", None) + return bool(run and run.cancelled) -def _begin_build_run() -> None: - """Clear the cancelled flag as a new run starts, so one cancellation cannot - label the next build.""" - global _build_cancelled - with _active_build_lock: - _build_cancelled = False +def _background_build(function): + """Capacity checks share the engines but never own the user's Cancel button.""" + @functools.wraps(function) + def wrapped(*args, **kwargs): + previous = getattr(_build_context, "cancellable", True) + _build_context.cancellable = False + try: + return function(*args, **kwargs) + finally: + _build_context.cancellable = previous + return wrapped + + +def _stop_build_process(proc) -> None: + if sys.platform == "win32": + subprocess.run(["taskkill", "/T", "/F", "/PID", str(proc.pid)], + capture_output=True, check=False) + else: + proc.terminate() def _cancel_active_build() -> bool: @@ -1731,18 +1746,16 @@ def _cancel_active_build() -> bool: long-lived `fbuild-daemon` child, so terminating the parent alone leaves the work running and the log silent. """ - global _build_cancelled with _active_build_lock: - proc = _active_build_proc - if proc is None or proc.poll() is not None: + run = next((run for run in reversed(_active_builds) + if run.cancellable and not run.cancelled), None) + if run is None: return False - _build_cancelled = True + run.cancelled = True + proc = run.proc try: - if sys.platform == "win32": - subprocess.run(["taskkill", "/T", "/F", "/PID", str(proc.pid)], - capture_output=True, check=False) - else: - proc.terminate() + if proc is not None and proc.poll() is None: + _stop_build_process(proc) except Exception: return False return True @@ -1801,6 +1814,39 @@ def _format_duration(seconds: float) -> str: return f"{hours}h {minutes:02d}m {secs:02d}s" +def _with_build_run(function): + """Keep one cancellation token across phases and streaming worker changes.""" + @functools.wraps(function) + def wrapped(*args, **kwargs): + if getattr(_build_context, "run", None) is not None: + return (yield from function(*args, **kwargs)) + run = _BuildRun(getattr(_build_context, "cancellable", True)) + generator = function(*args, **kwargs) + with _active_build_lock: + _active_builds.append(run) + try: + while True: + previous = getattr(_build_context, "run", None) + _build_context.run = run + try: + line = next(generator) + except StopIteration as result: + return result.value + finally: + _build_context.run = previous + yield line + finally: + previous = getattr(_build_context, "run", None) + _build_context.run = run + try: + generator.close() + finally: + _build_context.run = previous + with _active_build_lock: + _active_builds.remove(run) + return wrapped + + def _reports_total_time(compile_upload): """Wrap a compile/upload generator so every run ends with one `[time] total ...` line, whichever engine ran it. @@ -1818,9 +1864,12 @@ def _reports_total_time(compile_upload): it never compiled or flashed anything, and a duration printed beside "DID NOT RUN" would read as a build that took that long.""" @functools.wraps(compile_upload) + @_with_build_run def timed(*args, **kwargs): started = time.monotonic() rc, phase = yield from compile_upload(*args, **kwargs) + if _build_was_cancelled(): + rc = _BUILD_CANCELLED if phase != "busy": yield f" [time] total {_format_duration(time.monotonic() - started)}\n" return rc, phase @@ -1849,12 +1898,16 @@ def _missing_engine(label, engine, hint): return -1, "compile" +@_with_build_run def _run_phase(label, args, sink=None, cwd=None, tool_env=None): """Run one build-tool phase (arduino-cli or fbuild), yielding its output lines; returns the exit code. If `sink` (a list) is given, each output line is also appended to it so the caller can inspect the phase output (e.g. to parse the flash/RAM size report).""" - _begin_build_run() + run = _build_context.run + if run.cancelled: + yield "\n*** CANCELLED *** Stopped at your request.\n" + return _BUILD_CANCELLED # A None here means an engine binary went unresolved upstream. Report it as # a launch failure rather than raising out of the generator mid-stream: the # caller is streaming this to a log or the Output console, and a TypeError @@ -1863,6 +1916,9 @@ def _run_phase(label, args, sink=None, cwd=None, tool_env=None): yield f"[error] {label}: no build tool to run — the engine binary was not found\n" return -1 yield f"\n=== {label} ===\n$ {' '.join(str(arg) for arg in args)}\n" + if run.cancelled: + yield "\n*** CANCELLED *** Stopped at your request.\n" + return _BUILD_CANCELLED started = time.monotonic() try: proc = subprocess.Popen( @@ -1874,26 +1930,36 @@ def _run_phase(label, args, sink=None, cwd=None, tool_env=None): except Exception as e: yield f"[error] failed to launch {args[0]}: {e}\n" return -1 - _register_build(proc) - for line in _iter_stream_lines(proc.stdout): - if sink is not None: - sink.append(line) - # Evidence that this build is alive. Only a running holder reaches here, - # so a generator abandoned mid-stream stops stamping and eventually - # becomes reclaimable — see `_FbuildBuildLock`. A no-op for the - # arduino-cli path, which shares this helper but holds no lock. - _fbuild_build_lock.touch() - yield line - proc.wait() + with _active_build_lock: + run.proc = proc + cancelled = run.cancelled + if cancelled: + _stop_build_process(proc) + try: + for line in _iter_stream_lines(proc.stdout): + if sink is not None: + sink.append(line) + # Evidence that this build is alive. Only a running holder reaches here, + # so a generator abandoned mid-stream stops stamping and eventually + # becomes reclaimable — see `_FbuildBuildLock`. A no-op for the + # arduino-cli path, which shares this helper but holds no lock. + _fbuild_build_lock.touch() + yield line + proc.wait() + finally: + with _active_build_lock: + run.proc = None + if proc.poll() is None: + _stop_build_process(proc) + proc.wait() elapsed = time.monotonic() - started - _register_build(None) - if _build_was_cancelled(): + if run.cancelled: # Said before the exit code, and in a form `parseStatus` checks first: # a killed process exits non-zero, and reporting that as a build failure # would send the user looking for a fault in a graph they simply changed # their mind about. - yield "\n*** CANCELLED *** Stopped at your request — nothing was sent to the board.\n" - return proc.returncode + yield "\n*** CANCELLED *** Stopped at your request.\n" + return _BUILD_CANCELLED yield f"[{label} exit code: {proc.returncode} · {_format_duration(elapsed)}]\n" return proc.returncode diff --git a/src/App.tsx b/src/App.tsx index dbcf3998f..636065322 100644 --- a/src/App.tsx +++ b/src/App.tsx @@ -347,12 +347,13 @@ export default function App() { const lastSnapshotRef = useRef('') useEffect(() => { const timer = setInterval(() => { - const { nodes, edges, graphData, graphs, activeGraphId, buildProfile, trusted, performanceDeck, displayDocuments } = useGraphStore.getState() - if (nodes.length === 0) return - const serialized = JSON.stringify({ nodes, edges, graphData, graphs, activeGraphId, buildProfile, trusted, performanceDeck, displayDocuments }) + if (!workspaceHydrated.current || isMusicLibraryRestoring()) return + const workspace = captureWorkspace(useGraphStore.getState()) + if (workspace.nodes.length === 0) return + const serialized = JSON.stringify(workspace) if (serialized === lastSnapshotRef.current) return lastSnapshotRef.current = serialized - pushSnapshot({ nodes, edges, graphData, graphs, activeGraphId, buildProfile, trusted, performanceDeck, displayDocuments }) + pushSnapshot(workspace) }, SNAPSHOT_INTERVAL) return () => clearInterval(timer) }, []) diff --git a/src/components/Recover/RecoverPopup.tsx b/src/components/Recover/RecoverPopup.tsx index bddb592b7..e71911c8e 100644 --- a/src/components/Recover/RecoverPopup.tsx +++ b/src/components/Recover/RecoverPopup.tsx @@ -1,6 +1,6 @@ import { useState } from 'react' import { useUiStore } from '../../state/uiStore' -import { useGraphStore } from '../../state/graphStore' +import { restoreWorkspaceSnapshot } from '../../state/graphStore' import { loadSnapshots } from '../../state/snapshotHistory' import styles from './RecoverPopup.module.css' @@ -36,8 +36,7 @@ export default function RecoverPopup() { tone: 'danger', }) if (!ok) return - const { nodes, edges, graphData, graphs, activeGraphId, buildProfile, performanceDeck } = snap.workspace - useGraphStore.getState().loadGraph(nodes, edges, { graphData, graphs, activeGraphId, buildProfile, performanceDeck }) + restoreWorkspaceSnapshot(snap.workspace) setStatus(`Restored recovery snapshot from ${relativeTime(snap.timestamp)}`, 'success') closeRecover() } diff --git a/src/state/__tests__/graphStore.test.ts b/src/state/__tests__/graphStore.test.ts index bf5ed68c9..402e9983a 100644 --- a/src/state/__tests__/graphStore.test.ts +++ b/src/state/__tests__/graphStore.test.ts @@ -546,6 +546,7 @@ describe('graphStore — grouping', () => { reset([node('sc', 'SolidColor', { r: 0, g: 0, b: 255 })], []) const gid = useGraphStore.getState().createGroup('Blue', ['sc']) const rootBefore = useGraphStore.getState().nodes + useGraphStore.temporal.getState().clear() useGraphStore.temporal.setState({ pastStates: [{ nodes: rootBefore, edges: [] }], futureStates: [] }) useGraphStore.getState().enterGraph(gid) diff --git a/src/state/__tests__/workspaceHistory.test.ts b/src/state/__tests__/workspaceHistory.test.ts new file mode 100644 index 000000000..cbd866669 --- /dev/null +++ b/src/state/__tests__/workspaceHistory.test.ts @@ -0,0 +1,116 @@ +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' +import { ROOT_GRAPH_ID, restoreWorkspaceSnapshot, useGraphStore, type StudioNode } from '../graphStore' +import { NODE_LIBRARY } from '../nodeLibrary' +import { clearPatternContentTrustForTests, isContentKnown } from '../patterns/patternTrust' +import { createDisplayDocument } from '../displays/displayEditor' +import { captureWorkspace } from '../workspacePersistence' +import { loadSnapshots, pushSnapshot } from '../snapshotHistory' +import { registerMusicLibraryPersistence, type PersistedMusicEntry } from '../player/musicLibraryPersistence' + +function node(id: string, nodeType = 'SolidColor', properties: Record = {}): StudioNode { + const definition = NODE_LIBRARY.find((entry) => entry.type === nodeType)! + return { + id, type: 'studioNode', position: { x: 0, y: 0 }, + data: { label: nodeType, nodeType, category: definition?.category ?? 'pattern', properties, + inputs: definition?.inputs ?? [], outputs: definition?.outputs ?? [] }, + } as StudioNode +} + +describe('workspace history boundaries', () => { + beforeEach(() => { + vi.useFakeTimers() + useGraphStore.getState().loadGraph([], []) + clearPatternContentTrustForTests() + }) + afterEach(() => { + vi.runOnlyPendingTimers() + vi.useRealTimers() + registerMusicLibraryPersistence(() => [], () => undefined) + }) + + it('never pushes the previous project after loading and clearing history', () => { + useGraphStore.getState().loadGraph([node('project-a')], []) + useGraphStore.getState().updateNodeProperty('project-a', 'r', 99) + useGraphStore.getState().loadGraph([node('project-b')], []) + useGraphStore.temporal.getState().clear() + vi.advanceTimersByTime(500) + expect(useGraphStore.temporal.getState().pastStates).toEqual([]) + useGraphStore.temporal.getState().undo() + expect(useGraphStore.getState().nodes.some((entry) => entry.id === 'project-b')).toBe(true) + expect(useGraphStore.getState().nodes.some((entry) => entry.id === 'project-a')).toBe(false) + useGraphStore.getState().updateNodeProperty('project-b', 'r', 12) + useGraphStore.temporal.getState().undo() + expect(useGraphStore.getState().nodes.find((entry) => entry.id === 'project-b')?.data.properties.r).toBeUndefined() + }) + + it('flushes root edits before entering a group and keeps each history in its graph', async () => { + useGraphStore.getState().loadGraph([node('root'), node('group', 'Group', { groupId: 'g' })], [], { + graphs: { [ROOT_GRAPH_ID]: { id: ROOT_GRAPH_ID, name: 'Main' }, g: { id: 'g', name: 'Group' } }, + graphData: { g: { nodes: [node('inner', 'SolidColor', { r: 0 })], edges: [] } }, + }) + useGraphStore.getState().updateNodeProperty('root', 'r', 10) + useGraphStore.getState().enterGraph('g') + await Promise.resolve() + vi.advanceTimersByTime(500) + expect(useGraphStore.temporal.getState().pastStates).toEqual([]) + useGraphStore.getState().updateNodeProperty('inner', 'r', 20) + useGraphStore.temporal.getState().undo() + expect(useGraphStore.getState().nodes.map((entry) => entry.id)).toEqual(['inner']) + expect(useGraphStore.getState().nodes[0].data.properties.r).toBe(0) + useGraphStore.getState().enterGraph(ROOT_GRAPH_ID) + await Promise.resolve() + useGraphStore.temporal.getState().undo() + expect(useGraphStore.getState().nodes.find((entry) => entry.id === 'root')?.data.properties.r).toBeUndefined() + }) + + it('does not trust or remember foreign code restored by undo', () => { + const foreign = node('foreign', 'Code', { code: 'return 123456789;' }) + useGraphStore.getState().loadGraph([foreign], [], { trusted: false }) + expect(useGraphStore.getState().trusted).toBe(false) + useGraphStore.getState().deleteNode('foreign') + expect(useGraphStore.getState().trusted).toBe(true) + useGraphStore.temporal.getState().undo() + expect(useGraphStore.getState().trusted).toBe(false) + expect(isContentKnown([foreign])).toBe(false) + useGraphStore.temporal.getState().redo() + useGraphStore.temporal.getState().undo() + expect(useGraphStore.getState().trusted).toBe(false) + expect(isContentKnown([foreign])).toBe(false) + useGraphStore.getState().setTrusted(true) + expect(isContentKnown([foreign])).toBe(true) + }) + + it('restores complete recovery content and reverses it with undo and redo', () => { + let music: PersistedMusicEntry[] = [{ id: 'track-a', name: 'A' } as PersistedMusicEntry] + registerMusicLibraryPersistence(() => music, (entries) => { music = entries }) + useGraphStore.getState().loadGraph([node('before')], [], { musicLibrary: music }) + const before = captureWorkspace(useGraphStore.getState()) + const display = createDisplayDocument('screen', 240, 320) + const foreign = node('foreign', 'Code', { code: 'return 987654321;' }) + const recoveredMusic = [{ id: 'track-b', name: 'B' } as PersistedMusicEntry] + const workspace = { + nodes: [foreign, node('panel', 'TransportDisplay', { displayId: 'screen', tftLayout: 'Custom design' })], + edges: [], trusted: false, displayDocuments: { screen: display }, musicLibrary: recoveredMusic, + } + restoreWorkspaceSnapshot(workspace) + vi.advanceTimersByTime(500) + expect(useGraphStore.getState().trusted).toBe(false) + expect(useGraphStore.getState().displayDocuments.screen).toEqual(display) + expect(music).toEqual(recoveredMusic) + expect(useGraphStore.temporal.getState().pastStates).toHaveLength(1) + useGraphStore.temporal.getState().undo() + expect(captureWorkspace(useGraphStore.getState())).toEqual(before) + useGraphStore.temporal.getState().redo() + expect(useGraphStore.getState().trusted).toBe(false) + expect(useGraphStore.getState().displayDocuments.screen).toEqual(display) + expect(music).toEqual(recoveredMusic) + expect(isContentKnown([foreign])).toBe(false) + }) + + it('includes music metadata in captured rolling snapshots', () => { + const music = [{ id: 'track', name: 'Saved track' } as PersistedMusicEntry] + registerMusicLibraryPersistence(() => music, () => undefined) + pushSnapshot(captureWorkspace(useGraphStore.getState())) + expect(loadSnapshots()[0].workspace.musicLibrary).toEqual(music) + }) +}) diff --git a/src/state/graphStore.ts b/src/state/graphStore.ts index d645ea72f..c0737f8d3 100644 --- a/src/state/graphStore.ts +++ b/src/state/graphStore.ts @@ -64,6 +64,7 @@ import { deriveControlShape, } from './player/performanceDeck' import { restoreMusicLibrary, type PersistedMusicEntry } from './player/musicLibraryPersistence' +import { captureWorkspace, type PersistedWorkspace } from './workspacePersistence' import { isPlacedWidget, normalizeDisplayDocument, @@ -456,6 +457,9 @@ interface GraphState { * undo instead of leaving it for the orphan sweep. */ type HistorySlice = Pick + // Ordinary steps carry graph content and trust. Recovery checkpoints also + // carry all workspace extras; musicLibrary (including []) identifies them. + & Partial // Scene-level sources that are not bench parts but still stay in the parent // graph when a selection is encapsulated: each is one resource the whole scene @@ -1218,6 +1222,7 @@ function restoreStashedHistory(graphId: string): void { const scope = { nodes, edges, graphData, activeGraphId } const currentRoot = { nodes: rootGraphNodes(scope), edges: rootGraphEdges(scope) } const rebase = (snapshots: Partial[]) => snapshots.map((snapshot) => { + if (snapshot.musicLibrary !== undefined) return snapshot if (graphId !== ROOT_GRAPH_ID) return { ...snapshot, graphData, displayDocuments } const historicRoot = { nodes: snapshot.nodes ?? currentRoot.nodes, @@ -1548,6 +1553,7 @@ function restoreStashedDisplayHistory(displayId: string): void { stashedDisplayHistory.delete(displayId) const { nodes, edges, graphData, displayDocuments } = useGraphStore.getState() const rebase = (snapshots: Partial[]) => snapshots.map((snapshot) => { + if (snapshot.musicLibrary !== undefined) return snapshot const target = snapshot.displayDocuments?.[displayId] const rebasedDocuments = { ...displayDocuments } if (target) rebasedDocuments[displayId] = target @@ -1656,6 +1662,7 @@ function scheduleOrphanGraphPrune(): void { // from the first call in a burst, and only push a snapshot once the burst has // gone quiet, using the most recent currentState/deltaState by then. let flushHistoryDebounce = (): void => {} +let applyingHistory = false function debounceHandleSet void>( fn: Fn, @@ -2785,72 +2792,82 @@ export const useGraphStore = create()( }, loadGraph: (nodes, edges, workspace) => { + flushHistoryDebounce() + preDragHistoryState = undefined + const temporalApi = useGraphStore.temporal.getState() + const wasTracking = temporalApi.isTracking + temporalApi.pause() // Replacing the workspace invalidates every graph's history, stashed // stacks included — otherwise entering a group whose id happens to // recur in the newly loaded workspace would restore the old project's - // undo steps. Callers clear the live stacks themselves right after. + // undo steps. The live stacks are cleared below as part of the load. clearStashedGraphHistory() // A load is not an edit: the board link's identity is never assigned // to a project as it opens (see the board link subscription below). - whileLoadingWorkspace(() => set(() => { - // Restore the active graph plus every stashed pattern-group subgraph. - const active = normalizeLoadedGraph(nodes, edges) - const graphData: Record = {} - for (const [id, content] of Object.entries(workspace?.graphData ?? {})) { - graphData[id] = normalizeLoadedGraph(content.nodes ?? [], content.edges ?? []) - } - const graphs: Record = { - [ROOT_GRAPH_ID]: { id: ROOT_GRAPH_ID, name: 'Main' }, - ...(workspace?.graphs ?? {}), - } - const activeGraphId = workspace?.activeGraphId ?? ROOT_GRAPH_ID - // Sweep out subgraphs a previous session orphaned — undo history - // doesn't survive a load, so reachability from the loaded content - // alone decides what stays. - const pruned = pruneOrphanGraphData( - { nodes: active.nodes, graphData, graphs, activeGraphId }, - [], - ) - const loadedGraphData = pruned?.graphData ?? graphData - const buildProfile = normalizeBuildProfile(workspace?.buildProfile) - let displayDocuments = normalizeDisplayDocuments(workspace?.displayDocuments) - const rootContent = syncDisplayNodesInContent( - activeGraphId === ROOT_GRAPH_ID - ? { nodes: active.nodes, edges: active.edges } - : loadedGraphData[ROOT_GRAPH_ID] ?? { nodes: [], edges: [] }, - displayDocuments, - ) - displayDocuments = pruneOrphanDisplayDocuments(displayDocuments, rootContent.nodes) - if (activeGraphId !== ROOT_GRAPH_ID) loadedGraphData[ROOT_GRAPH_ID] = rootContent - const rootNodes = activeGraphId === ROOT_GRAPH_ID - ? ensureRootBoardNode(rootContent.nodes) - : active.nodes - return { - ...active, - nodes: rootNodes, - ...(activeGraphId === ROOT_GRAPH_ID ? { edges: rootContent.edges } : {}), - graphData: loadedGraphData, - graphs: pruned?.graphs ?? graphs, - activeGraphId, - buildProfile, - selectedNodeId: null, - // Missing/undefined = trusted: this predates the trust field, so - // it's the user's own prior local work, not imported content. - trusted: workspace?.trusted ?? true, - // Missing/malformed = an empty deck — pre-existing saves, share - // links, and JSON imports created before this field all fall - // back safely rather than throwing. - performanceDeck: normalizeDeckConfig(workspace?.performanceDeck), - displayDocuments, - panicActive: false, - panicRestoreValues: null, - } - })) - noteLoadedBoardLink() - restoreMusicLibrary(workspace?.musicLibrary) - // The active board is a session choice about the project that was - // open; the one that just loaded starts on its primary. - useUiStore.getState().setActiveBoardId(primaryBoardId(rootGraphNodes(useGraphStore.getState()))) + try { + whileLoadingWorkspace(() => set(() => { + // Restore the active graph plus every stashed pattern-group subgraph. + const active = normalizeLoadedGraph(nodes, edges) + const graphData: Record = {} + for (const [id, content] of Object.entries(workspace?.graphData ?? {})) { + graphData[id] = normalizeLoadedGraph(content.nodes ?? [], content.edges ?? []) + } + const graphs: Record = { + [ROOT_GRAPH_ID]: { id: ROOT_GRAPH_ID, name: 'Main' }, + ...(workspace?.graphs ?? {}), + } + const activeGraphId = workspace?.activeGraphId ?? ROOT_GRAPH_ID + // Sweep out subgraphs a previous session orphaned — undo history + // doesn't survive a load, so reachability from the loaded content + // alone decides what stays. + const pruned = pruneOrphanGraphData( + { nodes: active.nodes, graphData, graphs, activeGraphId }, + [], + ) + const loadedGraphData = pruned?.graphData ?? graphData + const buildProfile = normalizeBuildProfile(workspace?.buildProfile) + let displayDocuments = normalizeDisplayDocuments(workspace?.displayDocuments) + const rootContent = syncDisplayNodesInContent( + activeGraphId === ROOT_GRAPH_ID + ? { nodes: active.nodes, edges: active.edges } + : loadedGraphData[ROOT_GRAPH_ID] ?? { nodes: [], edges: [] }, + displayDocuments, + ) + displayDocuments = pruneOrphanDisplayDocuments(displayDocuments, rootContent.nodes) + if (activeGraphId !== ROOT_GRAPH_ID) loadedGraphData[ROOT_GRAPH_ID] = rootContent + const rootNodes = activeGraphId === ROOT_GRAPH_ID + ? ensureRootBoardNode(rootContent.nodes) + : active.nodes + return { + ...active, + nodes: rootNodes, + ...(activeGraphId === ROOT_GRAPH_ID ? { edges: rootContent.edges } : {}), + graphData: loadedGraphData, + graphs: pruned?.graphs ?? graphs, + activeGraphId, + buildProfile, + selectedNodeId: null, + // Missing/undefined = trusted: this predates the trust field, so + // it's the user's own prior local work, not imported content. + trusted: workspace?.trusted ?? true, + // Missing/malformed = an empty deck — pre-existing saves, share + // links, and JSON imports created before this field all fall + // back safely rather than throwing. + performanceDeck: normalizeDeckConfig(workspace?.performanceDeck), + displayDocuments, + panicActive: false, + panicRestoreValues: null, + } + })) + noteLoadedBoardLink() + restoreMusicLibrary(workspace?.musicLibrary) + // The active board is a session choice about the project that was + // open; the one that just loaded starts on its primary. + useUiStore.getState().setActiveBoardId(primaryBoardId(rootGraphNodes(useGraphStore.getState()))) + temporalApi.clear() + } finally { + if (wasTracking) temporalApi.resume() + } }, duplicateNode: (id) => @@ -3059,7 +3076,8 @@ export const useGraphStore = create()( return active ? { edges: kept } : withRootContent(s, { edges: kept }) }), - enterGraph: (id) => + enterGraph: (id) => { + flushHistoryDebounce() set((s) => { if (id === s.activeGraphId || !s.graphs[id]) return s // Stash the current graph, load the target. Graph navigation is not @@ -3089,7 +3107,8 @@ export const useGraphStore = create()( activeGraphId: id, selectedNodeId: null, } - }), + }) + }, createGroup: (name, nodeIds, options) => { // Every id this action mints (the Group node, its GroupOutput/GroupInput @@ -3714,6 +3733,7 @@ export const useGraphStore = create()( edges: s.edges, graphData: s.graphData, displayDocuments: s.displayDocuments, + trusted: s.trusted, }), // Treat states as equal (don't snapshot) while any node is mid-drag — // but remember the state from just before the drag started, since the @@ -3768,19 +3788,53 @@ useGraphStore.subscribe(() => { if (orphanPruneTimer) scheduleOrphanGraphPrune() }) -/* - * A project's trust follows what it holds (see patternTrust.ts). - * - * While it is trusted, everything in it is work made on this machine, so it - * is remembered as it is made: a formula typed here, or a pattern saved from - * here, is known the next time it appears anywhere. While it is untrusted, - * it becomes trusted again the moment nothing unknown is left in it - which - * is also how a loaded file or share link made only of known content opens - * trusted, since load paths start everything from elsewhere untrusted and - * this decides. Paths that bring unknown content in (a pattern drop, a paste - * from an untrusted project) set `trusted: false` in the same update, so - * that content is never remembered on the way in. - */ +// History may bring back content that was removed before it was trusted. +// Only explicit trust and local edits may add new known fingerprints. +const rawUndo = useGraphStore.temporal.getState().undo +const rawRedo = useGraphStore.temporal.getState().redo +const rawClear = useGraphStore.temporal.getState().clear +function applyHistory(direction: 'undo' | 'redo', steps = 1): void { + flushHistoryDebounce() + for (let step = 0; step < steps; step++) { + const history = useGraphStore.temporal.getState() + const target = (direction === 'undo' ? history.pastStates : history.futureStates).at(-1) + if (!target) break + const inverse = target.musicLibrary !== undefined ? captureWorkspace(useGraphStore.getState()) : undefined + applyingHistory = true + try { + if (direction === 'undo') rawUndo() + else rawRedo() + } finally { + applyingHistory = false + } + if (inverse) { + restoreMusicLibrary(target.musicLibrary) + clearStashedGraphHistory() + noteLoadedBoardLink() + useUiStore.getState().setActiveBoardId(primaryBoardId(rootGraphNodes(useGraphStore.getState()))) + const key = direction === 'undo' ? 'futureStates' : 'pastStates' + const stack = useGraphStore.temporal.getState()[key] + useGraphStore.temporal.setState({ [key]: [...stack.slice(0, -1), inverse] }) + } + } +} +useGraphStore.temporal.setState({ + undo: (steps) => applyHistory('undo', steps), + redo: (steps) => applyHistory('redo', steps), + clear: () => { + flushHistoryDebounce() + preDragHistoryState = undefined + rawClear() + }, +}) + +/** Recovery replaces a workspace, but retains one explicit, complete undo step. */ +export function restoreWorkspaceSnapshot(workspace: PersistedWorkspace): void { + const previous = captureWorkspace(useGraphStore.getState()) + useGraphStore.getState().loadGraph(workspace.nodes, workspace.edges, workspace) + useGraphStore.temporal.setState({ pastStates: [previous], futureStates: [] }) +} + /* * A template's controls connect themselves once there is something to control. * @@ -3812,10 +3866,25 @@ useGraphStore.subscribe((state, previous) => { } }) +/* + * A project's trust follows what it holds (see patternTrust.ts). + * + * While it is trusted, everything in it is work made on this machine, so it + * is remembered as it is made: a formula typed here, or a pattern saved from + * here, is known the next time it appears anywhere. While it is untrusted, + * it becomes trusted again the moment nothing unknown is left in it - which + * is also how a loaded file or share link made only of known content opens + * trusted, since load paths start everything from elsewhere untrusted and + * this decides. Paths that bring unknown content in (a pattern drop, a paste + * from an untrusted project) set `trusted: false` in the same update, so + * that content is never remembered on the way in. + */ useGraphStore.subscribe((state, previous) => { if (state.nodes === previous.nodes && state.graphData === previous.graphData && state.trusted === previous.trusted) return - if (state.trusted) { + if (applyingHistory && !isContentKnown(state.nodes, state.graphData)) { + if (state.trusted) useGraphStore.setState({ trusted: false }) + } else if (state.trusted) { rememberContent(state.nodes, state.graphData) } else if (isContentKnown(state.nodes, state.graphData)) { useGraphStore.setState({ trusted: true })