Skip to content

Commit 4fbf6b7

Browse files
TheodoreSpeaksclaude
authored andcommitted
fix(cli-auth): wait for the workspace list before allowing approval
The picker fell back to "No workspace (personal key)" while the workspace query was in flight, and Connect stayed live through that window. A fast click approved a personal key with no default workspace — when the same click a moment later would have issued a workspace-scoped key. The fallback read as an answer rather than a pending state, so the card could promise one outcome and deliver another. Connect is now disabled until the list resolves, the trigger shows a loading label (a placeholder would not show, since the fallback always counts as a selection), and the explanatory line no longer asserts the personal-key outcome before it is known. Failure is treated as degraded rather than fatal: the picker disables but Connect stays enabled and the copy says a personal key will be issued, so a transient list failure cannot strand a waiting terminal. Tests cover the pending, loaded, admin-binding, and error states; the two loading assertions fail against the previous implementation. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01EsThUPqZXjwuuyRjBbmVkj
1 parent a4db1d2 commit 4fbf6b7

2 files changed

Lines changed: 163 additions & 4 deletions

File tree

Lines changed: 138 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,138 @@
1+
/**
2+
* @vitest-environment jsdom
3+
*/
4+
import { act } from 'react'
5+
import { createRoot, type Root } from 'react-dom/client'
6+
import { beforeEach, describe, expect, it, vi } from 'vitest'
7+
8+
const { mockApprove, mockUseWorkspaces, mockPush } = vi.hoisted(() => ({
9+
mockApprove: vi.fn(),
10+
mockUseWorkspaces: vi.fn(),
11+
mockPush: vi.fn(),
12+
}))
13+
14+
vi.mock('next/navigation', () => ({
15+
useRouter: () => ({ push: mockPush }),
16+
}))
17+
18+
vi.mock('nuqs', () => ({
19+
useQueryStates: () => [
20+
{
21+
request: 'a'.repeat(43),
22+
challenge: 'b'.repeat(43),
23+
pairing: 'ABCD-2345',
24+
scope: 'platform',
25+
workspace: null,
26+
},
27+
],
28+
}))
29+
30+
vi.mock('@/hooks/queries/cli-auth', () => ({
31+
useApproveCliAuth: () => ({
32+
mutate: mockApprove,
33+
isPending: false,
34+
isSuccess: false,
35+
isError: false,
36+
error: null,
37+
}),
38+
}))
39+
40+
vi.mock('@/hooks/queries/workspace', () => ({
41+
useWorkspacesWithMetadata: mockUseWorkspaces,
42+
}))
43+
44+
import { CliAuthView } from '@/app/cli/auth/cli-auth-view'
45+
46+
let container: HTMLDivElement
47+
let root: Root
48+
49+
function render() {
50+
container = document.createElement('div')
51+
document.body.appendChild(container)
52+
root = createRoot(container)
53+
act(() => {
54+
root.render(<CliAuthView />)
55+
})
56+
}
57+
58+
/** The primary CTA is the only button whose label mentions connecting. */
59+
function connectButton(): HTMLButtonElement {
60+
const buttons = [...container.querySelectorAll('button')] as HTMLButtonElement[]
61+
const button = buttons.find((b) => /connect/i.test(b.textContent ?? ''))
62+
if (!button) throw new Error('Connect button not found')
63+
return button
64+
}
65+
66+
const LOADED = {
67+
isPending: false,
68+
isError: false,
69+
data: {
70+
workspaces: [
71+
{ id: 'ws_admin', name: 'Acme', permissions: 'admin' },
72+
{ id: 'ws_member', name: 'Other', permissions: 'write' },
73+
],
74+
lastActiveWorkspaceId: 'ws_admin',
75+
},
76+
}
77+
78+
describe('CliAuthView workspace loading', () => {
79+
beforeEach(() => {
80+
vi.clearAllMocks()
81+
})
82+
83+
it('blocks Connect until the workspace list resolves', () => {
84+
// The regression: while pending, the picker falls back to the personal
85+
// option, so an early click approved a personal key when the same click a
86+
// moment later would have bound the key to the user's workspace.
87+
mockUseWorkspaces.mockReturnValue({ isPending: true, isError: false, data: undefined })
88+
render()
89+
90+
expect(connectButton().disabled).toBe(true)
91+
expect(container.textContent).toContain('Loading workspaces')
92+
expect(container.textContent).not.toContain('No workspace (personal key)')
93+
})
94+
95+
it('does not present the personal-key wording as the answer while loading', () => {
96+
mockUseWorkspaces.mockReturnValue({ isPending: true, isError: false, data: undefined })
97+
render()
98+
99+
expect(container.textContent).toContain('Checking which workspaces')
100+
expect(container.textContent).not.toContain('Issues a personal key')
101+
})
102+
103+
it('enables Connect and preselects the last active workspace once loaded', () => {
104+
mockUseWorkspaces.mockReturnValue(LOADED)
105+
render()
106+
107+
expect(connectButton().disabled).toBe(false)
108+
expect(container.textContent).toContain('Acme')
109+
expect(container.textContent).toContain('only reach Acme')
110+
})
111+
112+
it('binds the key to the workspace when the approver is an admin', () => {
113+
mockUseWorkspaces.mockReturnValue(LOADED)
114+
render()
115+
act(() => {
116+
connectButton().click()
117+
})
118+
119+
expect(mockApprove).toHaveBeenCalledWith(
120+
expect.objectContaining({
121+
scope: 'platform',
122+
workspaceId: 'ws_admin',
123+
bindKeyToWorkspace: true,
124+
}),
125+
expect.anything()
126+
)
127+
})
128+
129+
it('still lets the user connect when the workspace list fails', () => {
130+
// A personal key is degraded but usable; blocking entirely would strand a
131+
// terminal on a transient list failure.
132+
mockUseWorkspaces.mockReturnValue({ isPending: false, isError: true, data: undefined })
133+
render()
134+
135+
expect(connectButton().disabled).toBe(false)
136+
expect(container.textContent).toContain('Could not load your workspaces')
137+
})
138+
})

apps/sim/app/cli/auth/cli-auth-view.tsx

Lines changed: 25 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -59,6 +59,18 @@ export function CliAuthView() {
5959

6060
const { request } = resolution
6161

62+
/**
63+
* Approval must wait for the workspace list.
64+
*
65+
* Until it arrives there is no selection to show, and the fallback would read
66+
* as "No workspace (personal key)" — a real answer, not a pending one. Leaving
67+
* Connect live through that window let a fast click approve a personal key
68+
* with no default workspace, when a moment later the same click would have
69+
* bound the key to the user's workspace. Blocking is the only way the card
70+
* can promise what it is about to do.
71+
*/
72+
const loadingWorkspaces = isPlatform && workspaces.isPending
73+
6274
// The terminal's suggestion, then the user's last active workspace. Derived at
6375
// render rather than synced into state through an effect, so the first paint
6476
// after the list loads already shows the right row.
@@ -91,23 +103,32 @@ export function CliAuthView() {
91103
options={options}
92104
value={workspaceId ?? PERSONAL_VALUE}
93105
onChange={setSelected}
94-
disabled={workspaces.isLoading}
106+
disabled={loadingWorkspaces || workspaces.isError}
107+
// A placeholder only shows when nothing is selected, and the
108+
// fallback value always counts as a selection — so the loading
109+
// state has to override the rendered label outright.
110+
displayLabel={loadingWorkspaces ? 'Loading workspaces…' : undefined}
95111
placeholder='Select a workspace'
96112
searchable={options.length > 8}
97113
searchPlaceholder='Search workspaces'
98114
fullWidth
99115
dropdownWidth='trigger'
100116
/>
101117
<p className='text-[var(--text-muted)] text-caption'>
102-
{bindsToWorkspace
103-
? `Issues a key that can only reach ${chosen.name}.`
104-
: 'Issues a personal key tied to your account, defaulting to this workspace. Workspace-scoped keys need admin.'}
118+
{loadingWorkspaces
119+
? 'Checking which workspaces you can issue a key for…'
120+
: workspaces.isError
121+
? 'Could not load your workspaces. Connecting still works and issues a personal key; reload to pick a default workspace.'
122+
: bindsToWorkspace
123+
? `Issues a key that can only reach ${chosen.name}.`
124+
: 'Issues a personal key tied to your account, defaulting to this workspace. Workspace-scoped keys need admin.'}
105125
</p>
106126
</div>
107127
)}
108128
<AuthSubmitButton
109129
type='button'
110130
loading={approve.isPending || approve.isSuccess}
131+
disabled={loadingWorkspaces}
111132
loadingLabel='Connecting'
112133
onClick={() =>
113134
approve.mutate(

0 commit comments

Comments
 (0)