From 97c682c7b7e1750b76668bc7cc2cc115cbf375ee Mon Sep 17 00:00:00 2001 From: Neil Date: Thu, 17 Sep 2026 22:03:51 -0700 Subject: [PATCH] Revert "fix(editor): evict stale mirrored file tabs (#21363)" This reverts commit 07e8c851b8b03651468459d9a63c285329e6e105. The eviction keys on `selector_not_found`, which this repo documents twice as UNKNOWN rather than absence: - `remote-browser-stream-errors.ts`: "it means 'I could not resolve this right now', which is UNKNOWN, not proof the target is gone. Its producer is a live worktree scan behind a 1s-TTL cache ... a slow scan can surface it transiently. Treating that as permanent would strand the pane forever, which is the exact bug this file exists to prevent." - `web-runtime-session-tab-lifecycle.ts`, added by #21277: "'selector_not_found' is a transient worktree resolver state (e.g. during scans or cache warm-up) and must not become a durable close tombstone." Two unambiguous absence codes exist for this purpose -- `tab_not_found` and `terminal_tab_not_found` -- and #21277 had just finished excluding `selector_not_found` from them. This keyed on the excluded one. Consequences, after roughly 3.75s of retries: 1. `closeFile` deletes `editorDrafts[fileId]` with no dirty check and no confirmation, so a transient resolver blip discards unsaved edits. 2. `closeFile` calls `notifyHostOfMirroredEditorClose`, so the host closes its copy too -- the eviction is not local and not recoverable. The `!ownerNotReady` guard does not cover this: `ownerNotReady` means the host is still connecting, while `selector_not_found` is emitted for a cold resolver cache or an unhydrated catalog, which is a different state. #21041 is still open. A correct fix keys on the two definitive absence codes, refuses to evict a tab that has a draft, and has a test proving a dirty mirrored tab survives `selector_not_found`. --- .../editor/useEditorPanelContentState.ts | 3 +- .../useEditorPanelFileLoadRetry.test.tsx | 32 ------------------- .../editor/useEditorPanelFileLoadRetry.ts | 19 ----------- 3 files changed, 1 insertion(+), 53 deletions(-) diff --git a/src/renderer/src/components/editor/useEditorPanelContentState.ts b/src/renderer/src/components/editor/useEditorPanelContentState.ts index 93c33d0961c..ec1a0fc6b1b 100644 --- a/src/renderer/src/components/editor/useEditorPanelContentState.ts +++ b/src/renderer/src/components/editor/useEditorPanelContentState.ts @@ -1,6 +1,6 @@ import { useCallback, useLayoutEffect, useRef, useState } from 'react' import type { OpenFile } from '@/store/slices/editor' -import { useAppStore } from '@/store' +import type { useAppStore } from '@/store' import type { DiffContent, FileContent } from './editor-panel-content-types' import { useEditorPanelExternalContentEvents, @@ -194,7 +194,6 @@ export function useEditorPanelContentState({ fileLoadRetryAttemptsRef, loadFileContent, openFilesRef, - closeFile: useAppStore.getState().closeFile, setFileContents }) diff --git a/src/renderer/src/components/editor/useEditorPanelFileLoadRetry.test.tsx b/src/renderer/src/components/editor/useEditorPanelFileLoadRetry.test.tsx index fe40142c645..6a29017d654 100644 --- a/src/renderer/src/components/editor/useEditorPanelFileLoadRetry.test.tsx +++ b/src/renderer/src/components/editor/useEditorPanelFileLoadRetry.test.tsx @@ -49,7 +49,6 @@ function Harness({ attemptsRef, isVisible = true, loadFileContent, - closeFile = vi.fn(), setFileContents }: { file: OpenFile @@ -57,7 +56,6 @@ function Harness({ attemptsRef: { current: Record } isVisible?: boolean loadFileContent: (filePath: string, id: string) => Promise - closeFile?: (fileId: string) => void setFileContents: ( updater: (prev: Record) => Record ) => void @@ -68,7 +66,6 @@ function Harness({ fileLoadRetryAttemptsRef: attemptsRef, loadFileContent: loadFileContent as never, openFilesRef: { current: [file] }, - closeFile, setFileContents: setFileContents as never }) return null @@ -106,35 +103,6 @@ describe('useEditorPanelFileLoadRetry — owner-not-ready bounding (#6648)', () expect(shouldRetryFileLoadError('Access denied: outside allowed directories')).toBe(false) }) - it('evicts a mirrored tab after selector resolution stays missing', () => { - const file = makeFile({ mirroredFromRuntimeSession: true }) - const attemptsRef = { current: { [file.id]: 3 } } - const closeFile = vi.fn() - const fileContents: Record = { - [file.id]: { content: '', isBinary: false, loadError: 'selector_not_found' } - } - - container = document.createElement('div') - document.body.appendChild(container) - root = createRoot(container) - act(() => { - root?.render( - undefined)} - closeFile={closeFile} - setFileContents={(updater) => { - updater(fileContents) - }} - /> - ) - }) - - expect(closeFile).toHaveBeenCalledWith(file.id) - }) - it('does not spend retry budget when hiding cancels a pending retry', () => { setTimeoutSpy.mockRestore() setTimeoutSpy = vi.spyOn(window, 'setTimeout') diff --git a/src/renderer/src/components/editor/useEditorPanelFileLoadRetry.ts b/src/renderer/src/components/editor/useEditorPanelFileLoadRetry.ts index 57a9483218e..9fb96ad80aa 100644 --- a/src/renderer/src/components/editor/useEditorPanelFileLoadRetry.ts +++ b/src/renderer/src/components/editor/useEditorPanelFileLoadRetry.ts @@ -7,7 +7,6 @@ import { } from './editor-panel-content-types' const FILE_LOAD_RETRY_DELAYS_MS = [250, 1000, 2500] -const noopCloseFile = (): void => {} // Why: a remote host can take a while to finish connecting. The owner-not-ready // check is a pure local store read (it throws before any network call until the // SSH repo hydrates), so poll it at a steady cadence — but cap the wait so a @@ -31,14 +30,9 @@ type UseEditorPanelFileLoadRetryParams = { relativePath?: string ) => Promise openFilesRef: MutableRefObject - closeFile?: (fileId: string) => void setFileContents: Dispatch>> } -function isSelectorNotFoundError(message: string): boolean { - return message.trim().toLowerCase() === 'selector_not_found' -} - export function shouldRetryFileLoadError(message: string): boolean { // Terminal: the owner-not-ready budget is spent; only an explicit Retry should // restart it, never the automatic backoff. @@ -60,7 +54,6 @@ export function useEditorPanelFileLoadRetry({ fileLoadRetryAttemptsRef, loadFileContent, openFilesRef, - closeFile = noopCloseFile, setFileContents }: UseEditorPanelFileLoadRetryParams): void { const activeFileLoadRetryId = activeFile?.id ?? null @@ -82,16 +75,6 @@ export function useEditorPanelFileLoadRetry({ ? OWNER_NOT_READY_RETRY_LIMIT : FILE_LOAD_RETRY_DELAYS_MS.length if (retryCount >= retryLimit) { - if ( - !ownerNotReady && - isSelectorNotFoundError(activeFileLoadError) && - activeFile?.mirroredFromRuntimeSession === true - ) { - // A host-mirrored file whose worktree stays unresolvable after the normal - // read retries is stale; evict it before snapshots can select it again. - closeFile(activeFileLoadRetryId) - return - } // Why: the remote host never finished connecting. Replace the transient // "still connecting" text with a truthful terminal message so it does not // look like it is still retrying; Retry starts a fresh budget (#6648). @@ -143,8 +126,6 @@ export function useEditorPanelFileLoadRetry({ }, [ activeFileLoadRetryId, activeFileLoadError, - activeFile?.mirroredFromRuntimeSession, - closeFile, fileLoadRetryAttemptsRef, loadFileContent, openFilesRef,