Compare commits

..

1 Commits

Author SHA1 Message Date
OpenClaude Worker 3
d51256df6f fix: replace isDeepStrictEqual with navigation-aware options comparison
The select cursor highlight was broken because isDeepStrictEqual in
use-select-navigation.ts and use-multi-select-state.ts would fail when
options contained identity-unstable properties (JSX label elements,
function onChange callbacks, computed disabled booleans). This caused
the reset logic to fire on every re-render, resetting focusedValue
back to the first option.

Replace isDeepStrictEqual with optionsNavigateEqual which only compares
properties that affect navigation behavior: value, disabled, and type.
ReactNode labels and function callbacks are intentionally excluded as
they are identity-unstable but don't change navigation semantics.

Fixes #472
2026-04-08 13:58:18 +05:30
5 changed files with 35 additions and 118 deletions

View File

@@ -1,5 +1,4 @@
import { useCallback, useState } from 'react' import { useCallback, useState } from 'react'
import { isDeepStrictEqual } from 'util'
import { useRegisterOverlay } from '../../context/overlayContext.js' import { useRegisterOverlay } from '../../context/overlayContext.js'
import type { InputEvent } from '../../ink/events/input-event.js' import type { InputEvent } from '../../ink/events/input-event.js'
// eslint-disable-next-line custom-rules/prefer-use-keybindings -- raw space/arrow multiselect input // eslint-disable-next-line custom-rules/prefer-use-keybindings -- raw space/arrow multiselect input
@@ -9,6 +8,7 @@ import {
normalizeFullWidthSpace, normalizeFullWidthSpace,
} from '../../utils/stringUtils.js' } from '../../utils/stringUtils.js'
import type { OptionWithDescription } from './select.js' import type { OptionWithDescription } from './select.js'
import { optionsNavigateEqual } from './use-select-navigation.js'
import { useSelectNavigation } from './use-select-navigation.js' import { useSelectNavigation } from './use-select-navigation.js'
export type UseMultiSelectStateProps<T> = { export type UseMultiSelectStateProps<T> = {
@@ -174,7 +174,7 @@ export function useMultiSelectState<T>({
// and the deleted ui/useMultiSelectState.ts — without this, MCPServerDesktopImportDialog // and the deleted ui/useMultiSelectState.ts — without this, MCPServerDesktopImportDialog
// keeps colliding servers checked after getAllMcpConfigs() resolves. // keeps colliding servers checked after getAllMcpConfigs() resolves.
const [lastOptions, setLastOptions] = useState(options) const [lastOptions, setLastOptions] = useState(options)
if (options !== lastOptions && !isDeepStrictEqual(options, lastOptions)) { if (options !== lastOptions && !optionsNavigateEqual(options, lastOptions)) {
setSelectedValues(defaultValue) setSelectedValues(defaultValue)
setLastOptions(options) setLastOptions(options)
} }

View File

@@ -6,10 +6,34 @@ import {
useRef, useRef,
useState, useState,
} from 'react' } from 'react'
import { isDeepStrictEqual } from 'util'
import OptionMap from './option-map.js' import OptionMap from './option-map.js'
import type { OptionWithDescription } from './select.js' import type { OptionWithDescription } from './select.js'
/**
* Compare two option arrays for structural equality on properties that
* affect navigation behavior. ReactNode `label` and function `onChange`
* are intentionally excluded — they are identity-unstable (new reference
* each render) but don't change navigation semantics.
*/
export function optionsNavigateEqual<T>(
a: OptionWithDescription<T>[],
b: OptionWithDescription<T>[],
): boolean {
if (a.length !== b.length) return false
for (let i = 0; i < a.length; i++) {
const ao = a[i]!
const bo = b[i]!
if (
ao.value !== bo.value ||
ao.disabled !== bo.disabled ||
ao.type !== bo.type
) {
return false
}
}
return true
}
type State<T> = { type State<T> = {
/** /**
* Map where key is option's value and value is option's index. * Map where key is option's value and value is option's index.
@@ -524,7 +548,7 @@ export function useSelectNavigation<T>({
const [lastOptions, setLastOptions] = useState(options) const [lastOptions, setLastOptions] = useState(options)
if (options !== lastOptions && !isDeepStrictEqual(options, lastOptions)) { if (options !== lastOptions && !optionsNavigateEqual(options, lastOptions)) {
dispatch({ dispatch({
type: 'reset', type: 'reset',
state: createDefaultState({ state: createDefaultState({

View File

@@ -238,7 +238,6 @@ import { usePromptsFromClaudeInChrome } from 'src/hooks/usePromptsFromClaudeInCh
import { getTipToShowOnSpinner, recordShownTip } from 'src/services/tips/tipScheduler.js'; import { getTipToShowOnSpinner, recordShownTip } from 'src/services/tips/tipScheduler.js';
import type { Theme } from 'src/utils/theme.js'; import type { Theme } from 'src/utils/theme.js';
import { isPromptTypingSuppressionActive } from './replInputSuppression.js'; import { isPromptTypingSuppressionActive } from './replInputSuppression.js';
import { shouldRunStartupChecks } from './replStartupGates.js';
import { checkAndDisableBypassPermissionsIfNeeded, checkAndDisableAutoModeIfNeeded, useKickOffCheckAndDisableBypassPermissionsIfNeeded, useKickOffCheckAndDisableAutoModeIfNeeded } from 'src/utils/permissions/bypassPermissionsKillswitch.js'; import { checkAndDisableBypassPermissionsIfNeeded, checkAndDisableAutoModeIfNeeded, useKickOffCheckAndDisableBypassPermissionsIfNeeded, useKickOffCheckAndDisableAutoModeIfNeeded } from 'src/utils/permissions/bypassPermissionsKillswitch.js';
import { SandboxManager } from 'src/utils/sandbox/sandbox-adapter.js'; import { SandboxManager } from 'src/utils/sandbox/sandbox-adapter.js';
import { SANDBOX_NETWORK_ACCESS_TOOL_NAME } from 'src/cli/structuredIO.js'; import { SANDBOX_NETWORK_ACCESS_TOOL_NAME } from 'src/cli/structuredIO.js';
@@ -793,8 +792,10 @@ export function REPL({
// accepts, and only then is the REPL component mounted and this effect runs. // accepts, and only then is the REPL component mounted and this effect runs.
// This ensures that plugin installations from repository and user settings only // This ensures that plugin installations from repository and user settings only
// happen after explicit user consent to trust the current working directory. // happen after explicit user consent to trust the current working directory.
// Deferring startup checks is handled below (after promptTypingSuppressionActive useEffect(() => {
// is declared) to avoid temporal dead zone issues. if (isRemoteSession) return;
void performStartupChecks(setAppState);
}, [setAppState, isRemoteSession]);
// Allow Claude in Chrome MCP to send prompts through MCP notifications // Allow Claude in Chrome MCP to send prompts through MCP notifications
// and sync permission mode changes to the Chrome extension // and sync permission mode changes to the Chrome extension
@@ -1428,25 +1429,6 @@ export function REPL({
const activeRemote = sshRemote.isRemoteMode ? sshRemote : directConnect.isRemoteMode ? directConnect : remoteSession; const activeRemote = sshRemote.isRemoteMode ? sshRemote : directConnect.isRemoteMode ? directConnect : remoteSession;
const [pastedContents, setPastedContents] = useState<Record<number, PastedContent>>({}); const [pastedContents, setPastedContents] = useState<Record<number, PastedContent>>({});
const [submitCount, setSubmitCount] = useState(0); const [submitCount, setSubmitCount] = useState(0);
// Defer startup checks until the user has submitted their first message.
// A timeout or grace period is insufficient (issue #363): if the user pauses
// before typing, startup checks can still fire and recommendation dialogs
// steal focus. Only the user's first submission guarantees the prompt was
// the first thing they interacted with.
const startupChecksStartedRef = React.useRef(false);
const hasHadFirstSubmission = (submitCount ?? 0) > 0;
useEffect(() => {
if (isRemoteSession) return;
if (startupChecksStartedRef.current) return;
if (!shouldRunStartupChecks({
isRemoteSession,
hasStarted: startupChecksStartedRef.current,
hasHadFirstSubmission,
})) return;
startupChecksStartedRef.current = true;
void performStartupChecks(setAppState);
}, [setAppState, isRemoteSession, hasHadFirstSubmission]);
// Ref instead of state to avoid triggering React re-renders on every // Ref instead of state to avoid triggering React re-renders on every
// streaming text_delta. The spinner reads this via its animation timer. // streaming text_delta. The spinner reads this via its animation timer.
const responseLengthRef = useRef(0); const responseLengthRef = useRef(0);
@@ -2079,14 +2061,13 @@ export function REPL({
if (allowDialogsWithAnimation && showRemoteCallout) return 'remote-callout'; if (allowDialogsWithAnimation && showRemoteCallout) return 'remote-callout';
// LSP plugin recommendation (lowest priority - non-blocking suggestion) // LSP plugin recommendation (lowest priority - non-blocking suggestion)
// Suppress during startup window to prevent stealing focus from the prompt (issue #363) if (allowDialogsWithAnimation && lspRecommendation) return 'lsp-recommendation';
if (allowDialogsWithAnimation && lspRecommendation && startupChecksStartedRef.current) return 'lsp-recommendation';
// Plugin hint from CLI/SDK stderr (same priority band as LSP rec) // Plugin hint from CLI/SDK stderr (same priority band as LSP rec)
if (allowDialogsWithAnimation && hintRecommendation && startupChecksStartedRef.current) return 'plugin-hint'; if (allowDialogsWithAnimation && hintRecommendation) return 'plugin-hint';
// Desktop app upsell (max 3 launches, lowest priority) // Desktop app upsell (max 3 launches, lowest priority)
if (allowDialogsWithAnimation && showDesktopUpsellStartup && startupChecksStartedRef.current) return 'desktop-upsell'; if (allowDialogsWithAnimation && showDesktopUpsellStartup) return 'desktop-upsell';
return undefined; return undefined;
} }
const focusedInputDialog = getFocusedInputDialog(); const focusedInputDialog = getFocusedInputDialog();

View File

@@ -1,53 +0,0 @@
import { describe, expect, test } from 'bun:test'
import { shouldRunStartupChecks } from './replStartupGates.js'
describe('shouldRunStartupChecks', () => {
test('runs checks after first message submission', () => {
expect(shouldRunStartupChecks({
isRemoteSession: false,
hasStarted: false,
hasHadFirstSubmission: true,
})).toBe(true)
})
test('skips checks in remote sessions even after submission', () => {
expect(shouldRunStartupChecks({
isRemoteSession: true,
hasStarted: false,
hasHadFirstSubmission: true,
})).toBe(false)
})
test('skips checks if already started', () => {
expect(shouldRunStartupChecks({
isRemoteSession: false,
hasStarted: true,
hasHadFirstSubmission: true,
})).toBe(false)
})
test('does not run checks before first submission', () => {
expect(shouldRunStartupChecks({
isRemoteSession: false,
hasStarted: false,
hasHadFirstSubmission: false,
})).toBe(false)
})
test('does not run checks when idle before first submission', () => {
expect(shouldRunStartupChecks({
isRemoteSession: false,
hasStarted: false,
hasHadFirstSubmission: false,
})).toBe(false)
})
test('skips checks in remote session regardless of other conditions', () => {
expect(shouldRunStartupChecks({
isRemoteSession: true,
hasStarted: false,
hasHadFirstSubmission: false,
})).toBe(false)
})
})

View File

@@ -1,35 +0,0 @@
/**
* Startup gates for the REPL.
*
* Prevents startup plugin checks and recommendation dialogs from stealing
* focus before the user has interacted with the prompt.
*
* This addresses the root cause of issue #363: on mount, performStartupChecks
* triggers plugin loading, which populates trackedFiles, which triggers
* useLspPluginRecommendation to surface an LSP recommendation dialog. Since
* promptTypingSuppressionActive is false before the user has typed anything,
* getFocusedInputDialog() returns the dialog, unmounting PromptInput entirely.
*
* The fix gates startup checks on actual prompt interaction. A pure timeout
* or grace period is insufficient because pausing before typing would still
* allow dialogs to steal focus. Only the user's first submission guarantees
* the prompt is no longer in the vulnerable pre-interaction window.
*/
/**
* Determines whether startup checks should run.
*
* Startup checks are deferred until the user has submitted their first
* message. This guarantees the prompt was the first thing the user interacted
* with, so no recommendation dialog can steal focus before the first keystroke.
*/
export function shouldRunStartupChecks(options: {
isRemoteSession: boolean;
hasStarted: boolean;
hasHadFirstSubmission: boolean;
}): boolean {
if (options.isRemoteSession) return false;
if (options.hasStarted) return false;
if (!options.hasHadFirstSubmission) return false;
return true;
}