diff --git a/src/workspaces/officerdev/src/apps/FileBrowser/FileBrowserApp/files-route.test.ts b/src/workspaces/officerdev/src/apps/FileBrowser/FileBrowserApp/files-route.test.ts index 36d24b30..f4fdf22b 100644 --- a/src/workspaces/officerdev/src/apps/FileBrowser/FileBrowserApp/files-route.test.ts +++ b/src/workspaces/officerdev/src/apps/FileBrowser/FileBrowserApp/files-route.test.ts @@ -1,5 +1,12 @@ import { describe, expect, test } from 'bun:test'; -import { decodeFolderPath, encodeFolderPath, folderFromPathname, folderHref } from './files-route'; +import { + decodeFolderPath, + encodeFolderPath, + folderFromPathname, + folderHref, + relativeToFolder, + resolveInFolder, +} from './files-route'; // The folder is in the URL path now, so every character a filesystem allows has to survive a round trip // through a URL. This is the file that says it does. @@ -103,3 +110,43 @@ describe('encode/decode directly', () => { expect(decodeFolderPath('')).toBe('/'); }); }); + +describe('view param, relative to the folder in the path', () => { + test('a direct child collapses to its name', () => { + expect(relativeToFolder('/archive/Tests/docs', '/archive/Tests/docs/agent-coordination.md')).toBe( + 'agent-coordination.md', + ); + expect(relativeToFolder('/', '/notes.md')).toBe('notes.md'); + }); + + test('and resolves back to exactly where it came from', () => { + for (const [folder, abs] of [ + ['/archive/Tests/docs', '/archive/Tests/docs/agent-coordination.md'], + ['/', '/notes.md'], + ['/a b', '/a b/c (2).md'], + ] as const) { + expect(resolveInFolder(folder, relativeToFolder(folder, abs))).toBe(abs); + } + }); + + test('something nested deeper stays absolute rather than becoming a slashed name', () => { + // `a/b.md` would read as a path when it is meant to be a name. Nothing is gained by the ambiguity. + expect(relativeToFolder('/docs', '/docs/sub/b.md')).toBe('/docs/sub/b.md'); + }); + + test('something outside the folder stays absolute — the widget opens files from anywhere', () => { + expect(relativeToFolder('/docs', '/elsewhere/b.md')).toBe('/elsewhere/b.md'); + // And a near-miss must not be treated as inside: /docs2 is not under /docs. + expect(relativeToFolder('/docs', '/docs2/b.md')).toBe('/docs2/b.md'); + }); + + test('an absolute value still resolves as itself, so old links keep working', () => { + expect(resolveInFolder('/docs', '/archive/Tests/docs/agent-coordination.md')).toBe( + '/archive/Tests/docs/agent-coordination.md', + ); + }); + + test('empty in, empty out', () => { + expect(resolveInFolder('/docs', '')).toBe(''); + }); +}); diff --git a/src/workspaces/officerdev/src/apps/FileBrowser/FileBrowserApp/files-route.ts b/src/workspaces/officerdev/src/apps/FileBrowser/FileBrowserApp/files-route.ts index e4f8ee7a..645f3cbe 100644 --- a/src/workspaces/officerdev/src/apps/FileBrowser/FileBrowserApp/files-route.ts +++ b/src/workspaces/officerdev/src/apps/FileBrowser/FileBrowserApp/files-route.ts @@ -59,3 +59,35 @@ export function folderFromPathname(pathname: string): string { if (!pathname.startsWith(`${FILES_ROUTE_BASE}/`)) return '/'; return decodeFolderPath(pathname.slice(FILES_ROUTE_BASE.length + 1)); } + +// ── `?view=` is relative to the folder in the path ── +// +// The pathname already says which folder you are in, so repeating it in the query was noise: +// `/files/archive/Tests/docs?view=%2Farchive%2FTests%2Fdocs%2Fnotes.md` says the same thing twice, and +// the encoded copy is the unreadable half. +// +// Both forms are accepted on the way in. An ABSOLUTE value still resolves as itself, which covers three +// real cases: links made before this change, `?ephemeral=` pointing at a generated file that may live +// elsewhere, and anything genuinely outside the folder being browsed. Relative is what we WRITE; absolute +// is what we still READ. + +/** A `?view=`/`?ephemeral=` value as an absolute path, given the folder it was recorded against. */ +export function resolveInFolder(folder: string, value: string): string { + if (!value) return ''; + if (value.startsWith('/')) return value; + return folder === '/' ? `/${value}` : `${folder}/${value}`; +} + +/** + * The shortest value that still names `absPath` from `folder`. + * + * Only a DIRECT child collapses to a bare name. Something nested deeper stays absolute rather than + * becoming `a/b/c.md`, because a relative value containing a slash reads like a path when it is really a + * name, and there is nothing to gain from the ambiguity. + */ +export function relativeToFolder(folder: string, absPath: string): string { + const prefix = folder === '/' ? '/' : `${folder}/`; + if (!absPath.startsWith(prefix)) return absPath; + const rest = absPath.slice(prefix.length); + return rest && !rest.includes('/') ? rest : absPath; +} diff --git a/src/workspaces/officerdev/src/apps/FileBrowser/FileBrowserApp/useFileBrowserApp.ts b/src/workspaces/officerdev/src/apps/FileBrowser/FileBrowserApp/useFileBrowserApp.ts index 9199b59c..b3470898 100644 --- a/src/workspaces/officerdev/src/apps/FileBrowser/FileBrowserApp/useFileBrowserApp.ts +++ b/src/workspaces/officerdev/src/apps/FileBrowser/FileBrowserApp/useFileBrowserApp.ts @@ -5,7 +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 { folderFromPathname, folderHref } from './files-route'; +import { folderFromPathname, folderHref, relativeToFolder } from './files-route'; import { useTasks, type TaskSummary } from '../useTasks'; import { useAgents, type AgentSummary } from '../useAgents'; import { useUserState } from 'state/useUserState'; @@ -355,8 +355,9 @@ export const useFileBrowserApp = (basePath: string, rootOverride?: string, urlPa void handleExtract(entry); return; } - const filePath = currentPath === '/' ? `/${entry.name}` : `${currentPath}/${entry.name}`; - setViewerParams({ view: filePath }); + // Just the name: the folder is already in the pathname, so `?view=` naming it again was the same + // fact twice, in its least readable form. + setViewerParams({ view: entry.name }); }; const handleCreateDir = async (name: string) => { @@ -578,7 +579,12 @@ export const useFileBrowserApp = (basePath: string, rootOverride?: string, urlPa const { audioPath } = await files.tts(filePath, { saveNextTo: true }); toast.dismiss(toastId); refresh(); - setViewerParams({ view: filePath, ephemeral: audioPath }); + // `ephemeral` goes through the same shortener rather than being assumed local — where the TTS + // artifact lands depends on `saveNextTo`, and it stays absolute when it is not a direct child. + setViewerParams({ + view: relativeToFolder(currentPath, filePath), + ephemeral: relativeToFolder(currentPath, audioPath), + }); } catch { toast.error('Failed to generate speech audio', { id: toastId }); } diff --git a/src/workspaces/officerdev/src/apps/FileBrowser/FileBrowserWidget/useFileBrowserWidget.ts b/src/workspaces/officerdev/src/apps/FileBrowser/FileBrowserWidget/useFileBrowserWidget.ts index d0177c06..2f15c5ff 100644 --- a/src/workspaces/officerdev/src/apps/FileBrowser/FileBrowserWidget/useFileBrowserWidget.ts +++ b/src/workspaces/officerdev/src/apps/FileBrowser/FileBrowserWidget/useFileBrowserWidget.ts @@ -1,4 +1,5 @@ import { useState, useEffect, useRef } from 'react'; +import { folderHref } from '../FileBrowserApp/files-route'; import { useNavigate } from 'react-router'; import { useFilesAPI, type DirEntry } from '../../../hooks/useFilesAPI'; import { useRecentFiles } from '../useRecentFiles'; @@ -50,7 +51,12 @@ export const useFileBrowserWidget = () => { const openFile = (path: string, name: string) => { addRecent(path, name); - navigate(`/files?view=${encodeURIComponent(path)}`); + // Navigate to the file's FOLDER and name it from there. Before the folder moved into the pathname + // this could hand `/files` an absolute `?view=` and let the screen sort it out; now the pathname is + // the folder, so a recent file from elsewhere has to say which one — otherwise it opens the file with + // the browser sitting in home, which is not where the file is. + const folder = path.slice(0, path.lastIndexOf('/')) || '/'; + navigate(`${folderHref(folder)}?view=${encodeURIComponent(name)}`); }; const isSearching = searchQuery.trim().length > 0; diff --git a/src/workspaces/officerdev/src/hooks/useFileViewerPanels/Providers.tsx b/src/workspaces/officerdev/src/hooks/useFileViewerPanels/Providers.tsx index 3b652093..e82a6f91 100644 --- a/src/workspaces/officerdev/src/hooks/useFileViewerPanels/Providers.tsx +++ b/src/workspaces/officerdev/src/hooks/useFileViewerPanels/Providers.tsx @@ -1,18 +1,26 @@ import type { ReactNode } from 'react'; import { useCallback } from 'react'; -import { useSearchParams } from 'react-router'; +import { useLocation, useSearchParams } from 'react-router'; import { useFilesRefresh } from '../../channels'; +import { + folderFromPathname, + relativeToFolder, + resolveInFolder, +} from '../../apps/FileBrowser/FileBrowserApp/files-route'; import { FileViewerProvider } from '../../apps/FileViewer'; import { EmbeddableChat } from '../../apps/Chat/EmbeddableChat'; type SetSearchParams = ReturnType[1]; -const onReplaceView = (setSearchParams: SetSearchParams) => +// `folder` is the one the pathname names, so what goes in the query is a name rather than a second copy +// of the path. Anything not directly inside it stays absolute, which the reader resolves either way. +const onReplaceView = + (setSearchParams: SetSearchParams, folder: string) => (viewPath: string, viewRoot: string, ephemeralPath: string, ephemeralRoot: string) => { setSearchParams((prev) => { const next = new URLSearchParams(prev); - next.set('view', viewPath); - next.set('ephemeral', ephemeralPath); + next.set('view', relativeToFolder(folder, viewPath)); + next.set('ephemeral', relativeToFolder(folder, ephemeralPath)); next.set('ephemeralRoot', ephemeralRoot); next.delete('ephemeral2'); next.delete('ephemeral2Root'); @@ -23,20 +31,29 @@ const onReplaceView = (setSearchParams: SetSearchParams) => export function ViewerProvider({ children }: { children: ReactNode }) { const [searchParams, setSearchParams] = useSearchParams(); - const viewPath = searchParams.get('view') ?? ''; + const { pathname } = useLocation(); + // `?view=` is a name within the folder the pathname names. An absolute value still resolves as itself, + // so links made before this change — and anything genuinely outside the folder — keep working. + const viewPath = resolveInFolder(folderFromPathname(pathname), searchParams.get('view') ?? ''); const fileName = viewPath.split('/').pop() ?? ''; + const folder = folderFromPathname(pathname); const onOpenFile = (filePath: string, root: string) => { setSearchParams((prev) => { const next = new URLSearchParams(prev); - next.set('ephemeral', filePath); + next.set('ephemeral', relativeToFolder(folder, filePath)); next.set('ephemeralRoot', root); return next; }); }; return ( - + {children} ); @@ -44,7 +61,8 @@ export function ViewerProvider({ children }: { children: ReactNode }) { export function EphemeralProvider({ children }: { children: ReactNode }) { const [searchParams, setSearchParams] = useSearchParams(); - const ephemeralPath = searchParams.get('ephemeral') ?? ''; + const { pathname } = useLocation(); + const ephemeralPath = resolveInFolder(folderFromPathname(pathname), searchParams.get('ephemeral') ?? ''); const ephemeralRoot = searchParams.get('ephemeralRoot') ?? 'home'; const fileName = ephemeralPath.split('/').pop() ?? ''; @@ -59,7 +77,13 @@ export function EphemeralProvider({ children }: { children: ReactNode }) { }; return ( - + {children} ); @@ -67,13 +91,20 @@ export function EphemeralProvider({ children }: { children: ReactNode }) { export function Ephemeral2Provider({ children }: { children: ReactNode }) { const [searchParams, setSearchParams] = useSearchParams(); - const ephemeral2Path = searchParams.get('ephemeral2') ?? ''; + const { pathname } = useLocation(); + const ephemeral2Path = resolveInFolder(folderFromPathname(pathname), searchParams.get('ephemeral2') ?? ''); const ephemeral2Root = searchParams.get('ephemeral2Root') ?? 'home'; const ephemeral2Auto = searchParams.get('ephemeral2Auto') === '1'; const fileName = ephemeral2Path.split('/').pop() ?? ''; return ( - + {children} ); @@ -85,15 +116,14 @@ export const ChatEphemeralBody = () => { const chatContext = searchParams.get('chatContext') ?? ''; const chatType = searchParams.get('chatType') as 'file' | 'folder' | null; - const cwdPath = chatType === 'file' - ? chatContext.substring(0, chatContext.lastIndexOf('/')) || '/' - : chatContext; + const cwdPath = chatType === 'file' ? chatContext.substring(0, chatContext.lastIndexOf('/')) || '/' : chatContext; const tag = chatType === 'file' ? 'file' : 'folder'; const path = chatContext.replace(/^\//, ''); - const message = chatType === 'file' - ? `[${tag}: ${path}] Let's talk about this file` - : `[${tag}: ${path || '/'}] consider, for this session, this directory as your current working directory`; + const message = + chatType === 'file' + ? `[${tag}: ${path}] Let's talk about this file` + : `[${tag}: ${path || '/'}] consider, for this session, this directory as your current working directory`; const handleMessageComplete = useCallback(() => { bumpFilesRefresh();