From 9a3bf1293cfc7d03bef243249e30e0264a6f3aae Mon Sep 17 00:00:00 2001 From: Brennan Benson <79079362+brennanb2025@users.noreply.github.com> Date: Mon, 6 Jul 2026 17:13:21 -0700 Subject: [PATCH] Mobile file tree follow-ups: reconnect no-blank + old-desktop files.list fallback (#7599) Co-authored-by: Orca --- .../src/files/MobileFileExplorerPanel.test.ts | 171 ++++++++++++++++++ mobile/src/files/MobileFileExplorerPanel.tsx | 54 +++++- mobile/src/files/file-list-fallback.test.ts | 65 +++++++ mobile/src/files/file-list-fallback.ts | 73 ++++++++ 4 files changed, 361 insertions(+), 2 deletions(-) create mode 100644 mobile/src/files/file-list-fallback.test.ts create mode 100644 mobile/src/files/file-list-fallback.ts diff --git a/mobile/src/files/MobileFileExplorerPanel.test.ts b/mobile/src/files/MobileFileExplorerPanel.test.ts index b86e722f1..0d003e9f4 100644 --- a/mobile/src/files/MobileFileExplorerPanel.test.ts +++ b/mobile/src/files/MobileFileExplorerPanel.test.ts @@ -224,4 +224,175 @@ describe('MobileFileExplorerPanel', () => { expect(renderedText(renderer)).toContain('README.md') expect(renderedText(renderer)).not.toContain('Waiting for desktop...') }) + + it('refreshes the root in the background after reconnect without blanking the tree', async () => { + const client = createMockClient({ + '': [entry('src', true), entry('README.md')] + }) + mockTransport.client = client + + const renderer = await renderExplorer() + expect(renderedText(renderer)).toContain('README.md') + + mockTransport.client = null + mockTransport.connectionState = 'disconnected' + await updateExplorer(renderer) + + let resolveReload: ((response: RpcResponse) => void) | undefined + const reconnectedClient: MockClient = { + sendRequest: vi.fn( + () => + new Promise((resolve) => { + resolveReload = resolve + }) + ) + } + mockTransport.client = reconnectedClient + mockTransport.connectionState = 'connected' + await updateExplorer(renderer) + + expect(reconnectedClient.sendRequest).toHaveBeenCalledTimes(1) + expect(renderedText(renderer)).toContain('src') + expect(renderedText(renderer)).toContain('README.md') + + await act(async () => { + resolveReload?.(ok([entry('src', true), entry('README.md'), entry('CHANGELOG.md')])) + }) + expect(renderedText(renderer)).toContain('CHANGELOG.md') + }) + + it('keeps the cached tree when a post-reconnect root refresh fails', async () => { + const client = createMockClient({ + '': [entry('src', true), entry('README.md')] + }) + mockTransport.client = client + + const renderer = await renderExplorer() + expect(renderedText(renderer)).toContain('README.md') + + mockTransport.client = null + mockTransport.connectionState = 'disconnected' + await updateExplorer(renderer) + + const failingClient: MockClient = { + sendRequest: vi.fn(async () => { + throw new Error('refresh failed') + }) + } + mockTransport.client = failingClient + mockTransport.connectionState = 'connected' + await updateExplorer(renderer) + + expect(failingClient.sendRequest).toHaveBeenCalledTimes(1) + expect(renderedText(renderer)).toContain('src') + expect(renderedText(renderer)).toContain('README.md') + expect(renderedText(renderer)).not.toContain('refresh failed') + }) + + it('falls back to the capped files.list against desktops without files.readDir', async () => { + const legacyClient: MockClient = { + sendRequest: vi.fn(async (method: string): Promise => { + if (method === 'files.readDir') { + return { + id: 'response-id', + ok: false, + error: { + code: 'forbidden', + message: "Method 'files.readDir' is not available to mobile clients" + }, + _meta: { runtimeId: 'runtime-id' } + } + } + return { + id: 'response-id', + ok: true, + result: { + files: [ + { relativePath: 'src/app.ts', basename: 'app.ts', kind: 'text' }, + { relativePath: 'README.md', basename: 'README.md', kind: 'text' } + ], + totalCount: 2, + truncated: false + }, + _meta: { runtimeId: 'runtime-id' } + } + }) + } + mockTransport.client = legacyClient + + const renderer = await renderExplorer() + + expect(legacyClient.sendRequest).toHaveBeenCalledWith('files.list', { + worktree: 'id:worktree-a' + }) + expect(renderedText(renderer)).toContain('src') + expect(renderedText(renderer)).toContain('README.md') + + await pressByLabel(renderer, 'Open folder src') + expect(renderedText(renderer)).toContain('app.ts') + // Every directory comes from the synthesized cache: one readDir attempt + // plus one files.list call total, no per-directory RPCs afterwards. + expect(legacyClient.sendRequest).toHaveBeenCalledTimes(2) + }) + + it('surfaces the legacy cap note when the files.list fallback is truncated', async () => { + const legacyClient: MockClient = { + sendRequest: vi.fn(async (method: string): Promise => { + if (method === 'files.readDir') { + return { + id: 'response-id', + ok: false, + error: { code: 'method_not_found', message: 'Unknown method' }, + _meta: { runtimeId: 'runtime-id' } + } + } + return { + id: 'response-id', + ok: true, + result: { + files: [{ relativePath: 'README.md', basename: 'README.md', kind: 'text' }], + totalCount: 6000, + truncated: true + }, + _meta: { runtimeId: 'runtime-id' } + } + }) + } + mockTransport.client = legacyClient + + const renderer = await renderExplorer() + + expect(renderedText(renderer)).toContain('README.md') + expect(renderedText(renderer)).toContain('Showing first 5000') + }) + + it('reports the files.list failure when the fallback itself fails', async () => { + const legacyClient: MockClient = { + sendRequest: vi.fn(async (method: string): Promise => { + if (method === 'files.readDir') { + return { + id: 'response-id', + ok: false, + error: { + code: 'forbidden', + message: "Method 'files.readDir' is not available to mobile clients" + }, + _meta: { runtimeId: 'runtime-id' } + } + } + return { + id: 'response-id', + ok: false, + error: { code: 'internal', message: 'legacy list failed' }, + _meta: { runtimeId: 'runtime-id' } + } + }) + } + mockTransport.client = legacyClient + + const renderer = await renderExplorer() + + expect(renderedText(renderer)).toContain('legacy list failed') + expect(renderedText(renderer)).not.toContain('not available to mobile clients') + }) }) diff --git a/mobile/src/files/MobileFileExplorerPanel.tsx b/mobile/src/files/MobileFileExplorerPanel.tsx index b777c1379..7e67bf81b 100644 --- a/mobile/src/files/MobileFileExplorerPanel.tsx +++ b/mobile/src/files/MobileFileExplorerPanel.tsx @@ -28,6 +28,11 @@ import { resetDirectoryLoadRevisions, type DirectoryLoadRevisions } from './directory-load-revisions' +import { + directoryCacheFromFileList, + isMobileMethodUnavailableError, + type LegacyFilesListResult +} from './file-list-fallback' import { fileExplorerStyles as styles } from './mobile-file-explorer-styles' import { MobileFileExplorerRow } from './mobile-file-explorer-row' import { navigateToMobileFilePreview } from './mobile-file-preview-navigation' @@ -53,6 +58,7 @@ export function MobileFileExplorerPanel(props: { const [expanded, setExpanded] = useState>(() => new Set()) const [loading, setLoading] = useState(true) const [error, setError] = useState(null) + const [legacyListTruncated, setLegacyListTruncated] = useState(false) const worktreeLabel = getWorktreeLabel(name, worktreeId) const loadDirectory = useCallback( @@ -82,8 +88,14 @@ export function MobileFileExplorerPanel(props: { return } + const hadLoadedRoot = + rootLoad && (getDirectoryCacheState(directoryCacheRef.current, '')?.entries.length ?? 0) > 0 if (rootLoad) { - setLoading(true) + // Why: a reconnect refresh must not blank an already browsable tree — + // the full-screen spinner unmounts the list and resets scroll. + if (!hadLoadedRoot) { + setLoading(true) + } setError(null) } setDirectoryCache((prev) => ({ @@ -100,6 +112,37 @@ export function MobileFileExplorerPanel(props: { relativePath }) if (!response.ok) { + // Why: desktops that predate the files.readDir mobile allowlist + // entry still serve the capped files.list; fall back so the Files + // tab keeps working until the desktop updates. + if ( + rootLoad && + isMobileMethodUnavailableError(response.error?.code, response.error?.message) + ) { + const legacy = await client.sendRequest('files.list', { + worktree: `id:${worktreeId}` + }) + if (legacy.ok) { + if ( + !isCurrentDirectoryLoad( + directoryLoadRevisionsRef.current, + scopeRef.current, + loadToken + ) + ) { + return + } + const legacyResult = (legacy as RpcSuccess).result as LegacyFilesListResult + setDirectoryCache(directoryCacheFromFileList(legacyResult.files)) + // Why: the capped list silently omits files past the cap — keep + // the legacy explorer's "Showing first 5000" note. + setLegacyListTruncated(legacyResult.truncated) + return + } + throw new Error( + legacy.error?.message || response.error?.message || 'Unable to load files' + ) + } throw new Error(response.error?.message || 'Unable to load files') } if ( @@ -108,6 +151,9 @@ export function MobileFileExplorerPanel(props: { return } const entries = (response as RpcSuccess).result as MobileDirEntry[] + if (rootLoad) { + setLegacyListTruncated(false) + } setDirectoryCache((prev) => ({ ...prev, [relativePath]: { entries } @@ -120,7 +166,9 @@ export function MobileFileExplorerPanel(props: { } const message = err instanceof Error ? err.message : 'Unable to load files' if (rootLoad) { - setError(message) + // Why: a failed background refresh keeps the cached tree browsable; + // only a cold load surfaces the full-screen error. + setError(hadLoadedRoot ? null : message) } else { setDirectoryCache((prev) => ({ ...prev, @@ -151,6 +199,7 @@ export function MobileFileExplorerPanel(props: { setExpanded(new Set()) setLoading(true) setError(null) + setLegacyListTruncated(false) }, [scope]) useEffect(() => { @@ -264,6 +313,7 @@ export function MobileFileExplorerPanel(props: { {worktreeLabel} + {legacyListTruncated ? ' - Showing first 5000' : ''} diff --git a/mobile/src/files/file-list-fallback.test.ts b/mobile/src/files/file-list-fallback.test.ts new file mode 100644 index 000000000..a69affc5f --- /dev/null +++ b/mobile/src/files/file-list-fallback.test.ts @@ -0,0 +1,65 @@ +import { describe, expect, it } from 'vitest' +import { directoryCacheFromFileList, isMobileMethodUnavailableError } from './file-list-fallback' +import { getDirectoryCacheState } from './file-tree' + +describe('isMobileMethodUnavailableError', () => { + it('detects old-desktop allowlist and missing-method failures', () => { + expect(isMobileMethodUnavailableError('forbidden', undefined)).toBe(true) + expect(isMobileMethodUnavailableError('method_not_found', undefined)).toBe(true) + expect( + isMobileMethodUnavailableError( + 'some_code', + "Method 'files.readDir' is not available to mobile clients" + ) + ).toBe(true) + expect(isMobileMethodUnavailableError('internal', 'boom')).toBe(false) + expect(isMobileMethodUnavailableError(undefined, undefined)).toBe(false) + }) +}) + +describe('directoryCacheFromFileList', () => { + it('synthesizes every ancestor directory from flat paths', () => { + const cache = directoryCacheFromFileList([ + { relativePath: 'src/lib/util.ts', basename: 'util.ts', kind: 'text' }, + { relativePath: 'src/app.ts', basename: 'app.ts', kind: 'text' }, + { relativePath: 'README.md', basename: 'README.md', kind: 'text' } + ]) + expect(cache['']?.entries).toEqual( + expect.arrayContaining([ + { name: 'src', isDirectory: true }, + { name: 'README.md', isDirectory: false } + ]) + ) + expect(cache['src']?.entries).toEqual( + expect.arrayContaining([ + { name: 'lib', isDirectory: true }, + { name: 'app.ts', isDirectory: false } + ]) + ) + expect(cache['src/lib']?.entries).toEqual([{ name: 'util.ts', isDirectory: false }]) + }) + + it('keeps a name a directory when it appears as both file and dir prefix', () => { + const cache = directoryCacheFromFileList([ + { relativePath: 'src', basename: 'src', kind: 'text' }, + { relativePath: 'src/app.ts', basename: 'app.ts', kind: 'text' } + ]) + expect(cache['']?.entries).toEqual([{ name: 'src', isDirectory: true }]) + }) + + it('returns an empty root for an empty list', () => { + const cache = directoryCacheFromFileList([]) + expect(cache['']?.entries).toEqual([]) + }) + + it('stores a __proto__ directory as an own key instead of mutating the prototype', () => { + const cache = directoryCacheFromFileList([ + { relativePath: '__proto__/pollute.js', basename: 'pollute.js', kind: 'text' } + ]) + expect(Object.getPrototypeOf(cache)).toBe(Object.prototype) + expect(cache['']?.entries).toEqual([{ name: '__proto__', isDirectory: true }]) + expect(getDirectoryCacheState(cache, '__proto__')?.entries).toEqual([ + { name: 'pollute.js', isDirectory: false } + ]) + }) +}) diff --git a/mobile/src/files/file-list-fallback.ts b/mobile/src/files/file-list-fallback.ts new file mode 100644 index 000000000..ef3041029 --- /dev/null +++ b/mobile/src/files/file-list-fallback.ts @@ -0,0 +1,73 @@ +// Fallback for desktops that predate files.readDir in the mobile RPC +// allowlist: synthesize the lazy directory cache from the flat, capped +// files.list result so the Files tab stays browsable against old desktops. +import type { DirectoryCache, MobileDirEntry } from './file-tree' + +export type LegacyMobileFileEntry = { + relativePath: string + basename: string + kind: 'text' | 'binary' +} + +export type LegacyFilesListResult = { + files: LegacyMobileFileEntry[] + totalCount: number + truncated: boolean +} + +// Same detection shape as isMobileGitUnavailable in mobile-git-status.ts: +// 'forbidden' = method exists but is not mobile-allowlisted on the old +// desktop; 'method_not_found' = desktop predates the method entirely. +export function isMobileMethodUnavailableError( + code: string | undefined, + message: string | undefined +): boolean { + return ( + code === 'forbidden' || + code === 'method_not_found' || + message?.includes('not available to mobile clients') === true + ) +} + +export function directoryCacheFromFileList(files: LegacyMobileFileEntry[]): DirectoryCache { + const childrenByDir = new Map>() + const ensureDir = (path: string): Map => { + let children = childrenByDir.get(path) + if (!children) { + children = new Map() + childrenByDir.set(path, children) + } + return children + } + ensureDir('') + for (const file of files) { + const parts = file.relativePath.split('/').filter(Boolean) + let parentPath = '' + parts.forEach((name, index) => { + const isDirectory = index < parts.length - 1 + const children = ensureDir(parentPath) + children.set(name, children.get(name) === true || isDirectory) + parentPath = parentPath ? `${parentPath}/${name}` : name + if (isDirectory) { + ensureDir(parentPath) + } + }) + } + // Why: plain `cache[path] = ...` with a '__proto__' path segment mutates the + // object's prototype instead of storing the directory; fromEntries always + // creates own keys. + return Object.fromEntries( + Array.from(childrenByDir, ([path, children]) => [ + path, + { + entries: Array.from( + children, + ([name, isDirectory]): MobileDirEntry => ({ + name, + isDirectory + }) + ) + } + ]) + ) +}