From f7a2c626c9a62993265127488cbaa9b7e8e350cc Mon Sep 17 00:00:00 2001 From: EthanHealy01 <80844253+EthanHealy01@users.noreply.github.com> Date: Thu, 27 Aug 2026 10:12:48 +0000 Subject: [PATCH] Persist the workbench session across the editor/processor switch (#7654) ## What Switching editor -> processor (or reloading) unmounts every editor provider, which emptied the workbench. This PR mirrors the workbench into per-tab sessionStorage and refills an empty one from that record on the next mount: - **Files, selection, view and active document survive** the shell switch and reloads. Each recorded file is resolved to its *current leaf* version on restore, so a file versioned by a policy or another tab comes back at its latest state. - **The switch back lands where the user left**: the processor sidebar's "editor" button consumes a one-shot return path saved at switch time. - **The app switch respects unsaved changes**: `useOtherAppSwitch` (proprietary + saas) now routes through `requestNavigation`, so the same warning guards it as any other navigation. - Desktop shadows `WorkbenchSessionPersistence` with a stub (OS-launched files own boot there). ## How to test I've run through each of these manually: - Upload several PDFs in the editor, select a couple, and switch to the Active Files grid. Click "Open PDF Processor" in the sidebar footer, then switch back to the editor. The same files, selection and view should return, and you should land on the editor page you left. - Open a document in the viewer, then reload the tab. The workbench should refill and come back on the viewer with the same document active. - With unsaved changes in a tool, click the processor switch. The unsaved-changes warning should appear, and the switch should only proceed if you confirm. - Open a second browser tab with different files. Each tab should restore its own workbench independently (the record is per-tab sessionStorage). - While in the processor, delete one of the open files from storage, then switch back. The remaining files should restore and a warning toast should report "Restored X of Y files". --------- Co-authored-by: Claude --- .../public/locales/en-US/translation.toml | 4 + .../src/core/components/AppProviders.tsx | 2 + .../WorkbenchSessionPersistence.test.tsx | 475 ++++++++++++++++++ .../session/WorkbenchSessionPersistence.tsx | 301 +++++++++++ .../editor/src/core/constants/featureFlags.ts | 3 + .../src/core/contexts/NavigationContext.tsx | 9 + .../src/core/extensions/accountLogout.ts | 6 + frontend/editor/src/core/pages/HomePage.tsx | 15 +- .../core/services/workbenchSession.test.ts | 193 +++++++ .../src/core/services/workbenchSession.ts | 173 +++++++ .../tests/stubbed/engine-capabilities.spec.ts | 6 + .../stubbed/workbench-session-restore.spec.ts | 207 ++++++++ .../session/WorkbenchSessionPersistence.tsx | 4 + .../src/desktop/extensions/accountLogout.ts | 5 + .../editor/src/portal/components/Sidebar.tsx | 3 +- .../proprietary/auth/spring/UseSession.tsx | 6 + .../usePolicyAutoRun.reentry.test.tsx | 119 +++++ .../src/proprietary/constants/featureFlags.ts | 3 + .../proprietary/extensions/accountLogout.ts | 6 + .../hooks/useOtherAppSwitch.test.tsx | 55 ++ .../proprietary/hooks/useOtherAppSwitch.ts | 16 +- .../src/saas/auth/AuthProvider.test.tsx | 58 ++- frontend/editor/src/saas/auth/UseSession.tsx | 13 + .../src/saas/hooks/useOtherAppSwitch.ts | 16 +- 24 files changed, 1689 insertions(+), 9 deletions(-) create mode 100644 frontend/editor/src/core/components/session/WorkbenchSessionPersistence.test.tsx create mode 100644 frontend/editor/src/core/components/session/WorkbenchSessionPersistence.tsx create mode 100644 frontend/editor/src/core/services/workbenchSession.test.ts create mode 100644 frontend/editor/src/core/services/workbenchSession.ts create mode 100644 frontend/editor/src/core/tests/stubbed/workbench-session-restore.spec.ts create mode 100644 frontend/editor/src/desktop/components/session/WorkbenchSessionPersistence.tsx create mode 100644 frontend/editor/src/proprietary/components/policies/usePolicyAutoRun.reentry.test.tsx create mode 100644 frontend/editor/src/proprietary/hooks/useOtherAppSwitch.test.tsx diff --git a/frontend/editor/public/locales/en-US/translation.toml b/frontend/editor/public/locales/en-US/translation.toml index 1b81a551f1..540836032f 100644 --- a/frontend/editor/public/locales/en-US/translation.toml +++ b/frontend/editor/public/locales/en-US/translation.toml @@ -11797,6 +11797,10 @@ title = "Watermark Text" image = "Image" text = "Text" +[workbench.sessionRestore] +none = "Your previous files are no longer stored on this device." +partial = "Restored {{restored}} of {{total}} files. The rest are no longer stored on this device." + [workbenchBar] activeFiles = "Active Files" annotations = "Annotations" diff --git a/frontend/editor/src/core/components/AppProviders.tsx b/frontend/editor/src/core/components/AppProviders.tsx index 01b2e42a82..ed1194eb4b 100644 --- a/frontend/editor/src/core/components/AppProviders.tsx +++ b/frontend/editor/src/core/components/AppProviders.tsx @@ -39,6 +39,7 @@ import { RedactionProvider } from "@app/contexts/RedactionContext"; import { FormFillProvider } from "@app/tools/formFill/FormFillContext"; import { FolderFileContextProvider } from "@app/contexts/FolderFileContext"; import { FolderProvider } from "@app/contexts/FolderContext"; +import { WorkbenchSessionPersistence } from "@app/components/session/WorkbenchSessionPersistence"; // Component to initialize scarf tracking (must be inside AppConfigProvider) function ScarfTrackingInitializer() { @@ -163,6 +164,7 @@ export function AppProviders({ + {children} diff --git a/frontend/editor/src/core/components/session/WorkbenchSessionPersistence.test.tsx b/frontend/editor/src/core/components/session/WorkbenchSessionPersistence.test.tsx new file mode 100644 index 0000000000..b05928a93e --- /dev/null +++ b/frontend/editor/src/core/components/session/WorkbenchSessionPersistence.test.tsx @@ -0,0 +1,475 @@ +import { describe, it, expect, vi, beforeEach, afterEach } from "vitest"; +import { render, waitFor, act } from "@testing-library/react"; + +const mocks = vi.hoisted(() => ({ + getLeafStirlingFileStubs: vi.fn(), + alert: vi.fn(), + setActiveFileId: vi.fn(), + restoreWorkbench: vi.fn(), + workbench: "viewer" as string, + authUser: null as { id: string } | null, + authLoading: false, + pathname: "/editor", + activeFileId: null as string | null, +})); + +vi.mock("@app/services/fileStorage", () => ({ + fileStorage: { getLeafStirlingFileStubs: mocks.getLeafStirlingFileStubs }, +})); +vi.mock("@app/components/toast", () => ({ alert: mocks.alert })); +vi.mock("@app/contexts/NavigationContext", () => ({ + useNavigationState: () => ({ workbench: mocks.workbench }), + useNavigationActions: () => ({ + actions: { restoreWorkbench: mocks.restoreWorkbench }, + }), +})); +vi.mock("react-router-dom", () => ({ + useLocation: () => ({ pathname: mocks.pathname }), +})); +vi.mock("@app/auth/UseSession", () => ({ + useAuth: () => ({ user: mocks.authUser, loading: mocks.authLoading }), +})); +vi.mock("@app/contexts/ViewerContext", () => ({ + useViewer: () => ({ + activeFileId: mocks.activeFileId, + setActiveFileId: mocks.setActiveFileId, + }), +})); + +import { WorkbenchSessionPersistence } from "@app/components/session/WorkbenchSessionPersistence"; +import { fingerprintOwner } from "@app/services/workbenchSession"; +import { + FileStoreContext, + FileActionsContext, +} from "@app/contexts/file/contexts"; +import type { StirlingFileStub } from "@app/types/fileContext"; + +const SESSION_KEY = "stirling.workbench.session"; + +function stub( + id: string, + originalFileId: string, + versionNumber = 1, +): StirlingFileStub { + return { id, originalFileId, versionNumber, name: `${id}.pdf` } as never; +} + +// A minimal stand-in for the FileContext store: mutable state plus subscribers. +function makeStore(open: StirlingFileStub[] = [], selected: string[] = []) { + const listeners = new Set<() => void>(); + const state = { + files: { + ids: open.map((s) => s.id), + byId: Object.fromEntries(open.map((s) => [s.id, s])), + }, + ui: { selectedFileIds: selected }, + }; + return { + state, + getState: () => state as never, + subscribe: (listener: () => void) => { + listeners.add(listener); + return () => listeners.delete(listener); + }, + // reopenView waits on this to know the restored bytes have landed. + selectors: { + getFiles: (ids: string[]) => ids.map((id) => ({ id })), + } as never, + notify: () => listeners.forEach((listener) => listener()), + }; +} + +const actions = { + addStirlingFileStubs: vi.fn().mockResolvedValue([]), + setSelectedFiles: vi.fn(), +}; + +function mount(store: ReturnType) { + return render( + + + + + , + ); +} + +beforeEach(() => { + // The shared setup stubs crypto.subtle.digest to one constant for every input, so every account + // would fingerprint alike - and ownership is exactly what these tests are about. + vi.spyOn(globalThis.crypto.subtle, "digest").mockImplementation( + async (_algorithm: AlgorithmIdentifier, data: BufferSource) => { + const bytes = ArrayBuffer.isView(data) + ? new Uint8Array(data.buffer, data.byteOffset, data.byteLength) + : new Uint8Array(data); + let hash = 0x811c9dc5; + for (const byte of bytes) { + hash = Math.imul(hash ^ byte, 0x01000193) >>> 0; + } + const out = new Uint8Array(32); + for (let i = 0; i < out.length; i++) { + hash = Math.imul(hash ^ i, 0x01000193) >>> 0; + out[i] = hash & 0xff; + } + return out.buffer; + }, + ); + sessionStorage.clear(); + vi.clearAllMocks(); + actions.addStirlingFileStubs.mockResolvedValue([]); + mocks.getLeafStirlingFileStubs.mockResolvedValue([]); + mocks.workbench = "viewer"; + mocks.authUser = null; + mocks.authLoading = false; + mocks.pathname = "/editor"; + mocks.activeFileId = null; +}); +afterEach(() => vi.useRealTimers()); + +describe("restore", () => { + it("refills an empty workbench with each file's current leaf, in saved order", async () => { + sessionStorage.setItem( + SESSION_KEY, + JSON.stringify({ + v: 2, + fileIds: ["root-a", "root-b"], + selectedFileIds: ["root-b"], + }), + ); + // root-a forked while the user was away: v3 must win over the stale v1 leaf. + mocks.getLeafStirlingFileStubs.mockResolvedValue([ + stub("a-v1", "root-a", 1), + stub("a-v3", "root-a", 3), + stub("root-b", "root-b", 1), + ]); + + mount(makeStore()); + + await waitFor(() => + expect(actions.addStirlingFileStubs).toHaveBeenCalled(), + ); + const restored = actions.addStirlingFileStubs.mock.calls[0][0]; + expect(restored.map((s: StirlingFileStub) => s.id)).toEqual([ + "a-v3", + "root-b", + ]); + expect(actions.setSelectedFiles).toHaveBeenCalledWith(["root-b"]); + expect(mocks.alert).not.toHaveBeenCalled(); + }); + + it("does not touch a workbench that already holds files", async () => { + sessionStorage.setItem( + SESSION_KEY, + JSON.stringify({ v: 2, fileIds: ["root-a"], selectedFileIds: [] }), + ); + mount(makeStore([stub("already-open", "already-open")])); + + await act(async () => {}); + expect(actions.addStirlingFileStubs).not.toHaveBeenCalled(); + }); + + it("restores what still exists and says how much is gone", async () => { + sessionStorage.setItem( + SESSION_KEY, + JSON.stringify({ + v: 2, + fileIds: ["root-a", "gone"], + selectedFileIds: [], + }), + ); + mocks.getLeafStirlingFileStubs.mockResolvedValue([ + stub("root-a", "root-a"), + ]); + + mount(makeStore()); + + await waitFor(() => expect(mocks.alert).toHaveBeenCalled()); + expect(actions.addStirlingFileStubs.mock.calls[0][0]).toHaveLength(1); + expect(mocks.alert.mock.calls[0][0].alertType).toBe("warning"); + }); + + it("does not say 'the rest' when nothing at all could be restored", async () => { + sessionStorage.setItem( + SESSION_KEY, + JSON.stringify({ + v: 2, + fileIds: ["gone-1", "gone-2"], + selectedFileIds: [], + }), + ); + mocks.getLeafStirlingFileStubs.mockResolvedValue([]); + + mount(makeStore()); + + await waitFor(() => expect(mocks.alert).toHaveBeenCalled()); + expect(mocks.alert.mock.calls[0][0].title).toBe( + "workbench.sessionRestore.none", + ); + }); + + it("does nothing when no session was recorded", async () => { + mount(makeStore()); + await act(async () => {}); + expect(actions.addStirlingFileStubs).not.toHaveBeenCalled(); + expect(mocks.getLeafStirlingFileStubs).not.toHaveBeenCalled(); + }); + + it("reopens the document the user was viewing, at its current version", async () => { + sessionStorage.setItem( + SESSION_KEY, + JSON.stringify({ + v: 2, + fileIds: ["root-a"], + selectedFileIds: ["root-a"], + workbench: "fileEditor", + activeFileId: "root-a", + }), + ); + mocks.getLeafStirlingFileStubs.mockResolvedValue([ + stub("a-v2", "root-a", 2), + ]); + + mount(makeStore()); + + await waitFor(() => expect(mocks.setActiveFileId).toHaveBeenCalled()); + expect(mocks.setActiveFileId).toHaveBeenCalledWith("a-v2"); + expect(mocks.restoreWorkbench).toHaveBeenCalledWith("fileEditor"); + }); + + it("leaves a URL-owned view to the return path", async () => { + sessionStorage.setItem( + SESSION_KEY, + JSON.stringify({ + v: 2, + fileIds: ["root-a"], + selectedFileIds: [], + workbench: "myFiles", + }), + ); + mocks.getLeafStirlingFileStubs.mockResolvedValue([ + stub("root-a", "root-a"), + ]); + + mount(makeStore()); + + await waitFor(() => + expect(actions.addStirlingFileStubs).toHaveBeenCalled(), + ); + expect(mocks.restoreWorkbench).not.toHaveBeenCalled(); + }); +}); + +describe("whose workbench it is", () => { + // Records hold a fingerprint of the owner, never the account id. + const record = async (userId: string | null) => + sessionStorage.setItem( + SESSION_KEY, + JSON.stringify({ + v: 2, + fileIds: ["root-a"], + selectedFileIds: [], + userId: userId == null ? null : await fingerprintOwner(userId), + }), + ); + + it("does not open one user's workbench for the next person in the tab", async () => { + await record("user-a"); + mocks.authUser = { id: "user-b" }; + mocks.getLeafStirlingFileStubs.mockResolvedValue([ + stub("root-a", "root-a"), + ]); + + mount(makeStore()); + + await act(async () => {}); + expect(actions.addStirlingFileStubs).not.toHaveBeenCalled(); + // The record is theirs now - the previous person's files are gone from it, so they cannot + // resurface later in the session. + const taken = JSON.parse(sessionStorage.getItem(SESSION_KEY)!); + expect(taken.fileIds).toEqual([]); + expect(taken.userId).toBe(await fingerprintOwner("user-b")); + }); + + it("reopens it for the user who left it", async () => { + await record("user-a"); + mocks.authUser = { id: "user-a" }; + mocks.getLeafStirlingFileStubs.mockResolvedValue([ + stub("root-a", "root-a"), + ]); + + mount(makeStore()); + + await waitFor(() => + expect(actions.addStirlingFileStubs).toHaveBeenCalled(), + ); + }); + + it("waits for the session before deciding", async () => { + await record("user-a"); + mocks.authUser = null; + mocks.authLoading = true; + mocks.getLeafStirlingFileStubs.mockResolvedValue([ + stub("root-a", "root-a"), + ]); + + mount(makeStore()); + + await act(async () => {}); + // Neither restored nor discarded - who is signed in is not known yet. + expect(actions.addStirlingFileStubs).not.toHaveBeenCalled(); + expect(sessionStorage.getItem(SESSION_KEY)).not.toBeNull(); + }); +}); + +describe("a lost session that comes back", () => { + const rerenderWith = ( + view: ReturnType, + store: ReturnType, + ) => + view.rerender( + + + + + , + ); + + it("survives a blip on the identity check", async () => { + // A failed /auth/me - flaky wifi, a backend redeploy, a refreshSession() that did not land - + // briefly reads as nobody signed in. It must not be mistaken for signing out. + sessionStorage.setItem( + SESSION_KEY, + JSON.stringify({ + v: 2, + fileIds: ["root-a"], + selectedFileIds: [], + userId: await fingerprintOwner("user-a"), + }), + ); + mocks.authUser = { id: "user-a" }; + const store = makeStore([stub("f1", "f1")]); + const view = mount(store); + await act(async () => {}); + + mocks.authUser = null; + rerenderWith(view, store); + await act(async () => {}); + + expect(sessionStorage.getItem(SESSION_KEY)).not.toBeNull(); + + // ...and once the identity is back, the workbench is still being recorded. + mocks.authUser = { id: "user-a" }; + rerenderWith(view, store); + // Let the fingerprint land: writes hold off while a known identity has none yet. + await act(async () => {}); + store.state.files.ids = ["f2" as never]; + store.state.files.byId = { f2: stub("f2", "root-b") } as never; + act(() => store.notify()); + view.unmount(); + expect(JSON.parse(sessionStorage.getItem(SESSION_KEY)!).fileIds).toEqual([ + "root-b", + ]); + }); +}); + +describe("on the login screen", () => { + it("neither restores nor records - signing out must not rebuild the workbench there", async () => { + mocks.pathname = "/login"; + sessionStorage.setItem( + SESSION_KEY, + JSON.stringify({ v: 2, fileIds: ["root-a"], selectedFileIds: [] }), + ); + mocks.getLeafStirlingFileStubs.mockResolvedValue([ + stub("root-a", "root-a"), + ]); + + const store = makeStore(); + const { unmount } = mount(store); + await act(async () => {}); + expect(actions.addStirlingFileStubs).not.toHaveBeenCalled(); + + // And the unmount flush must not write either. + store.state.files.ids = ["f1" as never]; + store.state.files.byId = { f1: stub("f1", "f1") } as never; + act(() => store.notify()); + unmount(); + expect(JSON.parse(sessionStorage.getItem(SESSION_KEY)!).fileIds).toEqual([ + "root-a", + ]); + }); +}); + +describe("writer", () => { + it("mirrors the open files and selection as original ids, debounced", async () => { + vi.useFakeTimers(); + const store = makeStore(); + mount(store); + + store.state.files.ids = ["v2" as never]; + store.state.files.byId = { v2: stub("v2", "root-a", 2) } as never; + store.state.ui.selectedFileIds = ["v2"]; + act(() => store.notify()); + + await act(async () => { + await vi.advanceTimersByTimeAsync(300); + }); + expect(JSON.parse(sessionStorage.getItem(SESSION_KEY)!)).toMatchObject({ + fileIds: ["root-a"], + selectedFileIds: ["root-a"], + workbench: "viewer", + }); + }); + + it("records the current view, so the return lands where the user left", async () => { + vi.useFakeTimers(); + mocks.workbench = "fileEditor"; + const store = makeStore([stub("f1", "f1")]); + mount(store); + + act(() => store.notify()); + await act(async () => { + await vi.advanceTimersByTimeAsync(300); + }); + expect(JSON.parse(sessionStorage.getItem(SESSION_KEY)!).workbench).toBe( + "fileEditor", + ); + }); + + it("writes nothing until the restore has settled", () => { + vi.useFakeTimers(); + sessionStorage.setItem( + SESSION_KEY, + JSON.stringify({ v: 2, fileIds: ["root-a"], selectedFileIds: [] }), + ); + // Restore is still awaiting storage, so this mount's empty state is not the truth. + mocks.getLeafStirlingFileStubs.mockReturnValue(new Promise(() => {})); + + const store = makeStore(); + const { unmount } = mount(store); + act(() => store.notify()); + unmount(); + + expect(JSON.parse(sessionStorage.getItem(SESSION_KEY)!).fileIds).toEqual([ + "root-a", + ]); + }); + + it("flushes on unmount, so the state at the shell switch survives", () => { + vi.useFakeTimers(); + const store = makeStore(); + const { unmount } = mount(store); + + store.state.files.ids = ["f1" as never]; + store.state.files.byId = { f1: stub("f1", "f1") } as never; + act(() => store.notify()); + unmount(); + + expect(JSON.parse(sessionStorage.getItem(SESSION_KEY)!).fileIds).toEqual([ + "f1", + ]); + }); +}); diff --git a/frontend/editor/src/core/components/session/WorkbenchSessionPersistence.tsx b/frontend/editor/src/core/components/session/WorkbenchSessionPersistence.tsx new file mode 100644 index 0000000000..86bf72502c --- /dev/null +++ b/frontend/editor/src/core/components/session/WorkbenchSessionPersistence.tsx @@ -0,0 +1,301 @@ +// The editor/processor shell switch unmounts every editor provider, and a reload starts from nothing: +// this mirrors the workbench into sessionStorage and refills an empty one from that record on mount. +import { useCallback, useContext, useEffect, useRef, useState } from "react"; +import { useTranslation } from "react-i18next"; +import { + FileStoreContext, + type FileStateStore, +} from "@app/contexts/file/contexts"; +import { useFileActions } from "@app/contexts/FileContext"; +import { + useNavigationActions, + useNavigationState, +} from "@app/contexts/NavigationContext"; +import { useViewer } from "@app/contexts/ViewerContext"; +import { useAuth } from "@app/auth/UseSession"; +import { useLocation } from "react-router-dom"; +import { isAuthRoute } from "@app/constants/routes"; +import { fileStorage } from "@app/services/fileStorage"; +import { alert } from "@app/components/toast"; +import { WORKBENCH_SESSION_RESTORE } from "@app/constants/featureFlags"; +import { + beginRestoredView, + clearWorkbenchSession, + fingerprintOwner, + resumeWorkbenchSession, + endRestoredView, + isSeedableView, + originalIdOf, + readWorkbenchSession, + writeWorkbenchSession, +} from "@app/services/workbenchSession"; +import type { WorkbenchType } from "@app/types/workbench"; +import type { StirlingFileStub } from "@app/types/fileContext"; +import type { FileId } from "@app/types/file"; + +const WRITE_DEBOUNCE_MS = 300; + +// Current leaf per original id; a forked chain resolves to the highest version. +function leafByOriginalId( + leaves: StirlingFileStub[], +): Map { + const map = new Map(); + for (const leaf of leaves) { + const key = originalIdOf(leaf); + const current = map.get(key); + if (!current || (leaf.versionNumber ?? 1) > (current.versionNumber ?? 1)) { + map.set(key, leaf); + } + } + return map; +} + +/** How long to wait for the NEXT file to hydrate before giving up on holding the view. Restarted on + * each arrival, so a slow device with large documents keeps the view as long as it makes progress. */ +const SETTLE_TIMEOUT_MS = 5000; + +/** Released a beat late, so effects reacting to the same commit still see the restore in progress. */ +const RELEASE_GRACE_MS = 250; + +/** + * Reopen the recorded view, then hold the restore guard until the files have hydrated. + * + * The view is written ONCE. Re-asserting it after hydration would also overwrite a view the user + * picked in the meantime; holding the guard is what keeps HomePage's defaults off it instead. + */ +function reopenView( + store: FileStateStore, + reopen: (view: WorkbenchType) => void, + { + view, + fileCount, + token, + }: { view: WorkbenchType; fileCount: number; token: number }, +): void { + reopen(view); + const loaded = () => + store.selectors.getFiles(store.getState().files.ids).length; + + const release = () => + setTimeout(() => endRestoredView(token), RELEASE_GRACE_MS); + if (loaded() >= fileCount) { + release(); + return; + } + + let timer: ReturnType; + const stop = () => { + clearTimeout(timer); + unsubscribe(); + release(); + }; + const waitForNext = () => { + clearTimeout(timer); + timer = setTimeout(stop, SETTLE_TIMEOUT_MS); + }; + + let seen = loaded(); + const unsubscribe = store.subscribe(() => { + const now = loaded(); + if (now >= fileCount) return stop(); + // Progress, not completion: give the remaining files a fresh window. + if (now > seen) { + seen = now; + waitForNext(); + } + }); + waitForNext(); +} + +export function WorkbenchSessionPersistence() { + const store = useContext(FileStoreContext); + const { actions } = useFileActions(); + const { workbench } = useNavigationState(); + const { actions: navigationActions } = useNavigationActions(); + const { activeFileId, setActiveFileId } = useViewer(); + const { user, loading: authLoading } = useAuth(); + // Login/signup mount the editor's providers too. Nothing there is the user's workbench, so this + // records nothing and restores nothing - otherwise signing out rebuilds it on the login screen. + const onAuthRoute = isAuthRoute(useLocation().pathname); + const userId = user?.id != null ? String(user.id) : null; + // Fingerprinted, never stored raw - see fingerprintOwner. Computed asynchronously, so writes + // hold off until it lands rather than stamping the record "nobody's" and then failing its own + // ownership check. + const [owner, setOwner] = useState(null); + useEffect(() => { + if (userId == null) { + setOwner(null); + return; + } + let cancelled = false; + void fingerprintOwner(userId).then((fingerprint) => { + if (!cancelled) setOwner(fingerprint); + }); + return () => { + cancelled = true; + }; + }, [userId]); + const { t } = useTranslation(); + // Captured before the writer below can overwrite it with the empty boot state. + const [saved] = useState(readWorkbenchSession); + const restoreStarted = useRef(false); + // Until the restore has run, this mount's empty state is not the truth to record. + const restoreSettled = useRef(false); + + // Published so a build's restore setting is legible without reading the bundle. + useEffect(() => { + document.documentElement.dataset.workbenchRestore = String( + WORKBENCH_SESSION_RESTORE, + ); + }, []); + + const write = useCallback(() => { + if (!store || !restoreSettled.current) return; + // A known identity whose fingerprint has not landed yet: wait, do not stamp it as nobody's. + if (userId != null && owner == null) return; + const state = store.getState(); + const toOriginal = (id: FileId): string | null => { + const stub = state.files.byId[id]; + return stub ? originalIdOf(stub) : null; + }; + const isPresent = (id: string | null): id is string => id !== null; + writeWorkbenchSession({ + fileIds: state.files.ids.map(toOriginal).filter(isPresent), + selectedFileIds: state.ui.selectedFileIds + .map(toOriginal) + .filter(isPresent), + workbench, + userId: owner, + activeFileId: activeFileId + ? (toOriginal(activeFileId as FileId) ?? undefined) + : undefined, + }); + }, [store, workbench, activeFileId, userId, owner]); + + // Read by the file subscription, which must not resubscribe on every view change. + const writeRef = useRef(write); + writeRef.current = write; + + useEffect(() => { + if (!store || onAuthRoute) return; + // This mount is a new session: undo any suspension left by a sign-out in this page's lifetime. + resumeWorkbenchSession(); + let timer: ReturnType | undefined; + const unsubscribe = store.subscribe(() => { + clearTimeout(timer); + timer = setTimeout(() => writeRef.current(), WRITE_DEBOUNCE_MS); + }); + return () => { + clearTimeout(timer); + // Flush, so the state at the moment of the shell switch is what survives. + writeRef.current(); + unsubscribe(); + }; + }, [store, onAuthRoute]); + + // Changing view touches no file state, so the subscription above never sees it. + useEffect(() => write(), [write]); + + useEffect(() => { + if (restoreStarted.current) return; + if (onAuthRoute) return; + // Who is signed in decides whether this record is theirs to reopen, so settle that first. + if (authLoading) return; + restoreStarted.current = true; + + const nothingToDo = + !WORKBENCH_SESSION_RESTORE || + !store || + !saved || + saved.fileIds.length === 0 || + store.getState().files.ids.length > 0; + if (nothingToDo) { + restoreSettled.current = true; + return; + } + + void (async () => { + // A tab can outlive a sign-out (the logout clears it, but a 401 bounce or an expiry does + // not), and the next person to sign in here must not open the last person's documents. + const currentOwner = + userId == null ? null : await fingerprintOwner(userId); + if ((saved.userId ?? null) !== currentOwner) { + clearWorkbenchSession(); + restoreSettled.current = true; + return; + } + + // Held while the files land: they are added one at a time, and each landing re-runs the + // default-view heuristic, which must not overwrite the recorded view mid-restore. + let held: number | null = null; + try { + // Resolve each id to its CURRENT leaf: a policy or another tab may have versioned it since. + const leaves = leafByOriginalId( + await fileStorage.getLeafStirlingFileStubs(), + ); + const stubs = saved.fileIds + .map((id) => leaves.get(id)) + .filter((stub): stub is StirlingFileStub => stub !== undefined); + + if (stubs.length > 0) { + const view = isSeedableView(saved.workbench) ? saved.workbench : null; + if (view) held = beginRestoredView(); + // The same entry point My Files uses, so a restored file is governed by the same rules as + // any other file entering the workbench - including whether a policy has already run on it. + await actions.addStirlingFileStubs(stubs); + const selected = saved.selectedFileIds + .map((id) => leaves.get(id)?.id) + .filter((id): id is FileId => id !== undefined); + if (selected.length > 0) actions.setSelectedFiles(selected); + // After the files land: the viewer drops an active id it cannot find. + const active = saved.activeFileId + ? leaves.get(saved.activeFileId)?.id + : undefined; + if (active) setActiveFileId(active as string); + if (view && held !== null) { + reopenView(store, navigationActions.restoreWorkbench, { + view, + fileCount: stubs.length, + token: held, + }); + held = null; // reopenView owns the release from here. + } + } + + const missing = saved.fileIds.length - stubs.length; + if (missing > 0) { + alert({ + alertType: "warning", + title: + stubs.length === 0 + ? t( + "workbench.sessionRestore.none", + "Your previous files are no longer stored on this device.", + ) + : t( + "workbench.sessionRestore.partial", + "Restored {{restored}} of {{total}} files. The rest are no longer stored on this device.", + { restored: stubs.length, total: saved.fileIds.length }, + ), + }); + } + } finally { + if (held !== null) endRestoredView(held); + // Even a failed restore must release the writer, or the record freezes for the session. + restoreSettled.current = true; + } + })(); + }, [ + saved, + store, + actions, + navigationActions, + setActiveFileId, + t, + authLoading, + userId, + onAuthRoute, + ]); + + return null; +} diff --git a/frontend/editor/src/core/constants/featureFlags.ts b/frontend/editor/src/core/constants/featureFlags.ts index a60770ac3a..8981dce2d6 100644 --- a/frontend/editor/src/core/constants/featureFlags.ts +++ b/frontend/editor/src/core/constants/featureFlags.ts @@ -11,3 +11,6 @@ // Annotated as `boolean` (not the literal `false`) so call sites aren't treated // as constant/unreachable conditions by the type checker and linter. export const WATCHED_FOLDERS_ENABLED: boolean = false; + +// Refill an empty workbench from the tab's last session (survives a provider remount or a reload). +export const WORKBENCH_SESSION_RESTORE: boolean = true; diff --git a/frontend/editor/src/core/contexts/NavigationContext.tsx b/frontend/editor/src/core/contexts/NavigationContext.tsx index a7cb95d349..f158991378 100644 --- a/frontend/editor/src/core/contexts/NavigationContext.tsx +++ b/frontend/editor/src/core/contexts/NavigationContext.tsx @@ -94,6 +94,9 @@ export interface NavigationWarningHandlers { // Navigation context actions interface export interface NavigationContextActions { setWorkbench: (workbench: WorkbenchType) => void; + /** Reopen a view the user already had, bypassing the unsaved-changes prompt that + * guards a user-initiated switch - a restore is not the user leaving anything. */ + restoreWorkbench: (workbench: WorkbenchType) => void; setSelectedTool: (toolId: ToolId | null) => void; setToolAndWorkbench: ( toolId: ToolId | null, @@ -221,6 +224,10 @@ export const NavigationProvider: React.FC<{ [state.workbench, state.hasUnsavedChanges], ); + const restoreWorkbench = useCallback((workbench: WorkbenchType) => { + dispatch({ type: "SET_WORKBENCH", payload: { workbench } }); + }, []); + const setSelectedTool = useCallback((toolId: ToolId | null) => { dispatch({ type: "SET_SELECTED_TOOL", payload: { toolId } }); }, []); @@ -402,6 +409,7 @@ export const NavigationProvider: React.FC<{ const actions: NavigationContextActions = useMemo( () => ({ setWorkbench, + restoreWorkbench, setSelectedTool, setToolAndWorkbench, setHasUnsavedChanges, @@ -419,6 +427,7 @@ export const NavigationProvider: React.FC<{ }), [ setWorkbench, + restoreWorkbench, setSelectedTool, setToolAndWorkbench, setHasUnsavedChanges, diff --git a/frontend/editor/src/core/extensions/accountLogout.ts b/frontend/editor/src/core/extensions/accountLogout.ts index e4eddd7274..df4e8f4273 100644 --- a/frontend/editor/src/core/extensions/accountLogout.ts +++ b/frontend/editor/src/core/extensions/accountLogout.ts @@ -1,3 +1,5 @@ +import { suspendWorkbenchSession } from "@app/services/workbenchSession"; + type SignOutFn = () => Promise; interface AccountLogoutDeps { @@ -21,6 +23,10 @@ export function useAccountLogout() { "1", ); } + // The tab outlives the session; the next person to sign in here must not + // inherit this workbench. Suspends writing too - signing out unmounts the + // editor, and its flush would otherwise write the record straight back. + suspendWorkbenchSession(); await signOut(); } finally { redirectToLogin(); diff --git a/frontend/editor/src/core/pages/HomePage.tsx b/frontend/editor/src/core/pages/HomePage.tsx index e0c5303447..0d7a571f98 100644 --- a/frontend/editor/src/core/pages/HomePage.tsx +++ b/frontend/editor/src/core/pages/HomePage.tsx @@ -15,6 +15,7 @@ import { useNavigationState, useNavigationActions, } from "@app/contexts/NavigationContext"; +import { isApplyingRestoredView } from "@app/services/workbenchSession"; import { useViewer } from "@app/contexts/ViewerContext"; import { useLocation, useNavigate } from "react-router-dom"; import AppsIcon from "@mui/icons-material/AppsRounded"; @@ -161,8 +162,14 @@ export default function HomePage() { if (navigationState.workbench !== "myFiles") { actions.setWorkbench("myFiles"); } - } else if (navigationState.workbench === "myFiles") { - // Leaving the file manager - drop back to a sensible default. + } else if ( + navigationState.workbench === "myFiles" && + !isApplyingRestoredView() + ) { + // The URL no longer supports the file manager - drop back to a sensible default. Stays a + // state check rather than a transition one: HomePage remounts without NavigationContext + // (a share link, a login bounce), and the view has to be corrected on arrival too. + // Skipped mid-restore, which is reopening a recorded view onto files still loading. actions.setWorkbench(activeFiles.length > 1 ? "fileEditor" : "viewer"); } }, [ @@ -204,7 +211,9 @@ export default function HomePage() { navigationState.workbench, ); - if (action) { + // A session restore fills an empty workbench too, but it already knows which view the user + // left - so it wins over this heuristic rather than being overwritten by it. + if (action && !isApplyingRestoredView()) { actions.setWorkbench(action.workbench); if (typeof action.activeFileIndex === "number") { setActiveFileIndex(action.activeFileIndex); diff --git a/frontend/editor/src/core/services/workbenchSession.test.ts b/frontend/editor/src/core/services/workbenchSession.test.ts new file mode 100644 index 0000000000..d9feec8d1c --- /dev/null +++ b/frontend/editor/src/core/services/workbenchSession.test.ts @@ -0,0 +1,193 @@ +import { describe, it, expect, beforeEach, vi } from "vitest"; +import { + originalIdOf, + readWorkbenchSession, + writeWorkbenchSession, + saveEditorReturnPath, + takeEditorReturnPath, + isSeedableView, + clearWorkbenchSession, + suspendWorkbenchSession, + resumeWorkbenchSession, +} from "@app/services/workbenchSession"; +import type { StirlingFileStub } from "@app/types/fileContext"; + +const SESSION_KEY = "stirling.workbench.session"; + +beforeEach(() => { + sessionStorage.clear(); + resumeWorkbenchSession(); +}); + +describe("workbench session record", () => { + it("round-trips the open files and selection", () => { + writeWorkbenchSession({ fileIds: ["a", "b"], selectedFileIds: ["b"] }); + expect(readWorkbenchSession()).toMatchObject({ + fileIds: ["a", "b"], + selectedFileIds: ["b"], + }); + }); + + it("returns null when nothing was recorded", () => { + expect(readWorkbenchSession()).toBeNull(); + }); + + it("rejects a malformed record instead of throwing", () => { + sessionStorage.setItem(SESSION_KEY, "not json"); + expect(readWorkbenchSession()).toBeNull(); + sessionStorage.setItem( + SESSION_KEY, + JSON.stringify({ v: 2, fileIds: "nope" }), + ); + expect(readWorkbenchSession()).toBeNull(); + }); + + it("drops non-string ids and defaults a missing selection", () => { + sessionStorage.setItem( + SESSION_KEY, + JSON.stringify({ v: 2, fileIds: ["a", 7, null, "b"] }), + ); + expect(readWorkbenchSession()).toMatchObject({ + fileIds: ["a", "b"], + selectedFileIds: [], + }); + }); +}); + +describe("editor return path", () => { + it("is consumed by the first take", () => { + saveEditorReturnPath("/compress?x=1"); + expect(takeEditorReturnPath()).toBe("/compress?x=1"); + expect(takeEditorReturnPath()).toBeNull(); + }); +}); + +describe("originalIdOf", () => { + it("prefers the original id and falls back to the file id", () => { + expect( + originalIdOf({ id: "v3", originalFileId: "root" } as StirlingFileStub), + ).toBe("root"); + expect( + originalIdOf({ id: "v1", originalFileId: "" } as StirlingFileStub), + ).toBe("v1"); + }); +}); + +describe("record hygiene", () => { + it("discards a record written by an older schema", () => { + // No version stamp: a shape this build no longer understands. + sessionStorage.setItem( + SESSION_KEY, + JSON.stringify({ fileIds: ["a"], selectedFileIds: [] }), + ); + expect(readWorkbenchSession()).toBeNull(); + + // v1 recorded userId before it meant anything, so those must go too rather than + // look like a workbench that legitimately belongs to an anonymous session. + sessionStorage.setItem( + SESSION_KEY, + JSON.stringify({ + v: 1, + fileIds: ["a"], + selectedFileIds: [], + userId: null, + }), + ); + expect(readWorkbenchSession()).toBeNull(); + }); + + it("drops the previous record when a write fails, rather than leaving it stale", () => { + writeWorkbenchSession({ fileIds: ["old"], selectedFileIds: [] }); + const setItem = vi + .spyOn(Storage.prototype, "setItem") + .mockImplementation(() => { + throw new Error("QuotaExceededError"); + }); + + writeWorkbenchSession({ fileIds: ["new"], selectedFileIds: [] }); + setItem.mockRestore(); + + // Better to restore nothing than to restore a workbench the user has moved on from. + expect(readWorkbenchSession()).toBeNull(); + }); + + it("records who the workbench belonged to", () => { + writeWorkbenchSession({ + fileIds: ["a"], + selectedFileIds: [], + userId: "user-1", + }); + expect(readWorkbenchSession()?.userId).toBe("user-1"); + }); + + it("stays gone after sign-out, even though the teardown writes once more", () => { + writeWorkbenchSession({ fileIds: ["a", "b"], selectedFileIds: [] }); + + suspendWorkbenchSession(); + // Signing out unmounts the editor, whose flush writes the workbench one last time - + // with no user attached. Clearing alone let that recreate the record. + writeWorkbenchSession({ + fileIds: ["a", "b"], + selectedFileIds: [], + userId: null, + }); + + expect(sessionStorage.getItem(SESSION_KEY)).toBeNull(); + }); + + it("keeps the owner when the identity is momentarily unknown", () => { + // A sign-out teardown and a failed /auth/me both write with no user attached. Losing the + // owner here would make the record unrestorable for the person it belongs to. + writeWorkbenchSession({ + fileIds: ["a", "b"], + selectedFileIds: [], + userId: "user-a", + }); + + writeWorkbenchSession({ + fileIds: ["a", "b"], + selectedFileIds: [], + userId: null, + }); + + expect(readWorkbenchSession()?.userId).toBe("user-a"); + }); + + it("still records for a genuinely anonymous session", () => { + // Core has no auth at all, so null is the normal owner there and must keep working. + writeWorkbenchSession({ + fileIds: ["a"], + selectedFileIds: [], + userId: null, + }); + expect(readWorkbenchSession()?.fileIds).toEqual(["a"]); + }); + + it("records again once a new editor session starts", () => { + suspendWorkbenchSession(); + resumeWorkbenchSession(); + writeWorkbenchSession({ fileIds: ["a"], selectedFileIds: [] }); + expect(readWorkbenchSession()?.fileIds).toEqual(["a"]); + }); + + it("clears on request", () => { + writeWorkbenchSession({ fileIds: ["a"], selectedFileIds: [] }); + clearWorkbenchSession(); + expect(readWorkbenchSession()).toBeNull(); + }); +}); + +describe("views the restore may reopen", () => { + it("accepts the workbench views a session can land on", () => { + expect(isSeedableView("viewer")).toBe(true); + expect(isSeedableView("fileEditor")).toBe(true); + expect(isSeedableView("pageEditor")).toBe(true); + }); + + it("leaves URL-owned and tool-owned views alone", () => { + // HomePage pins myFiles to /files and bounces it elsewhere; custom views belong to a tool. + expect(isSeedableView("myFiles")).toBe(false); + expect(isSeedableView("custom:compare")).toBe(false); + expect(isSeedableView(undefined)).toBe(false); + }); +}); diff --git a/frontend/editor/src/core/services/workbenchSession.ts b/frontend/editor/src/core/services/workbenchSession.ts new file mode 100644 index 0000000000..67f6d1abfc --- /dev/null +++ b/frontend/editor/src/core/services/workbenchSession.ts @@ -0,0 +1,173 @@ +// The tab's last editor session (open files, selection, view), so a processor switch or reload +// does not cost the user their workbench. sessionStorage on purpose: per-tab, tabs never clobber. + +import type { StirlingFileStub } from "@app/types/fileContext"; + +const SESSION_KEY = "stirling.workbench.session"; +/** Bumped when the record's shape or meaning changes, so an old one is discarded rather than + * half-read. v2: `userId` became meaningful - v1 records were written without a real owner and + * would otherwise look like they belonged to an anonymous session forever. */ +const SESSION_VERSION = 2; +const RETURN_PATH_KEY = "stirling.workbench.editorReturnPath"; + +// All ids are ORIGINAL file ids - a file's stable identity across versions. +export interface WorkbenchSession { + fileIds: string[]; + selectedFileIds: string[]; + /** Which view was on screen. Absent for a record written before this was tracked. */ + workbench?: string; + activeFileId?: string; + /** Fingerprint of who the workbench belonged to, so the next person in this tab does not + * inherit it. Never the account id itself - see {@link fingerprintOwner}. */ + userId?: string | null; +} + +/** A file's stable identity across versions - what the session records. */ +export function originalIdOf(stub: StirlingFileStub): string { + return stub.originalFileId || (stub.id as string); +} + +export function readWorkbenchSession(): WorkbenchSession | null { + try { + const raw = sessionStorage.getItem(SESSION_KEY); + if (!raw) return null; + const parsed = JSON.parse(raw) as Partial & { + v?: number; + }; + if (parsed.v !== SESSION_VERSION) return null; + if (!Array.isArray(parsed.fileIds)) return null; + return { + fileIds: parsed.fileIds.filter((id) => typeof id === "string"), + selectedFileIds: Array.isArray(parsed.selectedFileIds) + ? parsed.selectedFileIds.filter((id) => typeof id === "string") + : [], + workbench: + typeof parsed.workbench === "string" ? parsed.workbench : undefined, + activeFileId: + typeof parsed.activeFileId === "string" + ? parsed.activeFileId + : undefined, + userId: typeof parsed.userId === "string" ? parsed.userId : null, + }; + } catch { + return null; + } +} + +// Sign-out clears the record, but signing out also tears the editor down - and that teardown +// flushes the workbench one last time, recreating what we just deleted (with no user attached). +// So a sign-out has to stop writing too, not merely clear. +let writesSuspended = false; + +/** Sign-out: drop the record and stop recording, so the teardown cannot put it back. */ +export function suspendWorkbenchSession(): void { + writesSuspended = true; + clearWorkbenchSession(); +} + +/** A fresh editor mount is a new session, so recording starts again. */ +export function resumeWorkbenchSession(): void { + writesSuspended = false; +} + +export function writeWorkbenchSession(session: WorkbenchSession): void { + if (writesSuspended) return; + try { + // Never downgrade a known owner to "nobody". Signing out and a failed identity check both + // read as no user, and dropping the owner would either hand the workbench to whoever signs + // in next or lose it for the person it belongs to. Keeping the owner leaves the restore's + // ownership check to decide, which it does with a settled identity. + const owner = session.userId ?? readWorkbenchSession()?.userId ?? null; + sessionStorage.setItem( + SESSION_KEY, + JSON.stringify({ ...session, userId: owner, v: SESSION_VERSION }), + ); + } catch { + // Storage refused (quota, privacy mode). setItem is atomic, so the PREVIOUS record would + // survive and restore an older workbench - drop it, so the failure is "no restore" instead. + clearWorkbenchSession(); + } +} + +/** + * A one-way fingerprint of the signed-in user. Owners are only ever compared, never read back, so + * the account id itself never needs to reach storage. Falls back to a non-cryptographic digest + * where SubtleCrypto is absent (a self-hosted instance served over plain http): the fingerprint + * only has to tell two accounts sharing one tab apart, and the files it gates are reachable from + * My Files regardless, since IndexedDB is per-origin. + */ +export async function fingerprintOwner(userId: string): Promise { + if (globalThis.crypto?.subtle) { + const digest = await globalThis.crypto.subtle.digest( + "SHA-256", + new TextEncoder().encode(userId), + ); + return Array.from(new Uint8Array(digest)) + .map((byte) => byte.toString(16).padStart(2, "0")) + .join("") + .slice(0, 32); + } + let hash = 0x811c9dc5; + for (let i = 0; i < userId.length; i++) { + hash ^= userId.charCodeAt(i); + hash = Math.imul(hash, 0x01000193) >>> 0; + } + return `fnv-${hash.toString(16)}`; +} + +/** Drop the record: on sign-out, and whenever it would otherwise be restored for the wrong person. */ +export function clearWorkbenchSession(): void { + try { + sessionStorage.removeItem(SESSION_KEY); + } catch { + // A record we cannot remove is also one we cannot read. + } +} + +/** Views a restore may seed directly. "myFiles" is URL-owned (HomePage pins it to /files) and a + * custom view belongs to its tool - the editor return path restores those instead. */ +const SEEDABLE_VIEWS = ["viewer", "fileEditor", "pageEditor"]; + +// Raised while a restore is applying its recorded view, so writers that pick a default view from +// whatever is loaded at the time defer to the restore rather than race it. +let applyingRestoredView = false; +let restoreGeneration = 0; + +/** Returns a token for endRestoredView, so a stale release cannot end a newer restore. */ +export function beginRestoredView(): number { + applyingRestoredView = true; + return ++restoreGeneration; +} + +export function endRestoredView(token: number): void { + if (token === restoreGeneration) applyingRestoredView = false; +} + +export function isApplyingRestoredView(): boolean { + return applyingRestoredView; +} + +export function isSeedableView( + view: string | undefined, +): view is "viewer" | "fileEditor" | "pageEditor" { + return view !== undefined && SEEDABLE_VIEWS.includes(view); +} + +export function saveEditorReturnPath(path: string): void { + try { + sessionStorage.setItem(RETURN_PATH_KEY, path); + } catch { + // Best-effort: the switch back just lands on the editor root. + } +} + +/** One-shot: consumed by the switch back so a stale path cannot linger. */ +export function takeEditorReturnPath(): string | null { + try { + const path = sessionStorage.getItem(RETURN_PATH_KEY); + if (path !== null) sessionStorage.removeItem(RETURN_PATH_KEY); + return path; + } catch { + return null; + } +} diff --git a/frontend/editor/src/core/tests/stubbed/engine-capabilities.spec.ts b/frontend/editor/src/core/tests/stubbed/engine-capabilities.spec.ts index 6107fb5276..c5b24dd6ac 100644 --- a/frontend/editor/src/core/tests/stubbed/engine-capabilities.spec.ts +++ b/frontend/editor/src/core/tests/stubbed/engine-capabilities.spec.ts @@ -107,6 +107,12 @@ test.describe("engine capabilities", { tag: "@engine-capability" }, () => { await uploadFiles(page, SAMPLE_PDF); + // Dropped before the reload boots, so it cannot reopen the file for us: the eye + // below toggles, and whether the restore runs is a build flag this spec does not own. + await page.addInitScript(() => + sessionStorage.removeItem("stirling.workbench.session"), + ); + // Full reload: FileContext rehydrates from IndexedDB, not from memory. await page.reload({ waitUntil: "domcontentloaded" }); diff --git a/frontend/editor/src/core/tests/stubbed/workbench-session-restore.spec.ts b/frontend/editor/src/core/tests/stubbed/workbench-session-restore.spec.ts new file mode 100644 index 0000000000..48765392fe --- /dev/null +++ b/frontend/editor/src/core/tests/stubbed/workbench-session-restore.spec.ts @@ -0,0 +1,207 @@ +import { test, expect } from "@app/tests/helpers/stub-test-base"; +import { uploadFiles } from "@app/tests/helpers/ui-helpers"; +import path from "path"; + +const FIXTURES_DIR = path.join(import.meta.dirname, "../test-fixtures"); +const SAMPLES = [ + "compare_sample_a.pdf", + "compare_sample_b.pdf", + "sample.pdf", + "rotated-pages.pdf", + "annotations_out_of_order.pdf", +].map((name) => path.join(FIXTURES_DIR, name)); + +// Read from the running app, not imported: a spec resolves @app/* to a different layer than +// the browser build does, so an imported WORKBENCH_SESSION_RESTORE can disagree with reality. +async function restoreEnabled( + page: import("@playwright/test").Page, +): Promise { + await page.waitForFunction( + () => document.documentElement.dataset.workbenchRestore !== undefined, + null, + { timeout: 20000 }, + ); + return page.evaluate( + () => document.documentElement.dataset.workbenchRestore === "true", + ); +} + +const NO_RESTORE = "this build ships the workbench restore off"; + +// Switching editor -> processor unmounts every editor provider; the session record +// in sessionStorage is what brings the workbench back on return. +test.describe("Workbench survives the editor/processor switch", () => { + test.use({ + stubOptions: { + enableLogin: true, + user: { + id: 44, + username: "owner", + email: "owner@example.com", + role: "ROLE_USER", + portalAccess: true, + }, + }, + seedJwt: true, + }); + + test("open files and the library return after a round-trip", async ({ + page, + }) => { + test.skip(!(await restoreEnabled(page)), NO_RESTORE); + + // Portal endpoints the processor shell fetches on mount. + for (const [pattern, json] of [ + ["**/api/v1/policies", []], + ["**/api/v1/policies/runs", []], + ["**/api/v1/policies/overview", { pipelines: [] }], + ["**/api/v1/sources", { sources: [] }], + ["**/api/v1/team/my", []], + ] as const) { + await page.route(pattern, (route) => route.fulfill({ json })); + } + + await uploadFiles(page, SAMPLES); + await expect(page.locator(".file-sidebar-file-item")).toHaveCount( + SAMPLES.length, + { timeout: 15000 }, + ); + + // Uploading lands on the file grid, not the viewer - so the return has a + // view it can get wrong (NavigationContext boots to "viewer"). + await expect( + page.getByRole("radio", { name: /Active Files/i }), + ).toBeChecked(); + + // Out through the sidebar footer switch - the real user path. + await page.getByRole("button", { name: "Open PDF Processor" }).click(); + await expect(page).toHaveURL(/\/processor/, { timeout: 15000 }); + + // Split the two halves of the feature: if this fails the writer is at fault, + // if it passes but the view below is wrong the seeding is. + expect( + await page.evaluate(() => ({ + session: JSON.parse( + sessionStorage.getItem("stirling.workbench.session") ?? "{}", + ), + returnPath: sessionStorage.getItem( + "stirling.workbench.editorReturnPath", + ), + })), + ).toMatchObject({ + session: { workbench: "fileEditor" }, + returnPath: "/editor", + }); + + // Load the editor cold. Every provider mounts from nothing here, which is + // the loss the restore has to cover on the way back. + await page.goto("/editor", { waitUntil: "domcontentloaded" }); + + await expect(page.locator(".file-sidebar-file-item")).toHaveCount( + SAMPLES.length, + { timeout: 20000 }, + ); + await expect(page.getByText(/compare_sample_a/i).first()).toBeVisible(); + await expect( + page.getByRole("radio", { name: /Active Files/i }), + ).toBeChecked({ timeout: 15000 }); + await expect(page.locator(".file-sidebar-loading")).toHaveCount(0, { + timeout: 15000, + }); + }); +}); + +test.describe("The view survives a reload", () => { + test.use({ + stubOptions: { + enableLogin: true, + user: { + id: 44, + username: "owner", + email: "o@e.com", + role: "ROLE_USER", + portalAccess: true, + }, + }, + seedJwt: true, + }); + + const currentView = (page: import("@playwright/test").Page) => + page.evaluate(() => { + const r = Array.from( + document.querySelectorAll("input[type=radio]"), + ).find((x) => x.checked); + return r?.value ?? "none"; + }); + + test("comes back on the same view the user left", async ({ page }) => { + test.skip(!(await restoreEnabled(page)), NO_RESTORE); + + await uploadFiles(page, SAMPLES.slice(0, 3)); + await expect(page.locator(".file-sidebar-file-item")).toHaveCount(3, { + timeout: 15000, + }); + + // Open a document, so the view under test is the viewer rather than the grid. + await page + .getByRole("button", { name: /Open in Viewer/i }) + .first() + .click({ force: true }); + await expect + .poll(() => currentView(page), { timeout: 10000 }) + .toBe("viewer"); + + // Whatever the workbench settled on is what a reload must reproduce. + const before = await currentView(page); + await page.waitForTimeout(600); + + await page.reload({ waitUntil: "domcontentloaded" }); + await expect(page.locator(".file-sidebar-file-item")).toHaveCount(3, { + timeout: 20000, + }); + await page.waitForTimeout(3000); + expect(await currentView(page)).toBe(before); + }); + + // The conjunction neither neighbour covers: the spec above proves the VIEW comes back, + // engine-capabilities proves stored bytes decode, and nothing proved that the file the + // restore reopened is one whose pixels actually arrive. + test("a file the restore reopened renders its pages", async ({ page }) => { + test.setTimeout(120_000); + test.skip(!(await restoreEnabled(page)), NO_RESTORE); + + await uploadFiles(page, SAMPLES.slice(0, 3)); + await expect(page.locator(".file-sidebar-file-item")).toHaveCount(3, { + timeout: 15_000, + }); + + await page + .getByRole("button", { name: /Open in Viewer/i }) + .first() + .click({ force: true }); + await expect + .poll(() => currentView(page), { timeout: 10_000 }) + .toBe("viewer"); + // Let the record settle: the writer debounces, so a reload can outrun it. + await page.waitForTimeout(600); + + await page.reload({ waitUntil: "domcontentloaded" }); + await expect(page.locator(".file-sidebar-file-item")).toHaveCount(3, { + timeout: 30_000, + }); + + // A tile that decoded has non-zero naturalWidth. The restore resolves each recorded id + // to its current leaf, so an empty tile here means it reopened something unreadable. + const tile = page + .locator('[data-page-index="0"]') + .first() + .locator('img[src^="blob:"]') + .first(); + await expect(tile).toBeAttached({ timeout: 30_000 }); + await expect + .poll(() => tile.evaluate((img: HTMLImageElement) => img.naturalWidth), { + timeout: 30_000, + }) + .toBeGreaterThan(0); + }); +}); diff --git a/frontend/editor/src/desktop/components/session/WorkbenchSessionPersistence.tsx b/frontend/editor/src/desktop/components/session/WorkbenchSessionPersistence.tsx new file mode 100644 index 0000000000..2957ac3735 --- /dev/null +++ b/frontend/editor/src/desktop/components/session/WorkbenchSessionPersistence.tsx @@ -0,0 +1,4 @@ +// Stub: desktop opens OS-launched files on boot; a session restore would collide with that. +export function WorkbenchSessionPersistence() { + return null; +} diff --git a/frontend/editor/src/desktop/extensions/accountLogout.ts b/frontend/editor/src/desktop/extensions/accountLogout.ts index b97b06ec6e..ca75c9c5e9 100644 --- a/frontend/editor/src/desktop/extensions/accountLogout.ts +++ b/frontend/editor/src/desktop/extensions/accountLogout.ts @@ -1,4 +1,5 @@ import { connectionModeService } from "@app/services/connectionModeService"; +import { suspendWorkbenchSession } from "@app/services/workbenchSession"; type SignOutFn = () => Promise; @@ -16,6 +17,10 @@ export function useAccountLogout() { redirectToLogin, }: AccountLogoutDeps): Promise => { try { + // The tab outlives the session; the next person to sign in here must not + // inherit this workbench. Suspends writing too - signing out unmounts the + // editor, and its flush would otherwise write the record straight back. + suspendWorkbenchSession(); await signOut(); const currentConfig = await connectionModeService.getCurrentConfig(); diff --git a/frontend/editor/src/portal/components/Sidebar.tsx b/frontend/editor/src/portal/components/Sidebar.tsx index 8ce7008d67..685fa32722 100644 --- a/frontend/editor/src/portal/components/Sidebar.tsx +++ b/frontend/editor/src/portal/components/Sidebar.tsx @@ -14,6 +14,7 @@ import { useUI } from "@portal/contexts/UIContext"; import { LinkAccountFooterItem } from "@portal/components/LinkAccountFooterItem"; import { EDITOR_URL, EDITOR_IS_SAME_APP } from "@portal/auth/editorUrl"; import { EDITOR_BASENAME } from "@app/routes/editorBasename"; +import { takeEditorReturnPath } from "@app/services/workbenchSession"; import { CloseIcon } from "@portal/components/icons"; import { GROUP_PROCESSOR, @@ -57,7 +58,7 @@ export function Sidebar() { // the switch stays client-side; an absolute EDITOR_URL (dev cross-app setup) // needs a full page load. const goToEditor = () => { - if (EDITOR_IS_SAME_APP) navigate(EDITOR_BASENAME); + if (EDITOR_IS_SAME_APP) navigate(takeEditorReturnPath() ?? EDITOR_BASENAME); else window.location.href = EDITOR_URL; }; diff --git a/frontend/editor/src/proprietary/auth/spring/UseSession.tsx b/frontend/editor/src/proprietary/auth/spring/UseSession.tsx index 289f2ec895..27a722b2d7 100644 --- a/frontend/editor/src/proprietary/auth/spring/UseSession.tsx +++ b/frontend/editor/src/proprietary/auth/spring/UseSession.tsx @@ -12,6 +12,7 @@ import { type AuthUser, type AuthTranslate, } from "@app/auth/types"; +import { suspendWorkbenchSession } from "@app/services/workbenchSession"; /** * Strip the configured base path so route comparisons work under subpath @@ -97,6 +98,11 @@ export function SpringAuthProvider({ const signOut = useCallback(async () => { try { setError(null); + // Signing out is deliberate, unlike an identity check that merely failed: drop the + // workbench record here and stop recording, so the teardown that follows cannot + // write it back for whoever signs in next. + suspendWorkbenchSession(); + const { error } = await springAuth.signOut(); // Always clear the in-memory session: springAuth.signOut() removes the diff --git a/frontend/editor/src/proprietary/components/policies/usePolicyAutoRun.reentry.test.tsx b/frontend/editor/src/proprietary/components/policies/usePolicyAutoRun.reentry.test.tsx new file mode 100644 index 0000000000..dc8ff6397d --- /dev/null +++ b/frontend/editor/src/proprietary/components/policies/usePolicyAutoRun.reentry.test.tsx @@ -0,0 +1,119 @@ +import { describe, it, expect, vi, beforeEach } from "vitest"; +import { renderHook, waitFor } from "@testing-library/react"; + +// A file can re-enter the workbench without being a new upload (My Files reopen, session restore). +// The persisted dispatch record must stop the upload policy (and its billing) firing a second time. + +const mocks = vi.hoisted(() => ({ + workspace: [] as Array<{ id: string; derivedFromTool?: boolean }>, + runStoredPolicy: vi.fn(), + getPolicyRun: vi.fn(), + listPolicyRuns: vi.fn(), + getStirlingFile: vi.fn(), +})); + +vi.mock("@app/hooks/useAiEngineEnabled", () => ({ + useAiEngineEnabled: () => true, +})); +vi.mock("@app/contexts/FileContext", () => ({ + useAllFiles: () => ({ fileStubs: mocks.workspace }), + useFileManagement: () => ({ + addFiles: vi.fn(), + updateStirlingFileStub: vi.fn(), + }), + useFileContext: () => ({ consumeFiles: vi.fn() }), +})); +vi.mock("@app/contexts/IndexedDBContext", () => ({ + useIndexedDB: () => ({ bumpRevision: vi.fn() }), +})); +vi.mock("@app/hooks/usePolicies", () => ({ + usePolicies: () => ({ + policies: { + security: { + configured: true, + status: "active", + backendId: "backend-security", + runOn: "upload", + order: 0, + outputMode: "new_version", + outputName: "", + }, + }, + }), +})); +vi.mock("@app/services/policyApi", () => ({ + runStoredPolicy: mocks.runStoredPolicy, + getPolicyRun: mocks.getPolicyRun, + listPolicyRuns: mocks.listPolicyRuns, + downloadPolicyOutput: vi.fn(), + resolvePolicyRunTarget: () => "saas", +})); +vi.mock("@app/services/fileStorage", () => ({ + fileStorage: { + getStirlingFile: mocks.getStirlingFile, + getStirlingFileStub: vi.fn().mockResolvedValue(null), + persistVersionedOutputs: vi.fn(), + updateFileMetadata: vi.fn().mockResolvedValue(true), + }, +})); + +import { usePolicyAutoRun } from "@app/components/policies/usePolicyAutoRun"; +import { + markDispatched, + resetPolicyRuns, +} from "@app/components/policies/policyRunStore"; + +beforeEach(() => { + localStorage.clear(); + resetPolicyRuns(); + vi.clearAllMocks(); + mocks.listPolicyRuns.mockResolvedValue([]); + mocks.getStirlingFile.mockResolvedValue( + new File(["x"], "doc.pdf", { type: "application/pdf" }), + ); + mocks.runStoredPolicy.mockResolvedValue("run-0"); + // Completed with no outputs: the run settles without the import machinery. + mocks.getPolicyRun.mockResolvedValue({ + runId: "run-0", + policyId: null, + status: "COMPLETED", + currentStep: 1, + stepCount: 1, + error: null, + outputs: [], + }); +}); + +describe("upload policies and files re-entering the workbench", () => { + it("does not re-run on a file the policy already ran on", async () => { + markDispatched("security", "already-enforced"); + mocks.workspace = [{ id: "already-enforced" }, { id: "fresh-upload" }]; + + renderHook(() => usePolicyAutoRun()); + + await waitFor(() => expect(mocks.runStoredPolicy).toHaveBeenCalledTimes(1)); + expect(mocks.getStirlingFile).toHaveBeenCalledWith("fresh-upload"); + expect(mocks.getStirlingFile).not.toHaveBeenCalledWith("already-enforced"); + }); + + it("stays silent when every file in the workbench has already been enforced", async () => { + markDispatched("security", "one"); + markDispatched("security", "two"); + mocks.workspace = [{ id: "one" }, { id: "two" }]; + + renderHook(() => usePolicyAutoRun()); + + // Give the dispatch effect a tick to (wrongly) fire before asserting silence. + await new Promise((resolve) => setTimeout(resolve, 50)); + expect(mocks.runStoredPolicy).not.toHaveBeenCalled(); + }); + + it("still skips a policy's own output, which is not an upload at all", async () => { + mocks.workspace = [{ id: "policy-output", derivedFromTool: true }]; + + renderHook(() => usePolicyAutoRun()); + + await new Promise((resolve) => setTimeout(resolve, 50)); + expect(mocks.runStoredPolicy).not.toHaveBeenCalled(); + }); +}); diff --git a/frontend/editor/src/proprietary/constants/featureFlags.ts b/frontend/editor/src/proprietary/constants/featureFlags.ts index e432b34599..84af388005 100644 --- a/frontend/editor/src/proprietary/constants/featureFlags.ts +++ b/frontend/editor/src/proprietary/constants/featureFlags.ts @@ -13,3 +13,6 @@ * Watched Folders implementation to navigate to). */ export const WATCHED_FOLDERS_ENABLED: boolean = false; + +// Refill an empty workbench from the tab's last session (survives the editor/processor switch). +export const WORKBENCH_SESSION_RESTORE: boolean = true; diff --git a/frontend/editor/src/proprietary/extensions/accountLogout.ts b/frontend/editor/src/proprietary/extensions/accountLogout.ts index 6ae83b8e26..8c4d9f15d7 100644 --- a/frontend/editor/src/proprietary/extensions/accountLogout.ts +++ b/frontend/editor/src/proprietary/extensions/accountLogout.ts @@ -1,3 +1,5 @@ +import { suspendWorkbenchSession } from "@app/services/workbenchSession"; + type SignOutFn = () => Promise; interface AccountLogoutDeps { @@ -21,6 +23,10 @@ export function useAccountLogout() { "1", ); } + // The tab outlives the session; the next person to sign in here must not + // inherit this workbench. Suspends writing too - signing out unmounts the + // editor, and its flush would otherwise write the record straight back. + suspendWorkbenchSession(); await signOut(); } finally { redirectToLogin(); diff --git a/frontend/editor/src/proprietary/hooks/useOtherAppSwitch.test.tsx b/frontend/editor/src/proprietary/hooks/useOtherAppSwitch.test.tsx new file mode 100644 index 0000000000..c2a81c53d9 --- /dev/null +++ b/frontend/editor/src/proprietary/hooks/useOtherAppSwitch.test.tsx @@ -0,0 +1,55 @@ +import { describe, it, expect, vi, beforeEach } from "vitest"; +import { renderHook } from "@testing-library/react"; + +const mocks = vi.hoisted(() => ({ + navigate: vi.fn(), + requestNavigation: vi.fn(), + portalAccess: true, +})); + +vi.mock("react-router-dom", () => ({ + useNavigate: () => mocks.navigate, + useLocation: () => ({ pathname: "/compress", search: "?mode=fast" }), +})); +vi.mock("@app/auth/context", () => ({ + useAuth: () => ({ portalAccess: mocks.portalAccess }), +})); +vi.mock("@app/contexts/NavigationContext", () => ({ + useNavigationActions: () => ({ + actions: { requestNavigation: mocks.requestNavigation }, + }), +})); + +import { useOtherAppSwitch } from "@app/hooks/useOtherAppSwitch"; +import { takeEditorReturnPath } from "@app/services/workbenchSession"; + +beforeEach(() => { + sessionStorage.clear(); + vi.clearAllMocks(); + mocks.portalAccess = true; +}); + +describe("useOtherAppSwitch", () => { + it("offers no switch without portal access", () => { + mocks.portalAccess = false; + const { result } = renderHook(() => useOtherAppSwitch()); + expect(result.current).toBeNull(); + }); + + it("routes the switch through the unsaved-changes guard", () => { + const { result } = renderHook(() => useOtherAppSwitch()); + result.current?.onOpen(); + + expect(mocks.navigate).not.toHaveBeenCalled(); + expect(mocks.requestNavigation).toHaveBeenCalledTimes(1); + }); + + it("records where to return to, then navigates to the processor", () => { + const { result } = renderHook(() => useOtherAppSwitch()); + result.current?.onOpen(); + mocks.requestNavigation.mock.calls[0][0](); + + expect(takeEditorReturnPath()).toBe("/compress?mode=fast"); + expect(mocks.navigate).toHaveBeenCalledWith("/processor"); + }); +}); diff --git a/frontend/editor/src/proprietary/hooks/useOtherAppSwitch.ts b/frontend/editor/src/proprietary/hooks/useOtherAppSwitch.ts index 8bf07b5c2f..dff36a06f4 100644 --- a/frontend/editor/src/proprietary/hooks/useOtherAppSwitch.ts +++ b/frontend/editor/src/proprietary/hooks/useOtherAppSwitch.ts @@ -1,6 +1,8 @@ -import { useNavigate } from "react-router-dom"; +import { useLocation, useNavigate } from "react-router-dom"; import { useAuth } from "@app/auth/context"; +import { useNavigationActions } from "@app/contexts/NavigationContext"; import { PORTAL_BASENAME } from "@app/routes/portalBasename"; +import { saveEditorReturnPath } from "@app/services/workbenchSession"; import { type NavFooterAppLink } from "@app/components/shared/navFooter/NavFooter"; /** @@ -10,6 +12,16 @@ import { type NavFooterAppLink } from "@app/components/shared/navFooter/NavFoote export function useOtherAppSwitch(): NavFooterAppLink | null { const { portalAccess } = useAuth(); const navigate = useNavigate(); + const location = useLocation(); + const { actions } = useNavigationActions(); if (!portalAccess) return null; - return { app: "processor", onOpen: () => navigate(PORTAL_BASENAME) }; + return { + app: "processor", + onOpen: () => + // Through the guard, so unsaved edits get the same warning as any other navigation. + actions.requestNavigation(() => { + saveEditorReturnPath(location.pathname + location.search); + navigate(PORTAL_BASENAME); + }), + }; } diff --git a/frontend/editor/src/saas/auth/AuthProvider.test.tsx b/frontend/editor/src/saas/auth/AuthProvider.test.tsx index ffeca5433d..bfd3bef352 100644 --- a/frontend/editor/src/saas/auth/AuthProvider.test.tsx +++ b/frontend/editor/src/saas/auth/AuthProvider.test.tsx @@ -1,6 +1,7 @@ import { act, render, waitFor } from "@testing-library/react"; import { beforeEach, describe, expect, it, vi } from "vitest"; import type { Session, User } from "@supabase/supabase-js"; +import { expectConsole } from "@app/tests/failOnConsole"; /** * Request-count tests for {@link AuthProvider}'s data loading. It used to fetch @@ -16,6 +17,7 @@ const createSignedUrl = vi.fn(); const storageFrom = vi.fn((_bucket: string) => ({ createSignedUrl })); const getSession = vi.fn(); const onAuthStateChange = vi.fn(); +const supabaseSignOut = vi.fn(); const unsubscribe = vi.fn(); vi.mock("@app/auth/supabase", () => ({ @@ -26,7 +28,7 @@ vi.mock("@app/auth/supabase", () => ({ refreshSession: vi .fn() .mockResolvedValue({ data: { session: null }, error: null }), - signOut: vi.fn().mockResolvedValue({ error: null }), + signOut: () => supabaseSignOut(), }, rpc: (...args: unknown[]) => rpc(...args), storage: { from: (bucket: string) => storageFrom(bucket) }, @@ -53,6 +55,8 @@ vi.mock("@app/services/userService", () => ({ // Imported after the mocks so the provider picks them up. const { AuthProvider, useAuth } = await import("@app/auth/UseSession"); +const { writeWorkbenchSession, readWorkbenchSession, resumeWorkbenchSession } = + await import("@app/services/workbenchSession"); /** Surfaces `loading` so a test can assert on it rather than on the container. */ function LoadingProbe() { @@ -337,3 +341,55 @@ describe("AuthProvider user-data loading", () => { expect(rpc).toHaveBeenCalledTimes(1); }); }); + +/** + * Signing out suspends workbench recording before the request, so a teardown cannot write the + * record back for whoever signs in next. When the request fails the session stands, and a + * still-signed-in user must not be left silently not recording. + */ +describe("a sign-out that fails", () => { + beforeEach(() => { + vi.clearAllMocks(); + sessionStorage.clear(); + resumeWorkbenchSession(); + getSession.mockResolvedValue({ + data: { session: makeSession() }, + error: null, + }); + onAuthStateChange.mockImplementation(() => ({ + data: { subscription: { unsubscribe } }, + })); + rpc.mockResolvedValue({ data: null, error: null }); + getProfilePictureMetadata.mockResolvedValue(null); + syncOAuthAvatar.mockResolvedValue(undefined); + synchronizeUserUpgrade.mockResolvedValue(undefined); + }); + + it("leaves the workbench still being recorded", async () => { + expectConsole.error(/\[Auth Debug\] Sign out error/); + supabaseSignOut.mockResolvedValue({ error: new Error("network down") }); + + let signOut: (() => Promise) | null = null; + function SignOutProbe() { + signOut = useAuth().signOut; + return null; + } + render( + + + , + ); + await waitFor(() => expect(signOut).not.toBeNull()); + + await act(async () => { + await signOut!(); + }); + + writeWorkbenchSession({ + fileIds: ["still-here"], + selectedFileIds: [], + userId: "user-a", + }); + expect(readWorkbenchSession()?.fileIds).toEqual(["still-here"]); + }); +}); diff --git a/frontend/editor/src/saas/auth/UseSession.tsx b/frontend/editor/src/saas/auth/UseSession.tsx index a1c4db2396..4bd3b99346 100644 --- a/frontend/editor/src/saas/auth/UseSession.tsx +++ b/frontend/editor/src/saas/auth/UseSession.tsx @@ -22,6 +22,10 @@ import { getProviderAvatarUrl, type ProfilePictureMetadata, } from "@app/services/avatarSyncService"; +import { + resumeWorkbenchSession, + suspendWorkbenchSession, +} from "@app/services/workbenchSession"; // Extend Supabase User to include optional username for compatibility export type User = SupabaseUser & { username?: string }; @@ -355,11 +359,19 @@ export function AuthProvider({ children }: { children: ReactNode }) { const signOut = async () => { try { setError(null); + // Signing out is deliberate, unlike an identity check that merely failed: drop the + // workbench record here and stop recording, so the teardown that follows cannot + // write it back for whoever signs in next. + suspendWorkbenchSession(); + const { error } = await supabase.auth.signOut(); if (error) { console.error("[Auth Debug] Sign out error:", error); setError(error); + // The sign-out did not happen and the session stands, so keep recording: + // otherwise a still-signed-in user silently stops persisting their workbench. + resumeWorkbenchSession(); } else { console.debug("[Auth Debug] Signed out successfully"); setSession(null); @@ -367,6 +379,7 @@ export function AuthProvider({ children }: { children: ReactNode }) { } catch (err) { console.error("[Auth Debug] Unexpected error during sign out:", err); setError(err as AuthError); + resumeWorkbenchSession(); } }; diff --git a/frontend/editor/src/saas/hooks/useOtherAppSwitch.ts b/frontend/editor/src/saas/hooks/useOtherAppSwitch.ts index 824e612bd8..67d0aa7d49 100644 --- a/frontend/editor/src/saas/hooks/useOtherAppSwitch.ts +++ b/frontend/editor/src/saas/hooks/useOtherAppSwitch.ts @@ -1,6 +1,8 @@ -import { useNavigate } from "react-router-dom"; +import { useLocation, useNavigate } from "react-router-dom"; import { usePortalAccess } from "@app/hooks/usePortalAccess"; +import { useNavigationActions } from "@app/contexts/NavigationContext"; import { PORTAL_BASENAME } from "@app/routes/portalBasename"; +import { saveEditorReturnPath } from "@app/services/workbenchSession"; import { type NavFooterAppLink } from "@app/components/shared/navFooter/NavFooter"; /** @@ -11,6 +13,16 @@ import { type NavFooterAppLink } from "@app/components/shared/navFooter/NavFoote export function useOtherAppSwitch(): NavFooterAppLink | null { const portalAccess = usePortalAccess(); const navigate = useNavigate(); + const location = useLocation(); + const { actions } = useNavigationActions(); if (!portalAccess) return null; - return { app: "processor", onOpen: () => navigate(PORTAL_BASENAME) }; + return { + app: "processor", + onOpen: () => + // Through the guard, so unsaved edits get the same warning as any other navigation. + actions.requestNavigation(() => { + saveEditorReturnPath(location.pathname + location.search); + navigate(PORTAL_BASENAME); + }), + }; }