Skip to content
Merged
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
Expand Up @@ -72,6 +72,7 @@ beforeEach(() => {
// Main hides the retained relay-era rows, so its own lists come back empty.
vi.stubGlobal('window', {
api: {
cache: { setGitHub: vi.fn(async () => undefined) },
repos: { list: vi.fn(async () => []) },
projects: { list: vi.fn(async () => []), listHostSetups: vi.fn(async () => []) },
projectGroups: { list: vi.fn(async () => []) },
Expand All @@ -93,7 +94,10 @@ beforeEach(() => {
},
projectGroups: [{ ...group, connectionId: 'ssh-1' }],
folderWorkspaces: [folder],
runtimeEnvironments: []
runtimeEnvironments: [],
startupWorktreeRefreshCompleted: false,
activeWorktreeId: null,
activeWorkspaceExecutionHostId: null
})
})

Expand Down Expand Up @@ -180,4 +184,156 @@ describe('a host that just moved to its managed server', () => {
expect(window.api.runtimeEnvironments.list).toHaveBeenCalledTimes(1)
expect(window.api.repos.list).toHaveBeenCalledTimes(1)
})

it.each(['repo.list', 'projectGroup.list', 'folderWorkspace.list'])(
'keeps the relay rows and retries after a failed %s refresh',
async (method) => {
const errorLog = vi.spyOn(console, 'error').mockImplementation(() => {})
const warningLog = vi.spyOn(console, 'warn').mockImplementation(() => {})
let failed = false
const call = vi.fn(async (args: RuntimeEnvironmentCallRequest) => {
if (args.method === method && !failed) {
failed = true
throw new Error('connection dropped during conversion catalog refresh')
}
return serverReply(args)
})
Object.assign(window.api.runtimeEnvironments, { call })
const managed = { kind: 'managed', environmentId: 'env-1' } as const
const targetId = `ssh-retry-${method}`
store.setState({
repos: [{ ...repo, connectionId: targetId, executionHostId: `ssh:${targetId}` }],
worktreesByRepo: {
'repo-1': [makeWorktree({ id: worktreeId, repoId: 'repo-1', hostId: `ssh:${targetId}` })]
},
projectGroups: [{ ...group, connectionId: targetId }]
})
applySshManagedServerTransition(targetId, undefined, managed)
await vi.waitFor(() => expect(errorLog).toHaveBeenCalled())
await new Promise((resolve) => setTimeout(resolve, 0))

expect(store.getState().worktreesByRepo['repo-1']).toHaveLength(1)
const callsBeforeReconnect = call.mock.calls.filter(([args]) => args.method === method).length
applySshManagedServerTransition(targetId, managed, {
kind: 'setting-up',
phase: 'connecting'
})
applySshManagedServerTransition(
targetId,
{ kind: 'setting-up', phase: 'connecting' },
managed
)
await vi.waitFor(() =>
expect(call.mock.calls.filter(([args]) => args.method === method).length).toBeGreaterThan(
callsBeforeReconnect
)
)
await vi.waitFor(() =>
expect(store.getState().folderWorkspaces.map((entry) => entry.executionHostId)).toEqual([
'runtime:env-1'
])
)
errorLog.mockRestore()
warningLog.mockRestore()
}
)

it('reloads the server names after a failed environment list on reconnect', async () => {
const warningLog = vi.spyOn(console, 'warn').mockImplementation(() => {})
const list = vi.mocked(window.api.runtimeEnvironments.list)
list.mockRejectedValueOnce(new Error('environment catalog unavailable'))
const managed = { kind: 'managed', environmentId: 'env-1' } as const
applySshManagedServerTransition('ssh-list-retry', undefined, managed)
await vi.waitFor(() => expect(warningLog).toHaveBeenCalled())
applySshManagedServerTransition('ssh-list-retry', managed, {
kind: 'setting-up',
phase: 'connecting'
})
applySshManagedServerTransition('ssh-list-retry', undefined, managed)
await vi.waitFor(() =>
expect(store.getState().runtimeEnvironments.map((entry) => entry.name)).toEqual([
'Box server'
])
)
expect(list).toHaveBeenCalledTimes(2)
warningLog.mockRestore()
})

it('replays a reconnect that arrived while the failed catalog read was still pending', async () => {
const warningLog = vi.spyOn(console, 'warn').mockImplementation(() => {})
const errorLog = vi.spyOn(console, 'error').mockImplementation(() => {})
const targetId = 'ssh-overlap'
const firstRead = Promise.withResolvers<never>()
let repoListCalls = 0
const call = vi.fn(async (args: RuntimeEnvironmentCallRequest) => {
if (args.method === 'repo.list' && ++repoListCalls === 1) {
return firstRead.promise
}
return serverReply(args)
})
Object.assign(window.api.runtimeEnvironments, { call })
store.setState({
repos: [{ ...repo, connectionId: targetId, executionHostId: `ssh:${targetId}` }],
worktreesByRepo: {
'repo-1': [
makeWorktree({ id: worktreeId, repoId: 'repo-1', hostId: `ssh:${targetId}` }),
makeWorktree({ id: worktreeId, repoId: 'repo-1', hostId: 'runtime:env-1' })
]
},
projectGroups: [{ ...group, connectionId: targetId }],
activeRepoId: 'repo-1',
activeWorktreeId: worktreeId,
activeWorkspaceExecutionHostId: `ssh:${targetId}`
})
const managed = { kind: 'managed', environmentId: 'env-1' } as const
const settingUp = { kind: 'setting-up', phase: 'connecting' } as const
applySshManagedServerTransition(targetId, undefined, managed)
await vi.waitFor(() => expect(repoListCalls).toBe(1))
applySshManagedServerTransition(targetId, managed, settingUp)
applySshManagedServerTransition(targetId, settingUp, managed)
firstRead.reject(new Error('half-open connection timed out'))

await vi.waitFor(() =>
expect(store.getState().activeWorkspaceExecutionHostId).toBe('runtime:env-1')
)
expect(repoListCalls).toBe(2)
warningLog.mockRestore()
errorLog.mockRestore()
})

it('drops a pending reconnect replay when the host returns to the relay', async () => {
const warningLog = vi.spyOn(console, 'warn').mockImplementation(() => {})
const firstList = Promise.withResolvers<never>()
const list = vi.mocked(window.api.runtimeEnvironments.list)
list.mockReturnValueOnce(firstList.promise)
const managed = { kind: 'managed', environmentId: 'env-1' } as const
const settingUp = { kind: 'setting-up', phase: 'connecting' } as const
applySshManagedServerTransition('ssh-overlap-relay', undefined, managed)
applySshManagedServerTransition('ssh-overlap-relay', managed, settingUp)
applySshManagedServerTransition('ssh-overlap-relay', settingUp, managed)
applySshManagedServerTransition('ssh-overlap-relay', managed, {
kind: 'relay',
reason: 'orcad_unavailable'
})
firstList.reject(new Error('environment catalog unavailable'))
await vi.waitFor(() => expect(warningLog).toHaveBeenCalled())
await new Promise((resolve) => setTimeout(resolve, 0))
expect(list).toHaveBeenCalledTimes(1)
warningLog.mockRestore()
})

it('abandons an in-flight conversion refresh when the host returns to the relay', async () => {
const pending =
Promise.withResolvers<Awaited<ReturnType<typeof window.api.runtimeEnvironments.list>>>()
vi.mocked(window.api.runtimeEnvironments.list).mockReturnValueOnce(pending.promise)
const managed = { kind: 'managed', environmentId: 'env-1' } as const
applySshManagedServerTransition('ssh-cancel-load', undefined, managed)
applySshManagedServerTransition('ssh-cancel-load', managed, {
kind: 'relay',
reason: 'orcad_unavailable'
})
pending.resolve([])
await new Promise((resolve) => setTimeout(resolve, 0))
expect(window.api.repos.list).not.toHaveBeenCalled()
})
})
Original file line number Diff line number Diff line change
Expand Up @@ -23,18 +23,18 @@ export function applySshManagedServerTransition(
next: ManagedServerStatus
): void {
if (next?.kind === 'managed') {
// Why not `previous`: every start, wake and update passes through setting-up, which is no
// change of owner. Only a new environment for this host needs its catalogs loaded.
if (loadedEnvironmentByTarget.get(targetId) !== next.environmentId) {
loadedEnvironmentByTarget.set(targetId, next.environmentId)
void loadManagedServerCatalogs(targetId, next.environmentId).catch((error: unknown) =>
console.warn('[ssh] Could not load the managed server catalogs:', error)
)
// Starts and wakes keep the same owner; only a new server or a failed load needs a refresh.
const current = catalogLoadByTarget.get(targetId)
if (current?.environmentId !== next.environmentId) {
startManagedServerCatalogLoad(targetId, next.environmentId)
} else if (current.pending && previous?.kind !== 'managed') {
// Why: a reconnect during a doomed read must not be lost; replay it once if that read fails.
current.reconnectedWhilePending = true
}
return
}
if (next?.kind === 'relay') {
loadedEnvironmentByTarget.delete(targetId)
catalogLoadByTarget.delete(targetId)
}
Comment on lines +26 to 38

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.

🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win

Fence catalog commits when the target enters relay.

When a managed-server catalog load is pending, the relay transition deletes catalogLoadByTarget, but it does not invalidate the repository catalog action. A pending fetchRepos can therefore pass its unchanged host-specific generation check and merge the old server's repositories into repos. The later load-identity check only prevents converted-row cleanup and workspace rehoming; it does not undo that repository commit.

Invalidate the pending catalog actions at this transition, or pass the load identity into their commit guards so obsolete results cannot commit after the target enters relay.

if (isNewMoveOffer(previous, next) && canMoveSshHostToManagedServer()) {
offerManagedServerMove(targetId, next.terminals)
Expand All @@ -59,24 +59,64 @@ export function applySshManagedServerTransition(
}
}

const loadedEnvironmentByTarget = new Map<string, string>()
type CatalogLoad = {
environmentId: string
pending: boolean
reconnectedWhilePending: boolean
}

const catalogLoadByTarget = new Map<string, CatalogLoad>()

function startManagedServerCatalogLoad(targetId: string, environmentId: string): void {
const load: CatalogLoad = { environmentId, pending: true, reconnectedWhilePending: false }
catalogLoadByTarget.set(targetId, load)
void loadManagedServerCatalogs(targetId, load).then(
() => {
load.pending = false
},
(error: unknown) => {
load.pending = false
console.warn('[ssh] Could not load the managed server catalogs:', error)
// Why: relay fallback and server replacement swap the record, which drops any replay.
if (catalogLoadByTarget.get(targetId) !== load) {
return
}
catalogLoadByTarget.delete(targetId)
if (load.reconnectedWhilePending) {
startManagedServerCatalogLoad(targetId, environmentId)
}
}
)
}

/** Loads a newly managed host's server and the local catalogs, then drops its relay-era rows. */
async function loadManagedServerCatalogs(targetId: string, environmentId: string): Promise<void> {
async function loadManagedServerCatalogs(targetId: string, load: CatalogLoad): Promise<void> {
const { environmentId } = load
const isCurrent = (): boolean => catalogLoadByTarget.get(targetId) === load
const store = useAppStore.getState()
try {
// Why: host badges read server names from this catalog, which a conversion does not refresh.
store.setRuntimeEnvironments(await window.api.runtimeEnvironments.list())
void store.refreshRuntimeEnvironmentStatus(environmentId)
} catch (error) {
console.warn('[ssh] Could not refresh the managed server list:', error)
// Why: host badges read server names from this catalog, which a conversion does not refresh.
const environments = await window.api.runtimeEnvironments.list()
if (!isCurrent()) {
return
}
store.setRuntimeEnvironments(environments)
void store.refreshRuntimeEnvironmentStatus(environmentId)
// Why local too: the host's relay-era rows come from the local catalog, which main now hides.
for (const runtimeEnvironmentId of [null, environmentId]) {
await store.fetchRepos({ runtimeEnvironmentId })
const options = { runtimeEnvironmentId, throwOnError: true }
await store.fetchRepos(options)
if (!isCurrent()) {
return
}
// Why groups before folders: folder workspaces are owned through their project groups.
await store.fetchProjectGroups({ runtimeEnvironmentId })
await store.fetchFolderWorkspaces({ runtimeEnvironmentId })
await store.fetchProjectGroups(options)
if (!isCurrent()) {
return
}
await store.fetchFolderWorkspaces(options)
if (!isCurrent()) {
return
}
}
// Why gated: startup runs its own scan once every host's catalog is in.
if (useAppStore.getState().startupWorktreeRefreshCompleted) {
Expand All @@ -86,6 +126,9 @@ async function loadManagedServerCatalogs(targetId: string, environmentId: string
.repos.filter((repo) => getRepoExecutionHostId(repo) === executionHostId)
await Promise.all(repos.map((repo) => store.fetchWorktrees(repo.id, { executionHostId })))
}
if (!isCurrent()) {
return
}
useAppStore.setState((state) => withoutConvertedSshHostRows(state, targetId))
rehomeActiveWorkspace(targetId, environmentId)
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,9 @@ export function createFolderWorkspaceCatalogActions(
})
} catch (err) {
console.error('Failed to fetch folder workspaces:', err)
if (options?.throwOnError) {
throw err
}
}
},

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,9 @@ export function createProjectGroupCatalogActions(
})
} catch (err) {
console.error('Failed to fetch project groups:', err)
if (options?.throwOnError) {
throw err
}
}
},

Expand Down
3 changes: 3 additions & 0 deletions src/renderer/src/store/repos/repo-catalog-actions.ts
Original file line number Diff line number Diff line change
Expand Up @@ -163,6 +163,9 @@ export function createRepoCatalogActions(
} catch (err) {
localCatalogOutcome = { status: 'rejected', reason: err }
console.error('Failed to fetch repos:', err)
if (options?.throwOnError) {
throw err
}
} finally {
settleLocalCatalog(localCatalogOutcome)
}
Expand Down
6 changes: 5 additions & 1 deletion src/renderer/src/store/repos/repo-state.ts
Original file line number Diff line number Diff line change
Expand Up @@ -141,7 +141,11 @@ export type AddRepoPathOptions = {
displayName?: string
}

export type RuntimeCatalogFetchOptions = { runtimeEnvironmentId?: string | null }
export type RuntimeCatalogFetchOptions = {
runtimeEnvironmentId?: string | null
/** Conversion must retain its source rows until every destination catalog loads. */
throwOnError?: boolean
}

export type RepoSlice = {
repos: readonly Repo[]
Expand Down
Loading
Loading