Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -0,0 +1,180 @@
// @vitest-environment happy-dom
import { act, useLayoutEffect, useRef, useState } from 'react'
import { createRoot } from 'react-dom/client'
import { afterEach, describe, expect, it, vi } from 'vitest'
import { useAppStore } from '@/store'
import { makeWorktree, TEST_REPO } from '@/store/slices/store-test-helpers'
import type { OpenFile } from '@/store/slices/editor'
import type * as RuntimeFileClient from '@/runtime/runtime-file-client'
import { createGlobalSettingsFixture } from '../../../../shared/global-settings-test-fixture'
import type { ExecutionHostId } from '../../../../shared/execution-host'
import type { FileContent } from './editor-panel-content-types'
import { createEditorSaveQueue } from './editor-save-queue'
import {
useEditorPanelFileContentLoader,
type EditorPanelFileContentLoader
} from './useEditorPanelFileContentLoader'

const calls = vi.hoisted(() => ({ read: vi.fn(), write: vi.fn() }))
vi.mock('@/runtime/runtime-file-client', async (importOriginal) => ({
...(await importOriginal<typeof RuntimeFileClient>()),
readRuntimeFileContent: calls.read,
writeRuntimeFile: calls.write
}))

function deferred<T>() {
let resolve!: (value: T) => void
const promise = new Promise<T>((done) => {
resolve = done
})
return { promise, resolve }
}

function ReadProbe({
file,
ready
}: {
file: OpenFile
ready: (load: EditorPanelFileContentLoader) => void
}) {
const [, setFileContents] = useState<Record<string, FileContent>>({})
const load = useEditorPanelFileContentLoader({
fileLoadRetryAttemptsRef: useRef({}),
fileReadGenerationCounterRef: useRef(0),
fileReadGenerationRef: useRef({}),
openFilesRef: useRef([file]),
outstandingFileReadsRef: useRef({}),
setFileContents
})
useLayoutEffect(() => {
ready(load)
}, [load, ready])
return null
}

afterEach(() => {
useAppStore.setState(useAppStore.getInitialState(), true)
calls.read.mockReset()
calls.write.mockReset()
})

describe('editor operations across host focus changes', () => {
it.each([
{ host: 'local', environment: null, connection: undefined },
{ host: 'runtime:file-host', environment: 'file-host', connection: undefined },
{ host: 'ssh:file-target', environment: null, connection: 'file-target' }
] satisfies {
host: ExecutionHostId
environment: string | null
connection: string | undefined
}[])('keeps delayed read and save on $host', async ({ host, environment, connection }) => {
globalThis.IS_REACT_ACT_ENVIRONMENT = true
useAppStore.setState(useAppStore.getInitialState(), true)
const worktreeId = 'repo1::/workspace/repo'
useAppStore.setState({
repos: [{ ...TEST_REPO, path: '/workspace/repo', connectionId: connection }],
worktreesByRepo: {
repo1: [
makeWorktree({
id: worktreeId,
repoId: 'repo1',
path: '/workspace/repo',
...(host === 'local' ? {} : { hostId: host }),
...(environment ? { runtimeOwnerEnvironmentId: environment } : {})
})
]
},
settings: createGlobalSettingsFixture({ activeRuntimeEnvironmentId: null }),
activeWorktreeId: worktreeId,
activeWorkspaceExecutionHostId: 'runtime:other-host',
sshConnectionStates: new Map([
[
'file-target',
{
targetId: 'file-target',
status: 'connected',
error: null,
reconnectAttempt: 0,
connectionGeneration: 7
}
]
]),
recordFeatureInteraction: vi.fn()
})
const fileId = useAppStore.getState().openFile({
filePath: '/workspace/repo/file.txt',
relativePath: 'file.txt',
worktreeId,
language: 'plaintext',
mode: 'edit'
})
const file = useAppStore.getState().openFiles.find((entry) => entry.id === fileId)
if (!file) {
throw new Error('Expected the opened file')
}
const read = deferred<FileContent>()
const write = deferred<void>()
calls.read.mockReturnValueOnce(read.promise)
calls.write.mockReturnValueOnce(write.promise)
const container = document.body.appendChild(document.createElement('div'))
const root = createRoot(container)
const queue = createEditorSaveQueue(useAppStore)
const probe: { load?: EditorPanelFileContentLoader } = {}
try {
act(() =>
root.render(
<ReadProbe
file={file}
ready={(load) => {
probe.load = load
}}
/>
)
)
const load = probe.load
if (!load) {
throw new Error('Expected the content loader')
}
const pendingRead = load(file.filePath, file.id, worktreeId, file.relativePath)
await vi.waitFor(() => expect(calls.read).toHaveBeenCalledTimes(1))
useAppStore.setState({
activeWorkspaceExecutionHostId: 'runtime:third-host',
settings: createGlobalSettingsFixture({ activeRuntimeEnvironmentId: 'third-host' })
})
expect(calls.read.mock.calls[0]?.[0]).toMatchObject({
settings: { activeRuntimeEnvironmentId: environment },
connectionId: connection
})
await act(async () => {
read.resolve({ content: 'original', isBinary: false })
await pendingRead
})
useAppStore.getState().setEditorDraft(fileId, 'saved draft')
useAppStore.getState().markFileDirty(fileId, true)
const pendingSave = queue.queueSave(file, 'saved draft')
await vi.waitFor(() => expect(calls.write).toHaveBeenCalledTimes(1))
useAppStore.setState({ activeWorkspaceExecutionHostId: 'runtime:fourth-host' })
expect(calls.write.mock.calls[0]?.[0]).toMatchObject({
settings: { activeRuntimeEnvironmentId: environment },
...(connection ? { connectionId: connection } : {}),
expectedExecutionHostId: connection ? host : 'local'
})
expect(calls.write.mock.calls[0]?.slice(1, 3)).toEqual([file.filePath, 'saved draft'])
write.resolve()
await pendingSave
expect(useAppStore.getState().openFiles.find((entry) => entry.id === fileId)?.isDirty).toBe(
false
)
expect(
useAppStore
.getState()
.unifiedTabsByWorktree[worktreeId]?.find((tab) => tab.entityId === fileId)
?.executionHostId
).toBe(host)
} finally {
queue.dispose()
act(() => root.unmount())
container.remove()
}
})
})
26 changes: 26 additions & 0 deletions src/renderer/src/lib/editor-file-operation-owner.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,8 @@ import {
getEditorFileOperationContext
} from './editor-file-operation-owner'
import { FLOATING_TERMINAL_WORKTREE_ID } from '../../../shared/constants'
import { makeWorktree, TEST_REPO } from '../store/slices/store-test-helpers'
import { createGlobalSettingsFixture } from '../../../shared/global-settings-test-fixture'

const worktreeId = 'repo::/remote/repo'

Expand Down Expand Up @@ -93,6 +95,30 @@ describe('editor file operation owner', () => {
expect(context.settings?.activeRuntimeEnvironmentId).toBe('hub-a')
})

it('keeps an unstamped local worktree local while another host is focused', () => {
useAppStore.setState({
repos: [{ ...TEST_REPO, id: 'repo', path: '/local/repo', displayName: 'Local repo' }],
worktreesByRepo: {
repo: [makeWorktree({ id: worktreeId, repoId: 'repo', path: '/local/repo' })]
},
activeWorktreeId: worktreeId,
activeWorkspaceExecutionHostId: 'runtime:hub-b',
settings: createGlobalSettingsFixture({ activeRuntimeEnvironmentId: null })
})

const provenance = captureEditorFileOperationProvenance(
useAppStore.getState(),
worktreeId,
undefined,
false
)

expect(provenance.generation.route).toEqual({
executionHostId: 'local',
runtimeEnvironmentId: null
})
})

it('rejects an open tab after the same environment id is re-paired', () => {
useAppStore.getState().setRuntimeEnvironments([runtimeEnvironment('hub-a', 1)])
useAppStore.setState({
Expand Down
21 changes: 19 additions & 2 deletions src/renderer/src/lib/editor-file-operation-owner.ts
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,8 @@ export type EditorFileOperationProvenance = {
type EditorOwnerState = Pick<
AppState,
| 'settings'
| 'activeWorktreeId'
| 'activeWorkspaceExecutionHostId'
| 'repos'
| 'worktreesByRepo'
| 'detectedWorktreesByRepo'
Expand Down Expand Up @@ -66,7 +68,7 @@ export function captureEditorFileOperationProvenance(
: ('local' as const),
runtimeEnvironmentId: hintedRuntimeEnvironmentId
}
: resolveWorktreeOperationRoute(state, worktreeId)
: resolveLegacyEditorFileRoute(state, worktreeId)
if (!route || (ownerHintProvided && (ownerHint?.trim() || null) !== route.runtimeEnvironmentId)) {
throw new Error(OWNER_CHANGED_MESSAGE)
}
Expand All @@ -78,6 +80,21 @@ export function captureEditorFileOperationProvenance(
}
}

function resolveLegacyEditorFileRoute(
state: EditorOwnerState,
worktreeId: string
): WorktreeOperationRoute | null {
// Why: a focused workspace selects where the UI is looking, not which host owns a file.
return resolveWorktreeOperationRoute(
{
...state,
activeWorktreeId: null,
activeWorkspaceExecutionHostId: null
},
worktreeId
)
}
Comment on lines +83 to +96

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nulling the active host here also removes the only disambiguator for a folder workspace id that exists on more than one host, so editor capture for such a folder now throws instead of resolving.

Technical details
# Ambiguous folder ids lose their only tie-breaker

Today a folder workspace keyed `folder:<uuid>` can carry two rows on different hosts (local + a runtime/SSH peer); this is supported and exercised by `src/renderer/src/store/slices/selected-host-active-workspace-identity.test.ts:114` and the collision contracts in `folder-workspace-owner-routed-mutations.test.ts`. For a folder key `resolveExplicitWorktreeOperationRouteResult` is always `missing` (it only scans worktrees/repos), so the legacy branch is the only resolver.

## Affected sites
- `editor-file-operation-owner.ts:83-96` — `resolveLegacyEditorFileRoute` sets `activeWorktreeId`/`activeWorkspaceExecutionHostId` to `null`.
- `editor-file-operation-owner.ts:71` — `captureEditorFileOperationProvenance` uses it for the no-hint branch; a `null` route throws `OWNER_CHANGED_MESSAGE` at `:72`.
- `editor-file-operation-owner.ts:135` — `resolveCurrentEditorRoute` uses it for the folder revalidation branch.
- `worktree-operation-route.ts:303` — `resolveFolderWorkspaceOperationRoute` calls `findFolderWorkspaceOwner(state, folderWorkspaceId)` with no host.
- `worktree-runtime-owner-index.ts:78-83,295-299` — with no host the bare id maps to `{ kind: 'ambiguous' }`, which `findIndexedFolderWorkspaceOwner` returns as `null`.

## Required outcome
- An editor file opened in a folder workspace whose id is published on multiple hosts must still resolve to the host the user selected (the pre-PR behavior via `resolveActiveWorkspaceRoute` / `resolveSelectedHostRoute`), rather than throwing on open and on every subsequent read/save.

## Suggested approach (optional)
- Preserve the active selection as a tie-breaker only when the folder id is genuinely ambiguous (e.g. resolve the folder owner for the active host before falling back to a host-blind lookup), or scope the focus-nulling to git-worktree ids where owner rows carry the host. A test mirroring `selected-host-active-workspace-identity.test.ts` at the editor-capture level would pin this.

## Open questions for the human (optional)
- Is a cross-host folder id a state you expect to persist beyond the catalog-hydration window? If not, is failing closed on open the intended tradeoff?


export function assertEditorFileOperationCurrent(
state: EditorOwnerState,
worktreeId: string,
Expand Down Expand Up @@ -115,7 +132,7 @@ function resolveCurrentEditorRoute(
// Why: ordinary folder workspaces have no published worktree row, so re-resolve their live
// folder owner after preserving the explicit-owner fail-closed contract above (#10251).
if (parseWorkspaceKey(worktreeId)?.type === 'folder') {
return resolveWorktreeOperationRoute(state, worktreeId)
return resolveLegacyEditorFileRoute(state, worktreeId)
}
return isWorktreePublished(state, worktreeId) ? provenance.generation.route : null
}
Expand Down
Loading
Loading