Add cancel flow for Claude account login (#6702)
Co-authored-by: Orca <help@stably.ai>
This commit is contained in:
parent
29df9a3ab5
commit
41e5c3918b
|
|
@ -1099,4 +1099,195 @@ describe('ClaudeAccountService credential capture', () => {
|
|||
vi.doUnmock('node:child_process')
|
||||
}
|
||||
})
|
||||
|
||||
it('cancels an in-flight Claude account add', async () => {
|
||||
vi.resetModules()
|
||||
const child = new EventEmitter() as EventEmitter & {
|
||||
stdout: PassThrough
|
||||
stderr: PassThrough
|
||||
kill: ReturnType<typeof vi.fn>
|
||||
}
|
||||
child.stdout = new PassThrough()
|
||||
child.stderr = new PassThrough()
|
||||
child.kill = vi.fn()
|
||||
const spawnMock = vi.fn(() => child)
|
||||
vi.doMock('node:child_process', () => ({ spawn: spawnMock }))
|
||||
|
||||
try {
|
||||
const { ClaudeAccountService } = await import('./service')
|
||||
let settings = {
|
||||
claudeManagedAccounts: [],
|
||||
activeClaudeManagedAccountId: null,
|
||||
activeClaudeManagedAccountIdsByRuntime: { host: null, wsl: {} }
|
||||
}
|
||||
const store = {
|
||||
getSettings: vi.fn(() => settings),
|
||||
updateSettings: vi.fn((updates: Partial<typeof settings>) => {
|
||||
settings = { ...settings, ...updates }
|
||||
return settings
|
||||
})
|
||||
}
|
||||
const runtimeAuth = {
|
||||
clearLastWrittenCredentialsJson: vi.fn(),
|
||||
forceMaterializeCurrentSelectionForRollback: vi.fn(async () => {})
|
||||
}
|
||||
const rateLimits = {
|
||||
evictInactiveClaudeCache: vi.fn(),
|
||||
refreshForClaudeAccountChange: vi.fn()
|
||||
}
|
||||
const service = new ClaudeAccountService(
|
||||
store as never,
|
||||
rateLimits as never,
|
||||
runtimeAuth as never
|
||||
)
|
||||
|
||||
const addPromise = service.addAccount()
|
||||
await vi.waitFor(() => {
|
||||
expect(spawnMock).toHaveBeenCalledTimes(1)
|
||||
})
|
||||
|
||||
expect(service.cancelPendingLogin()).toBe(true)
|
||||
await expect(addPromise).rejects.toThrow('Claude sign-in was cancelled.')
|
||||
expect(child.kill).toHaveBeenCalledTimes(1)
|
||||
expect(service.cancelPendingLogin()).toBe(false)
|
||||
expect(settings.claudeManagedAccounts).toEqual([])
|
||||
expect(child.stdout.listenerCount('data')).toBe(0)
|
||||
expect(child.stderr.listenerCount('data')).toBe(0)
|
||||
expect(child.listenerCount('error')).toBe(0)
|
||||
expect(child.listenerCount('close')).toBe(0)
|
||||
} finally {
|
||||
vi.doUnmock('node:child_process')
|
||||
}
|
||||
})
|
||||
|
||||
it('honors cancel before Claude login command starts', async () => {
|
||||
setPlatform('linux')
|
||||
vi.resetModules()
|
||||
let releaseKeychainRead: (value: string | null) => void = () => {}
|
||||
vi.mocked(readActiveClaudeKeychainCredentials).mockReturnValue(
|
||||
new Promise<string | null>((resolve) => {
|
||||
releaseKeychainRead = resolve
|
||||
})
|
||||
)
|
||||
const spawnMock = vi.fn()
|
||||
vi.doMock('node:child_process', () => ({ spawn: spawnMock }))
|
||||
|
||||
try {
|
||||
const { ClaudeAccountService } = await import('./service')
|
||||
let settings = {
|
||||
claudeManagedAccounts: [],
|
||||
activeClaudeManagedAccountId: null,
|
||||
activeClaudeManagedAccountIdsByRuntime: { host: null, wsl: {} }
|
||||
}
|
||||
const store = {
|
||||
getSettings: vi.fn(() => settings),
|
||||
updateSettings: vi.fn((updates: Partial<typeof settings>) => {
|
||||
settings = { ...settings, ...updates }
|
||||
return settings
|
||||
})
|
||||
}
|
||||
const runtimeAuth = {
|
||||
clearLastWrittenCredentialsJson: vi.fn(),
|
||||
forceMaterializeCurrentSelectionForRollback: vi.fn(async () => {})
|
||||
}
|
||||
const rateLimits = {
|
||||
evictInactiveClaudeCache: vi.fn(),
|
||||
refreshForClaudeAccountChange: vi.fn()
|
||||
}
|
||||
const service = new ClaudeAccountService(
|
||||
store as never,
|
||||
rateLimits as never,
|
||||
runtimeAuth as never
|
||||
)
|
||||
|
||||
const addPromise = service.addAccount()
|
||||
await vi.waitFor(() => {
|
||||
expect(readActiveClaudeKeychainCredentials).toHaveBeenCalled()
|
||||
})
|
||||
|
||||
expect(service.cancelPendingLogin()).toBe(true)
|
||||
expect(service.cancelPendingLogin()).toBe(false)
|
||||
expect(spawnMock).not.toHaveBeenCalled()
|
||||
releaseKeychainRead(null)
|
||||
await expect(addPromise).rejects.toThrow('Claude sign-in was cancelled.')
|
||||
expect(spawnMock).not.toHaveBeenCalled()
|
||||
expect(service.cancelPendingLogin()).toBe(false)
|
||||
expect(settings.claudeManagedAccounts).toEqual([])
|
||||
} finally {
|
||||
vi.doUnmock('node:child_process')
|
||||
}
|
||||
})
|
||||
|
||||
it('uses taskkill to cancel the Windows Claude login process tree', async () => {
|
||||
setPlatform('win32')
|
||||
vi.resetModules()
|
||||
vi.mocked(readActiveClaudeKeychainCredentials).mockResolvedValue(null)
|
||||
const child = new EventEmitter() as EventEmitter & {
|
||||
pid: number
|
||||
stdout: PassThrough
|
||||
stderr: PassThrough
|
||||
kill: ReturnType<typeof vi.fn>
|
||||
}
|
||||
child.pid = 1234
|
||||
child.stdout = new PassThrough()
|
||||
child.stderr = new PassThrough()
|
||||
child.kill = vi.fn()
|
||||
const taskkill = new EventEmitter() as EventEmitter & {
|
||||
unref: ReturnType<typeof vi.fn>
|
||||
}
|
||||
taskkill.unref = vi.fn()
|
||||
const spawnMock = vi.fn((command: string) => (command === 'taskkill.exe' ? taskkill : child))
|
||||
vi.doMock('node:child_process', () => ({ spawn: spawnMock }))
|
||||
|
||||
try {
|
||||
const { ClaudeAccountService } = await import('./service')
|
||||
let settings = {
|
||||
claudeManagedAccounts: [],
|
||||
activeClaudeManagedAccountId: null,
|
||||
activeClaudeManagedAccountIdsByRuntime: { host: null, wsl: {} }
|
||||
}
|
||||
const store = {
|
||||
getSettings: vi.fn(() => settings),
|
||||
updateSettings: vi.fn((updates: Partial<typeof settings>) => {
|
||||
settings = { ...settings, ...updates }
|
||||
return settings
|
||||
})
|
||||
}
|
||||
const runtimeAuth = {
|
||||
clearLastWrittenCredentialsJson: vi.fn(),
|
||||
forceMaterializeCurrentSelectionForRollback: vi.fn(async () => {})
|
||||
}
|
||||
const rateLimits = {
|
||||
evictInactiveClaudeCache: vi.fn(),
|
||||
refreshForClaudeAccountChange: vi.fn()
|
||||
}
|
||||
const service = new ClaudeAccountService(
|
||||
store as never,
|
||||
rateLimits as never,
|
||||
runtimeAuth as never
|
||||
)
|
||||
|
||||
const addPromise = service.addAccount()
|
||||
await vi.waitFor(() => {
|
||||
expect(spawnMock).toHaveBeenCalledWith(
|
||||
'claude',
|
||||
['auth', 'login', '--claudeai'],
|
||||
expect.objectContaining({ shell: true })
|
||||
)
|
||||
})
|
||||
|
||||
expect(service.cancelPendingLogin()).toBe(true)
|
||||
await expect(addPromise).rejects.toThrow('Claude sign-in was cancelled.')
|
||||
expect(child.kill).not.toHaveBeenCalled()
|
||||
expect(spawnMock).toHaveBeenCalledWith(
|
||||
'taskkill.exe',
|
||||
['/pid', '1234', '/t', '/f'],
|
||||
expect.objectContaining({ stdio: 'ignore', windowsHide: true })
|
||||
)
|
||||
expect(taskkill.unref).toHaveBeenCalled()
|
||||
expect(service.cancelPendingLogin()).toBe(false)
|
||||
} finally {
|
||||
vi.doUnmock('node:child_process')
|
||||
}
|
||||
})
|
||||
})
|
||||
|
|
|
|||
|
|
@ -83,6 +83,7 @@ function shellQuote(value: string): string {
|
|||
|
||||
export class ClaudeAccountService {
|
||||
private mutationQueue: Promise<unknown> = Promise.resolve()
|
||||
private cancelPendingClaudeLogin: (() => boolean) | null = null
|
||||
|
||||
constructor(
|
||||
private readonly store: Store,
|
||||
|
|
@ -118,6 +119,10 @@ export class ClaudeAccountService {
|
|||
return this.serializeMutation(() => this.doSelectAccount(accountId, target))
|
||||
}
|
||||
|
||||
cancelPendingLogin(): boolean {
|
||||
return this.cancelPendingClaudeLogin?.() ?? false
|
||||
}
|
||||
|
||||
private serializeMutation<T>(fn: () => Promise<T>): Promise<T> {
|
||||
const next = this.mutationQueue.then(fn, fn)
|
||||
this.mutationQueue = next.catch(() => {})
|
||||
|
|
@ -424,12 +429,26 @@ export class ClaudeAccountService {
|
|||
}
|
||||
): Promise<CapturedClaudeAuth> {
|
||||
const tempConfig = this.createTemporaryClaudeConfigDir(location)
|
||||
const loginAbortController = new AbortController()
|
||||
this.cancelPendingClaudeLogin = () => {
|
||||
if (loginAbortController.signal.aborted) {
|
||||
return false
|
||||
}
|
||||
loginAbortController.abort()
|
||||
return true
|
||||
}
|
||||
const previousLegacyKeychain = await readActiveClaudeKeychainCredentials()
|
||||
let captured: CapturedClaudeAuth | null = null
|
||||
let captureError: unknown = null
|
||||
let cleanupError: unknown = null
|
||||
try {
|
||||
await this.runClaudeCommand(['auth', 'login', '--claudeai'], tempConfig, LOGIN_TIMEOUT_MS)
|
||||
if (loginAbortController.signal.aborted) {
|
||||
throw new Error('Claude sign-in was cancelled.')
|
||||
}
|
||||
await this.runClaudeCommand(['auth', 'login', '--claudeai'], tempConfig, LOGIN_TIMEOUT_MS, {
|
||||
signal: loginAbortController.signal
|
||||
})
|
||||
this.cancelPendingClaudeLogin = null
|
||||
const status = await this.runClaudeCommand(
|
||||
['auth', 'status', '--json'],
|
||||
tempConfig,
|
||||
|
|
@ -463,6 +482,7 @@ export class ClaudeAccountService {
|
|||
}
|
||||
}
|
||||
this.removeTemporaryClaudeConfigDir(tempConfig)
|
||||
this.cancelPendingClaudeLogin = null
|
||||
}
|
||||
if (captureError) {
|
||||
throw captureError
|
||||
|
|
@ -862,7 +882,7 @@ export class ClaudeAccountService {
|
|||
args: string[],
|
||||
configDir: { windowsPath: string; linuxPath: string | null; wslDistro: string | null },
|
||||
timeoutMs: number,
|
||||
options?: { allowFailure?: boolean }
|
||||
options?: { allowFailure?: boolean; signal?: AbortSignal }
|
||||
): Promise<string> {
|
||||
return new Promise((resolvePromise, rejectPromise) => {
|
||||
const spawnConfig =
|
||||
|
|
@ -892,7 +912,10 @@ export class ClaudeAccountService {
|
|||
const child = spawn(spawnConfig.command, spawnConfig.args, {
|
||||
stdio: ['ignore', 'pipe', 'pipe'],
|
||||
shell: spawnConfig.shell,
|
||||
env: spawnConfig.env
|
||||
env: spawnConfig.env,
|
||||
// Why: Claude auth can leave browser/login descendants alive after denial.
|
||||
// A process group lets cancellation terminate the whole POSIX login tree.
|
||||
detached: process.platform !== 'win32'
|
||||
})
|
||||
|
||||
let settled = false
|
||||
|
|
@ -913,6 +936,7 @@ export class ClaudeAccountService {
|
|||
child.stderr.off('data', appendOutput)
|
||||
child.off('error', onError)
|
||||
child.off('close', onClose)
|
||||
options?.signal?.removeEventListener('abort', onAbort)
|
||||
}
|
||||
const settle = (callback: () => void): void => {
|
||||
if (settled) {
|
||||
|
|
@ -923,11 +947,36 @@ export class ClaudeAccountService {
|
|||
callback()
|
||||
}
|
||||
const timeoutError = new Error('Claude sign-in took too long to finish.')
|
||||
timeout = setTimeout(() => {
|
||||
const cancelError = new Error('Claude sign-in was cancelled.')
|
||||
const killChild = (): void => {
|
||||
if (process.platform === 'win32' && child.pid) {
|
||||
const taskkill = spawn('taskkill.exe', ['/pid', String(child.pid), '/t', '/f'], {
|
||||
stdio: 'ignore',
|
||||
windowsHide: true
|
||||
})
|
||||
taskkill.on('error', () => {})
|
||||
taskkill.unref()
|
||||
return
|
||||
}
|
||||
if (process.platform !== 'win32' && child.pid) {
|
||||
try {
|
||||
process.kill(-child.pid)
|
||||
return
|
||||
} catch {
|
||||
// Fall back to the direct child if the process group is unavailable.
|
||||
}
|
||||
}
|
||||
child.kill()
|
||||
}
|
||||
timeout = setTimeout(() => {
|
||||
killChild()
|
||||
settle(() => rejectPromise(timeoutError))
|
||||
}, timeoutMs)
|
||||
|
||||
const onAbort = (): void => {
|
||||
killChild()
|
||||
settle(() => rejectPromise(cancelError))
|
||||
}
|
||||
const onError = (error: Error): void => {
|
||||
settle(() => rejectPromise(error))
|
||||
}
|
||||
|
|
@ -952,6 +1001,11 @@ export class ClaudeAccountService {
|
|||
child.stderr.on('data', appendOutput)
|
||||
child.on('error', onError)
|
||||
child.on('close', onClose)
|
||||
if (options?.signal?.aborted) {
|
||||
onAbort()
|
||||
} else {
|
||||
options?.signal?.addEventListener('abort', onAbort, { once: true })
|
||||
}
|
||||
})
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -7,6 +7,7 @@ export function registerClaudeAccountHandlers(claudeAccounts: ClaudeAccountServi
|
|||
ipcMain.handle('claudeAccounts:add', (_event, args?: ClaudeAccountAddTarget) =>
|
||||
claudeAccounts.addAccount(args)
|
||||
)
|
||||
ipcMain.handle('claudeAccounts:cancelPendingLogin', () => claudeAccounts.cancelPendingLogin())
|
||||
ipcMain.handle('claudeAccounts:reauthenticate', (_event, args: { accountId: string }) =>
|
||||
claudeAccounts.reauthenticateAccount(args.accountId)
|
||||
)
|
||||
|
|
|
|||
|
|
@ -1901,6 +1901,7 @@ export type PreloadApi = {
|
|||
runtime?: 'host' | 'wsl'
|
||||
wslDistro?: string | null
|
||||
}) => Promise<ClaudeRateLimitAccountsState>
|
||||
cancelPendingLogin: () => Promise<boolean>
|
||||
reauthenticate: (args: { accountId: string }) => Promise<ClaudeRateLimitAccountsState>
|
||||
remove: (args: { accountId: string }) => Promise<ClaudeRateLimitAccountsState>
|
||||
select: (args: {
|
||||
|
|
|
|||
|
|
@ -1705,6 +1705,8 @@ const api = {
|
|||
list: (): Promise<unknown> => ipcRenderer.invoke('claudeAccounts:list'),
|
||||
add: (args?: { runtime?: 'host' | 'wsl'; wslDistro?: string | null }): Promise<unknown> =>
|
||||
ipcRenderer.invoke('claudeAccounts:add', args),
|
||||
cancelPendingLogin: (): Promise<boolean> =>
|
||||
ipcRenderer.invoke('claudeAccounts:cancelPendingLogin'),
|
||||
reauthenticate: (args: { accountId: string }): Promise<unknown> =>
|
||||
ipcRenderer.invoke('claudeAccounts:reauthenticate', args),
|
||||
remove: (args: { accountId: string }): Promise<unknown> =>
|
||||
|
|
|
|||
|
|
@ -15,7 +15,7 @@ import { Input } from '../ui/input'
|
|||
import { Label } from '../ui/label'
|
||||
import { Separator } from '../ui/separator'
|
||||
import { Select, SelectContent, SelectItem, SelectTrigger, SelectValue } from '../ui/select'
|
||||
import { AlertTriangle, Loader2, Plus, RefreshCw, Trash2 } from 'lucide-react'
|
||||
import { AlertTriangle, Loader2, Plus, RefreshCw, Trash2, X } from 'lucide-react'
|
||||
import { useAppStore } from '../../store'
|
||||
import { ClaudeIcon, GeminiIcon, OpenAIIcon, OpenCodeGoIcon } from '../status-bar/icons'
|
||||
import { toast } from 'sonner'
|
||||
|
|
@ -183,6 +183,10 @@ function getClaudeAccountErrorDescription(error: unknown): string {
|
|||
)
|
||||
}
|
||||
|
||||
function isClaudeAccountCancellation(error: unknown): boolean {
|
||||
return getClaudeAccountErrorDescription(error).toLowerCase() === 'claude sign-in was cancelled.'
|
||||
}
|
||||
|
||||
type LocalAccountRuntime = {
|
||||
runtime: 'host' | 'wsl'
|
||||
wslDistro?: string | null
|
||||
|
|
@ -550,6 +554,9 @@ export function AccountsPane({
|
|||
)
|
||||
}
|
||||
} catch (error) {
|
||||
if (isClaudeAccountCancellation(error)) {
|
||||
return
|
||||
}
|
||||
toast.error(
|
||||
translate(
|
||||
'auto.components.settings.AccountsPane.2743cdc0af',
|
||||
|
|
@ -608,29 +615,42 @@ export function AccountsPane({
|
|||
)}
|
||||
</p>
|
||||
</div>
|
||||
<Button
|
||||
variant="outline"
|
||||
size="xs"
|
||||
onClick={() =>
|
||||
void runClaudeAccountAction('adding', () =>
|
||||
window.api.claudeAccounts.add({
|
||||
runtime: accountRuntime.runtime,
|
||||
wslDistro: accountRuntime.wslDistro
|
||||
})
|
||||
)
|
||||
}
|
||||
disabled={
|
||||
claudeAction !== 'idle' || wslCapabilitiesLoading || accountRuntimeUnavailable
|
||||
}
|
||||
className="gap-1.5"
|
||||
>
|
||||
<div className="flex shrink-0 items-center gap-1.5">
|
||||
<Button
|
||||
variant="outline"
|
||||
size="xs"
|
||||
onClick={() =>
|
||||
void runClaudeAccountAction('adding', () =>
|
||||
window.api.claudeAccounts.add({
|
||||
runtime: accountRuntime.runtime,
|
||||
wslDistro: accountRuntime.wslDistro
|
||||
})
|
||||
)
|
||||
}
|
||||
disabled={
|
||||
claudeAction !== 'idle' || wslCapabilitiesLoading || accountRuntimeUnavailable
|
||||
}
|
||||
className="gap-1.5"
|
||||
>
|
||||
{claudeAction === 'adding' ? (
|
||||
<Loader2 className="size-3 animate-spin" />
|
||||
) : (
|
||||
<Plus className="size-3" />
|
||||
)}
|
||||
{translate('auto.components.settings.AccountsPane.b0e948a4f9', 'Add Account')}
|
||||
</Button>
|
||||
{claudeAction === 'adding' ? (
|
||||
<Loader2 className="size-3 animate-spin" />
|
||||
) : (
|
||||
<Plus className="size-3" />
|
||||
)}
|
||||
{translate('auto.components.settings.AccountsPane.b0e948a4f9', 'Add Account')}
|
||||
</Button>
|
||||
<Button
|
||||
variant="ghost"
|
||||
size="xs"
|
||||
onClick={() => void window.api.claudeAccounts.cancelPendingLogin()}
|
||||
className="gap-1.5 text-muted-foreground hover:text-foreground"
|
||||
>
|
||||
<X className="size-3" />
|
||||
{translate('auto.components.settings.AccountsPane.dbb9626ed1', 'Cancel')}
|
||||
</Button>
|
||||
) : null}
|
||||
</div>
|
||||
</div>
|
||||
|
||||
<div className="space-y-2">
|
||||
|
|
|
|||
|
|
@ -2470,6 +2470,7 @@ function createAccountsApi(): never {
|
|||
return {
|
||||
list: () => Promise.resolve(empty),
|
||||
add: () => Promise.resolve(empty),
|
||||
cancelPendingLogin: () => Promise.resolve(false),
|
||||
reauthenticate: () => Promise.resolve(empty),
|
||||
remove: () => Promise.resolve(empty),
|
||||
select: () => Promise.resolve(empty)
|
||||
|
|
|
|||
Loading…
Reference in New Issue