diff --git a/src/apps/officer-web/App.tsx b/src/apps/officer-web/App.tsx index cab25bd9..e146ebee 100644 --- a/src/apps/officer-web/App.tsx +++ b/src/apps/officer-web/App.tsx @@ -57,7 +57,11 @@ export function App() { } /> } /> } /> - } /> + {/* Splat, because the folder is the URL now: `/files/Tests/test folder`. The bare `/files` + still matches with an empty splat and means home, so every existing link and the dock + entry keep working. Only DIRECTORIES live here — an open file stays `?view=`, since it is + what is on top of you rather than where you are. */} + } /> } /> } /> } /> diff --git a/src/server.tsx b/src/server.tsx index 13dd1aa4..93e07b20 100644 --- a/src/server.tsx +++ b/src/server.tsx @@ -212,7 +212,17 @@ const serveBuilt = async (req: Request): Promise => { // A request that LOOKS like an asset and is not one must 404, never fall through to the shell. Serving // HTML with a JS content-type produces "Unexpected token '<'" and a blank page — a failure mode that // reads as a broken build rather than a missing file. - if (/\.(js|css|map|png|svg|ico|webmanifest|woff2?)$/.test(path)) { + // + // SINGLE-SEGMENT only, and that is load-bearing. The bundler emits everything to the root of `build/` + // — `chunk-.js`, `chunk-.css`, the shell — so a real build asset never has a second slash. + // Multi-segment assets (`public/**`, `/plugins//icon.png`) are served by Bun's `routes` table + // before this handler runs and never arrive here at all. + // + // Without the segment test, `/files/Tests/logo.png` — a client route naming a DIRECTORY that happens to + // end in `.png` — 404s instead of reaching the SPA. Rare, but it fails silently and looks like a broken + // deep link rather than a rule. + const isSingleSegment = path.indexOf('/', 1) === -1; + if (isSingleSegment && /\.(js|css|map|png|svg|ico|webmanifest|woff2?)$/.test(path)) { const file = Bun.file(join(BUILD_DIR, path.slice(1))); if (!(await file.exists())) return new Response('Not found', { status: 404 }); return new Response(file, { headers: { 'cache-control': 'public, max-age=31536000, immutable' } }); diff --git a/src/workspaces/officerdev/src/apps/FileBrowser/FileBrowserApp/FileBrowserApp.tsx b/src/workspaces/officerdev/src/apps/FileBrowser/FileBrowserApp/FileBrowserApp.tsx index 580f091a..db921dec 100644 --- a/src/workspaces/officerdev/src/apps/FileBrowser/FileBrowserApp/FileBrowserApp.tsx +++ b/src/workspaces/officerdev/src/apps/FileBrowser/FileBrowserApp/FileBrowserApp.tsx @@ -26,7 +26,7 @@ export const FileBrowserApp = ({ basePath = '/', rootOverride, urlPath }: FileBr path={fileBrowserManager.currentPath} onNavigate={handleNavigate} basePath={basePath} - searchForPath={fileBrowserManager.searchForPath} + hrefForPath={fileBrowserManager.hrefForPath} /> diff --git a/src/workspaces/officerdev/src/apps/FileBrowser/FileBrowserApp/components/Breadcrumb.tsx b/src/workspaces/officerdev/src/apps/FileBrowser/FileBrowserApp/components/Breadcrumb.tsx index 39779c3a..66ca788d 100644 --- a/src/workspaces/officerdev/src/apps/FileBrowser/FileBrowserApp/components/Breadcrumb.tsx +++ b/src/workspaces/officerdev/src/apps/FileBrowser/FileBrowserApp/components/Breadcrumb.tsx @@ -6,20 +6,22 @@ type BreadcrumbProps = { onNavigate: (path: string) => void; basePath?: string; /** - * Query string for a folder, when this browser owns the address bar. Given, the crumbs are real links - * and cmd-click opens the folder in a new tab; absent (a dashboard panel, the widget) they stay buttons, + * Where a folder lives, when this browser owns the address bar. Given, the crumbs are real links and + * cmd-click opens the folder in a new tab; absent (a dashboard panel, the widget) they stay buttons, * because that browser's location is not in the URL and there would be nothing for the new tab to read. + * + * A `To` rather than a query string since 2026-08-15 — the folder is the pathname now. */ - searchForPath?: ((path: string) => string) | null; + hrefForPath?: ((path: string) => { pathname: string; search: string }) | null; }; -export const Breadcrumb = ({ path, onNavigate, basePath = '/', searchForPath }: BreadcrumbProps) => { +export const Breadcrumb = ({ path, onNavigate, basePath = '/', hrefForPath }: BreadcrumbProps) => { const relativePath = basePath !== '/' && path.startsWith(basePath) ? path.slice(basePath.length) : path; const segments = relativePath.split('/').filter(Boolean); const Crumb = ({ to, children, className }: { to: string; children: React.ReactNode; className: string }) => - searchForPath ? ( - + hrefForPath ? ( + {children} ) : ( 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 new file mode 100644 index 00000000..36d24b30 --- /dev/null +++ b/src/workspaces/officerdev/src/apps/FileBrowser/FileBrowserApp/files-route.test.ts @@ -0,0 +1,105 @@ +import { describe, expect, test } from 'bun:test'; +import { decodeFolderPath, encodeFolderPath, folderFromPathname, folderHref } 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. + +describe('folderHref', () => { + test('the base folder is the bare route, with no trailing slash', () => { + expect(folderHref('/')).toBe('/files'); + expect(folderHref('')).toBe('/files'); + }); + + test('the case that prompted this', () => { + expect(folderHref('/Tests/test_folder/Kanban (copy 2)')).toBe('/files/Tests/test_folder/Kanban%20(copy%202)'); + }); + + test('parentheses are left readable; only the space is escaped', () => { + // encodeURIComponent deliberately leaves ( ) ! \' * ~ alone. That is why the example above stays + // legible in the address bar instead of turning into %28copy%202%29. + expect(folderHref('/a (b)')).toBe('/files/a%20(b)'); + }); + + test('separators survive, contents are escaped', () => { + // The whole reason for encoding per segment: encodeURIComponent('/a/b') would eat the slashes. + expect(folderHref('/a/b')).toBe('/files/a/b'); + }); +}); + +describe('round trip', () => { + test('every character a filename can legally carry comes back unchanged', () => { + const names = [ + 'Kanban (copy 2)', + 'a b', + 'hash#tag', + 'question?mark', + 'percent%20literal', + 'amp&and', + 'plus+plus', + "quote'single", + 'semi;colon', + 'at@sign', + 'equals=sign', + 'brack[et]', + 'emoji🦆duck', + 'accénts', + '.dotfile', + 'dash-under_score', + ]; + for (const name of names) { + const path = `/Tests/${name}`; + expect(folderFromPathname(folderHref(path))).toBe(path); + } + }); + + test('a hash in a name does not truncate the path', () => { + // The one that would silently lose everything after it if the segment were not encoded, because a + // browser treats a bare # as the fragment. + expect(folderHref('/a#b/c')).toBe('/files/a%23b/c'); + expect(folderFromPathname('/files/a%23b/c')).toBe('/a#b/c'); + }); + + test('a literal percent is not mistaken for an escape', () => { + expect(folderHref('/100%')).toBe('/files/100%25'); + expect(folderFromPathname('/files/100%25')).toBe('/100%'); + }); +}); + +describe('folderFromPathname', () => { + test('the bare route is the base folder', () => { + expect(folderFromPathname('/files')).toBe('/'); + }); + + test('a trailing slash is the base folder, not an empty segment', () => { + expect(folderFromPathname('/files/')).toBe('/'); + }); + + test('anything outside the route answers the base rather than making the caller check', () => { + expect(folderFromPathname('/chat')).toBe('/'); + expect(folderFromPathname('/filesystem/x')).toBe('/'); + expect(folderFromPathname('/')).toBe('/'); + }); + + test('malformed encoding is kept verbatim rather than throwing', () => { + // decodeURIComponent('%zz') raises a URIError. A hand-mangled URL should land you somewhere odd, not + // take the screen down with an uncaught exception. + expect(() => folderFromPathname('/files/%zz')).not.toThrow(); + expect(folderFromPathname('/files/%zz')).toBe('/%zz'); + }); + + test('empty segments collapse, so a double slash is not a nameless folder', () => { + expect(folderFromPathname('/files//a//b')).toBe('/a/b'); + }); +}); + +describe('encode/decode directly', () => { + test('encodeFolderPath returns no leading slash, so it can be joined', () => { + expect(encodeFolderPath('/a/b')).toBe('a/b'); + expect(encodeFolderPath('/')).toBe(''); + }); + + test('decodeFolderPath is always absolute', () => { + expect(decodeFolderPath('a/b')).toBe('/a/b'); + expect(decodeFolderPath('')).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 new file mode 100644 index 00000000..e4f8ee7a --- /dev/null +++ b/src/workspaces/officerdev/src/apps/FileBrowser/FileBrowserApp/files-route.ts @@ -0,0 +1,61 @@ +// The folder you are looking at, as a URL path. +// +// `/files/Tests/test folder/Kanban (copy 2)` rather than `/files?path=/Tests/...`. The folder is WHERE +// YOU ARE, which is what a path is for; the viewer stays in `?view=` because it is what is on top of you, +// not where you are. That split is the same one `setViewerParams` has always documented, and it is what +// keeps closing a pane from moving you. +// +// ── Why encode per segment ── +// +// A file name may contain anything the filesystem allows except `/` and NUL — `#`, `?`, `%`, spaces, +// quotes, emoji. Encoding the whole path at once would eat the separators; encoding per segment keeps +// them and escapes the rest. `encodeURIComponent` leaves `(`, `)`, `!`, `'`, `*` and `~` alone, which is +// why `Kanban (copy 2)` survives readably while the space becomes `%20`. + +export const FILES_ROUTE_BASE = '/files'; + +/** + * A folder path (`/a/b c`) as URL path segments (`a/b%20c`). No leading slash — the caller joins it to + * the route base, and an empty string is the base folder. + */ +export function encodeFolderPath(path: string): string { + return path.split('/').filter(Boolean).map(encodeURIComponent).join('/'); +} + +/** + * The inverse: URL path segments back to a folder path, always absolute. + * + * A segment that is not valid percent-encoding is kept verbatim rather than throwing — + * `decodeURIComponent('%zz')` raises a URIError, and a malformed URL should land you somewhere odd, not + * take the whole screen down with an uncaught exception. + */ +export function decodeFolderPath(segments: string): string { + const parts = segments + .split('/') + .filter(Boolean) + .map((segment) => { + try { + return decodeURIComponent(segment); + } catch { + return segment; + } + }); + return parts.length ? `/${parts.join('/')}` : '/'; +} + +/** The full pathname for a folder. The base folder is the bare route, not a trailing slash. */ +export function folderHref(path: string): string { + const encoded = encodeFolderPath(path); + return encoded ? `${FILES_ROUTE_BASE}/${encoded}` : FILES_ROUTE_BASE; +} + +/** + * The folder a `/files/...` pathname refers to. + * + * Anything that is not under the base returns `/`, so a caller never has to check first. + */ +export function folderFromPathname(pathname: string): string { + if (pathname === FILES_ROUTE_BASE) return '/'; + if (!pathname.startsWith(`${FILES_ROUTE_BASE}/`)) return '/'; + return decodeFolderPath(pathname.slice(FILES_ROUTE_BASE.length + 1)); +} diff --git a/src/workspaces/officerdev/src/apps/FileBrowser/FileBrowserApp/useFileBrowserApp.ts b/src/workspaces/officerdev/src/apps/FileBrowser/FileBrowserApp/useFileBrowserApp.ts index f2f70804..9199b59c 100644 --- a/src/workspaces/officerdev/src/apps/FileBrowser/FileBrowserApp/useFileBrowserApp.ts +++ b/src/workspaces/officerdev/src/apps/FileBrowser/FileBrowserApp/useFileBrowserApp.ts @@ -1,10 +1,11 @@ import { useState, useEffect, useRef } from 'react'; -import { useSearchParams, useNavigate } from 'react-router'; +import { useSearchParams, useNavigate, useLocation } from 'react-router'; import { toast } from 'sonner'; 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 { useTasks, type TaskSummary } from '../useTasks'; import { useAgents, type AgentSummary } from '../useAgents'; import { useUserState } from 'state/useUserState'; @@ -14,23 +15,15 @@ import { useNewDashboardDraft } from '../../Dashboards/useNewDashboardDraft'; import { copyToClipboard, canReadClipboard } from 'helpers/clipboard'; /** - * Where the browser is looking, when it is the one browser that owns the address bar. `urlPath` is opt-in - * because the same app mounts as a dashboard panel, and a dashboard can hold two of them — one shared - * param would move both. The /files screen holds exactly one, so there it is the source of truth. + * The old query param. Kept only to recognise and redirect links made before 2026-08-15 — the folder + * lives in the URL PATH now (`/files/Tests/test folder`). Nothing writes it. */ export const FILES_PATH_PARAM = 'path'; -/** The base folder is the bare address, so it drops the param rather than spelling itself out. */ -const withPath = (prev: URLSearchParams, path: string, basePath: string) => { - const next = new URLSearchParams(prev); - if (path === basePath) next.delete(FILES_PATH_PARAM); - else next.set(FILES_PATH_PARAM, path); - return next; -}; - export const useFileBrowserApp = (basePath: string, rootOverride?: string, urlPath = false) => { const { user } = useAuth(); const navigate = useNavigate(); + const location = useLocation(); const openNewDashboard = useNewDashboardDraft(); const [searchParams, setSearchParams] = useSearchParams(); const homeRoot = 'home'; @@ -42,27 +35,40 @@ export const useFileBrowserApp = (basePath: string, rootOverride?: string, urlPa // visit still opens at home, because a URL with no `path` means the base. Either way the main // (unscoped) browser mirrors its folder into files/currentPath so the Create Dashboard flow can // default a new dashboard's cwd to it. - const currentPath = urlPath ? (searchParams.get(FILES_PATH_PARAM) ?? basePath) : localPath; + const currentPath = urlPath ? folderFromPathname(location.pathname) : localPath; const setCurrentPath = (path: string) => { - if (urlPath) setSearchParams((prev) => withPath(prev, path, basePath)); + // The search string rides along, so walking into a folder does not close an open pane — the same + // property the old `withPath` had, now expressed as "change the pathname, keep the query". + if (urlPath) navigate({ pathname: folderHref(path), search: location.search }); else setLocalPath(path); if (!scoped) setGlobalPath(path); }; - /** Query string for a folder — the current params with `path` set, so an open viewer survives a crumb. */ - const searchForPath = (path: string) => withPath(searchParams, path, basePath).toString(); + /** + * Where a crumb points. A real `To` rather than a query string, because the folder is the pathname now; + * the current search rides along so cmd-clicking a crumb keeps whatever is open. + */ + const hrefForPath = (path: string) => ({ pathname: folderHref(path), search: location.search }); /** * The ephemeral viewer params replace each other wholesale: opening a file closes whatever overlay was - * open. `path` is not one of them — it is where you are, not what is on top of it — so it survives. + * open. The folder is no longer among them — it is the pathname — so it survives without being copied + * across, which is what used to be needed here. */ - const setViewerParams = (params: Record) => - setSearchParams((prev) => { - const next = new URLSearchParams(params); - const keep = prev.get(FILES_PATH_PARAM); - if (keep) next.set(FILES_PATH_PARAM, keep); - return next; - }); + const setViewerParams = (params: Record) => setSearchParams(new URLSearchParams(params)); + + /** + * Links from before the folder moved into the path still say `?path=/Tests`. Rewrite them once, in + * place, so an old bookmark or a stale dock entry lands where it meant to instead of silently opening + * home. `replace` so it does not become a back-button step. + */ + const legacyPath = urlPath ? searchParams.get(FILES_PATH_PARAM) : null; + useEffect(() => { + if (!legacyPath) return; + const next = new URLSearchParams(searchParams); + next.delete(FILES_PATH_PARAM); + navigate({ pathname: folderHref(legacyPath), search: next.toString() }, { replace: true }); + }, [legacyPath]); const [entries, setEntries] = useState([]); const [rootDir, setRootDir] = useState(''); const [loading, setLoading] = useState(true); @@ -852,7 +858,7 @@ export const useFileBrowserApp = (basePath: string, rootOverride?: string, urlPa currentPath, setCurrentPath, // Null when this browser does not own the address bar — the breadcrumb then stays buttons. - searchForPath: urlPath ? searchForPath : null, + hrefForPath: urlPath ? hrefForPath : null, // Directory listing visibleEntries, loading,