diff --git a/src/renderer/src/store/slices/editor-markdown-preview-read-authority.test.ts b/src/renderer/src/store/slices/editor-markdown-preview-read-authority.test.ts new file mode 100644 index 00000000000..5298a7b5890 --- /dev/null +++ b/src/renderer/src/store/slices/editor-markdown-preview-read-authority.test.ts @@ -0,0 +1,129 @@ +import { describe, expect, it, vi } from 'vitest' +import { createEditorStore } from './editor-slice-test-harness' +import type { OpenFile } from './editor' +import { isClientLocalReadOnlyTab } from '@/lib/local-file-access' + +vi.mock('sonner', () => ({ toast: { error: vi.fn() } })) +vi.mock('@/runtime/close-mirrored-editor-tab', () => ({ + notifyHostOfMirroredEditorClose: vi.fn() +})) + +const PREVIEW_INPUT = { + filePath: '/tmp/task/brief.md', + relativePath: '/tmp/task/brief.md', + worktreeId: 'wt-1', + runtimeEnvironmentId: 'env-1', + language: 'markdown' +} + +function grantedSourceTab(grantId: string): OpenFile { + return { + id: 'brief-source', + ...PREVIEW_INPUT, + isDirty: false, + readOnly: true, + terminalArtifactGrantId: grantId, + mode: 'edit' + } +} + +describe('markdown preview read authority (#31034)', () => { + it('reads a terminal-opened host file with its source tab grant', () => { + const store = createEditorStore() + store.setState({ openFiles: [grantedSourceTab('grant-1')] }) + + store.getState().openMarkdownPreview(PREVIEW_INPUT, { sourceFileId: 'brief-source' }) + + expect(store.getState().openFiles.at(-1)).toMatchObject({ + id: 'markdown-preview::brief-source', + mode: 'markdown-preview', + runtimeEnvironmentId: 'env-1', + terminalArtifactGrantId: 'grant-1', + readOnly: true + }) + }) + + it('moves an open preview to the source tab fresh grant', () => { + const store = createEditorStore() + store.setState({ openFiles: [grantedSourceTab('grant-1')] }) + store.getState().openMarkdownPreview(PREVIEW_INPUT, { sourceFileId: 'brief-source' }) + + store.setState({ + openFiles: [ + grantedSourceTab('grant-2'), + ...store.getState().openFiles.filter((file) => file.id !== 'brief-source') + ] + }) + store.getState().openMarkdownPreview(PREVIEW_INPUT, { sourceFileId: 'brief-source' }) + + const previews = store + .getState() + .openFiles.filter((file) => file.id === 'markdown-preview::brief-source') + expect(previews).toEqual([ + expect.objectContaining({ terminalArtifactGrantId: 'grant-2', fileContentReloadNonce: 1 }) + ]) + }) + + it('keeps a client-local file preview on this computer in an SSH workspace', () => { + const store = createEditorStore() + store.setState({ + openFiles: [ + { + id: 'local-notes', + filePath: '/Users/me/notes.md', + relativePath: '/Users/me/notes.md', + worktreeId: 'wt-ssh', + runtimeEnvironmentId: null, + language: 'markdown', + isDirty: false, + readOnly: true, + mode: 'edit' + } + ] + }) + + store.getState().openMarkdownPreview( + { + filePath: '/Users/me/notes.md', + relativePath: '/Users/me/notes.md', + worktreeId: 'wt-ssh', + runtimeEnvironmentId: null, + language: 'markdown' + }, + { sourceFileId: 'local-notes' } + ) + + expect(store.getState().openFiles.at(-1)).toMatchObject({ + mode: 'markdown-preview', + runtimeEnvironmentId: null, + readOnly: true + }) + const preview = store.getState().openFiles.at(-1) + expect(preview?.terminalArtifactGrantId).toBeUndefined() + expect(preview && isClientLocalReadOnlyTab(preview)).toBe(true) + }) + + it('keeps the grant when a preview is retargeted after its source tab closed', () => { + const store = createEditorStore() + store.setState({ openFiles: [grantedSourceTab('grant-1')] }) + store.getState().openMarkdownPreview(PREVIEW_INPUT, { sourceFileId: 'brief-source' }) + store.setState({ + openFiles: store.getState().openFiles.filter((file) => file.id !== 'brief-source') + }) + + store + .getState() + .openMarkdownPreview(PREVIEW_INPUT, { sourceFileId: 'brief-source', anchor: 'plan' }) + + const [retargeted] = store.getState().openFiles + expect(retargeted?.fileContentReloadNonce).toBeUndefined() + expect(store.getState().openFiles).toEqual([ + expect.objectContaining({ + id: 'markdown-preview::brief-source', + markdownPreviewAnchor: 'plan', + terminalArtifactGrantId: 'grant-1', + readOnly: true + }) + ]) + }) +}) diff --git a/src/renderer/src/store/slices/editor/actions/markdown-preview-actions.ts b/src/renderer/src/store/slices/editor/actions/markdown-preview-actions.ts index 8872ccdb7d5..095831b399c 100644 --- a/src/renderer/src/store/slices/editor/actions/markdown-preview-actions.ts +++ b/src/renderer/src/store/slices/editor/actions/markdown-preview-actions.ts @@ -20,6 +20,23 @@ import { recordPromotedEditorTabsInLayoutOwner } from '../tabs/editor-tab-layout-owner' +type FileReadAuthority = Pick< + OpenFile, + 'externalSshTargetId' | 'terminalArtifactGrantId' | 'readOnly' +> + +/** A preview reads its source's file, so it must read with the same host, grant and client-local flag (#31034). */ +function inheritFileReadAuthority( + file: FileReadAuthority, + source: FileReadAuthority | undefined +): FileReadAuthority { + return { + externalSshTargetId: file.externalSshTargetId ?? source?.externalSshTargetId, + terminalArtifactGrantId: file.terminalArtifactGrantId ?? source?.terminalArtifactGrantId, + readOnly: file.readOnly === true || source?.readOnly === true ? true : undefined + } +} + export function createMarkdownPreviewActions( set: EditorSet, get: EditorGet @@ -96,9 +113,12 @@ export function createMarkdownPreviewActions( ['edit'] ) const id = `markdown-preview::${sourceFileId}` - const externalSshTargetId = - file.externalSshTargetId ?? - initialState.openFiles.find((openFile) => openFile.id === sourceFileId)?.externalSshTargetId + // Why: with the source tab closed, a retarget keeps the authority the preview already reads with. + const { externalSshTargetId, terminalArtifactGrantId, readOnly } = inheritFileReadAuthority( + file, + initialState.openFiles.find((openFile) => openFile.id === sourceFileId) ?? + initialState.openFiles.find((openFile) => openFile.id === id) + ) const anchor = options?.anchor || undefined set((s) => { const existing = s.openFiles.find((openFile) => openFile.id === id) @@ -107,11 +127,17 @@ export function createMarkdownPreviewActions( const activeResult = buildEditorActiveResult(s, worktreeId, id) if (existing) { + // Why: a fresh grant means the file was re-resolved; cached bytes may predate it. + const grantChanged = + terminalArtifactGrantId !== undefined && + existing.terminalArtifactGrantId !== terminalArtifactGrantId const needsUpdate = existing.relativePath !== file.relativePath || existing.filePath !== file.filePath || existing.language !== file.language || existing.externalSshTargetId !== externalSshTargetId || + existing.terminalArtifactGrantId !== terminalArtifactGrantId || + existing.readOnly !== readOnly || existing.markdownPreviewSourceFileId !== sourceFileId || existing.markdownPreviewAnchor !== anchor || existing.mode !== 'markdown-preview' @@ -127,6 +153,11 @@ export function createMarkdownPreviewActions( language: file.language, runtimeEnvironmentId, externalSshTargetId, + terminalArtifactGrantId, + readOnly, + ...(grantChanged + ? { fileContentReloadNonce: (openFile.fileContentReloadNonce ?? 0) + 1 } + : {}), markdownPreviewSourceFileId: sourceFileId, markdownPreviewAnchor: anchor, mode: 'markdown-preview' as const @@ -147,6 +178,8 @@ export function createMarkdownPreviewActions( isDirty: false, runtimeEnvironmentId, externalSshTargetId, + terminalArtifactGrantId, + readOnly, markdownPreviewSourceFileId: sourceFileId, markdownPreviewAnchor: anchor, mode: 'markdown-preview' diff --git a/src/renderer/src/store/slices/editor/types/editor-files-slice.ts b/src/renderer/src/store/slices/editor/types/editor-files-slice.ts index f062e6914a5..0f7dcada607 100644 --- a/src/renderer/src/store/slices/editor/types/editor-files-slice.ts +++ b/src/renderer/src/store/slices/editor/types/editor-files-slice.ts @@ -78,7 +78,8 @@ export type EditorFilesSlice = { | 'language' | 'runtimeEnvironmentId' | 'externalSshTargetId' - >, + > & + Partial>, options?: { anchor?: string | null; targetGroupId?: string; sourceFileId?: string } ) => void makePreviewFilePermanent: (fileId: string, tabId?: string) => void