Prevent background worktree creation from stealing focus (#6403)
Avoid forcing the active view back to the creation panel when background worktree preflight completes after the user has already navigated away. - Add a `revealCreationSurface` option to `continueBackgroundWorktreeCreation` to skip switching views. - Check both `activeView` and `activePendingCreationId` to determine if the creation UI is actually visible. - Use this option when kicking off background creation for GitHub work items.
This commit is contained in:
parent
01417f689e
commit
1f85822b34
|
|
@ -151,7 +151,8 @@ describe('createGitHubWorkItemWorkspaceInBackground', () => {
|
|||
startupPlan: null,
|
||||
quickPrompt: '',
|
||||
quickTelemetry: null
|
||||
})
|
||||
}),
|
||||
{ revealCreationSurface: false }
|
||||
)
|
||||
})
|
||||
|
||||
|
|
@ -573,7 +574,8 @@ describe('createGitHubWorkItemWorkspaceInBackground', () => {
|
|||
pushTarget: { remote: 'origin', branch: 'feature/from-pr' },
|
||||
branchNameOverride: 'feature/from-pr',
|
||||
compareBaseRef: 'main'
|
||||
})
|
||||
}),
|
||||
{ revealCreationSurface: false }
|
||||
)
|
||||
})
|
||||
|
||||
|
|
|
|||
|
|
@ -260,7 +260,7 @@ export async function createGitHubWorkItemWorkspaceInBackground(
|
|||
quickTelemetry
|
||||
}
|
||||
|
||||
deps.continueBackgroundCreate(creationId, request)
|
||||
deps.continueBackgroundCreate(creationId, request, { revealCreationSurface: false })
|
||||
return { kind: 'background-started' }
|
||||
} catch (error) {
|
||||
if (!deps.hasPendingCreate(creationId)) {
|
||||
|
|
|
|||
|
|
@ -1,13 +1,23 @@
|
|||
import { readFileSync } from 'node:fs'
|
||||
import { join } from 'node:path'
|
||||
import { describe, expect, it, vi } from 'vitest'
|
||||
import { beforeEach, describe, expect, it, vi } from 'vitest'
|
||||
import type { WorktreeCreationRequest } from '@/lib/pending-worktree-creation'
|
||||
|
||||
type TestActiveView = 'terminal' | 'tasks'
|
||||
|
||||
const store = {
|
||||
settings: { activeRuntimeEnvironmentId: null as string | null },
|
||||
activeView: 'terminal' as TestActiveView,
|
||||
activePendingCreationId: 'creation-1' as string | null,
|
||||
repos: [] as { id: string; connectionId?: string | null }[],
|
||||
beginPendingWorktreeCreation: vi.fn(),
|
||||
updatePendingWorktreeCreation: vi.fn(),
|
||||
pendingWorktreeCreations: { 'creation-1': { creationId: 'creation-1' } },
|
||||
pendingWorktreeCreations: { 'creation-1': { creationId: 'creation-1' } } as Record<
|
||||
string,
|
||||
{ creationId: string; request?: WorktreeCreationRequest }
|
||||
>,
|
||||
removePendingWorktreeCreation: vi.fn(),
|
||||
updateWorktreeMeta: vi.fn(),
|
||||
setActivePendingWorktreeCreation: vi.fn(),
|
||||
setActiveView: vi.fn(),
|
||||
setSidebarOpen: vi.fn(),
|
||||
|
|
@ -37,6 +47,18 @@ vi.mock('@/lib/new-workspace', () => ({
|
|||
ensureAgentStartupInTerminal: vi.fn()
|
||||
}))
|
||||
|
||||
vi.mock('sonner', () => ({
|
||||
toast: {
|
||||
error: vi.fn()
|
||||
}
|
||||
}))
|
||||
|
||||
import { toast } from 'sonner'
|
||||
import {
|
||||
activateAndRevealWorktree,
|
||||
ensureWorktreeHasInitialTerminal
|
||||
} from '@/lib/worktree-activation'
|
||||
import { queueNewWorkspaceTerminalFocus } from '@/lib/new-workspace-terminal-focus'
|
||||
import {
|
||||
beginBackgroundWorktreePreparation,
|
||||
continueBackgroundWorktreeCreation,
|
||||
|
|
@ -45,6 +67,17 @@ import {
|
|||
|
||||
const FLOW_SOURCE = readFileSync(join(__dirname, 'worktree-creation-flow.ts'), 'utf8')
|
||||
|
||||
beforeEach(() => {
|
||||
vi.clearAllMocks()
|
||||
store.settings.activeRuntimeEnvironmentId = null
|
||||
store.activeView = 'terminal'
|
||||
store.activePendingCreationId = 'creation-1'
|
||||
store.repos = []
|
||||
store.pendingWorktreeCreations = { 'creation-1': { creationId: 'creation-1' } }
|
||||
store.createWorktree.mockImplementation(() => new Promise(() => {}))
|
||||
vi.mocked(ensureWorktreeHasInitialTerminal).mockReturnValue('tab-1')
|
||||
})
|
||||
|
||||
function makeRequest(overrides: Partial<WorktreeCreationRequest> = {}): WorktreeCreationRequest {
|
||||
return {
|
||||
repoId: 'repo-1',
|
||||
|
|
@ -60,6 +93,11 @@ function makeRequest(overrides: Partial<WorktreeCreationRequest> = {}): Worktree
|
|||
}
|
||||
}
|
||||
|
||||
async function flushAsyncWorktreeCreation(): Promise<void> {
|
||||
await Promise.resolve()
|
||||
await Promise.resolve()
|
||||
}
|
||||
|
||||
function sourceBetween(source: string, startPattern: string, endPattern: string): string {
|
||||
const start = source.indexOf(startPattern)
|
||||
expect(start).toBeGreaterThanOrEqual(0)
|
||||
|
|
@ -122,6 +160,9 @@ describe('staged background worktree creation', () => {
|
|||
it('replaces the staged request before the create starts', () => {
|
||||
store.updatePendingWorktreeCreation.mockClear()
|
||||
store.createWorktree.mockClear()
|
||||
store.setActivePendingWorktreeCreation.mockClear()
|
||||
store.setActiveView.mockClear()
|
||||
store.setSidebarOpen.mockClear()
|
||||
|
||||
const request = makeRequest({ setupDecision: 'run' })
|
||||
const started = continueBackgroundWorktreeCreation('creation-1', request)
|
||||
|
|
@ -141,6 +182,82 @@ describe('staged background worktree creation', () => {
|
|||
expect(createCall?.[1]).toBe('feature')
|
||||
expect(createCall?.[3]).toBe('run')
|
||||
expect(createCall?.[18]).toBe('creation-1')
|
||||
expect(store.setActivePendingWorktreeCreation).toHaveBeenCalledWith('creation-1')
|
||||
expect(store.setActiveView).toHaveBeenCalledWith('terminal')
|
||||
expect(store.setSidebarOpen).toHaveBeenCalledWith(true)
|
||||
})
|
||||
|
||||
it('can continue without revealing a staged create after background preflight', () => {
|
||||
store.updatePendingWorktreeCreation.mockClear()
|
||||
store.createWorktree.mockClear()
|
||||
store.setActivePendingWorktreeCreation.mockClear()
|
||||
store.setActiveView.mockClear()
|
||||
store.setSidebarOpen.mockClear()
|
||||
|
||||
const request = makeRequest({ setupDecision: 'run' })
|
||||
const started = continueBackgroundWorktreeCreation('creation-1', request, {
|
||||
revealCreationSurface: false
|
||||
})
|
||||
|
||||
expect(started).toBe(true)
|
||||
expect(store.updatePendingWorktreeCreation).toHaveBeenCalledWith(
|
||||
'creation-1',
|
||||
expect.objectContaining({
|
||||
phase: 'fetching',
|
||||
request
|
||||
})
|
||||
)
|
||||
expect(store.createWorktree).toHaveBeenCalledTimes(1)
|
||||
expect(store.setActivePendingWorktreeCreation).not.toHaveBeenCalled()
|
||||
expect(store.setActiveView).not.toHaveBeenCalled()
|
||||
expect(store.setSidebarOpen).not.toHaveBeenCalled()
|
||||
})
|
||||
|
||||
it('does not reveal a completed staged create after the user leaves the creation surface', async () => {
|
||||
store.activeView = 'tasks'
|
||||
store.createWorktree.mockResolvedValueOnce({
|
||||
worktree: {
|
||||
id: 'wt-1',
|
||||
repoId: 'repo-1'
|
||||
}
|
||||
})
|
||||
|
||||
const started = continueBackgroundWorktreeCreation('creation-1', makeRequest(), {
|
||||
revealCreationSurface: false
|
||||
})
|
||||
|
||||
expect(started).toBe(true)
|
||||
await flushAsyncWorktreeCreation()
|
||||
expect(activateAndRevealWorktree).not.toHaveBeenCalled()
|
||||
expect(ensureWorktreeHasInitialTerminal).toHaveBeenCalledWith(
|
||||
store,
|
||||
'wt-1',
|
||||
undefined,
|
||||
undefined,
|
||||
undefined,
|
||||
undefined
|
||||
)
|
||||
expect(queueNewWorkspaceTerminalFocus).not.toHaveBeenCalled()
|
||||
expect(store.removePendingWorktreeCreation).toHaveBeenCalledWith('creation-1')
|
||||
})
|
||||
|
||||
it('toasts a staged create error after the user leaves the creation surface', async () => {
|
||||
store.activeView = 'tasks'
|
||||
store.createWorktree.mockRejectedValueOnce(new Error('create failed'))
|
||||
|
||||
const started = continueBackgroundWorktreeCreation('creation-1', makeRequest(), {
|
||||
revealCreationSurface: false
|
||||
})
|
||||
|
||||
expect(started).toBe(true)
|
||||
await flushAsyncWorktreeCreation()
|
||||
expect(store.updatePendingWorktreeCreation).toHaveBeenCalledWith(
|
||||
'creation-1',
|
||||
expect.objectContaining({
|
||||
status: 'error'
|
||||
})
|
||||
)
|
||||
expect(toast.error).toHaveBeenCalledTimes(1)
|
||||
})
|
||||
})
|
||||
|
||||
|
|
|
|||
|
|
@ -18,6 +18,10 @@ import type { CreateWorktreeResult } from '../../../shared/types'
|
|||
import type { WorktreeCreationRequest } from '@/lib/pending-worktree-creation'
|
||||
import { createBrowserUuid } from '@/lib/browser-uuid'
|
||||
|
||||
type ContinueBackgroundWorktreeCreationOptions = {
|
||||
revealCreationSurface?: boolean
|
||||
}
|
||||
|
||||
// Why: mirrors the startup-opt the composer used to build inline. The renderer
|
||||
// only seeds the first terminal when the backend did not already spawn it.
|
||||
function buildStartupOpt(
|
||||
|
|
@ -51,6 +55,13 @@ function getWorktreeCreationIndeterminate(request: WorktreeCreationRequest): boo
|
|||
return getActiveRuntimeTarget(useAppStore.getState().settings).kind !== 'local'
|
||||
}
|
||||
|
||||
// Why: activePendingCreationId can outlive the terminal route when the user
|
||||
// switches app views; only the terminal route renders the creation panel.
|
||||
function isPendingCreationSurfaceVisible(creationId: string): boolean {
|
||||
const state = useAppStore.getState()
|
||||
return state.activeView === 'terminal' && state.activePendingCreationId === creationId
|
||||
}
|
||||
|
||||
function revealPendingCreation(
|
||||
creationId: string,
|
||||
request: WorktreeCreationRequest,
|
||||
|
|
@ -151,7 +162,7 @@ async function executeWorktreeCreation(
|
|||
})
|
||||
// Why: only toast when the panel isn't already showing this error (the user
|
||||
// navigated away), so a visible failure isn't announced twice.
|
||||
if (useAppStore.getState().activePendingCreationId !== creationId) {
|
||||
if (!isPendingCreationSurfaceVisible(creationId)) {
|
||||
toast.error(message)
|
||||
}
|
||||
return
|
||||
|
|
@ -183,7 +194,7 @@ async function executeWorktreeCreation(
|
|||
|
||||
// `createWorktree` already inserted the real worktree row. Whether we steal
|
||||
// the view depends on whether the user is still watching this creation.
|
||||
const stillActive = useAppStore.getState().activePendingCreationId === creationId
|
||||
const stillActive = isPendingCreationSurfaceVisible(creationId)
|
||||
|
||||
let activation: ActivateAndRevealResult | false = false
|
||||
let primaryTabId: string | null
|
||||
|
|
@ -258,7 +269,8 @@ export function beginBackgroundWorktreePreparation(request: WorktreeCreationRequ
|
|||
/** Continue a staged pending entry once async preflight has produced a final request. */
|
||||
export function continueBackgroundWorktreeCreation(
|
||||
creationId: string,
|
||||
request: WorktreeCreationRequest
|
||||
request: WorktreeCreationRequest,
|
||||
options: ContinueBackgroundWorktreeCreationOptions = {}
|
||||
): boolean {
|
||||
const store = useAppStore.getState()
|
||||
if (!store.pendingWorktreeCreations[creationId]) {
|
||||
|
|
@ -270,9 +282,13 @@ export function continueBackgroundWorktreeCreation(
|
|||
error: undefined,
|
||||
request
|
||||
})
|
||||
store.setActivePendingWorktreeCreation(creationId)
|
||||
store.setActiveView('terminal')
|
||||
store.setSidebarOpen(true)
|
||||
// Why: background work-item preflight can finish after the user moved on; keep
|
||||
// the pending row alive without reselecting the creation panel in that case.
|
||||
if (options.revealCreationSurface !== false) {
|
||||
store.setActivePendingWorktreeCreation(creationId)
|
||||
store.setActiveView('terminal')
|
||||
store.setSidebarOpen(true)
|
||||
}
|
||||
void executeWorktreeCreation(creationId, request)
|
||||
return true
|
||||
}
|
||||
|
|
|
|||
Loading…
Reference in New Issue