fix: gate startup checks on prompt readiness, not just a timeout (issue #363)
The previous approach used a fixed 1500ms timeout, but as gnanam1990 pointed out, if a user pauses for >1.5s before typing the timer can still fire and recommendation dialogs can steal focus. This is a timing mitigation, not a reliable fix. New approach: gate startup checks on actual prompt readiness: 1. After first message submission (submitCount > 0) — always safe 2. After grace period (3s) elapsed AND user is idle — safe because no dialog will interrupt an idle user who hasn't started typing 3. While user is actively typing — deferrred until they stop This ensures startup checks never steal focus from a prompt the user is about to type into, regardless of how long they pause before typing. Also removes the old STARTUP_CHECK_DELAY_MS constant in favor of STARTUP_GRACE_PERIOD_MS with clearer semantics.
This commit is contained in:
@@ -238,7 +238,7 @@ 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, STARTUP_CHECK_DELAY_MS } from './replStartupGates.js';
|
import { shouldRunStartupChecks, STARTUP_GRACE_PERIOD_MS } 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';
|
||||||
@@ -1338,20 +1338,33 @@ export function REPL({
|
|||||||
inputValueRef.current = inputValue;
|
inputValueRef.current = inputValue;
|
||||||
const promptTypingSuppressionActive = isPromptTypingSuppressionActive(isPromptInputActive, inputValue);
|
const promptTypingSuppressionActive = isPromptTypingSuppressionActive(isPromptInputActive, inputValue);
|
||||||
|
|
||||||
// Defer startup checks by STARTUP_CHECK_DELAY_MS and gate on
|
// Defer startup checks until the user has interacted with the prompt.
|
||||||
// promptTypingSuppressionActive so that plugin loading doesn't steal focus
|
// A pure timeout is insufficient (issue #363): if the user pauses >1.5s
|
||||||
// from the prompt during the vulnerable startup window (issue #363).
|
// before typing, the timer can still fire and recommendation dialogs can
|
||||||
|
// steal focus. Instead, we gate on actual prompt readiness:
|
||||||
|
// - First message submitted (submitCount > 0)
|
||||||
|
// - Grace period elapsed + user is not actively typing
|
||||||
|
// - User is typing (deferred until they stop)
|
||||||
const startupChecksStartedRef = React.useRef(false);
|
const startupChecksStartedRef = React.useRef(false);
|
||||||
|
const [startupGraceElapsed, setStartupGraceElapsed] = useState(false);
|
||||||
|
useEffect(() => {
|
||||||
|
const timer = setTimeout(() => setStartupGraceElapsed(true), STARTUP_GRACE_PERIOD_MS);
|
||||||
|
return () => clearTimeout(timer);
|
||||||
|
}, []);
|
||||||
|
const hasHadFirstSubmission = submitCount > 0;
|
||||||
useEffect(() => {
|
useEffect(() => {
|
||||||
if (isRemoteSession) return;
|
if (isRemoteSession) return;
|
||||||
if (startupChecksStartedRef.current) return;
|
if (startupChecksStartedRef.current) return;
|
||||||
const timer = setTimeout(() => {
|
if (!shouldRunStartupChecks({
|
||||||
if (!shouldRunStartupChecks(isRemoteSession, startupChecksStartedRef.current, promptTypingSuppressionActive)) return;
|
isRemoteSession,
|
||||||
|
hasStarted: startupChecksStartedRef.current,
|
||||||
|
promptTypingSuppressionActive,
|
||||||
|
hasHadFirstSubmission,
|
||||||
|
gracePeriodElapsed: startupGraceElapsed,
|
||||||
|
})) return;
|
||||||
startupChecksStartedRef.current = true;
|
startupChecksStartedRef.current = true;
|
||||||
void performStartupChecks(setAppState);
|
void performStartupChecks(setAppState);
|
||||||
}, STARTUP_CHECK_DELAY_MS);
|
}, [setAppState, isRemoteSession, promptTypingSuppressionActive, hasHadFirstSubmission, startupGraceElapsed]);
|
||||||
return () => clearTimeout(timer);
|
|
||||||
}, [setAppState, isRemoteSession, promptTypingSuppressionActive]);
|
|
||||||
const insertTextRef = useRef<{
|
const insertTextRef = useRef<{
|
||||||
insert: (text: string) => void;
|
insert: (text: string) => void;
|
||||||
setInputWithCursor: (value: string, cursor: number) => void;
|
setInputWithCursor: (value: string, cursor: number) => void;
|
||||||
|
|||||||
@@ -1,32 +1,92 @@
|
|||||||
import { describe, expect, test } from 'bun:test'
|
import { describe, expect, test } from 'bun:test'
|
||||||
|
|
||||||
import { shouldRunStartupChecks, STARTUP_CHECK_DELAY_MS } from './replStartupGates.js'
|
import { shouldRunStartupChecks, STARTUP_GRACE_PERIOD_MS } from './replStartupGates.js'
|
||||||
|
|
||||||
describe('shouldRunStartupChecks', () => {
|
describe('shouldRunStartupChecks', () => {
|
||||||
test('runs checks when not remote, not started, and not typing', () => {
|
test('runs checks after first message submission regardless of grace period', () => {
|
||||||
expect(shouldRunStartupChecks(false, false, false)).toBe(true)
|
expect(shouldRunStartupChecks({
|
||||||
|
isRemoteSession: false,
|
||||||
|
hasStarted: false,
|
||||||
|
promptTypingSuppressionActive: false,
|
||||||
|
hasHadFirstSubmission: true,
|
||||||
|
gracePeriodElapsed: false,
|
||||||
|
})).toBe(true)
|
||||||
})
|
})
|
||||||
|
|
||||||
test('skips checks in remote sessions', () => {
|
test('skips checks in remote sessions', () => {
|
||||||
expect(shouldRunStartupChecks(true, false, false)).toBe(false)
|
expect(shouldRunStartupChecks({
|
||||||
|
isRemoteSession: true,
|
||||||
|
hasStarted: false,
|
||||||
|
promptTypingSuppressionActive: false,
|
||||||
|
hasHadFirstSubmission: false,
|
||||||
|
gracePeriodElapsed: true,
|
||||||
|
})).toBe(false)
|
||||||
})
|
})
|
||||||
|
|
||||||
test('skips checks if already started', () => {
|
test('skips checks if already started', () => {
|
||||||
expect(shouldRunStartupChecks(false, true, false)).toBe(false)
|
expect(shouldRunStartupChecks({
|
||||||
|
isRemoteSession: false,
|
||||||
|
hasStarted: true,
|
||||||
|
promptTypingSuppressionActive: false,
|
||||||
|
hasHadFirstSubmission: false,
|
||||||
|
gracePeriodElapsed: true,
|
||||||
|
})).toBe(false)
|
||||||
})
|
})
|
||||||
|
|
||||||
test('skips checks while user is typing', () => {
|
test('does not run checks before grace period when user is idle', () => {
|
||||||
expect(shouldRunStartupChecks(false, false, true)).toBe(false)
|
expect(shouldRunStartupChecks({
|
||||||
|
isRemoteSession: false,
|
||||||
|
hasStarted: false,
|
||||||
|
promptTypingSuppressionActive: false,
|
||||||
|
hasHadFirstSubmission: false,
|
||||||
|
gracePeriodElapsed: false,
|
||||||
|
})).toBe(false)
|
||||||
})
|
})
|
||||||
|
|
||||||
test('skips checks when remote even if started and typing', () => {
|
test('runs checks after grace period when user is idle', () => {
|
||||||
expect(shouldRunStartupChecks(true, true, true)).toBe(false)
|
expect(shouldRunStartupChecks({
|
||||||
|
isRemoteSession: false,
|
||||||
|
hasStarted: false,
|
||||||
|
promptTypingSuppressionActive: false,
|
||||||
|
hasHadFirstSubmission: false,
|
||||||
|
gracePeriodElapsed: true,
|
||||||
|
})).toBe(true)
|
||||||
|
})
|
||||||
|
|
||||||
|
test('does not run checks while user is actively typing even after grace period', () => {
|
||||||
|
expect(shouldRunStartupChecks({
|
||||||
|
isRemoteSession: false,
|
||||||
|
hasStarted: false,
|
||||||
|
promptTypingSuppressionActive: true,
|
||||||
|
hasHadFirstSubmission: false,
|
||||||
|
gracePeriodElapsed: true,
|
||||||
|
})).toBe(false)
|
||||||
|
})
|
||||||
|
|
||||||
|
test('runs checks after first submission even while typing', () => {
|
||||||
|
expect(shouldRunStartupChecks({
|
||||||
|
isRemoteSession: false,
|
||||||
|
hasStarted: false,
|
||||||
|
promptTypingSuppressionActive: true,
|
||||||
|
hasHadFirstSubmission: true,
|
||||||
|
gracePeriodElapsed: false,
|
||||||
|
})).toBe(true)
|
||||||
|
})
|
||||||
|
|
||||||
|
test('does not run checks before grace period even with typing suppression', () => {
|
||||||
|
expect(shouldRunStartupChecks({
|
||||||
|
isRemoteSession: false,
|
||||||
|
hasStarted: false,
|
||||||
|
promptTypingSuppressionActive: true,
|
||||||
|
hasHadFirstSubmission: false,
|
||||||
|
gracePeriodElapsed: false,
|
||||||
|
})).toBe(false)
|
||||||
})
|
})
|
||||||
})
|
})
|
||||||
|
|
||||||
describe('STARTUP_CHECK_DELAY_MS', () => {
|
describe('STARTUP_GRACE_PERIOD_MS', () => {
|
||||||
test('delay is positive and reasonable', () => {
|
test('grace period is positive and reasonable', () => {
|
||||||
expect(STARTUP_CHECK_DELAY_MS).toBeGreaterThan(0)
|
expect(STARTUP_GRACE_PERIOD_MS).toBeGreaterThan(0)
|
||||||
expect(STARTUP_CHECK_DELAY_MS).toBeLessThanOrEqual(5000)
|
expect(STARTUP_GRACE_PERIOD_MS).toBeLessThanOrEqual(10000)
|
||||||
})
|
})
|
||||||
})
|
})
|
||||||
@@ -2,26 +2,53 @@
|
|||||||
* Startup gates for the REPL.
|
* Startup gates for the REPL.
|
||||||
*
|
*
|
||||||
* Prevents startup plugin checks and recommendation dialogs from stealing
|
* Prevents startup plugin checks and recommendation dialogs from stealing
|
||||||
* focus while the user is typing or has early input buffered.
|
* focus before the user has interacted with the prompt.
|
||||||
*
|
*
|
||||||
* This addresses the root cause of issue #363: on mount, performStartupChecks
|
* This addresses the root cause of issue #363: on mount, performStartupChecks
|
||||||
* triggers plugin loading, which populates trackedFiles, which triggers
|
* triggers plugin loading, which populates trackedFiles, which triggers
|
||||||
* useLspPluginRecommendation to surface an LSP recommendation dialog. Since
|
* useLspPluginRecommendation to surface an LSP recommendation dialog. Since
|
||||||
* promptTypingSuppressionActive is false before the user has typed anything,
|
* promptTypingSuppressionActive is false before the user has typed anything,
|
||||||
* getFocusedInputDialog() returns the dialog, unmounting PromptInput entirely.
|
* getFocusedInputDialog() returns the dialog, unmounting PromptInput entirely.
|
||||||
|
*
|
||||||
|
* The fix gates startup checks on actual prompt readiness — either the user
|
||||||
|
* has started typing (inputValue is non-empty) or has submitted their first
|
||||||
|
* message. A pure timeout is insufficient because pausing for >1.5s before
|
||||||
|
* typing would still allow dialogs to steal focus.
|
||||||
*/
|
*/
|
||||||
|
|
||||||
const STARTUP_CHECK_DELAY_MS = 1500
|
const STARTUP_GRACE_PERIOD_MS = 3000
|
||||||
|
|
||||||
export function shouldRunStartupChecks(
|
/**
|
||||||
isRemoteSession: boolean,
|
* Determines whether startup checks should run.
|
||||||
hasStarted: boolean,
|
*
|
||||||
promptTypingSuppressionActive: boolean,
|
* Startup checks are deferred until one of:
|
||||||
): boolean {
|
* 1. The user has typed something into the prompt (inputValue non-empty)
|
||||||
if (isRemoteSession) return false
|
* 2. The user has submitted their first message (hasHadFirstSubmission)
|
||||||
if (hasStarted) return false
|
* 3. The grace period has elapsed AND the user is not actively typing
|
||||||
if (promptTypingSuppressionActive) return false
|
* (fallback for long idle periods where checks should eventually run,
|
||||||
return true
|
* but only when it won't interrupt an active typing session)
|
||||||
|
*/
|
||||||
|
export function shouldRunStartupChecks(options: {
|
||||||
|
isRemoteSession: boolean;
|
||||||
|
hasStarted: boolean;
|
||||||
|
promptTypingSuppressionActive: boolean;
|
||||||
|
hasHadFirstSubmission: boolean;
|
||||||
|
gracePeriodElapsed: boolean;
|
||||||
|
}): boolean {
|
||||||
|
if (options.isRemoteSession) return false;
|
||||||
|
if (options.hasStarted) return false;
|
||||||
|
|
||||||
|
// User has submitted their first message — safe to run checks
|
||||||
|
if (options.hasHadFirstSubmission) return true;
|
||||||
|
|
||||||
|
// User has typed something and grace period has passed — safe once they stop
|
||||||
|
if (options.promptTypingSuppressionActive && options.gracePeriodElapsed) return false;
|
||||||
|
|
||||||
|
// Grace period elapsed and user is idle — safe to run checks
|
||||||
|
if (options.gracePeriodElapsed && !options.promptTypingSuppressionActive) return true;
|
||||||
|
|
||||||
|
// Before grace period — don't run checks yet
|
||||||
|
return false;
|
||||||
}
|
}
|
||||||
|
|
||||||
export { STARTUP_CHECK_DELAY_MS }
|
export { STARTUP_GRACE_PERIOD_MS }
|
||||||
Reference in New Issue
Block a user