mirror of
https://github.com/stablyai/orca.git
synced 2026-09-23 00:02:29 +00:00
This reverts commit 07e8c851b8.
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`.
268 lines
8.8 KiB
TypeScript
268 lines
8.8 KiB
TypeScript
// @vitest-environment happy-dom
|
|
|
|
import { act } from 'react'
|
|
import { createRoot, type Root } from 'react-dom/client'
|
|
import { afterEach, beforeEach, describe, expect, it, vi, type MockInstance } from 'vitest'
|
|
import type { OpenFile } from '@/store/slices/editor'
|
|
import {
|
|
WORKTREE_OWNER_NOT_READY_ERROR,
|
|
WORKTREE_OWNER_UNREACHABLE_ERROR,
|
|
type FileContent
|
|
} from './editor-panel-content-types'
|
|
import {
|
|
OWNER_NOT_READY_RETRY_DELAY_MS,
|
|
OWNER_NOT_READY_RETRY_LIMIT,
|
|
shouldRetryFileLoadError,
|
|
useEditorPanelFileLoadRetry
|
|
} from './useEditorPanelFileLoadRetry'
|
|
|
|
// Why: the real setFileContents replaces the map; a key the hook deletes must
|
|
// disappear. A merge (Object.assign) would silently keep a stale loadError.
|
|
function replaceFileContents(
|
|
target: Record<string, FileContent>,
|
|
next: Record<string, FileContent>
|
|
): void {
|
|
for (const key of Object.keys(target)) {
|
|
delete target[key]
|
|
}
|
|
Object.assign(target, next)
|
|
}
|
|
|
|
function makeFile(overrides: Partial<OpenFile> = {}): OpenFile {
|
|
return {
|
|
id: 'tab-1',
|
|
filePath: '/home/user/project/src/index.ts',
|
|
relativePath: 'src/index.ts',
|
|
worktreeId: 'repo-ssh::/home/user/project',
|
|
language: 'typescript',
|
|
isDirty: false,
|
|
mode: 'edit',
|
|
...overrides
|
|
}
|
|
}
|
|
|
|
// Drives the real hook with a controllable fileContents store, mirroring how
|
|
// useEditorPanelContentState wires it up.
|
|
function Harness({
|
|
file,
|
|
fileContents,
|
|
attemptsRef,
|
|
isVisible = true,
|
|
loadFileContent,
|
|
setFileContents
|
|
}: {
|
|
file: OpenFile
|
|
fileContents: Record<string, FileContent>
|
|
attemptsRef: { current: Record<string, number> }
|
|
isVisible?: boolean
|
|
loadFileContent: (filePath: string, id: string) => Promise<void>
|
|
setFileContents: (
|
|
updater: (prev: Record<string, FileContent>) => Record<string, FileContent>
|
|
) => void
|
|
}): null {
|
|
useEditorPanelFileLoadRetry({
|
|
activeFile: isVisible ? file : null,
|
|
fileContents,
|
|
fileLoadRetryAttemptsRef: attemptsRef,
|
|
loadFileContent: loadFileContent as never,
|
|
openFilesRef: { current: [file] },
|
|
setFileContents: setFileContents as never
|
|
})
|
|
return null
|
|
}
|
|
|
|
describe('useEditorPanelFileLoadRetry — owner-not-ready bounding (#6648)', () => {
|
|
let container: HTMLDivElement | null = null
|
|
let root: Root | null = null
|
|
let setTimeoutSpy: MockInstance
|
|
|
|
beforeEach(() => {
|
|
vi.useFakeTimers()
|
|
// Run scheduled retries immediately so we can exhaust the budget without
|
|
// waiting ~2 minutes of real time.
|
|
setTimeoutSpy = vi.spyOn(window, 'setTimeout').mockImplementation(((fn: () => void) => {
|
|
fn()
|
|
return 0 as unknown as ReturnType<typeof setTimeout>
|
|
}) as typeof window.setTimeout)
|
|
})
|
|
|
|
afterEach(() => {
|
|
if (root) {
|
|
act(() => root?.unmount())
|
|
}
|
|
container?.remove()
|
|
container = null
|
|
root = null
|
|
setTimeoutSpy.mockRestore()
|
|
vi.useRealTimers()
|
|
})
|
|
|
|
it('classifies retryable vs terminal errors', () => {
|
|
expect(shouldRetryFileLoadError(WORKTREE_OWNER_NOT_READY_ERROR)).toBe(true)
|
|
expect(shouldRetryFileLoadError(WORKTREE_OWNER_UNREACHABLE_ERROR)).toBe(false)
|
|
expect(shouldRetryFileLoadError('Access denied: outside allowed directories')).toBe(false)
|
|
})
|
|
|
|
it('does not spend retry budget when hiding cancels a pending retry', () => {
|
|
setTimeoutSpy.mockRestore()
|
|
setTimeoutSpy = vi.spyOn(window, 'setTimeout')
|
|
const file = makeFile()
|
|
const attemptsRef = { current: {} as Record<string, number> }
|
|
const fileContents: Record<string, FileContent> = {
|
|
[file.id]: { content: '', isBinary: false, loadError: WORKTREE_OWNER_NOT_READY_ERROR }
|
|
}
|
|
const loadFileContent = vi.fn(async () => undefined)
|
|
const setFileContents = vi.fn((updater) => updater(fileContents))
|
|
|
|
container = document.createElement('div')
|
|
document.body.appendChild(container)
|
|
root = createRoot(container)
|
|
|
|
act(() => {
|
|
root?.render(
|
|
<Harness
|
|
file={file}
|
|
fileContents={fileContents}
|
|
attemptsRef={attemptsRef}
|
|
loadFileContent={loadFileContent}
|
|
setFileContents={setFileContents as never}
|
|
/>
|
|
)
|
|
})
|
|
expect(loadFileContent).not.toHaveBeenCalled()
|
|
expect(attemptsRef.current[file.id]).toBeUndefined()
|
|
|
|
act(() => {
|
|
root?.render(
|
|
<Harness
|
|
file={file}
|
|
fileContents={fileContents}
|
|
attemptsRef={attemptsRef}
|
|
isVisible={false}
|
|
loadFileContent={loadFileContent}
|
|
setFileContents={setFileContents as never}
|
|
/>
|
|
)
|
|
})
|
|
act(() => vi.advanceTimersByTime(OWNER_NOT_READY_RETRY_DELAY_MS))
|
|
expect(loadFileContent).not.toHaveBeenCalled()
|
|
expect(attemptsRef.current[file.id]).toBeUndefined()
|
|
|
|
act(() => {
|
|
root?.render(
|
|
<Harness
|
|
file={file}
|
|
fileContents={fileContents}
|
|
attemptsRef={attemptsRef}
|
|
loadFileContent={loadFileContent}
|
|
setFileContents={setFileContents as never}
|
|
/>
|
|
)
|
|
})
|
|
act(() => vi.advanceTimersByTime(OWNER_NOT_READY_RETRY_DELAY_MS))
|
|
expect(loadFileContent).toHaveBeenCalledOnce()
|
|
expect(attemptsRef.current[file.id]).toBe(1)
|
|
})
|
|
|
|
it('stops after the budget and shows a truthful terminal message, then Retry re-arms', () => {
|
|
const file = makeFile()
|
|
const attemptsRef = { current: {} as Record<string, number> }
|
|
// The owner never hydrates: every retry re-fails with owner-not-ready.
|
|
const fileContents: Record<string, FileContent> = {
|
|
[file.id]: { content: '', isBinary: false, loadError: WORKTREE_OWNER_NOT_READY_ERROR }
|
|
}
|
|
const setFileContents = (
|
|
updater: (prev: Record<string, FileContent>) => Record<string, FileContent>
|
|
): void => {
|
|
replaceFileContents(fileContents, updater(fileContents))
|
|
}
|
|
// loadFileContent (the retry callback) clears then re-fails as owner-not-ready.
|
|
const loadFileContent = vi.fn(async (_filePath: string, id: string) => {
|
|
fileContents[id] = { content: '', isBinary: false, loadError: WORKTREE_OWNER_NOT_READY_ERROR }
|
|
})
|
|
|
|
container = document.createElement('div')
|
|
document.body.appendChild(container)
|
|
root = createRoot(container)
|
|
|
|
// Re-render the hook repeatedly; each render that still sees the
|
|
// owner-not-ready error schedules (and our spy immediately runs) one retry.
|
|
for (let i = 0; i < OWNER_NOT_READY_RETRY_LIMIT + 2; i++) {
|
|
act(() => {
|
|
root?.render(
|
|
<Harness
|
|
file={file}
|
|
fileContents={{ ...fileContents }}
|
|
attemptsRef={attemptsRef}
|
|
loadFileContent={loadFileContent}
|
|
setFileContents={setFileContents}
|
|
/>
|
|
)
|
|
})
|
|
if (fileContents[file.id]?.loadError === WORKTREE_OWNER_UNREACHABLE_ERROR) {
|
|
break
|
|
}
|
|
}
|
|
|
|
// Budget honored: retried at most the limit, then went terminal.
|
|
expect(loadFileContent.mock.calls.length).toBeLessThanOrEqual(OWNER_NOT_READY_RETRY_LIMIT)
|
|
expect(fileContents[file.id]?.loadError).toBe(WORKTREE_OWNER_UNREACHABLE_ERROR)
|
|
|
|
// The terminal error is not auto-retried.
|
|
const callsAfterTerminal = loadFileContent.mock.calls.length
|
|
act(() => {
|
|
root?.render(
|
|
<Harness
|
|
file={file}
|
|
fileContents={{ ...fileContents }}
|
|
attemptsRef={attemptsRef}
|
|
loadFileContent={loadFileContent}
|
|
setFileContents={setFileContents}
|
|
/>
|
|
)
|
|
})
|
|
expect(loadFileContent.mock.calls.length).toBe(callsAfterTerminal)
|
|
|
|
// Retry (reloadContent) clears the attempt budget for a fresh start.
|
|
delete attemptsRef.current[file.id]
|
|
expect(attemptsRef.current[file.id]).toBeUndefined()
|
|
})
|
|
|
|
it('stops immediately once the read succeeds (no terminal message)', () => {
|
|
const file = makeFile()
|
|
const attemptsRef = { current: {} as Record<string, number> }
|
|
const fileContents: Record<string, FileContent> = {
|
|
[file.id]: { content: '', isBinary: false, loadError: WORKTREE_OWNER_NOT_READY_ERROR }
|
|
}
|
|
const setFileContents = (
|
|
updater: (prev: Record<string, FileContent>) => Record<string, FileContent>
|
|
): void => {
|
|
replaceFileContents(fileContents, updater(fileContents))
|
|
}
|
|
// The repo hydrates on the first retry: the read now succeeds.
|
|
const loadFileContent = vi.fn(async (_filePath: string, id: string) => {
|
|
fileContents[id] = { content: 'remote', isBinary: false }
|
|
})
|
|
|
|
container = document.createElement('div')
|
|
document.body.appendChild(container)
|
|
root = createRoot(container)
|
|
|
|
act(() => {
|
|
root?.render(
|
|
<Harness
|
|
file={file}
|
|
fileContents={{ ...fileContents }}
|
|
attemptsRef={attemptsRef}
|
|
loadFileContent={loadFileContent}
|
|
setFileContents={setFileContents}
|
|
/>
|
|
)
|
|
})
|
|
|
|
expect(loadFileContent).toHaveBeenCalledTimes(1)
|
|
expect(fileContents[file.id]?.loadError).toBeUndefined()
|
|
expect(fileContents[file.id]?.content).toBe('remote')
|
|
})
|
|
})
|