From 74495c25ab66828ed1102a94d102e0e1cf6e0b0e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andr=C3=A9=20Padez?= Date: Sat, 15 Aug 2026 22:58:00 +0000 Subject: [PATCH] a file browser panel reopens where you left it MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Navigate into a subfolder of a dashboard's directory, refresh, and you were back at the top. The /files screen keeps its folder in the URL and never had this; a panel had nowhere to put it. ── Made shared, because chat is next ── The editor's tab memory from the last commit was one panel's version of a pattern the terminal had already invented. Rather than write a third, `usePanelMemory` now holds the mechanics — claim once, key per panel, drop on real close — and each panel supplies only a payload. The editor moved onto it and lost 50 lines; the file browser stores `{ path }`; chat will store whatever chat needs. Restoring is SYNCHRONOUS, in the initial state rather than an effect, and can be: `WorkspaceView` renders nothing until dashboard state has loaded, so the remembered folder is already there on the first render. An effect would list the base folder first and visibly jump to the right one. ── Two ways it could strand you, both closed ── A remembered folder OUTSIDE the panel's base is discarded on read. A dashboard scoped to ~/Tests must not reopen at ~/other because it was rescoped or the folder moved. The existing guard catches that after the fact; this stops it being requested. A remembered folder that no longer EXISTS was the new failure this feature introduced — the listing throws, a toast appears, and every reload lands in the same dead folder, because that is what was remembered. The first listing now falls back to the base and rewrites the memory. Deliberately first-listing-only: later in a session a failure is far more likely transient, and bouncing someone out of a deep folder over one bad response is worse than the error they can already read. `files` registered in PANEL_STATE_PREFIXES, without which the PATCH is a 400. tsgo clean, frontend builds, 846 pass / 7 fail unchanged. Co-Authored-By: Claude Opus 5 --- src/servers/api/dashboards/dashboards.ts | 2 +- .../src/apps/CodeEditor/use-editor-tabs.ts | 81 +++---------------- .../FileBrowserApp/FileBrowserApp.tsx | 9 ++- .../FileBrowserPanelWrapper.tsx | 11 ++- .../FileBrowserApp/useFileBrowserApp.ts | 55 ++++++++++++- .../src/components/Workspace/index.ts | 1 + .../components/Workspace/use-panel-memory.ts | 79 ++++++++++++++++++ 7 files changed, 158 insertions(+), 80 deletions(-) create mode 100644 src/workspaces/officerdev/src/components/Workspace/use-panel-memory.ts diff --git a/src/servers/api/dashboards/dashboards.ts b/src/servers/api/dashboards/dashboards.ts index 67e7e5c0..ae563abf 100644 --- a/src/servers/api/dashboards/dashboards.ts +++ b/src/servers/api/dashboards/dashboards.ts @@ -20,7 +20,7 @@ export const dashboardsRouter = createRouter(); // `editor` joined 2026-08-15: which files a code-editor panel has open, and which is in front. Note an // unmatched key is a 400 rather than a silent 200 — so a new panel prefix MUST be added here or its state // vanishes loudly instead of quietly, which is the whole reason that branch exists. -const PANEL_STATE_PREFIXES = ['tmux', 'nvim', 'claude-code', 'editor']; +const PANEL_STATE_PREFIXES = ['tmux', 'nvim', 'claude-code', 'editor', 'files']; // A `LayoutNode` is an object. Everything from the PATCH body down to jsonb is `unknown`, so without this // the only thing standing between a wrong-shaped body and the layout columns is the CHECK constraint — diff --git a/src/workspaces/officerdev/src/apps/CodeEditor/use-editor-tabs.ts b/src/workspaces/officerdev/src/apps/CodeEditor/use-editor-tabs.ts index 30f569a0..e0ba7659 100644 --- a/src/workspaces/officerdev/src/apps/CodeEditor/use-editor-tabs.ts +++ b/src/workspaces/officerdev/src/apps/CodeEditor/use-editor-tabs.ts @@ -1,82 +1,21 @@ -import { useRef } from 'react'; -import { useDashboardState } from 'state/useDashboardState'; -import { usePanelClose, type WorkspaceIdentity } from '../../components/Workspace'; +import { usePanelMemory, type WorkspaceIdentity } from '../../components/Workspace'; /** * Which files a code-editor panel has open, remembered across a reload. * - * A panel's tabs lived in component state, so a refresh emptied it — you came back to "Open a file from - * the explorer to start editing" with no sign of what you had been doing. The terminal solved the same - * problem earlier by keeping a `panelId -> sessionId` map in dashboard state; this is that map with a - * different payload, and it is deliberately the same shape so there is one pattern to learn rather than - * two to compare. + * The mechanics — claim once, key per panel, drop on close — live in `usePanelMemory`, which the file + * browser and chat use too. What is specific to the editor is only the payload, and one decision: * - * ── What is stored, and what is not ── + * PATHS, not contents. The disk is the truth. Restoring a remembered body would show a version that may + * be hours stale, or resurrect a file something has since deleted. Each path is read fresh on restore and + * one that no longer reads is dropped rather than opened as an empty buffer named after a missing file. * - * PATHS, not contents. The file on disk is the truth: restoring a remembered body would mean showing a - * version that may be hours stale, or resurrecting a file something has since deleted. On restore each - * path is read fresh, and one that no longer reads is dropped rather than opened empty. - * - * The cost is that UNSAVED edits still do not survive a reload — they did not before either, since the - * whole tab went with them. Keeping them means persisting buffers, which is a different feature with its - * own hard question (what happens when the disk AND the buffer have both changed?) and is not smuggled - * in here. + * The cost, stated rather than hidden: unsaved edits still do not survive a reload. They did not before + * either — the whole tab went with them. Keeping them means persisting buffers, which has its own hard + * question (the disk AND the buffer have both changed — now what?) and is not smuggled in here. */ export type EditorPanelState = { open: string[]; active: string | null }; -const EMPTY: Record = {}; - -/** `ws-editor-{dashboardId}`, or `-default` for a panel on a screen. Mirrors `terminalStateKey`. */ -export function editorStateKey(workspace: WorkspaceIdentity | null): string { - const id = workspace?.kind === 'dashboard' ? workspace.id : 'default'; - return `ws-editor-${id}`; -} - export function useEditorTabs(panelId: string | undefined, workspace: WorkspaceIdentity | null) { - const { value: bag, setValue: setBag } = useDashboardState>( - editorStateKey(workspace), - EMPTY, - ); - - const saved = panelId ? bag[panelId] : undefined; - - /** - * The remembered set is claimed ONCE, on the first render that has it. - * - * After that this panel is the authority. Re-reading would fight the user: every tab they open writes - * back into this same bag, so a later read would arrive as an "update" telling them to reopen what they - * had just closed. - */ - const restored = useRef(null); - const claimed = useRef(false); - if (!claimed.current && saved) { - claimed.current = true; - restored.current = saved; - } - - const remember = (state: EditorPanelState) => { - if (!panelId) return; - setBag((prev) => { - const before = prev[panelId]; - // Cheap equality on a small ordered payload. Without it, every render caused by a keystroke would - // PATCH the identical value back to the server. - if (before && before.active === state.active && before.open.join('\n') === state.open.join('\n')) { - return prev; - } - return { ...prev, [panelId]: state }; - }); - }; - - // A panel closed for good should not leave its tab list behind. `usePanelClose` fires on a real close - // and on nothing else — not a drag, not a swap, not a mobile panel switch. - usePanelClose(panelId ?? '', () => { - if (!panelId) return; - setBag((prev) => { - if (!(panelId in prev)) return prev; - const { [panelId]: _drop, ...rest } = prev; - return rest; - }); - }); - - return { restored: restored.current, remember }; + return usePanelMemory('editor', panelId, workspace); } diff --git a/src/workspaces/officerdev/src/apps/FileBrowser/FileBrowserApp/FileBrowserApp.tsx b/src/workspaces/officerdev/src/apps/FileBrowser/FileBrowserApp/FileBrowserApp.tsx index db921dec..645e9e04 100644 --- a/src/workspaces/officerdev/src/apps/FileBrowser/FileBrowserApp/FileBrowserApp.tsx +++ b/src/workspaces/officerdev/src/apps/FileBrowser/FileBrowserApp/FileBrowserApp.tsx @@ -13,10 +13,15 @@ type FileBrowserAppProps = { rootOverride?: string; /** Put the current folder in `?path=`. Opt-in — only the /files screen is guaranteed one browser. */ urlPath?: boolean; + /** + * Identifies this panel so the folder it is showing survives a reload. Absent on the /files screen, + * where the folder is the URL and already does. + */ + panelId?: string; }; -export const FileBrowserApp = ({ basePath = '/', rootOverride, urlPath }: FileBrowserAppProps) => { - const fileBrowserManager = useFileBrowserApp(basePath, rootOverride, urlPath); +export const FileBrowserApp = ({ basePath = '/', rootOverride, urlPath, panelId }: FileBrowserAppProps) => { + const fileBrowserManager = useFileBrowserApp(basePath, rootOverride, urlPath, panelId); const { handleNavigate } = fileBrowserManager; return ( diff --git a/src/workspaces/officerdev/src/apps/FileBrowser/FileBrowserApp/FileBrowserPanelWrapper.tsx b/src/workspaces/officerdev/src/apps/FileBrowser/FileBrowserApp/FileBrowserPanelWrapper.tsx index e6e19113..3206f583 100644 --- a/src/workspaces/officerdev/src/apps/FileBrowser/FileBrowserApp/FileBrowserPanelWrapper.tsx +++ b/src/workspaces/officerdev/src/apps/FileBrowser/FileBrowserApp/FileBrowserPanelWrapper.tsx @@ -1,7 +1,7 @@ import { useWorkspace, cwdToPath } from '../../../components/Workspace'; import { FileBrowserApp } from './FileBrowserApp'; -export const FileBrowserPanelWrapper = () => { +export const FileBrowserPanelWrapper = ({ panelId }: { panelId: string }) => { const { cwd, workspace } = useWorkspace(); const basePath = cwdToPath(cwd); @@ -11,5 +11,12 @@ export const FileBrowserPanelWrapper = () => { // (cwd `~`) sits on dashboards too, so `basePath === '/'` would catch the wrong browsers. const urlPath = workspace?.kind === 'screen' && workspace.id === 'files'; - return ; + return ( + + ); }; diff --git a/src/workspaces/officerdev/src/apps/FileBrowser/FileBrowserApp/useFileBrowserApp.ts b/src/workspaces/officerdev/src/apps/FileBrowser/FileBrowserApp/useFileBrowserApp.ts index 0be2b610..bf700605 100644 --- a/src/workspaces/officerdev/src/apps/FileBrowser/FileBrowserApp/useFileBrowserApp.ts +++ b/src/workspaces/officerdev/src/apps/FileBrowser/FileBrowserApp/useFileBrowserApp.ts @@ -5,6 +5,7 @@ import { useFilesAPI, type DirEntry } from '../../../hooks/useFilesAPI'; import { usePinnedFiles } from '../usePinnedFiles'; import { EDITOR_FILE_PARAM } from '../../CodeEditor/useEditorState'; import { getFileType } from '../../FileViewer'; +import { usePanelMemory, useWorkspace } from '../../../components/Workspace'; import { folderFromPathname, folderHref, relativeToFolder, withoutViewerParams } from './files-route'; import { useTasks, type TaskSummary } from '../useTasks'; import { useAgents, type AgentSummary } from '../useAgents'; @@ -20,7 +21,7 @@ import { copyToClipboard, canReadClipboard } from 'helpers/clipboard'; */ export const FILES_PATH_PARAM = 'path'; -export const useFileBrowserApp = (basePath: string, rootOverride?: string, urlPath = false) => { +export const useFileBrowserApp = (basePath: string, rootOverride?: string, urlPath = false, panelId?: string) => { const { user } = useAuth(); const navigate = useNavigate(); const location = useLocation(); @@ -28,7 +29,31 @@ export const useFileBrowserApp = (basePath: string, rootOverride?: string, urlPa const [searchParams, setSearchParams] = useSearchParams(); const homeRoot = 'home'; const [, setGlobalPath] = useUserState('files/currentPath', '/'); - const [localPath, setLocalPath] = useState(basePath); + /** + * A panel's folder, remembered across a reload. + * + * The /files screen keeps this in the URL and needs none of it. A PANEL had nowhere to put it, so a + * refresh always dropped you back at the dashboard's base folder however deep you had navigated. + * + * Restored synchronously in the initial state rather than in an effect: `WorkspaceView` renders nothing + * until dashboard state has loaded, so the remembered folder is already here on the first render. An + * effect would list the base folder first and visibly jump. + * + * A remembered folder OUTSIDE the panel's base is discarded. A dashboard scoped to ~/Tests must not + * reopen at ~/other because it was rescoped, or because the folder moved — the existing guard below + * catches it after the fact, this stops it being requested at all. + */ + const { workspace } = useWorkspace(); + const { restored: rememberedFolder, remember: rememberFolder } = usePanelMemory<{ path: string }>( + 'files', + panelId, + workspace, + ); + const [localPath, setLocalPath] = useState(() => { + const saved = rememberedFolder?.path; + if (!saved || (basePath !== '/' && !saved.startsWith(basePath))) return basePath; + return saved; + }); const scoped = basePath !== '/'; // Navigation is session-local for a panel — it always opens at its base and never restores the last // path. On /files it is the URL instead, so back/forward work and a folder can be linked to; a fresh @@ -42,7 +67,13 @@ export const useFileBrowserApp = (basePath: string, rootOverride?: string, urlPa // another. Only the OVERLAY params go; anything else in the query (global UI state) is left alone, // which is the distinction a wholesale reset kept getting wrong. if (urlPath) navigate({ pathname: folderHref(path), search: withoutViewerParams(location.search) }); - else setLocalPath(path); + else { + setLocalPath(path); + // Written here rather than in an effect on `currentPath`, so it records a NAVIGATION and not the + // initial render. An effect would fire once on mount with the restored value and persist it back, + // which is harmless but writes on every panel that never moves. + rememberFolder({ path }); + } if (!scoped) setGlobalPath(path); }; @@ -133,6 +164,8 @@ export const useFileBrowserApp = (basePath: string, rootOverride?: string, urlPa const entryPath = (name: string) => (currentPath === '/' ? `/${name}` : `${currentPath}/${name}`); const selectedPaths = () => Array.from(selected).map(entryPath); + const firstListing = useRef(true); + const refresh = async () => { setLoading(true); try { @@ -145,8 +178,22 @@ export const useFileBrowserApp = (basePath: string, rootOverride?: string, urlPa if (data.rootDir) setRootDir(data.rootDir); } catch (err: any) { console.error('[FileBrowser] refresh error:', err); - toast.error(err?.message || 'Failed to load directory'); + // A RESTORED folder that no longer exists would otherwise strand the panel: the listing fails, the + // toast appears, and every reload lands in the same dead folder because that is what was remembered. + // Falling back to the base is the only exit that does not need the user to know what happened. + // + // Narrow on purpose — first listing only. Later in the session a failure is far more likely to be + // transient, and bouncing someone out of a deep folder over one bad response would be worse than + // the error they can already see. + if (firstListing.current && currentPathRef.current !== basePath) { + toast.error('That folder is gone — back to the start'); + setCurrentPath(basePath); + rememberFolder({ path: basePath }); + } else { + toast.error(err?.message || 'Failed to load directory'); + } } finally { + firstListing.current = false; setLoading(false); } }; diff --git a/src/workspaces/officerdev/src/components/Workspace/index.ts b/src/workspaces/officerdev/src/components/Workspace/index.ts index 00286aae..28342807 100644 --- a/src/workspaces/officerdev/src/components/Workspace/index.ts +++ b/src/workspaces/officerdev/src/components/Workspace/index.ts @@ -37,5 +37,6 @@ export { cwdToPath } from './cwd-path'; export { usePanelFullscreen } from './panel-fullscreen'; export { usePanelConfig } from './usePanelConfig'; export { usePanelClose } from './usePanelClose'; +export { usePanelMemory, panelStateKey } from './use-panel-memory'; export { WorkspaceView } from './WorkspaceView'; export { WorkspaceLayout } from './WorkspaceLayout'; diff --git a/src/workspaces/officerdev/src/components/Workspace/use-panel-memory.ts b/src/workspaces/officerdev/src/components/Workspace/use-panel-memory.ts new file mode 100644 index 00000000..aba44d4e --- /dev/null +++ b/src/workspaces/officerdev/src/components/Workspace/use-panel-memory.ts @@ -0,0 +1,79 @@ +import { useRef } from 'react'; +import { useDashboardState } from 'state/useDashboardState'; +import { usePanelClose } from './usePanelClose'; +import type { WorkspaceIdentity } from './workspace-identity'; + +/** + * A panel's own scrap of state, remembered across a reload. + * + * Panels keep what they are showing in component state, so a refresh empties them: the code editor came + * back with no tabs, the file browser back at its base folder. The terminal solved this shape first — a + * `panelId -> sessionId` map in dashboard state under `ws-{prefix}-{dashboardId}` — and this is that idea + * with the payload left open, so the next panel that needs it does not invent a third variant. + * + * ── The two mistakes it exists to prevent ── + * + * **Reading more than once.** Every write from this panel lands in the same bag it reads from, so a second + * read arrives as an instruction: reopen the tab you just closed, go back to the folder you just left. + * The saved value is CLAIMED once, on the first render that has one, and ignored forever after. This panel + * is the authority from then on. + * + * **Writing before reading.** The first render usually has nothing yet — no tabs open, base folder — and + * persisting that would overwrite the very state about to be restored. Callers guard their write on the + * restore having happened; `usePanelMemory` cannot do it for them because only the caller knows when its + * restore is complete. + * + * Restoring is synchronous rather than an effect, and can be: `WorkspaceView` returns null until dashboard + * state has loaded, so by the time a panel renders at all, its remembered value is already here. That is + * what makes a file browser open directly at the right folder instead of visibly jumping to it. + * + * Keyed per PANEL, not per dashboard, so two of the same app side by side keep separate memories. A panel + * closed for good drops its entry — `usePanelClose` fires on a real close and not on a drag, a swap or a + * mobile panel switch. + */ + +const EMPTY: Record = {}; + +/** `ws-{prefix}-{dashboardId}`, or `-default` for a panel on a screen. Mirrors `terminalStateKey`. */ +export function panelStateKey(prefix: string, workspace: WorkspaceIdentity | null): string { + const id = workspace?.kind === 'dashboard' ? workspace.id : 'default'; + return `ws-${prefix}-${id}`; +} + +export function usePanelMemory(prefix: string, panelId: string | undefined, workspace: WorkspaceIdentity | null) { + const { value: bag, setValue: setBag } = useDashboardState>( + panelStateKey(prefix, workspace), + EMPTY as Record, + ); + + const saved = panelId ? bag[panelId] : undefined; + + const restored = useRef(null); + const claimed = useRef(false); + if (!claimed.current && saved !== undefined) { + claimed.current = true; + restored.current = saved; + } + + const remember = (next: T) => { + if (!panelId) return; + setBag((prev) => { + // Structural equality on a deliberately tiny payload — a path, a short list of them. Without it + // every render that recomputes the same value would PATCH it back to the server. Key order is + // stable because these objects are built in one place, by one literal. + if (panelId in prev && JSON.stringify(prev[panelId]) === JSON.stringify(next)) return prev; + return { ...prev, [panelId]: next }; + }); + }; + + usePanelClose(panelId ?? '', () => { + if (!panelId) return; + setBag((prev) => { + if (!(panelId in prev)) return prev; + const { [panelId]: _drop, ...rest } = prev; + return rest as Record; + }); + }); + + return { restored: restored.current, remember }; +}