From 342b5357dc59db99dbaa8153b68d566b849d49f3 Mon Sep 17 00:00:00 2001 From: Neil <4138956+nwparker@users.noreply.github.com> Date: Tue, 27 Oct 2026 05:52:30 -0700 Subject: [PATCH] fix(renderer): keep render-phase guards in state so discarded renders revert them (#34414) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit QuickCommandsPane, useAddRepoCloneFlow, UntitledFileRenameDialog and the workspace board column resize hook guarded a render-phase setState with a ref written in the same render. When a sibling suspends, React discards that render and its queued updates but keeps the ref write, so the retry skipped the adjustment (no Add dialog, empty clone destination, stale save-as name, stale column width). Track each guard in state instead, and sync the column-resize handler refs in a layout effect. The other two sites from the issue (WorktreeMetaDialog prevIsOpenRef and use-native-chat-draft lastScopeKey) no longer exist on main: the dialog remounts a keyed draft and the chat draft reads via useSyncExternalStore. Fixes #25527 Co-authored-by: Matheus Breguêz --- .../editor/UntitledFileRenameDialog.tsx | 15 ++- ...phase-adjustment-discarded-render.test.tsx | 109 ++++++++++++++++++ .../components/settings/QuickCommandsPane.tsx | 8 +- .../use-workspace-kanban-column-resize.ts | 19 +-- .../components/sidebar/useAddRepoCloneFlow.ts | 10 +- 5 files changed, 140 insertions(+), 21 deletions(-) create mode 100644 src/renderer/src/components/render-phase-adjustment-discarded-render.test.tsx diff --git a/src/renderer/src/components/editor/UntitledFileRenameDialog.tsx b/src/renderer/src/components/editor/UntitledFileRenameDialog.tsx index bd8c252fd16..05b7de70870 100644 --- a/src/renderer/src/components/editor/UntitledFileRenameDialog.tsx +++ b/src/renderer/src/components/editor/UntitledFileRenameDialog.tsx @@ -39,7 +39,7 @@ export function UntitledFileRenameDialog({ const [error, setError] = useState(null) const nameInputRef = useRef(null) const focusFrameRef = useRef(null) - const seededOpenStateRef = useRef({ open: false, baseName, worktreePath }) + const [seededOpenState, setSeededOpenState] = useState({ open: false, baseName, worktreePath }) const mountedRef = useMountedRef() const displayError = externalError ?? error @@ -64,15 +64,18 @@ export function UntitledFileRenameDialog({ // Seed before painting; Radix’s open lifecycle owns focus without a post-render effect. if (open) { - const seeded = seededOpenStateRef.current - if (!seeded.open || seeded.baseName !== baseName || seeded.worktreePath !== worktreePath) { - seededOpenStateRef.current = { open: true, baseName, worktreePath } + if ( + !seededOpenState.open || + seededOpenState.baseName !== baseName || + seededOpenState.worktreePath !== worktreePath + ) { + setSeededOpenState({ open: true, baseName, worktreePath }) setName(baseName) setDir(worktreePath) setError(null) } - } else if (seededOpenStateRef.current.open) { - seededOpenStateRef.current = { open: false, baseName, worktreePath } + } else if (seededOpenState.open) { + setSeededOpenState({ open: false, baseName, worktreePath }) } const handleBrowse = useCallback(async () => { diff --git a/src/renderer/src/components/render-phase-adjustment-discarded-render.test.tsx b/src/renderer/src/components/render-phase-adjustment-discarded-render.test.tsx new file mode 100644 index 00000000000..e65c30f3db5 --- /dev/null +++ b/src/renderer/src/components/render-phase-adjustment-discarded-render.test.tsx @@ -0,0 +1,109 @@ +// @vitest-environment happy-dom +import { act, cleanup, fireEvent, render, screen } from '@testing-library/react' +import { Suspense, use, type ReactNode } from 'react' +import { afterEach, beforeEach, expect, it, vi } from 'vitest' +import { getDefaultSettings } from '../../../shared/constants' +import { ConfirmationDialogContext } from './confirmation-dialog-context' +import { UntitledFileRenameDialog } from './editor/UntitledFileRenameDialog' +import { QuickCommandsPane } from './settings/QuickCommandsPane' +import { useWorkspaceKanbanColumnResize } from './sidebar/use-workspace-kanban-column-resize' +import { useAddRepoCloneFlow } from './sidebar/useAddRepoCloneFlow' + +beforeEach(() => { + vi.useFakeTimers() +}) + +afterEach(() => { + cleanup() + vi.useRealTimers() +}) + +function SuspendUntil({ promise }: { promise: Promise }): null { + use(promise) + return null +} + +// A sibling suspends after the adjusting render, so React discards that render and retries it. +async function renderThenDiscardUpdate( + initial: ReactNode, + update: ReactNode +): Promise> { + const view = render({initial}) + let release = (): void => {} + const suspended = new Promise((resolve) => (release = resolve)) + await act(async () => + view.rerender( + + {update} + + + ) + ) + await act(async () => { + release() + await vi.advanceTimersByTimeAsync(1000) + }) + return view +} + +function KanbanColumn({ width, onCommit }: { width: number; onCommit: (w: number) => void }) { + const resize = useWorkspaceKanbanColumnResize(width, onCommit) + return ( +
+ {resize.columnWidth} +
+ ) +} + +it('applies a committed board column width', async () => { + const onCommit = vi.fn() + await renderThenDiscardUpdate( + , + + ) + expect(screen.getByTestId('column').textContent).toBe('420') + fireEvent.keyDown(screen.getByTestId('column'), { key: 'ArrowRight' }) + expect(onCommit).toHaveBeenLastCalledWith(440) +}) + +function CloneDestination({ workspaceDir }: { workspaceDir?: string }) { + const flow = useAddRepoCloneFlow({ + step: 'clone', + activeRuntimeEnvironmentId: null, + workspaceDir, + fetchWorktrees: vi.fn(), + onGitRepoReady: vi.fn() + }) + return
{flow.cloneDestination}
+} + +it('fills the clone destination once settings hydrate', async () => { + await renderThenDiscardUpdate( + , + + ) + expect(screen.getByTestId('destination').textContent).toBe('/home/dev/orca') +}) + +it('seeds the save-as dialog with the opened file name', async () => { + const props = { worktreePath: '/repo', onClose: vi.fn(), onConfirm: vi.fn() } + await renderThenDiscardUpdate( + , + + ) + expect(screen.queryByDisplayValue('Untitled-2')).not.toBeNull() +}) + +it('opens the Add dialog for a quick-command deep link', async () => { + const settings = getDefaultSettings('/home/dev') + const confirm = async (): Promise => true + await renderThenDiscardUpdate( + + + , + + + + ) + expect(screen.queryByText('Add Quick Command')).not.toBeNull() +}) diff --git a/src/renderer/src/components/settings/QuickCommandsPane.tsx b/src/renderer/src/components/settings/QuickCommandsPane.tsx index 4265ef5fb20..653bf5fc1a2 100644 --- a/src/renderer/src/components/settings/QuickCommandsPane.tsx +++ b/src/renderer/src/components/settings/QuickCommandsPane.tsx @@ -1,4 +1,4 @@ -import { useCallback, useEffect, useMemo, useRef, useState } from 'react' +import { useCallback, useEffect, useMemo, useState } from 'react' import type { GlobalSettings } from '../../../../shared/global-settings-types' import type { TerminalQuickCommand } from '../../../../shared/terminal-quick-command-types' import { getTerminalQuickCommandScope } from '../../../../shared/terminal-quick-commands' @@ -125,7 +125,7 @@ export function QuickCommandsPane({ const hostOptions = getTerminalQuickCommandHostOptions(settings, runtimeEnvironments) const [editor, setEditor] = useState(null) - const consumedAddIntentSignalRef = useRef(0) + const [consumedAddIntentSignal, setConsumedAddIntentSignal] = useState(0) // Null means sticky All so newly added repos appear automatically, matching Tasks filtering. const [scopeSelection, setScopeSelection] = useState | null>(null) const [scopePopoverOpen, setScopePopoverOpen] = useState(false) @@ -196,10 +196,10 @@ export function QuickCommandsPane({ const intentSignal = addCommandIntentSignal if ( typeof intentSignal === 'number' && - shouldOpenQuickCommandAddIntent(intentSignal, consumedAddIntentSignalRef.current) + shouldOpenQuickCommandAddIntent(intentSignal, consumedAddIntentSignal) ) { // Consume deep-link add intent before paint so the editor opens without a pane-only flash. - consumedAddIntentSignalRef.current = intentSignal + setConsumedAddIntentSignal(intentSignal) setEditor({ mode: 'add', command: createDraftForCurrentFilter(), diff --git a/src/renderer/src/components/sidebar/use-workspace-kanban-column-resize.ts b/src/renderer/src/components/sidebar/use-workspace-kanban-column-resize.ts index 03bd998c678..73f4fed6202 100644 --- a/src/renderer/src/components/sidebar/use-workspace-kanban-column-resize.ts +++ b/src/renderer/src/components/sidebar/use-workspace-kanban-column-resize.ts @@ -1,4 +1,4 @@ -import { useCallback, useEffect, useRef, useState } from 'react' +import { useCallback, useEffect, useLayoutEffect, useRef, useState } from 'react' import type React from 'react' import { WORKSPACE_BOARD_COLUMN_WIDTH_STEP, @@ -21,6 +21,7 @@ export function useWorkspaceKanbanColumnResize( ) const [isResizingColumn, setIsResizingColumn] = useState(false) const nextCommittedWidth = clampWorkspaceBoardColumnWidth(committedWidth) + const [renderedCommittedWidth, setRenderedCommittedWidth] = useState(nextCommittedWidth) const committedWidthRef = useRef(nextCommittedWidth) const commitWidthRef = useRef(onCommitWidth) const resizingRef = useRef(false) @@ -30,16 +31,20 @@ export function useWorkspaceKanbanColumnResize( const frameRef = useRef(null) commitWidthRef.current = onCommitWidth - if (committedWidthRef.current !== nextCommittedWidth) { + if (renderedCommittedWidth !== nextCommittedWidth) { + setRenderedCommittedWidth(nextCommittedWidth) + if (!resizingRef.current && columnWidth !== nextCommittedWidth) { + // Apply external widths before rendering children; retain the local draft during active resize. + setColumnWidth(nextCommittedWidth) + } + } + // Sync handler refs after commit so a discarded render cannot leave them on an abandoned width. + useLayoutEffect(() => { committedWidthRef.current = nextCommittedWidth if (!resizingRef.current) { draftWidthRef.current = nextCommittedWidth - if (columnWidth !== nextCommittedWidth) { - // Apply external widths before rendering children; retain the local draft during active resize. - setColumnWidth(nextCommittedWidth) - } } - } + }, [nextCommittedWidth]) const resetDocumentStyles = useCallback(() => { document.body.style.cursor = '' diff --git a/src/renderer/src/components/sidebar/useAddRepoCloneFlow.ts b/src/renderer/src/components/sidebar/useAddRepoCloneFlow.ts index 7ae333e52d0..0090f3d042d 100644 --- a/src/renderer/src/components/sidebar/useAddRepoCloneFlow.ts +++ b/src/renderer/src/components/sidebar/useAddRepoCloneFlow.ts @@ -63,7 +63,7 @@ export function useAddRepoCloneFlow({ // Reject stale callbacks after a clone flow is reset or superseded. const cloneGenRef = useRef(0) // Allow late settings hydration to fill the default once per clone-step entry. - const cloneStepAutoFilledRef = useRef(false) + const [cloneStepAutoFilled, setCloneStepAutoFilled] = useState(false) useEffect(() => { if (!isCloning) { @@ -77,13 +77,15 @@ export function useAddRepoCloneFlow({ cloneDestination, activeRuntimeEnvironmentId, workspaceDir, - cloneStepAutoFilled: cloneStepAutoFilledRef.current + cloneStepAutoFilled }) if (step !== 'clone') { - cloneStepAutoFilledRef.current = false + if (cloneStepAutoFilled) { + setCloneStepAutoFilled(false) + } } else if (cloneDestinationAutoFill) { // Late defaults may seed local destinations; remote clone paths remain user-entered. - cloneStepAutoFilledRef.current = true + setCloneStepAutoFilled(true) setCloneDestination(cloneDestinationAutoFill.destination) }