diff --git a/mobile/src/session/mobile-file-preview-lifecycle.test.ts b/mobile/src/session/mobile-file-preview-lifecycle.test.ts index db23a20a0f7..bd01b5e1aa6 100644 --- a/mobile/src/session/mobile-file-preview-lifecycle.test.ts +++ b/mobile/src/session/mobile-file-preview-lifecycle.test.ts @@ -223,4 +223,67 @@ describe('mobile file preview lifetime', () => { } } ) + + it('keeps a ready preview mounted across a revisit and a failed refresh', async () => { + const session = previewSession() + const tab = fileTab('revisit') + const ready: FileDocState = { + status: 'ready', + kind: 'file', + content: 'Same', + truncated: false, + byteLength: 4 + } + try { + session.open(tab, ready) + const unchanged = session.read(tab) + expect(session.docs().get(tab.id)).toBe(ready) + session.reply(0, textReply('Same')) + await unchanged + expect(session.docs().get(tab.id)).toBe(ready) + + const failed = session.read(tab) + session.reply(1, refusedReply()) + await failed + expect(session.docs().get(tab.id)).toBe(ready) + + const changed = session.read(tab) + session.reply(2, textReply('Changed')) + await changed + expect(session.docs().get(tab.id)).toMatchObject({ + status: 'ready', + kind: 'file', + content: 'Changed' + }) + } finally { + session.dispose() + } + }) + + it('keeps the newer file reread when an older one finishes later', async () => { + const session = previewSession() + const tab = fileTab('raced') + const ready: FileDocState = { + status: 'ready', + kind: 'file', + content: 'Cached', + truncated: false, + byteLength: 6 + } + try { + session.open(tab, ready) + const older = session.read(tab) + const newer = session.read(tab) + expect(session.docs().get(tab.id)).toBe(ready) + session.reply(1, textReply('New content')) + await newer + const current = session.docs().get(tab.id) + session.reply(0, textReply('Old content')) + await older + expect(session.docs().get(tab.id)).toBe(current) + expect(current).toMatchObject({ content: 'New content' }) + } finally { + session.dispose() + } + }) }) diff --git a/mobile/src/session/mobile-markdown-document-lifetime.test.ts b/mobile/src/session/mobile-markdown-document-lifetime.test.ts index 749a53eeadc..de813c3489c 100644 --- a/mobile/src/session/mobile-markdown-document-lifetime.test.ts +++ b/mobile/src/session/mobile-markdown-document-lifetime.test.ts @@ -37,6 +37,11 @@ function mountDocuments(readReply: () => Promise) { readers = useMobileSessionDocumentReaders( mountFixture({ worktreeId: 'folder-workspace', + markdownDocsRef: { + get current() { + return docs + } + }, client, setMarkdownDocs: setDocs, setFileDocs: () => {} diff --git a/mobile/src/session/mobile-session-active-document-retap.ts b/mobile/src/session/mobile-session-active-document-retap.ts new file mode 100644 index 00000000000..080a826c6fe --- /dev/null +++ b/mobile/src/session/mobile-session-active-document-retap.ts @@ -0,0 +1,17 @@ +import type { FileDocState, MarkdownDocState, MobileSessionTab } from './mobile-session-route-types' + +/** Retap of the open markdown or file tab is not a reload; refresh is a separate control. */ +export function isCachedActiveDocumentRetap( + tab: MobileSessionTab, + activeSessionTabId: string | null, + docs: { + markdownDocs: ReadonlyMap + fileDocs: ReadonlyMap + } +): boolean { + if (tab.id !== activeSessionTabId || (tab.type !== 'markdown' && tab.type !== 'file')) { + return false + } + const cached = tab.type === 'markdown' ? docs.markdownDocs.get(tab.id) : docs.fileDocs.get(tab.id) + return cached !== undefined && cached.status !== 'error' +} diff --git a/mobile/src/session/mobile-session-document-read-settlement.ts b/mobile/src/session/mobile-session-document-read-settlement.ts new file mode 100644 index 00000000000..4f1f14a1b6f --- /dev/null +++ b/mobile/src/session/mobile-session-document-read-settlement.ts @@ -0,0 +1,177 @@ +import type { MobileFileMedia } from '../files/mobile-file-media' +import type { MobileDiffLine } from './mobile-diff-lines' +import type { + FileDocState, + MarkdownDocState, + ReadyFileDocState +} from './mobile-session-route-types' + +export function claimDocumentRead(sequences: Map, tabId: string): number { + const seq = (sequences.get(tabId) ?? 0) + 1 + sequences.set(tabId, seq) + return seq +} + +export function documentReadIsCurrent( + sequences: Map, + tabId: string, + seq: number +): boolean { + return sequences.get(tabId) === seq +} + +/** A dirty ready doc is a discard and must reload. A clean one stays up and refreshes behind it. */ +export function keepMarkdownVisibleDuringRead(current: MarkdownDocState | undefined): boolean { + return current?.status === 'ready' && !current.isDirty +} + +export function keepFileVisibleDuringRead(current: FileDocState | undefined): boolean { + return current?.status === 'ready' +} + +export function installDocumentReadPlaceholder( + prev: Map, + tabId: string, + loading: Doc, + keepVisible: (current: Doc | undefined) => boolean +): Map { + if (keepVisible(prev.get(tabId))) { + return prev + } + const next = new Map(prev) + next.set(tabId, loading) + return next +} + +export function commitDocumentRead( + prev: Map, + tabId: string, + loading: Doc, + nextDoc: Doc, + settle: (current: Doc, loading: Doc, nextDoc: Doc) => Doc | undefined +): Map { + const current = prev.get(tabId) + if (!current) { + return prev + } + const settled = settle(current, loading, nextDoc) + if (!settled || settled === current) { + return prev + } + const next = new Map(prev) + next.set(tabId, settled) + return next +} + +/** Edits and saves made during a reread win; unchanged content and metadata keep the view. */ +export function settleMarkdownRead( + current: MarkdownDocState, + loading: MarkdownDocState, + nextDoc: MarkdownDocState, + visibleAtStart: MarkdownDocState | undefined +): MarkdownDocState | undefined { + if (current.status === 'ready') { + if ( + current !== visibleAtStart || + current.isDirty || + current.saving || + nextDoc.status !== 'ready' + ) { + return undefined + } + if ( + ( + [ + 'content', + 'localContent', + 'baseVersion', + 'isDirty', + 'editable', + 'stale', + 'readOnlyReason', + 'truncated', + 'byteLength', + 'saving', + 'saveError' + ] as const + ).every((field) => current[field] === nextDoc[field]) + ) { + return undefined + } + return nextDoc + } + if (current !== loading) { + return undefined + } + return nextDoc +} + +export function settleFileRead( + current: FileDocState, + loading: FileDocState, + nextDoc: FileDocState +): FileDocState | undefined { + if (current.status === 'ready') { + if (nextDoc.status !== 'ready' || readyFileDocsMatch(current, nextDoc)) { + return undefined + } + return nextDoc + } + if (current !== loading) { + return undefined + } + return nextDoc +} + +function readyFileDocsMatch(current: ReadyFileDocState, next: FileDocState): boolean { + if (next.status !== 'ready' || current.kind !== next.kind) { + return false + } + switch (current.kind) { + case 'file': + return ( + next.kind === 'file' && + current.content === next.content && + current.truncated === next.truncated && + current.byteLength === next.byteLength + ) + case 'html': + return next.kind === 'html' && current.content === next.content + case 'image': + return next.kind === 'image' && current.dataUri === next.dataUri + case 'media': + return next.kind === 'media' && sameMedia(current.media, next.media) + case 'diff': + return ( + next.kind === 'diff' && + current.truncated === next.truncated && + sameDiffLines(current.lines, next.lines) + ) + } +} + +function sameMedia(current: MobileFileMedia, next: MobileFileMedia): boolean { + return ( + current.worktreeId === next.worktreeId && + current.relativePath === next.relativePath && + current.mimeType === next.mimeType + ) +} + +function sameDiffLines( + current: readonly MobileDiffLine[], + next: readonly MobileDiffLine[] +): boolean { + if (current.length !== next.length) { + return false + } + return current.every((line, index) => { + const other = next[index] + return ( + line.kind === other?.kind && + line.text === other?.text && + line.oldLineNumber === other?.oldLineNumber && + line.newLineNumber === other?.newLineNumber + ) + }) +} diff --git a/mobile/src/session/use-mobile-session-document-readers.test.ts b/mobile/src/session/use-mobile-session-document-readers.test.ts index 70de6de02a3..c7b089148b7 100644 --- a/mobile/src/session/use-mobile-session-document-readers.test.ts +++ b/mobile/src/session/use-mobile-session-document-readers.test.ts @@ -14,6 +14,11 @@ async function readMarkdown(reply: RpcResponse): Promise[0]>({ worktreeId: 'wt-1', + markdownDocsRef: { + get current() { + return docs + } + }, client: { sendRequest: vi.fn(async (): Promise => reply) }, setMarkdownDocs: (update) => { docs = typeof update === 'function' ? update(docs) : update @@ -93,3 +98,305 @@ describe('useMobileSessionDocumentReaders markdown reads', () => { expect(doc).not.toHaveProperty('byteLength') }) }) + +function readyMarkdown( + content: string, + extra: Partial> = {} +): Extract { + return { + status: 'ready', + content, + localContent: content, + baseVersion: 'v1', + isDirty: false, + editable: true, + stale: false, + ...extra + } +} + +function deferredReply() { + let resolve: (reply: RpcResponse) => void = () => {} + const promise = new Promise((done) => { + resolve = done + }) + return { promise, resolve } +} + +function mountMarkdownReads( + initial: Map, + sendRequest: (method: string) => Promise +) { + let docs = new Map(initial) + const published: MarkdownDocState[] = [] + let readers: ReturnType | undefined + const hook = hookMount(() => { + readers = useMobileSessionDocumentReaders( + mountFixture[0]>({ + worktreeId: 'wt-1', + markdownDocsRef: { + get current() { + return docs + } + }, + client: { sendRequest: vi.fn(sendRequest) }, + setMarkdownDocs: (update) => { + const next = typeof update === 'function' ? update(docs) : update + const doc = next.get('tab-md') + if (doc && doc !== docs.get('tab-md')) { + published.push(doc) + } + docs = next + }, + setFileDocs: () => {} + }) + ) + }) + hook.mount() + const tab = mountFixture['readMarkdownTab']>[0]>({ + type: 'markdown', + id: 'tab-md', + relativePath: 'README.md', + isDirty: false + }) + return { + published, + get docs() { + return docs + }, + read: () => performHookAction(() => readers?.readMarkdownTab(tab)), + unmount: hook.unmount + } +} + +describe('useMobileSessionDocumentReaders markdown revalidation', () => { + it.each([ + { + reply: { editable: false, readOnlyReason: 'host_read_only' }, + expected: { editable: false, readOnlyReason: 'host_read_only' } + }, + { reply: { isDirty: true }, expected: { stale: true } }, + { + reply: { truncated: true, byteLength: 3_000_000 }, + expected: { truncated: true, byteLength: 3_000_000 } + } + ])( + 'applies metadata changes with unchanged content and version: $expected', + async ({ reply, expected }) => { + const initial = readyMarkdown('# head') + const harness = mountMarkdownReads(new Map([['tab-md', initial]]), async () => + readTabResult({ + content: '# head', + version: 'v1', + editable: true, + readOnlyReason: undefined, + ...reply + }) + ) + try { + await harness.read() + expect(harness.docs.get('tab-md')).toMatchObject(expected) + } finally { + harness.unmount() + } + } + ) + + it('does not overwrite a phone save that finishes during a background read', async () => { + const initial = readyMarkdown('# head') + const pending = deferredReply() + const harness = mountMarkdownReads(new Map([['tab-md', initial]]), () => pending.promise) + try { + const reading = harness.read() + const saved = readyMarkdown('saved phone draft', { baseVersion: 'v3' }) + harness.docs.set('tab-md', saved) + pending.resolve(readTabResult({ content: '# older server text', version: 'v2' })) + await reading + expect(harness.docs.get('tab-md')).toBe(saved) + } finally { + harness.unmount() + } + }) + + it('keeps a clean document mounted when the reread is unchanged', async () => { + const initial = readyMarkdown('# head') + const harness = mountMarkdownReads(new Map([['tab-md', initial]]), async () => + readTabResult({ content: '# head', version: 'v1', editable: true, readOnlyReason: undefined }) + ) + try { + await harness.read() + expect(harness.published).toEqual([]) + expect(harness.docs.get('tab-md')).toBe(initial) + } finally { + harness.unmount() + } + }) + + it('updates a clean document in place when desktop content changed', async () => { + const initial = readyMarkdown('# head') + const harness = mountMarkdownReads(new Map([['tab-md', initial]]), async () => + readTabResult({ content: '# next', version: 'v2' }) + ) + try { + await harness.read() + expect(harness.published.map((doc) => doc.status)).toEqual(['ready']) + expect(harness.docs.get('tab-md')).toMatchObject({ + status: 'ready', + content: '# next', + localContent: '# next', + baseVersion: 'v2', + isDirty: false + }) + } finally { + harness.unmount() + } + }) + + it('keeps the visible document when a reread fails', async () => { + const initial = readyMarkdown('# head') + const harness = mountMarkdownReads(new Map([['tab-md', initial]]), async () => ({ + id: 'frame-1', + ok: false, + error: { code: 'tab_not_found', message: 'No such tab' }, + _meta: META + })) + try { + await harness.read() + expect(harness.published).toEqual([]) + expect(harness.docs.get('tab-md')).toBe(initial) + } finally { + harness.unmount() + } + }) + + it('drops a reread that loses to an edit made while it was in flight', async () => { + const initial = readyMarkdown('# head') + const pending = deferredReply() + const harness = mountMarkdownReads(new Map([['tab-md', initial]]), () => pending.promise) + try { + const reading = harness.read() + const current = harness.docs.get('tab-md') + if (current?.status !== 'ready') { + throw new Error('Expected the clean document to stay visible') + } + harness.docs.set('tab-md', { ...current, localContent: 'phone draft', isDirty: true }) + pending.resolve(readTabResult({ content: '# server', version: 'v2' })) + await reading + expect(harness.docs.get('tab-md')).toMatchObject({ + status: 'ready', + content: '# head', + localContent: 'phone draft', + isDirty: true + }) + expect(harness.published.every((doc) => doc.status !== 'loading')).toBe(true) + } finally { + harness.unmount() + } + }) + + it('reloads a dirty document instead of keeping the draft', async () => { + const initial = readyMarkdown('# head', { localContent: 'phone draft', isDirty: true }) + const harness = mountMarkdownReads(new Map([['tab-md', initial]]), async () => + readTabResult({ content: '# server', version: 'v2' }) + ) + try { + await harness.read() + expect(harness.published.map((doc) => doc.status)).toEqual(['loading', 'ready']) + expect(harness.docs.get('tab-md')).toMatchObject({ + status: 'ready', + content: '# server', + localContent: '# server', + isDirty: false + }) + } finally { + harness.unmount() + } + }) + + it('keeps the newer reread when an older one finishes later', async () => { + const initial = readyMarkdown('# head') + const older = deferredReply() + let calls = 0 + const harness = mountMarkdownReads(new Map([['tab-md', initial]]), () => { + calls += 1 + return calls === 1 + ? older.promise + : Promise.resolve(readTabResult({ content: '# new', version: 'v2' })) + }) + try { + const first = harness.read() + const second = harness.read() + await second + const current = harness.docs.get('tab-md') + older.resolve(readTabResult({ content: '# old', version: 'v0' })) + await first + expect(harness.docs.get('tab-md')).toBe(current) + expect(current).toMatchObject({ content: '# new', baseVersion: 'v2' }) + expect(harness.published.every((doc) => doc.status !== 'loading')).toBe(true) + } finally { + harness.unmount() + } + }) + + it('updates from disk without unmounting when the desktop renderer is down', async () => { + const initial = readyMarkdown('# head') + const harness = mountMarkdownReads(new Map([['tab-md', initial]]), async (method) => + method === 'files.read' + ? { + id: 'frame-2', + ok: true, + result: { content: '# disk' }, + _meta: META + } + : { + id: 'frame-1', + ok: false, + error: { code: 'runtime_error', message: 'renderer_unavailable' }, + _meta: META + } + ) + try { + await harness.read() + expect(harness.published.map((doc) => doc.status)).toEqual(['ready']) + expect(harness.docs.get('tab-md')).toMatchObject({ + status: 'ready', + content: '# disk', + editable: false + }) + } finally { + harness.unmount() + } + }) + + it('keeps the visible document when disk fallback fails too', async () => { + const initial = readyMarkdown('# head') + const harness = mountMarkdownReads(new Map([['tab-md', initial]]), async () => ({ + id: 'frame-1', + ok: false, + error: { code: 'runtime_error', message: 'renderer_unavailable' }, + _meta: META + })) + try { + await harness.read() + expect(harness.published).toEqual([]) + expect(harness.docs.get('tab-md')).toBe(initial) + } finally { + harness.unmount() + } + }) + + it('does not restore a document closed while a reread was in flight', async () => { + const initial = readyMarkdown('# head') + const pending = deferredReply() + const harness = mountMarkdownReads(new Map([['tab-md', initial]]), () => pending.promise) + try { + const reading = harness.read() + harness.docs.delete('tab-md') + pending.resolve(readTabResult({ content: '# late', version: 'v2' })) + await reading + expect(harness.docs.has('tab-md')).toBe(false) + } finally { + harness.unmount() + } + }) +}) diff --git a/mobile/src/session/use-mobile-session-document-readers.ts b/mobile/src/session/use-mobile-session-document-readers.ts index be9f4d74f6d..49b037f2b97 100644 --- a/mobile/src/session/use-mobile-session-document-readers.ts +++ b/mobile/src/session/use-mobile-session-document-readers.ts @@ -1,4 +1,4 @@ -import { useCallback } from 'react' +import { useCallback, useRef } from 'react' import type { RpcFailure } from '../transport/types' import { resolveMobileFileTabDoc } from '../files/mobile-file-tab-doc' import { filePreviewTextRead } from '../files/mobile-file-preview-operations' @@ -7,18 +7,50 @@ import { buildMarkdownDiskFallbackDoc, shouldReadMarkdownFromDiskAfterReadTabFailure } from './mobile-markdown-disk-fallback' -import type { MobileSessionTab } from './mobile-session-route-types' +import { + claimDocumentRead, + commitDocumentRead, + documentReadIsCurrent, + installDocumentReadPlaceholder, + keepFileVisibleDuringRead, + keepMarkdownVisibleDuringRead, + settleFileRead, + settleMarkdownRead +} from './mobile-session-document-read-settlement' +import type { FileDocState, MarkdownDocState, MobileSessionTab } from './mobile-session-route-types' import type { MobileSessionTabApplicationModel } from './use-mobile-session-tab-application' export function useMobileSessionDocumentReaders(scope: MobileSessionTabApplicationModel) { - const { worktreeId, client, setMarkdownDocs, setFileDocs } = scope + const { worktreeId, client, markdownDocsRef, setMarkdownDocs, setFileDocs } = scope + const markdownReadSeqRef = useRef(new Map()) + const fileReadSeqRef = useRef(new Map()) const readMarkdownTab = useCallback( async (tab: Extract) => { if (!client) { return } - const loading = { status: 'loading' } as const - setMarkdownDocs((prev) => new Map(prev).set(tab.id, loading)) + const seq = claimDocumentRead(markdownReadSeqRef.current, tab.id) + const visibleAtStart = markdownDocsRef.current.get(tab.id) + // Why: swapping a visible document for `loading` unmounts the WebView and drops the caret. + const loading: MarkdownDocState = { status: 'loading' } + setMarkdownDocs((prev) => + installDocumentReadPlaceholder(prev, tab.id, loading, keepMarkdownVisibleDuringRead) + ) + const publish = (nextDoc: MarkdownDocState) => { + setMarkdownDocs((prev) => { + if (!documentReadIsCurrent(markdownReadSeqRef.current, tab.id, seq)) { + return prev + } + return commitDocumentRead( + prev, + tab.id, + loading, + nextDoc, + (current, placeholder, result) => + settleMarkdownRead(current, placeholder, result, visibleAtStart) + ) + }) + } try { const response = await markdownTabRead.request(client, { worktree: `id:${worktreeId}`, @@ -26,23 +58,17 @@ export function useMobileSessionDocumentReaders(scope: MobileSessionTabApplicati }) if (response.ok) { const result = markdownTabRead.interpret(response) - setMarkdownDocs((prev) => - prev.get(tab.id) === loading - ? new Map(prev).set(tab.id, { - status: 'ready', - content: result.content, - localContent: result.content, - baseVersion: result.version, - isDirty: false, - editable: result.editable === true, - stale: result.isDirty, - readOnlyReason: result.readOnlyReason, - ...(result.truncated === true - ? { truncated: true, byteLength: result.byteLength } - : {}) - }) - : prev - ) + publish({ + status: 'ready', + content: result.content, + localContent: result.content, + baseVersion: result.version, + isDirty: false, + editable: result.editable === true, + stale: result.isDirty, + readOnlyReason: result.readOnlyReason, + ...(result.truncated === true ? { truncated: true, byteLength: result.byteLength } : {}) + }) return } if (!shouldReadMarkdownFromDiskAfterReadTabFailure(response as RpcFailure)) { @@ -59,28 +85,19 @@ export function useMobileSessionDocumentReaders(scope: MobileSessionTabApplicati throw new Error('Unable to read markdown') } const fileResult = fallback.value - setMarkdownDocs((prev) => - prev.get(tab.id) === loading - ? new Map(prev).set( - tab.id, - buildMarkdownDiskFallbackDoc({ - content: fileResult.content, - truncated: fileResult.truncated, - byteLength: fileResult.byteLength, - tabIsDirty: tab.isDirty - }) - ) - : prev + publish( + buildMarkdownDiskFallbackDoc({ + content: fileResult.content, + truncated: fileResult.truncated, + byteLength: fileResult.byteLength, + tabIsDirty: tab.isDirty + }) ) } catch (err) { - setMarkdownDocs((prev) => - prev.get(tab.id) === loading - ? new Map(prev).set(tab.id, { - status: 'error', - message: documentReadErrorMessage(err, "Couldn't load markdown") - }) - : prev - ) + publish({ + status: 'error', + message: documentReadErrorMessage(err, "Couldn't load markdown") + }) } }, [client, worktreeId] @@ -91,18 +108,26 @@ export function useMobileSessionDocumentReaders(scope: MobileSessionTabApplicati if (!client) { return } - const loading = { status: 'loading' } as const - setFileDocs((prev) => new Map(prev).set(tab.id, loading)) + const seq = claimDocumentRead(fileReadSeqRef.current, tab.id) + const loading: FileDocState = { status: 'loading' } + setFileDocs((prev) => + installDocumentReadPlaceholder(prev, tab.id, loading, keepFileVisibleDuringRead) + ) + const publish = (nextDoc: FileDocState) => { + setFileDocs((prev) => { + if (!documentReadIsCurrent(fileReadSeqRef.current, tab.id, seq)) { + return prev + } + return commitDocumentRead(prev, tab.id, loading, nextDoc, settleFileRead) + }) + } try { const doc = await resolveMobileFileTabDoc(client, { worktreeId, relativePath: tab.relativePath, diffSource: tab.diffSource }) - // Closed tabs and newer reads release ownership of this reply. - setFileDocs((prev) => - prev.get(tab.id) === loading ? new Map(prev).set(tab.id, doc) : prev - ) + publish(doc) } catch (err) { const previewMessage = documentReadErrorMessage( err, @@ -110,11 +135,7 @@ export function useMobileSessionDocumentReaders(scope: MobileSessionTabApplicati ? "Couldn't load diff preview" : "Couldn't load file preview" ) - setFileDocs((prev) => - prev.get(tab.id) === loading - ? new Map(prev).set(tab.id, { status: 'error', message: previewMessage }) - : prev - ) + publish({ status: 'error', message: previewMessage }) } }, [client, worktreeId] diff --git a/mobile/src/session/use-mobile-session-tab-switching.test.ts b/mobile/src/session/use-mobile-session-tab-switching.test.ts new file mode 100644 index 00000000000..9f698273332 --- /dev/null +++ b/mobile/src/session/use-mobile-session-tab-switching.test.ts @@ -0,0 +1,172 @@ +import { describe, expect, it, vi } from 'vitest' + +vi.mock('../platform/haptics', () => ({ + triggerSelection: vi.fn() +})) +import { hookMount, performHookAction } from '../test-support/rpc-recording/hook-mount' +import { mountFixture } from '../test-support/rpc-recording/recorder-fixture-shape' +import { MobileTerminalDiagnostics } from './mobile-terminal-diagnostics' +import type { FileDocState, MarkdownDocState, MobileSessionTab } from './mobile-session-route-types' +import { isCachedActiveDocumentRetap } from './mobile-session-active-document-retap' +import { useMobileSessionTabSwitching } from './use-mobile-session-tab-switching' + +const markdownTab: MobileSessionTab = { + type: 'markdown', + id: 'md', + title: 'Notes', + filePath: '/Notes.md', + relativePath: 'Notes.md', + isDirty: false, + isActive: true, + documentVersion: 'v1' +} + +const fileTab: MobileSessionTab = { + type: 'file', + id: 'file', + title: 'Main', + filePath: '/Main.ts', + relativePath: 'Main.ts', + isDirty: false, + isActive: true +} + +const readyMarkdown: MarkdownDocState = { + status: 'ready', + content: '# Notes', + localContent: '# Notes', + baseVersion: 'v1', + isDirty: false, + editable: true +} + +function mountSwitching(args: { + activeSessionTabId: string | null + markdownDocs?: Map + fileDocs?: Map +}) { + const readMarkdownTab = vi.fn() + const readFileTab = vi.fn() + const setActiveSessionTabId = vi.fn() + let switching: ReturnType | undefined + const hook = hookMount(() => { + switching = useMobileSessionTabSwitching( + mountFixture[0]>({ + activeSessionTabId: args.activeSessionTabId, + markdownDocs: args.markdownDocs ?? new Map(), + fileDocs: args.fileDocs ?? new Map(), + readMarkdownTab, + readFileTab, + setActiveSessionTabId, + setActiveHandle: vi.fn(), + switchSessionTabRef: { current: null }, + terminalDiagnosticsRef: { current: new MobileTerminalDiagnostics() }, + pendingSelectionRef: { current: null }, + activeSessionTabTypeRef: { current: null }, + activeHandleRef: { current: null }, + client: null, + sessionTabs: [] + }) + ) + }) + hook.mount() + return { + readMarkdownTab, + readFileTab, + setActiveSessionTabId, + switchTo: (tab: MobileSessionTab) => { + performHookAction(() => switching?.switchSessionTab(tab)) + }, + unmount: hook.unmount + } +} + +describe('isCachedActiveDocumentRetap', () => { + it('ignores a retap only while the open markdown or file document is already on screen', () => { + const docs = { + markdownDocs: new Map([['md', readyMarkdown]]), + fileDocs: new Map([ + ['file', { status: 'ready', kind: 'html', content: '

Hi

' }] + ]) + } + expect(isCachedActiveDocumentRetap(markdownTab, 'md', docs)).toBe(true) + expect(isCachedActiveDocumentRetap(fileTab, 'file', docs)).toBe(true) + expect(isCachedActiveDocumentRetap(markdownTab, 'other', docs)).toBe(false) + expect( + isCachedActiveDocumentRetap(markdownTab, 'md', { + ...docs, + markdownDocs: new Map([['md', { status: 'error', message: 'Missing' }]]) + }) + ).toBe(false) + expect( + isCachedActiveDocumentRetap( + { + type: 'terminal', + id: 'term', + title: 'Shell', + terminal: 'pty-1', + isActive: true + }, + 'term', + docs + ) + ).toBe(false) + }) +}) + +describe('useMobileSessionTabSwitching active document retap', () => { + it('does not reload the markdown tab that is already showing', () => { + const harness = mountSwitching({ + activeSessionTabId: 'md', + markdownDocs: new Map([['md', readyMarkdown]]) + }) + try { + harness.switchTo(markdownTab) + expect(harness.readMarkdownTab).not.toHaveBeenCalled() + expect(harness.setActiveSessionTabId).not.toHaveBeenCalled() + } finally { + harness.unmount() + } + }) + + it('does not reload a file tab that is already showing', () => { + const harness = mountSwitching({ + activeSessionTabId: 'file', + fileDocs: new Map([['file', { status: 'loading' }]]) + }) + try { + harness.switchTo(fileTab) + expect(harness.readFileTab).not.toHaveBeenCalled() + expect(harness.setActiveSessionTabId).not.toHaveBeenCalled() + } finally { + harness.unmount() + } + }) + + it('rereads a cached markdown tab when switching back to it', () => { + const harness = mountSwitching({ + activeSessionTabId: 'file', + markdownDocs: new Map([['md', readyMarkdown]]) + }) + try { + harness.switchTo(markdownTab) + expect(harness.readMarkdownTab).toHaveBeenCalledExactlyOnceWith(markdownTab) + expect(harness.setActiveSessionTabId).toHaveBeenCalledExactlyOnceWith('md') + } finally { + harness.unmount() + } + }) + + it('rereads the open markdown tab after its document failed', () => { + const harness = mountSwitching({ + activeSessionTabId: 'md', + markdownDocs: new Map([['md', { status: 'error', message: 'Missing' }]]) + }) + try { + harness.switchTo(markdownTab) + expect(harness.readMarkdownTab).toHaveBeenCalledOnce() + } finally { + harness.unmount() + } + }) +}) diff --git a/mobile/src/session/use-mobile-session-tab-switching.ts b/mobile/src/session/use-mobile-session-tab-switching.ts index 29756ba3d9a..d370265728d 100644 --- a/mobile/src/session/use-mobile-session-tab-switching.ts +++ b/mobile/src/session/use-mobile-session-tab-switching.ts @@ -1,5 +1,6 @@ import { useCallback } from 'react' import { triggerSelection } from '../platform/haptics' +import { isCachedActiveDocumentRetap } from './mobile-session-active-document-retap' import { activateMobileSessionTab } from './mobile-session-tab-activation' import type { MobileSessionTab } from './mobile-session-route-types' import type { MobileSessionKeyboardStateModel } from './use-mobile-session-keyboard-state' @@ -12,7 +13,9 @@ export function useMobileSessionTabSwitching(scope: MobileSessionKeyboardStateMo defaultTerminalHandlesToLiveInput, setActiveHandle, setActiveSessionTabId, + activeSessionTabId, markdownDocs, + fileDocs, terminalUnsubsRef, terminalDiagnosticsRef, activeHandleRef, @@ -72,6 +75,9 @@ export function useMobileSessionTabSwitching(scope: MobileSessionKeyboardStateMo const switchSessionTab = useCallback( (tab: MobileSessionTab) => { + if (isCachedActiveDocumentRetap(tab, activeSessionTabId, { markdownDocs, fileDocs })) { + return + } if (tab.type === 'terminal') { if (typeof tab.terminal === 'string') { switchTab(tab.terminal) @@ -137,7 +143,17 @@ export function useMobileSessionTabSwitching(scope: MobileSessionKeyboardStateMo // Why: tab list lacks a reliable version for desktop clean saves; re-read on revisit unless the phone has a draft. void readMarkdownTab(tab) }, - [client, markdownDocs, readFileTab, readMarkdownTab, switchTab, unsubscribeTerminal, worktreeId] + [ + activeSessionTabId, + client, + fileDocs, + markdownDocs, + readFileTab, + readMarkdownTab, + switchTab, + unsubscribeTerminal, + worktreeId + ] ) // Ref to latest switchSessionTab so fetchSessionTabs can activate a synced browser tab without a dependency cycle. switchSessionTabRef.current = switchSessionTab diff --git a/mobile/src/test-support/rpc-recording/adapters/session-screen-read-mount-adapters.ts b/mobile/src/test-support/rpc-recording/adapters/session-screen-read-mount-adapters.ts index 5afe5253e73..691c356dec9 100644 --- a/mobile/src/test-support/rpc-recording/adapters/session-screen-read-mount-adapters.ts +++ b/mobile/src/test-support/rpc-recording/adapters/session-screen-read-mount-adapters.ts @@ -37,6 +37,11 @@ export function sessionScreenReadMountAdapters( mountFixture[0]>({ worktreeId: WORKSPACE, client, + markdownDocsRef: { + get current() { + return markdownDocs + } + }, setMarkdownDocs: (update) => { markdownDocs = typeof update === 'function' ? update(markdownDocs) : update },