Fix connect spinner stuck after a failed SSH connect

The run-picker Connect spinner never stopped when a connect failed. Root cause:
the `mountedRef` guard around the clearing setState. The ref's effect only set
it false on cleanup and never back to true on mount, so StrictMode's dev
mount→unmount→remount left it stuck false — the connect `finally` then skipped
clearing connectingHostIds and the row spun forever even after the error toast.

Remove the unnecessary mounted-guard entirely (a stale setState is a harmless
no-op in React 18) so the in-flight id is always cleared when the connect
settles. Add a regression test for the failed-connect path.
This commit is contained in:
Jinwoo-H
2026-07-09 03:02:20 -04:00
parent e0d577869d
commit 09f205acec
2 changed files with 32 additions and 17 deletions
@@ -676,6 +676,26 @@ describe('NewWorkspaceComposerCard folder task source mode', () => {
expect(apiMocks.sshConnect).toHaveBeenCalledWith({ targetId: 'bastion' })
})
it('stops the connecting indicator when the connect fails', async () => {
// A failed connect must clear the spinner and restore the Connect button so the user
// can retry — the row can't stay stuck on "Connecting" after the error.
apiMocks.sshConnect.mockRejectedValue(new Error('connection refused'))
current = renderCard({
projectHostSetupOptions: [localReadyHostOption, disconnectedDevboxNeedsSetupHostOption],
selectedProjectHostSetupId: 'setup-local'
})
openRunTargetPicker(current.container)
await act(async () => {
findConnectButton('Devbox')?.click()
})
const devboxButton = findConnectButton('Devbox')
expect(devboxButton?.disabled).toBe(false)
expect(devboxButton?.textContent).toContain('Connect')
expect(devboxButton?.textContent).not.toContain('Connecting')
})
it('opens the SSH host add dialog over the composer without leaving for Settings', () => {
current = renderCard({
projectHostSetupOptions: [localReadyHostOption, devboxNeedsSetupHostOption],
@@ -293,7 +293,6 @@ function WorkspaceRunTargetCombobox({
const [connectingHostIds, setConnectingHostIds] = React.useState<ReadonlySet<string>>(
() => new Set()
)
const mountedRef = React.useRef(true)
const readyHostOptions = React.useMemo(
() =>
hostOptions.filter(
@@ -321,12 +320,6 @@ function WorkspaceRunTargetCombobox({
'Per-Workspace Environment'
)
React.useEffect(() => {
return () => {
mountedRef.current = false
}
}, [])
const handleHostSelect = React.useCallback(
(setupId: string): void => {
if (!readyHostOptions.some((candidate) => candidate.id === setupId)) {
@@ -360,16 +353,18 @@ function WorkspaceRunTargetCombobox({
try {
await onConnectHost(option)
} finally {
if (mountedRef.current) {
setConnectingHostIds((current) => {
if (!current.has(option.hostId)) {
return current
}
const next = new Set(current)
next.delete(option.hostId)
return next
})
}
// Why: always clear the in-flight id when the connect settles (success, failure, or
// timeout) so the row's spinner stops. No mounted-guard — a stale setState on an
// unmounted component is a harmless no-op in React 18, and the guard's ref was the
// source of a StrictMode bug that left the spinner stuck.
setConnectingHostIds((current) => {
if (!current.has(option.hostId)) {
return current
}
const next = new Set(current)
next.delete(option.hostId)
return next
})
}
},
[connectingHostIds, onConnectHost]